Skip to content
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

Allow compilers and replicas to merge csr_attributes. #238

Merged
merged 2 commits into from
Mar 4, 2022
Merged

Allow compilers and replicas to merge csr_attributes. #238

merged 2 commits into from
Mar 4, 2022

Conversation

bwilcox
Copy link
Contributor

@bwilcox bwilcox commented Mar 3, 2022

This update allows compilers and replica servers being provisioned to respect the contents of an existing csr_attributes.yaml file.

@bwilcox bwilcox requested a review from a team as a code owner March 3, 2022 21:04
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Bill Wilcox seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@reidmv reidmv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this. One change request, but overall the logic and problem solving looks good 👍

@@ -324,23 +324,28 @@
]

$role_and_group =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change request:

If we're switching to csr_attributes.yaml pre-population for all servers, let's strip it out of this block entirely (remove $role_and_group and anything that operates on it), and instead add the needed peadm::util::insert_csr_extension_requests task run to $csr_yaml_jobs on line 215.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commit with the requested change.

@bwilcox bwilcox requested a review from reidmv March 3, 2022 22:35
Copy link
Contributor

@reidmv reidmv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! We'll get this merged once all the tests finish.

@reidmv reidmv added the feature label Mar 3, 2022
@reidmv reidmv merged commit ee611d7 into puppetlabs:main Mar 4, 2022
@bwilcox bwilcox deleted the csr_fix branch April 6, 2022 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants