(PUP-3534) Deprecate caching certificate authority verify methods #5484
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ping @joshcooper for review as you raised this ticket originally |
|
CLA signed by all contributors. |
|
Everything here looks good to me. It would be good to note what version of PE (and corresponding PUP version) transitioned away from these code paths for historical reference. |
added 3 commits
January 13, 2017 09:37
This commit deprecates Puppet::SSL::CertificateAuthority#list_certificates, as it is no longer used by anything. The PE License check code that originally used this been migrated to querying PuppetDB, and was removed from PE puppet in the PE 3.1 series. To avoid superflous warnings emanating from use of Puppet::SSL::CertificateAuthority#list, that method is updated to directly do an indirector search instead of indirectly through #list_certificates. This is what it originally did, before #list_certificates was added, and is backwards-compatible.
…cate Puppet::SSL::CertificateAuthority#x509_store is a caching layer for the x509 object that was used by the PE License code. This commit tags it as deprecated, as the PE License code that used it was removed in the PE 3.1 series, and it was questionably advisable back then to return a potentially stale x509 store anyway. We also update #verify to call #create_x509_store instead of #x509_store, skipping the cache layer and always creating a new object - the behavior before x509_store was introduced. This is semantically API backwards-compatible, and now we can remove #x509_store at a major version, as its only callers are also deprecated. Note #x509_store is API private, so we don't actually do a Puppet.deprecation_warning. The comment tag should be enough to warn off anyone trying to use it before it's removed.
…_alive? Puppet::SSL::CertificateAuthority#certificate_is_alive? was introduced to leverage the #x509_store cached x509 object for the PE License code. It is no longer used as the code that used it was removed in the PE 3.1 series, and
MosesMendoza
force-pushed
the
PUP-3534/master/deprecate_caching_certificate_authority_methods
branch
from
fa5a652
to
113a90d
Compare
|
@Iristyle good call - updated commit messages to note that the license code using these methods was removed in the PE 3.1 series. |
joshcooper
approved these changes
Jan 13, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR deprecates methods in
Puppet::SSL::CertificateAuthoritythat were used by the PE License library (but no longer are).#x509_store(and its callers) in particular is problematic because it can cache the x509_store object once and never refresh it, even if it is no longer valid.Commit outline: