(maint) Make builds reproducible #8916
Conversation
|
Can one of the admins verify this patch? |
|
|
|
just for clarity here, @jcharaoui has already signed the CLA here, it's @lamby that needs to sign it as well now. |
|
Done. |
lib/puppet/defaults.rb
Outdated
| @@ -806,7 +806,7 @@ def self.initialize_default_settings!(settings) | |||
| # We have to downcase the fqdn, because the current ssl stuff (as opposed to in master) doesn't have good facilities for | |||
| # manipulating naming. | |||
| :certname => { | |||
| :default => lambda { Puppet::Settings.default_certname.downcase }, | |||
| :default => lambda { ENV.has_key?('SOURCE_DATE_EPOCH') ? '(node\'s fully qualified domain name)' : Puppet::Settings.default_certname.downcase }, | |||
There was a problem hiding this comment.
I don't think this change is necessary anymore due to 5b954fd. You may also want to handle the srv_domain setting in a similar way, as currently it is sensitive to the domain fact on the host you're running the rake task on.
There was a problem hiding this comment.
To be clear, you mean we can safely back out this particular hunk (ie. for :certname), but we need to make a change almost identical to this one or make a change similar to 5b954fd that applies to srv_domain?
There was a problem hiding this comment.
Not @joshcooper, but how I read it it's the latter: apply a similar solution to srv_domain.
There was a problem hiding this comment.
Great. This isn't actually my PR permission-wise, so here's my diff: https://gist.github.com/lamby/00daaaf9e8f397ffcd3eb0b80185cdc7/raw @jcharaoui can you add this new hunk to your PR and force-push?
There was a problem hiding this comment.
I think what @joshcooper meant is that ideally a similar solution is used. 5b954fd changes it so https://puppet.com/docs/puppet/7/configuration.html#certname is rendered with a sane default. You can see in https://puppet.com/docs/puppet/7/configuration.html#srv-domain that it does indeed leak the domain name. Ideally the mechanism used to make the reference correct would also be applied during manpage generation. Your current solution can break the application at runtime.
Perhaps it's something that should be applied in https://github.com/puppetlabs/puppet/blob/main/tasks/manpages.rake or extracted to some common helper so the help face can apply it too.
There was a problem hiding this comment.
For those wondering why srv_domain isn't part of this PR anymore: it's fixed via #8933.
|
Hi @jcharaoui , thank you for your contribution! I think for the sake of consistency, we'd like to maintain how Would you be able to resubmit this PR with only the manpages change? Thank you! |
When the standard SOURCE_DATE_EPOCH environment variable is set, this patch causes the default certificate name and manpages to become deterministic, helping the reproducibility of Puppet builds. This patch has been in use in the Debian packaging of Puppet for some time, without issue.
jcharaoui
force-pushed
the
reproducible
branch
from
4486050
to
7e3ff62
Compare
|
@mhashizume Done! |
|
No problem! |
When the standard SOURCE_DATE_EPOCH environment variable is set, this patch causes the default certificate name and manpages to become deterministic, helping the reproducibility of Puppet builds.
This patch has been in use in the Debian packaging of Puppet for some time, without issue.