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

feat: manage the secrets annotated to be managed by the controller #331

Merged
merged 5 commits into from
Dec 13, 2019

Conversation

mashail
Copy link
Contributor

@mashail mashail commented Dec 8, 2019

No description provided.

Copy link
Collaborator

@mkmik mkmik left a comment

Choose a reason for hiding this comment

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

Thanks! Would it be possible to also add a test?

cmd/controller/controller.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mkmik
Copy link
Collaborator

mkmik commented Dec 10, 2019

Integration tests are failing.
You can run them locally with:

make integrationtest

@@ -253,6 +257,7 @@ func (c *Controller) unseal(key string) error {
secret.Data = newSecret.Data
secret.Type = newSecret.Type
secret.ObjectMeta.Annotations = newSecret.ObjectMeta.Annotations
secret.ObjectMeta.OwnerReferences = newSecret.ObjectMeta.OwnerReferences
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkmik I am thinking to append the OwnerRefernces and as it's now being overwritten. What do you think?

}, Timeout, PollingInterval).Should(WithTransform(getData, Equal(expected)))
Eventually(func() (*v1.Secret, error) {
return c.Secrets(ns).Get(secretName, metav1.GetOptions{})
}, Timeout, PollingInterval).Should(WithTransform(getFirstOwnerName, Equal(ss.GetName())))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkmik can you please help this check is failing as there are no owners. I tried it manually and it's updating the existing secret owners with SealedSecret as one among theme

Co-Authored-By: Marko Mikulicic <mkm@bitnami.com>
Copy link
Collaborator

@mkmik mkmik left a comment

Choose a reason for hiding this comment

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

I'll take a look at the tests, in the meantime I noticed a minor typo

integration/controller_test.go Outdated Show resolved Hide resolved
Co-Authored-By: Marko Mikulicic <mkm@bitnami.com>
@mashail
Copy link
Contributor Author

mashail commented Dec 11, 2019

make integrationtest

@mashail mashail closed this Dec 11, 2019
@mashail mashail reopened this Dec 11, 2019
@mashail
Copy link
Contributor Author

mashail commented Dec 11, 2019

Integration tests are failing.
You can run them locally with:

make integrationtest

Yes I did I told it's failing I don't what's the issue with my assertion. But let me check try second time. And I will get back to you. if I needed a help

@mashail mashail requested a review from mkmik December 13, 2019 21:03
Copy link
Collaborator

@mkmik mkmik left a comment

Choose a reason for hiding this comment

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

Thanks!

@mkmik
Copy link
Collaborator

mkmik commented Dec 13, 2019

bors r+

bors bot added a commit that referenced this pull request Dec 13, 2019
331: feat: manage the secrets annotated to be managed by the controller r=mkmik a=malmuzaini



Co-authored-by: Mashail Almuzaini <mashail_d@ymail.com>
@bors
Copy link
Contributor

bors bot commented Dec 13, 2019

Build succeeded

@bors bors bot merged commit e27c7d7 into bitnami-labs:master Dec 13, 2019
@mkmik mkmik added this to the v0.9.7 milestone Jan 29, 2020
@juan131 juan131 mentioned this pull request Feb 3, 2022
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.

None yet

2 participants