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

move unless into manage_security_corerules #1976

Conversation

SimonHoenscheid
Copy link
Contributor

@SimonHoenscheid SimonHoenscheid commented Nov 12, 2019

This PR moves the unless block inside the manage manage_security_corerules block because it is only needed if core rules are there. It also prevents the apache::security::rule_link define to be called if security core rules are not managed

@SimonHoenscheid SimonHoenscheid requested a review from a team as a code owner November 12, 2019 17:55
@florindragos
Copy link
Contributor

Hello @SimonHoenscheid,
Thanks for your contribution!
Can you provide some more info on why this is necessary?
From what I can see, the unless has been outside manage_security_corerules for a long time.

@tuxmea
Copy link
Contributor

tuxmea commented Nov 18, 2019

@florindragos : we recommend to only activate the security rules when security rules are set at all.
The actual code will try to set rules, even when $manage_security_crs is set to false.

@SimonHoenscheid
Copy link
Contributor Author

@florindragos I updated the description to make the intention more clear

@florindragos
Copy link
Contributor

got it! makes sense, was just wondering why it was not fixed earlier.

@florindragos florindragos merged commit 940dc4e into puppetlabs:master Nov 20, 2019
@SimonHoenscheid SimonHoenscheid deleted the unless_block_is_needed_if_security_core_rules_are_used branch April 9, 2020 16:18
cegeka-jenkins pushed a commit to cegeka/puppet-apache that referenced this pull request Jul 15, 2020
…is_needed_if_security_core_rules_are_used

move unless into manage_security_corerules
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.

3 participants