Updates to allow multiple rewrite rules and rewrite rule/condition pairs #373

amvapor · 2013-09-21T12:42:32Z

I believe I have setup tests and documentation correctly, but would love any comments to make this ready to merge.

…ng grouping and commenting

…ore intuitive and is consistent with other directives (i.e: aliases)

blkperl · 2013-09-21T19:06:44Z

Is this meant to replace the old rewrite variables? Please add a validate_hash() in vhost.pp for the new rewrites param.

amvapor · 2013-09-21T21:00:14Z

Yes, the intention is to create an entirely different way to handle rewrites, one where multiple rewrite rule/condition sets can be setup. I have just committed an addition of the validate_hash() for the rewrites hash but I don't have my full dev environment to run full test suite locally.

blkperl · 2013-09-21T21:17:35Z

Spec tests are failing now. Can you add deprecation warnings to the old rewrite variables?

amvapor · 2013-09-21T21:20:26Z

Yes, I see the failing test, I will get it all up to snuff in my proper development environment on Monday. I may play with it further this weekend, but I'm not counting on it.

amvapor · 2013-09-23T12:58:02Z

I think this is ready to merge now, I'd appreciate any comments on the last commit however, I can't seem to see a good way to validate all hashes in an array.

blkperl · 2013-09-24T21:31:53Z

Can you add warnings for the old params?

amvapor · 2013-09-25T14:41:20Z

Shouldn't be a problem. will do today.

amvapor · 2013-09-26T15:19:37Z

Sorry for the delay, but I can't find a great way to test the warnings, especially in the rspec tests.

blkperl · 2013-09-26T18:05:14Z

That's fine warnings don't need to be tested.

blkperl · 2013-09-26T18:06:26Z

manifests/vhost.pp

