-
Notifications
You must be signed in to change notification settings - Fork 112
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
Fixed a bug where key did not work on redhat due to incorrect pkg name #87
Fixed a bug where key did not work on redhat due to incorrect pkg name #87
Conversation
… name. Replaced the hardcoded package name with the package variable.
@@ -15,7 +15,7 @@ | |||
command => "/usr/sbin/dnssec-keygen -a HMAC-MD5 -r /dev/urandom -b 128 -n USER ${name}", | |||
cwd => "${cfg_dir}/bind.keys.d", | |||
require => [ | |||
Package['dnssec-tools','bind9'], | |||
Package['dnssec-tools',$package], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand, where does this variable come from? I don't see it in the namespace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is from dns::server::params I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So.. shouldn't it be dns::server::package or something?
Alternatively I would argue that this should just require 'dnssec-tools' and let the other classes require their own packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My apologies, when I was preparing a clean branch for the pull request I neglected to include the $package = $dns::server::params::package line in the file, so you're right, $package would have been undef. I pushed a new commit that fully qualifies it, opting to just fully qualify it in the Package require.
Regarding just requiring dnssec-tools, that is probably a better choice. I was just doing the minimum necessary to fix the bug and get it working on RedHat while attempting to preserve the function of the original code. If you'd rather go that route, I'll push a commit that removes the requirement for the bind package entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, the dnssec-tools is the actual requirement, as attached to the exec. It doesn't actually need to depend on bind.
…-pkgname Fixed a bug where key did not work on redhat due to incorrect pkg name
Replaced the hardcoded package name with the package variable.