-
Notifications
You must be signed in to change notification settings - Fork 229
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
Conditionally generate the CA cert. #783
base: master
Are you sure you want to change the base?
Conversation
6fc064c
to
a86918f
Compare
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.
Some minor suggestions, overall it makes sense to have a parameter for this.
manifests/init.pp
Outdated
@@ -536,6 +536,8 @@ | |||
# invokes when on static_file_content requests. | |||
# Defaults to undef | |||
# | |||
# $generate_ca_cert:: Defaults to true. When true, the a ca cert is generated. |
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.
# $generate_ca_cert:: Defaults to true. When true, the a ca cert is generated. | |
# $generate_ca_cert:: Whether or not a CA certificate is generated. |
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.
Will do.
Concat["${puppet::server::dir}/puppet.conf"], | ||
Exec['puppet_server_config-create_ssl_dir'], | ||
], | ||
if $puppet::generate_ca_cert { |
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 think you can move this up a bit so it also captures the if/else
block to determine $creates
and $command
. They're only used in this exec.
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.
ok, will do.
@@ -157,22 +157,24 @@ | |||
|
|||
# Generate a new CA and host cert if our host cert doesn't exist | |||
if $puppet::server::ca { |
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.
@ekohl I followed your suggestion. However, if this is the case, shouldn't there just be a $ca in the manifests/init.pp? There is no $ca option in manifests/init.pp now.
I just need this turned off so this module can complete doing its thing without failing about preexisting certs.
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.
@ekohl I followed your suggestion. However, if this is the case, shouldn't there just be a $ca in the manifests/init.pp? There is no $ca option in manifests/init.pp now.
We do have $server_ca
in init.pp
I just need this turned off so this module can complete doing its thing without failing about preexisting certs.
We have a creates
. Why isn't it picking that up?
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.
Hi @ekohl ,
We do have $server_ca in init.pp
You must be referring to this. I did see this, but this parameter has no effect on whether to generate a CA file.
We have a creates. Why isn't it picking that up?
Where in the code is this? I did not find a creates
parameter in the init.pp.
manifests/init.pp
Outdated
@@ -734,6 +736,8 @@ | |||
Optional[Integer[1]] $server_max_open_files = $puppet::params::server_max_open_files, | |||
Optional[Stdlib::Absolutepath] $server_versioned_code_id = undef, | |||
Optional[Stdlib::Absolutepath] $server_versioned_code_content = undef, | |||
Boolean $generate_ca_cert = $puppet::params::generate_ca_cert, | |||
|
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.
…puppet-puppet into make-build-ca-optional
Hi any news on this? Should I continue to pursue this PR or should I just leave it? |
My apologies for not getting to this. I've been really neglecting this module. Sadly there's a merge conflict now. If you're still interested, please rebase and resolve the conflict. |
No description provided.