Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Modules 5385] Include support for Apache 2.4 mod_authz_host directives in apache::mod::status #1667

Merged
merged 1 commit into from
Jan 2, 2018

Conversation

EmersonPrado
Copy link
Contributor

In the current state, creates helper templates mod/_allow and mod/_require with, respectively, Apache 2.2 and 2.4 host authorization logic. These aim to be used by various templates in mod/. Then, template mod/status.conf.erb is modified to use one of these helper templates depending on Apache version.
The _require helper template implements "Require" directives from the parameter requires provided by the calling class. If this is undef, then uses variable requires_defaults, also provided by the calling class. In order not to break existing systems, if requires == undef but allow_from is defined, uses the latter and issue a deprecation warning.

requires parameter can be either of:

  • undef
  • '' or 'unmanaged' (No "Require" directive)
  • String (ip <list>, host <list>, not <something>, etc.)
  • Array of strings (each with same syntax as standalone string)
  • Hash
    • Key requires - Array of strings (same rule as standalone array)
    • Key enforce (optional) - Either of "Any|All|None" for <Require*></Require*> block

Tested on RHEL 6 and 7 and Debian 7 and 8, but only in Puppet 3 (unfortunately, it's what I have here for the moment)
Test specs were not updated yet

@EmersonPrado
Copy link
Contributor Author

Now with spec tests for all reasonable options for requires parameter:

  • undef
  • Empty string
  • 'unmanaged'
  • String with 'ip ...' or 'host ...'
  • Array of strings
  • Hash with 'requires' array
    • Without 'enforce' key
    • With 'enforce' key

After lots of corrections in spec test (since I'm a beginner in that still) and a minor correction in helper template, all tests pass.

I'd like to read other people opinions about the path I'm going. In the absence of issues with that, I intend to replicate this in the other affected classes.

@@ -26,7 +26,8 @@
# }
#
class apache::mod::status (
Array $allow_from = ['127.0.0.1','::1'],
Optional[Array] $allow_from = undef,
$requires = undef,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check this with Optional[Variant[String, Array, Hash]]?

@EmersonPrado
Copy link
Contributor Author

@ekohl Good idea about validation. Done!

@EmersonPrado
Copy link
Contributor Author

Since this PR has already many commits and changes, I'll limit it only to apache::mod::status class, then work the other affected classes in new branches and PRs. If there are no other suggestions or other needs about this, I'll just wait for the merge to copy the implementation to the other classes.

@EmersonPrado EmersonPrado changed the title [WIP] [Modules 5385] Include support for Apache 2.4 mod_authz_host directives in mod/* templates [Modules 5385] Include support for Apache 2.4 mod_authz_host directives in apache::mod::status Aug 17, 2017
@david22swan
Copy link
Member

@EmersonPrado Could you please alter the readme to reflect your changes?
Also we would prefer you to squish your commits down into a single one.

Use "Allow" defaults for "Require" directives for mod::*

Include auxilliary template for "Allow" directives for mod::*

Use helper templates in mod/status.conf.erb / add "requires" parameter

Conflicts:
	manifests/mod/status.pp

Include defaults for "Allow" and "Require" directives in mod::status

Conflicts:
	manifests/mod/status.pp

Add comment for Puppet 4.2 in template mod/status.conf.erb

Include in template mod/_require.erb parsing of "requires" as string

Include in template mod/_require.erb parsing of "requires" as array

Include in template mod/_require.erb parsing of "requires" as hash

Include in template mod/_require.erb section "Require(Any|All|None)"

Allows undef for parameter apache::mod::status::allow_from

Add Require <String> context for Apache >= 2.4 in mod::status spec

Add missing parameter for Apache 2.4 contexts in mod::status spec test

Add function for "Require" directive in mod::status spec test

Iterate Require params in Apache 2.4 contexts in mod::status spec test

Include empty and "unmanaged" requires param in mod::status spec test

Include default case for requires param in mod::status spec test

Remove class variable reference in mod::status spec test

Change assignment to nil in mod::status spec test

Change undef test in mod::status spec test

Revert "Change undef test in mod::status spec test"

This reverts commit 57ac710.

Only change undef test in mod::status spec test

Change quoting of control char in mod::status spec test

Include in template mod/_require.erb parsing of "requires" as array

Include in template mod/_require.erb parsing of "requires" as hash

Include in template mod/_require.erb parsing of "requires" with enforce

Use each for array parsing in mod::status spec test

Correct hash references in mod::status spec test

Remove "prints" from "each" iterations in mod::status spec test

Split "each" one-liners in mod::status spec test

Use symbols in hash keys in mod::status spec test

Include "print" again in function returns in mod::status spec test

Use map+join to parse arrays in mod::status spec test

Correct indentations in helper template for apache 2.4

Include type validation for ...mod::status::requires

Align parameter indentation in ...mod::status

Document parameter "requires" in ...mod::status

Correct parameter description in mod::status class

Include in README parameter mod::status::requires

Add code tags in parameter mod::status::requires description in README

Include array/hash doc refs in mod::status::requires description in README
@EmersonPrado
Copy link
Contributor Author

@david22swan Done. Also rebased to master. Got some failures, but I think they're unrelated.

@david22swan
Copy link
Member

Yeah, those fails are related to a separate issue.

@david22swan david22swan merged commit 674bb6d into puppetlabs:master Jan 2, 2018
cegeka-jenkins pushed a commit to cegeka/puppet-apache that referenced this pull request Jul 15, 2020
[Modules 5385] Include support for Apache 2.4 mod_authz_host directives in apache::mod::status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants