-
Notifications
You must be signed in to change notification settings - Fork 731
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
Store transport keys and certificates in a single shared secret. #1198
Store transport keys and certificates in a single shared secret. #1198
Conversation
26d16ca
to
0b207ff
Compare
0b207ff
to
505f7b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I left a few minor comments.
Would be nice to unit test the certs pruning logic (can be in another PR?).
operators/pkg/controller/elasticsearch/certificates/transport/pod_secret.go
Outdated
Show resolved
Hide resolved
operators/pkg/controller/elasticsearch/certificates/transport/reconcile.go
Outdated
Show resolved
Hide resolved
operators/pkg/controller/elasticsearch/certificates/transport/transport_fixtures_test.go
Show resolved
Hide resolved
operators/pkg/controller/elasticsearch/initcontainer/prepare_fs_script.go
Outdated
Show resolved
Hide resolved
operators/pkg/controller/elasticsearch/initcontainer/prepare_fs_script.go
Outdated
Show resolved
Hide resolved
operators/pkg/controller/elasticsearch/certificates/transport/pod_secret_test.go
Outdated
Show resolved
Hide resolved
} | ||
} | ||
if len(keysToPrune) > 0 { | ||
log.Info("Pruning keys from certificates secret", "keys", keysToPrune) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have this unit tested.
Re the unit test for pruning, I wonder if it will be sufficiently different with StatefulSets it makes more sense to cover it there (note the method in question is also currently un-tested as it's going to be refactored come StatefulSet changes) |
I'm fine with keeping some unit tests for later (let's try not to forget them though) since we'll be breaking a lot of stuff. |
This facilitates a move to StatefulSets where the mounted secrets must be the same between all the Pods in the same StatefulSet
f97e775
to
3b24cd3
Compare
This facilitates a move to StatefulSets where the mounted secrets must be the same between all the Pods in the same StatefulSet.