added redirect match support #499

pablokbs · 2013-11-22T19:44:39Z

Please check it now

pablokbs · 2013-11-22T19:44:55Z

@igalic Sorry for the delay

igalic · 2013-11-22T20:54:59Z

spec/defines/vhost_spec.rb

+                '404',
+              ],
+              :redirectmatch_regexp   => [
+                '/\\.git(/.*|$)',


That's one \ too many. From the looks of the travis build error.

That's weird, I have that configuration working on an apache virtualhost:

Redirect rules

RedirectMatch 404 /.git(/.*|$)

http://www.foreclosuredatabank.com/.git

apenney · 2013-11-26T14:20:19Z

Unfortunately the tests are breaking on this. Can I ask you to take a look at travis? It looks like it'll be an easy one to fix, just have to tweak the test then "rake spec" to test it.

pablokbs · 2013-11-26T15:27:32Z

Removed the extra slash and it works on apache also :squirrel:

pablokbs · 2013-11-26T20:10:09Z

Do I have to do something to refresh the test? Travis is using the same code I think

igalic · 2013-11-26T20:46:32Z

I'd squash and force-push those two commits,

% git rebase -i HEAD~2
...
% git push -f origin redirect_match_support

pablokbs · 2013-11-26T21:09:05Z

Done

igalic · 2013-11-26T21:27:55Z

But it's still failing, https://travis-ci.org/puppetlabs/puppetlabs-apache/jobs/14568417#L1124

kclair · 2013-11-29T18:36:35Z

I'm a little confused by the implementation of this. So the intention is that the full redirect text (regex AND destination) are added to $redirectmatch_regexp ? For example, if I wanted to redirect ^/(.*.gif)$ to /images/gifs/$1, would I do this?

$redirectmatch_regexp => '^/(.*.gif)$ http://otherserver.com/images/gifs/$1`

I'm curious about why the different implementation than the redirect params, which break out source and dest. Seems like the redirect params should be consistent between Redirect and RedirectMatch.

pablokbs · 2013-11-30T13:36:27Z

Yes, exactly, is the same as redirect but you can use regular expresions.

@apenney @igalic I'm running rake spec on my computer now, I will test it here before uploading, thanks

pablokbs · 2013-12-01T00:01:40Z

I will need some help here, I can't make that test work, it's always failing no matter how I escape the characters, I'm using grep and it works. How does this test make the matching expression checking? Is there I way I can test it by hand? Every rake spec test takes about 10 minutes on my computer

pablokbs · 2013-12-05T14:32:55Z

Any help?

igalic · 2013-12-05T14:38:23Z

I'm looking at this now.

igalic · 2013-12-05T14:41:32Z

first off, this is how to run the spec tests:

igalic@levix ~/src/puppet/puppetlabs-apache (git)-[redirect_match] % rspec --format d --color --backtrace spec/defines/vhost_spec.rb -e 'redirect match rules'
Run options: include {:full_description=>/redirect\ match\ rules/}

apache::vhost
  os-independent items
    attribute resources
      redirect rules
        redirect match rules
          example at ./spec/defines/vhost_spec.rb:1015 (FAILED - 1)

Failures:

  1) apache::vhost os-independent items attribute resources redirect rules redirect match rules
     Failure/Error: it { should contain_file("25-#{title}.conf").with_content %r{  RedirectMatch 404 /\\\.git(/\.*|$)} }
     Puppet::Error:
       Could not parse for environment production: No file(s) found for import of '/home/igalic/src/puppet/puppetlabs-apache/spec/fixtures/manifests/site.pp' at line 3 on node levix
     # /usr/lib/ruby/vendor_ruby/puppet/parser/parser_support.rb:166:in `rescue in parse'
     # /usr/lib/ruby/vendor_ruby/puppet/parser/parser_support.rb:157:in `parse'

igalic · 2013-12-05T14:44:50Z

next, test rules here http://rubular.com/

igalic · 2013-12-05T14:56:00Z

diff --git spec/defines/vhost_spec.rb spec/defines/vhost_spec.rb
index 1bd1aff..2e3a03e 100644
--- spec/defines/vhost_spec.rb
+++ spec/defines/vhost_spec.rb
@@ -1002,6 +1002,7 @@ describe 'apache::vhost', :type => :define do
         end
         describe 'redirect match rules' do
           let :params do
