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

fix: update cert secret if exists #401

Merged
merged 6 commits into from
Dec 14, 2022
Merged

fix: update cert secret if exists #401

merged 6 commits into from
Dec 14, 2022

Conversation

busser
Copy link
Contributor

@busser busser commented Nov 15, 2022

Fixes #378.

@hashicorp-cla
Copy link

hashicorp-cla commented Nov 15, 2022

CLA assistant check
All committers have signed the CLA.

@busser
Copy link
Contributor Author

busser commented Nov 17, 2022

@tvoran no rush, but if you have time to take a look that would be great 🙂

Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

Thanks! This looks pretty good, just a thought about using the informer cache for the get.

helper/cert/source_gen.go Outdated Show resolved Hide resolved
@busser busser requested a review from tvoran December 7, 2022 10:10
@tvoran
Copy link
Member

tvoran commented Dec 8, 2022

Looks like we'll also need to update the tests, since make test crashes in TestGenSource_leader(). I bet the secret informer will need to be setup in that test, similar to TestGenSource_follower().

It would also be great to add another test that starts with an empty secret with labels and annotations, runs the cert generation, then checks that the updated secret's labels and annotations are intact. Maybe another variant of TestGenSource_leader()?

@busser
Copy link
Contributor Author

busser commented Dec 12, 2022

TestGenSource_leader() now fixed. Like you said, it just needed the informer to be setup.

I added a TestGenSource_leader_expiry(), which combines logic from TestGenSource_leader() and TestGenSource_expiry() to confirm that the secret retains its metadata when it is updated with a new bundle.

Copy link
Member

@tvoran tvoran 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 adding the tests!

@tvoran tvoran merged commit 180f065 into hashicorp:main Dec 14, 2022
@kenske
Copy link

kenske commented Jan 4, 2023

Great work, thanks! Any idea when this will get released?

@tvoran
Copy link
Member

tvoran commented Jan 4, 2023

@kenske Sometime later this month is the best estimate I can give right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vault-injector-certs Secret metadata is regularly overwritten, breaking any gitops flow
4 participants