+
+  # Deprecated backwards-compatibility
+  if $rewrite_rule {
+    warning('Apache::Vhost: parameter rewrite_rule is depricated in favor of rewrites')


deprecated is spelled wrong

blkperl · 2013-09-26T18:09:32Z

Does rewrite_rule and rewrite_cond still work after this PR? If not then this has to go in a major release otherwise this can go in the next minor release.

amvapor · 2013-09-26T18:25:02Z

the existing rewrite_rule and rewrite_cond continue to work. But if rewrites is defined, they stop working. You can see this logic in templates/vhost/_rewrites.erb

blkperl · 2013-10-01T03:34:06Z

@igalic Can you do a review?

igalic · 2013-10-01T07:13:19Z

sure! On it.

igalic · 2013-10-01T07:32:58Z

I have two criticisms:

Some of the examples are weak leaning on bad: I'm not a bad a fan of (.*) in RewriteRules when you do not actually use what you captured. In such cases it's enough to check for ^: everything has a beginning, even empty strings, and no need to waste CPU or memory going to the end of that string (note that the HTTP RFC gives no limit for the length of an URI.
The tests should test the examples you give and document to be truly useful.

amvapor · 2013-10-09T19:30:27Z

@igalic Thanks for the review, I appreciate it

Regarding point 1. I just reformatted an existing example to work with the new rewrites format. I'm not sure if I should be changing the example's intended use.

Regarding point 2. I will be committing a change to actually do a valid test shortly. Should all examples be tested? or only a subset?

igalic · 2013-10-10T04:36:00Z

@amvapor can you please look at the author (Arnold and root..) in 831be33 and fix it

igalic · 2013-10-10T04:38:52Z

@amvapor you may want to take another look at the author and committer fields (it says arnold and root) in that last commit (831be33) and fix them accordingly.

amvapor · 2013-11-14T21:08:43Z

I'm pretty sure that makes this reasonable to merge. If it doesn't please help me out with a link or two.

igalic · 2013-11-14T22:04:24Z

I'd rebase it against master, then squash the commits down to a reasonable amount.
See http://git-scm.com/book/en/Git-Tools-Rewriting-History for how to do that.

igalic · 2013-11-18T13:40:39Z

any news on this?

make all matches and notmatches arrays of regex and exact match strings ...because consistency and explicitness

…clarations

Altered changed ssl_proxyengine to match form of other SSL parameters. Fixed unescaped + in regex

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

…tra_newlines Remove extra empty lines in _directories template

Adding support for another mod_wsgi parameter (WSGIPythonPath)

…ng grouping and commenting Changing the syntax of the directives. Now the rewrite directive is more intuitive and is consistent with other directives (i.e: aliases) filling out the documentation a little more thoroughly filling out the documentation a little more thoroughly adding validation to the rewrites param adding better tests for the new rewrite settings make sure at least the first rewrite is a valid hash adding warnings for depricated parameters fixing spelling mistake setting up real tests of rewrites Setting up the ability to do multiple rewrites and conditions. Allowing grouping and commenting Changing the syntax of the directives. Now the rewrite directive is more intuitive and is consistent with other directives (i.e: aliases) filling out the documentation a little more thoroughly fixing a misstype renaming the rewrite_conds and rewrite_rules parameters to be singular to maintain consistency with previous standard

added peruser and event mpms Workaround for apxs-loaded modules On some OSes (FreeBSD) apxs tool is used to put LoadModule directives into httpd.conf during apache package (and apache modules) insallation/reinstallation. The apxs expects some LoadModule directives to be already present in httpd.conf (they may be commented-out) in order to decide where to put its own directives. This PR puts fake LoadModule directive (commented out) to httpd.conf. The $apxs_workaround boolean parameter in apache class decides, whether to use this workaround or not. This is used on FreeBSD, where apxs is used by ports package provider (and perhaps all other). Without this, the apache installation/reinstallation/deinstallation as well as installation of additional modules would fail. This PR was created in order to split puppetlabs#342 into smaller parts to make review process easier, see puppetlabs#342 (comment) Remove AllowOverride header for non-directories When adding a dir with a different provider than directory (e.g. files, or location), the AllowOverride header still added. This causes a warning in apache. Add unit tests Fixed a typo. The directory provider should be lower case to match the manifests. allow to choose the mpm_event mod from the init.pp added a mod/event.pp manifest and edited the spec test of the prefork/worker/event modules. Add initial support for nss module (no directives in vhost template yet) added $server_root parameter Previously, the $httpd_dir was used as ServerRoot in httpd.conf template. On some installations httpd_dir is not same as ServerRoot, for example on FreeBSD apache22 (installed via ports) has ServerRoot set to /usr/local by default. This PR adds $server_root parameter to apache::params. Its purpose is to be substituted in httpd.conf as ServerRoot. This parameter is used in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into smaller parts to make review process easier, see puppetlabs#342 (comment) Allow apache::mod to specify module id and path Add $id, $path and $lib_path parameters to apache::mod. These parameters are used in puppetlabs#342. The purpose of the whole thing is to plit puppetlabs#342 into smaller parts to make review process easier, see puppetlabs#342 (comment) This PR covers all changes introduced by puppetlabs#271 to manifests/mod.pp. I just don't have test/mod_load_params.pp as in puppetlabs#271. Add new params to apache::mod::mime class This changeset adds $mime_support_package and $mime_types_config parameters to class `apache::mod::mime`. Their purpose is following: - $mime_support_package - install this package, as it provides mime.conf file (definition of mime types used by mod_mime), - $mime_types_config - used to substitute TypesConfig in mod/mime.conf.erb template. This PR was created in order to plit puppetlabs#342 into smaller parts to make review process easier, see puppetlabs#342 (comment) Include mime if ssl is in use. Without the default_mods we still try to setup an SSL vhost that fails because AddType is used in the template. Rather than trying to make AddType conditional on the inclusion of the class we just require mime as a standard mod no matter what you say. Wrap the event test in a check for FreeBSD. It seems this only works on FreeBSD currently. In the future we'll rework testing to test for 2.2 vs 2.4 or something.

amvapor · 2013-11-19T16:03:42Z

@igalic I've either done it, or I've seriously messed up this branch... if it doesn't work this time, I'll close this pull request and applying the patch on a fresh branch of upstream master as a single commit and opening a new pull request.

igalic · 2013-11-19T16:33:12Z

We still have ~39 commits on this branch. Please rebase against master:

git remote add upstream git://github.com/puppetlabs/puppetlabs-apache
git remote update
git rebase upstream/master

And then squash your commits:

git rebase -i HEAD~number_of_your_commits

paul91 · 2013-11-25T20:34:09Z

Any progress? This feature would be really helpful.

igalic · 2013-11-25T22:54:15Z

@amvapor hasn't replied in quite some time. I can attempt to do the merge/squash myself if we don't here anything from them until tomorrow.

igalic · 2013-11-27T09:47:05Z

@amvapor I've merge --squashed your entire branch one commit https://github.com/igalic/puppetlabs-apache/commit/5830d0668bbd630f4c72f391559ebcfcbd85e888. Unfortunately, git did not preserve the authorship, except in the commit message, so I'm a little unhappy

amvapor · 2013-11-27T14:04:36Z

I'm getting lost on what's wrong and what I need to do to correct it, I squashed my commits down into amvapor@e8b1ea7 and it should be able to be merged. I don't particularly care about the authorship being in my name or not.

igalic · 2013-11-27T14:44:50Z

Either you force-push (git push -f) into the branch on github that makes up this pull request, or you close this one and create a new pull request from amvapor@e8b1ea7

igalic · 2013-12-05T15:00:57Z

@amvapor what's the status? Can you update this pull request, or close it and create one from amvapor@e8b1ea7 ?

amvapor · 2013-12-05T15:18:36Z

@igalic Just in the middle of some deployments, I will address this by end of week.

amvapor added 4 commits September 20, 2013 10:27

Setting up the ability to do multiple rewrites and conditions. Allowi…

5e5c90b

…ng grouping and commenting

Changing the syntax of the directives. Now the rewrite directive is m…

2b23fc6

…ore intuitive and is consistent with other directives (i.e: aliases)

filling out the documentation a little more thoroughly

68eb53d

filling out the documentation a little more thoroughly

f04024a

adding validation to the rewrites param

15cab8e

amvapor added 2 commits September 21, 2013 19:08

adding better tests for the new rewrite settings

7001d39

make sure at least the first rewrite is a valid hash

f5d0bcb

blkperl mentioned this pull request Sep 24, 2013

Enable multi-rewriterule feature #357

Closed

adding warnings for depricated parameters

b7e498c

blkperl reviewed Sep 26, 2013
View reviewed changes

fixing spelling mistake

fa78de0

setting up real tests of rewrites

28b27dd

Merge branch 'master' of https://github.com/amvapor/puppetlabs-apache

2fae0fe

Aaron Hicks and others added 12 commits November 18, 2013 22:47

Reworking broken match and notmatch code

9ad3e70

make all matches and notmatches arrays of regex and exact match strings ...because consistency and explicitness

remove unecessary " from around paths in Directory and ScriptAlias de…

8d8896c

…clarations

Fixing incorrect match strings and regexes

816b629

the ssl parameter needs to be true to test the SSL parameters.

4316ac0

Add missing SSL directive tests.

b6ccfb7

Altered changed ssl_proxyengine to match form of other SSL parameters. Fixed unescaped + in regex

Fixing broken test matches...

823e952

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

a0d2000

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

Merge pull request puppetlabs#473 from antaflos/directories_remove_ex…

ba1a98a

…tra_newlines Remove extra empty lines in _directories template

Merge pull request puppetlabs#458 from ksexton/wsgi

4c34942

Adding support for another mod_wsgi parameter (WSGIPythonPath)

Merging upstream and rebasing

543fc2c

igalic mentioned this pull request Nov 26, 2013

[discuss] Release what we have as 1.0.0? #505

Closed

igalic mentioned this pull request Dec 5, 2013

added redirect match support #499

Closed

amvapor closed this Dec 6, 2013

amvapor mentioned this pull request Dec 6, 2013

Setting up the ability to do multiple rewrites and conditions. Allowing ... #517

Merged

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

Handle compose files with services that have no image (puppetlabs#373)

2d3f4d6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to allow multiple rewrite rules and rewrite rule/condition pairs #373

Updates to allow multiple rewrite rules and rewrite rule/condition pairs #373

amvapor commented Sep 21, 2013

blkperl commented Sep 21, 2013

amvapor commented Sep 21, 2013

blkperl commented Sep 21, 2013

amvapor commented Sep 21, 2013

amvapor commented Sep 23, 2013

blkperl commented Sep 24, 2013

amvapor commented Sep 25, 2013

amvapor commented Sep 26, 2013

blkperl commented Sep 26, 2013

blkperl Sep 26, 2013

blkperl commented Sep 26, 2013

amvapor commented Sep 26, 2013

blkperl commented Oct 1, 2013

igalic commented Oct 1, 2013

igalic commented Oct 1, 2013

amvapor commented Oct 9, 2013

igalic commented Oct 10, 2013

igalic commented Oct 10, 2013

amvapor commented Nov 14, 2013

igalic commented Nov 14, 2013

igalic commented Nov 18, 2013

amvapor commented Nov 19, 2013

igalic commented Nov 19, 2013

paul91 commented Nov 25, 2013

igalic commented Nov 25, 2013

igalic commented Nov 27, 2013

amvapor commented Nov 27, 2013

igalic commented Nov 27, 2013

igalic commented Dec 5, 2013

amvapor commented Dec 5, 2013

Updates to allow multiple rewrite rules and rewrite rule/condition pairs #373

Updates to allow multiple rewrite rules and rewrite rule/condition pairs #373

Conversation

amvapor commented Sep 21, 2013

blkperl commented Sep 21, 2013

amvapor commented Sep 21, 2013

blkperl commented Sep 21, 2013

amvapor commented Sep 21, 2013

amvapor commented Sep 23, 2013

blkperl commented Sep 24, 2013

amvapor commented Sep 25, 2013

amvapor commented Sep 26, 2013

blkperl commented Sep 26, 2013

blkperl Sep 26, 2013

Choose a reason for hiding this comment

blkperl commented Sep 26, 2013

amvapor commented Sep 26, 2013

blkperl commented Oct 1, 2013

igalic commented Oct 1, 2013

igalic commented Oct 1, 2013

amvapor commented Oct 9, 2013

igalic commented Oct 10, 2013

igalic commented Oct 10, 2013

amvapor commented Nov 14, 2013

igalic commented Nov 14, 2013

igalic commented Nov 18, 2013

amvapor commented Nov 19, 2013

igalic commented Nov 19, 2013

paul91 commented Nov 25, 2013

igalic commented Nov 25, 2013

igalic commented Nov 27, 2013

amvapor commented Nov 27, 2013

igalic commented Nov 27, 2013

igalic commented Dec 5, 2013

amvapor commented Dec 5, 2013