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

Don't set forbidden zone options for zone type 'forward' #142

Merged

Conversation

antaflos
Copy link
Contributor

@antaflos antaflos commented May 8, 2019

When zonetype => forward is set for a dns::zone resource then ignore any
forbidden options that might be set as parameters when rendering the
named.zone.erb template. Such forbidden options are "file", "masters",
"allow-transfer", "allow-query", "notify" and "also_notify".

This prevents the module from generating an invalid named configuration
(in zones.conf) that would result in named failing to (re)start.

Fixes GH-141

@theforeman-bot
Copy link
Member

@antaflos, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).


This message was auto-generated by Foreman's prprocessor

When zonetype => forward is set for a dns::zone resource then ignore any
forbidden options that might be set as parameters when rendering the
named.zone.erb template. Such forbidden options are "file", "masters",
"allow-transfer", "allow-query", "notify" and "also_notify".

This prevents the module from generating an invalid named configuration
(in zones.conf) that would result in named failing to (re)start.

Fixes theforemanGH-141
@antaflos antaflos force-pushed the zonetype_forward_dont_set_forbidden_options branch from 35e6acb to 0993455 Compare May 8, 2019 14:37
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Should we do these checks in puppet code and hard error when it's passed in? I like hard failures in catalog compilation so you're aware that you're passing in an invalid configuration.

@antaflos
Copy link
Contributor Author

antaflos commented Jun 5, 2019

In this case I thought it may be easier and more efficient to make the template aware of the zone type and let it "do the right thing" on its own, without failing the catalog compilation.

The use case I thought of was this: when most zones are slave zones we can set defaults for the dns::zone type in Hiera or in a profile, like zonetype => slave and manage_file_name => true. Then we can override the zonetype default for specific zones by setting zonetype => forward and we don't have to set or change manage_file_name; it can remain at its set default value since it is just ignored for the forward zone type.

But if you want I can also change the behaviour like you suggested and fail with a hard error if this is the preferable way to go for this module.

@antaflos
Copy link
Contributor Author

antaflos commented Jul 5, 2019

Gently bumping this as well :)

@ekohl ekohl merged commit 9a0339b into theforeman:master Jul 16, 2019
@ekohl
Copy link
Member

ekohl commented Jul 16, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module can generate invalid configuration when zonetype => forward
3 participants