-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Stop storing stateless kubernetes keystores #21880
Conversation
Pinging @elastic/integrations-platforms (Team:Platforms) |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
d1bfa21
to
e5abbda
Compare
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
storedKeystore, _ := kr.kubernetesKeystores.LoadOrStore(namespace, k8sKeystore) | ||
return storedKeystore.(keystore.Keystore) |
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.
This way we are instantiating a new keystore on every call, this is why I mentioned that we could need to store an object that creates the keystore lazily.
But, I have been looking at the implementation of the k8s keystore and it actually doesn't keep any state at all, so maybe we can return a new one each time and don't keep a registry?
storedKeystore, _ := kr.kubernetesKeystores.LoadOrStore(namespace, k8sKeystore) | |
return storedKeystore.(keystore.Keystore) | |
return k8sKeystore |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Signed-off-by: chrismark <chrismarkou92@gmail.com>
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, only a suggestion about a comment.
@@ -76,11 +71,7 @@ func (kr *KubernetesKeystoresRegistry) GetKeystore(event bus.Event) keystore.Key | |||
} | |||
if namespace != "" { | |||
// either retrieve already stored keystore or create a new one for the namespace |
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.
This comment is not needed anymore.
// either retrieve already stored keystore or create a new one for the namespace |
Signed-off-by: chrismark <chrismarkou92@gmail.com>
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
(cherry picked from commit b2d1929)
(cherry picked from commit b2d1929)
(cherry picked from commit b2d1929)
* upstream/master: feat: package aliases for snapshots (elastic#21960) [DOC] Add firewall as possible troubleshooting issue (elastic#21743) [Filebeat] Add max_number_of_messages config parameter for S3 input (elastic#21993) [Elastic Agent] Fix missing elastic_agent event data (elastic#21994) Document auditbeat system process module config (elastic#21766) Update links (elastic#22012) dynamically find librpm (elastic#21936) Fix Istio docs (elastic#22019) [beats-tester][packaging] store packages in another location (elastic#21903) [Kubernetes] Remove redundant dockersock volume mount (elastic#22009) [Ingest Manager] Always try snapshot repo for agent upgrade (elastic#21951) Azure storage metricset values not inside the metricset name (elastic#21845) fix diskio and memory bugs under windows (elastic#21992) Fix TestDockerStart flaky test (elastic#21681) filebeat: add SSL options to checkpoint module (elastic#19560) Stop storing stateless kubernetes keystores (elastic#21880) [Elastic Agent] Fix named pipe communication on Windows 7 (elastic#21931) [Elastic Agent] Fix index for Agent monitoring to to elastic_agent. (elastic#21932)
What does this PR do?
This PR removes
kubernetesKeystores
since it does not store anything that actually needs to be shared.Related issues