+            ## XXX As mentioned above, rspec-puppet drops constructs like $1 and makes it hard to correctly escape \.
             default_params.merge({
               :redirectmatch_status => [
                 '404',
@@ -1012,7 +1013,7 @@ describe 'apache::vhost', :type => :define do
             })
           end

-          it { should contain_file("25-#{title}.conf").with_content %r{  RedirectMatch 404 /\\\.git(/\.*|$)} }
+          it { should contain_file("25-#{title}.conf").with_content %r{  RedirectMatch 404 } }
         end
         describe 'without a status' do
           let :params do

and the test:

igalic@levix ~/src/puppet/puppetlabs-apache (git)-[redirect_match] % rake spec_clean spec_prep
Cloning into 'spec/fixtures/modules/stdlib'...
remote: Counting objects: 4350, done.
remote: Compressing objects: 100% (2538/2538), done.
remote: Total 4350 (delta 1657), reused 3916 (delta 1272)
Receiving objects: 100% (4350/4350), 768.28 KiB | 272.00 KiB/s, done.
Resolving deltas: 100% (1657/1657), done.
Checking connectivity... done
Cloning into 'spec/fixtures/modules/concat'...
remote: Counting objects: 762, done.
remote: Compressing objects: 100% (538/538), done.
remote: Total 762 (delta 312), reused 617 (delta 197)
Receiving objects: 100% (762/762), 138.42 KiB | 213.00 KiB/s, done.
Resolving deltas: 100% (312/312), done.
Checking connectivity... done
igalic@levix ~/src/puppet/puppetlabs-apache (git)-[redirect_match] % rspec --format d --color --backtrace spec/defines/vhost_spec.rb -e 'redirect match rules'
Run options: include {:full_description=>/redirect\ match\ rules/}

apache::vhost
  os-independent items
    attribute resources
      redirect rules
        redirect match rules
          should contain File[25-rspec.example.com.conf] with content matching /  RedirectMatch 404 /

Finished in 5.63 seconds
1 example, 0 failures
rspec --format d --color --backtrace spec/defines/vhost_spec.rb -e   8,42s user 0,23s system 100% cpu 8,646 total
igalic@levix ~/src/puppet/puppetlabs-apache (git)-[redirect_match] %

igalic · 2013-12-05T14:57:41Z

But, in general, and I'm sorry I'm saying this only now: I have to agree with @kclair:

The parameters should be a list of hashes that break everything up, analogous to redirects, and analogous to all other directives.

igalic · 2013-12-05T14:59:10Z

Perhaps it would be even better to do this analogous to the multiple rewrites (#373) patch.

kclair · 2013-12-06T07:22:08Z

i'm not sure that i agree that it is analogous to rewrites because the directives (Rewrite and RewriteMatch) are actually different. To consolidate them would necessitate either a flag to indicate it's a RewriteMatch rather than a Rewrite, or trying to auto-detect a regex (ugh!).

pablokbs · 2013-12-08T00:00:57Z

Hello again

Sorry I didn't really understand your response. I see you've removed the regular expresion and the test went fine. I did the same at my computer and it works also. Do you want me to do that?

igalic · 2013-12-10T07:54:46Z

Yes. For now, please do that. This is a bug in rspec-puppet

When managing directories (or locations, files) through the `directories` parameter the resulting vhost configuration file contains a few empty lines too many. This cleans up these unnecessary empty lines, leaving just one between Directory blocks. This is a purely cosmetic change.

…er is used to specify additional directories to search for Python modules as described here: https://code.google.com/p/modwsgi/wiki/ConfigurationDirectives#WSGIPythonPath

(#479) Extend the _aliases.erb template so that the list of hashes in `apache::vhost`'s `aliases` parameter can contain the usual `alias` hashes but also `aliasmatch` hashes, which get translated to `AliasMatch` directives. Includes basic spec tests and updated documentation in README.md. Note that these spec tests are currently only skeletons because rspec-puppet drops constructs like '$1'. See the entries marked XXX.

Don't implicitly add a trailing slash to the path components of ScriptAlias directives. Includes updated spec tests. Fixes issue #489.

Remove the `authnz_ldap` key from $mod_packages in `apache::params` so an installation of the non-existent package `libapache2-mod-authz-ldap` is not attempted. Properly manage the `authnz_ldap.conf` file template and set the `LDAPVerifyServerCert` directive according to $verifyServerCert in `apache::mod::authnz_ldap`. Also update spec tests accordingly. This fixes only the superficial problems in issue #494. Better support for mod_ldap and mod_authnz_ldap is required, as discussed there.

add support for overriding ErrorDocument settings in both, vhost and directory context. With documentation and tests included this fixes #474

…edirect_match_support

…bs-apache into redirect_match_support Conflicts: spec/defines/vhost_spec.rb

blkperl · 2013-12-13T06:46:24Z

Looks like a few other commits snuck in, can you rebase this against master.

blkperl · 2013-12-13T06:48:13Z

Looks like you submitted a second PR for this in #521. I'll close this one then

igalic reviewed Nov 22, 2013
View reviewed changes

added redirect match support

5c17980

antaflos and others added 8 commits December 10, 2013 09:44

Add WSGIPythonPath as an optional parameter to mod_wsgi. This paramet…

739cd5f

…er is used to specify additional directories to search for Python modules as described here: https://code.google.com/p/modwsgi/wiki/ConfigurationDirectives#WSGIPythonPath

Disable default "allow from all" in vhost-directories

55801d5

No trailing slash for ScriptAlias path

58a83cd

Don't implicitly add a trailing slash to the path components of ScriptAlias directives. Includes updated spec tests. Fixes issue #489.

Restore test for multiple includes

989d243

Add support for overriding ErrorDocument

5de6e82

add support for overriding ErrorDocument settings in both, vhost and directory context. With documentation and tests included this fixes #474

Pablo Fredrikson added 3 commits December 10, 2013 09:44

added redirectmatch support

2c39ad5

Merge branch 'master' of github.com:pablokbs/puppetlabs-apache into r…

e235168

…edirect_match_support

Merge branch 'redirect_match_support' of github.com:pablokbs/puppetla…

cf1e6a0

…bs-apache into redirect_match_support Conflicts: spec/defines/vhost_spec.rb

blkperl closed this Dec 13, 2013

traylenator pushed a commit to traylenator/puppetlabs-apache that referenced this pull request Jun 7, 2022

Fix frozen string errors for tasks (puppetlabs#499)

3772c5b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added redirect match support #499

added redirect match support #499

pablokbs commented Nov 22, 2013

pablokbs commented Nov 22, 2013

igalic Nov 22, 2013

pablokbs Nov 23, 2013

apenney commented Nov 26, 2013

pablokbs commented Nov 26, 2013

pablokbs commented Nov 26, 2013

igalic commented Nov 26, 2013

pablokbs commented Nov 26, 2013

igalic commented Nov 26, 2013

kclair commented Nov 29, 2013

pablokbs commented Nov 30, 2013

pablokbs commented Dec 1, 2013

pablokbs commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

kclair commented Dec 6, 2013

pablokbs commented Dec 8, 2013

igalic commented Dec 10, 2013

blkperl commented Dec 13, 2013

blkperl commented Dec 13, 2013

added redirect match support #499

added redirect match support #499

Conversation

pablokbs commented Nov 22, 2013

pablokbs commented Nov 22, 2013

igalic Nov 22, 2013

Choose a reason for hiding this comment

pablokbs Nov 23, 2013

Choose a reason for hiding this comment

Redirect rules

apenney commented Nov 26, 2013

pablokbs commented Nov 26, 2013

pablokbs commented Nov 26, 2013

igalic commented Nov 26, 2013

pablokbs commented Nov 26, 2013

igalic commented Nov 26, 2013

kclair commented Nov 29, 2013

pablokbs commented Nov 30, 2013

pablokbs commented Dec 1, 2013

pablokbs commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

igalic commented Dec 5, 2013

kclair commented Dec 6, 2013

pablokbs commented Dec 8, 2013

igalic commented Dec 10, 2013

blkperl commented Dec 13, 2013

blkperl commented Dec 13, 2013