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

Make sure secret type does not get dropped after sealing and unsealing #88

Merged
merged 2 commits into from
Apr 27, 2018

Conversation

linoecarrillo
Copy link

@linoecarrillo linoecarrillo commented Apr 19, 2018

This addresses #86 and #87, where the secret type is not persisted throughout the sealing/unsealing process. Secret type used to be persisted before v0.6.0 when the whole secret was being sealed, but is now ignored and lost since only secret.Data key-value pairs are being sealed.

Fixes #86
Fixes #87

@linoecarrillo
Copy link
Author

Obviously this still needs some integration tests.

@anguslees anguslees added the bug label Apr 26, 2018
Copy link
Contributor

@anguslees anguslees left a comment

Choose a reason for hiding this comment

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

Tiny and probably zero-impact suggestion, otherwise lgtm!

@@ -36,6 +37,8 @@ type SealedSecret struct {
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec SealedSecretSpec `json:"spec"`

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a // +optional comment telling the autogen machinery that this is optional, just like: https://github.com/kubernetes/api/blob/99b35971cc9d07896d4428dc09ad3efacef8b21f/core/v1/types.go#L4613

Not so useful now, but will be if/when we bring in schema auto-generation.
@anguslees anguslees merged commit ac3d381 into bitnami-labs:master Apr 27, 2018
@anguslees
Copy link
Contributor

(Thanks for the fix, @linoplt !)

@mgk
Copy link

mgk commented Jun 6, 2018

Thanks for the fix! It is working well for us with secrets of type docker-registry and tls.

We've been pointing at latest for now, but would like to peg to a version number. Is a release on the way?

@anguslees
Copy link
Contributor

Is a release on the way?

I want to get #92 merged (in some form) first, since that moves the Type field added in this PR to a more future-proof schema. .. and better to collapse both those changes in a single release cycle, rather than break CRD compatibility in the middle.

Otherwise yes, consider me nagged to hurry up and finish #92 :P

icelynjennings added a commit to pusher/sealed-secrets that referenced this pull request Sep 20, 2018
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