(#23074) Fix cert inventory race condition#2032
Closed
hlindberg wants to merge 7 commits intopuppetlabs:masterfrom
Closed
(#23074) Fix cert inventory race condition#2032hlindberg wants to merge 7 commits intopuppetlabs:masterfrom
hlindberg wants to merge 7 commits intopuppetlabs:masterfrom
Conversation
This adds a reinventory command to the puppet cert application. Further work on this is to refactor the use of the #apply method found in Puppet::SSL::CertificateAuthority as it delegates to a subclass called Interface to do the work. This is all an application concern and it is confusing with the back and forth between the classes "the cert application", the CA and its Interface.
This makes a couple of the methods in the CertificateAuthority and related Interface classes easier to read / understand.
The regenerate inventory wrote a header that was not there in the original file.
There is a race condition that requires protection if the inventory performs rebuild of a missing inventory.txt file when adding a cert. Without the protection the inventory may end up with double entries or missing entries, and with the protection the processing of cert requests become serialized over all processors creating a performance problem. This change simply appends to the inventory.txt - if the file does not exists, it is atomically created. A users that wants to recreate the inventory should do so with the command `puppet cert reinventory` which may be executed on an inactive system.
The code was difficult to understand as the cert application called the apply function on the CertAuthority, which in turned created an instance of the Interface class, which in turn called back into the ca. As this construct was only used by the cert application, the ca's apply method was moved to the cert application. It is now more clear that the application interacts with the ca via an instance of its Interface class.
There were several tests attempting to test the cert application, the CertificateAuthority and its interface, as well as the inventory handling by use of excessive stubbing that prevents any real code from being tested. The failing overly mocked tests have been removed. Their value was questionable to begin with.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 fixes a race condition in cert inventory rebuild by making reinventory a new subcommand of the
puppet cert application. Appends to the inventory.txt are now atomic writes in append mode.
This also refactors the interaction betwen the cert application, CertificateAuthority and its Interface class.