fix: validate a user-supplied CA for the transport layer of Elasticsearch#8953
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Pull request overview
This PR adds validation for user-supplied CA certificates in the Elasticsearch transport layer to ensure certificates are valid before use.
Key changes:
- Introduces a
ValidateCustomCAfunction that checks certificate validity periods and key pair matching - Integrates validation into the CA reconciliation flow with user-facing error events
- Adds comprehensive test coverage for different validation failure scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/controller/elasticsearch/certificates/transport/ca.go | Adds validation call for custom CA certificates with error event reporting |
| pkg/controller/common/certificates/ca_secret.go | Implements ValidateCustomCA function with time-bound and key-matching checks |
| pkg/controller/common/certificates/ca_secret_test.go | Adds test cases for valid, expired, not-yet-valid, and key-mismatched CAs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c4b67c4 to
9c31a0c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
buildkite test this -f p=gke,E2E_TAGS=es,t=TestCustom* |
pebrc
left a comment
There was a problem hiding this comment.
LGTM. But it creates a behavioural gap between the transport and HTTP layer. We do not have a similar check for custom HTTP certificates. Example:
apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
name: quickstart
spec:
version: 9.2.0
http:
tls:
certificate:
secretName: custom-cert
Could you create a follow-up issue (and/or PR)?
3.3. has been branched off so you can merge this anytime now but it won't be included in the release. We can discuss whether we consider this bug significant enough to warrant a back port to the release branch. I am not sure it is. |
I totally agree with your assessment, I would even debate if this is a bug to begin with 🙂 Let's discuss this with the team |
What does this PR do?
This pull request ensures that when a user supplies a custom CA for the Elasticsearch transport layer, the operator validates the CA’s expiration and prevents the system from endlessly attempting to renew certificates with an expired CA. Instead, it emits a clear error or event when an expired CA is detected.
Why is it important?
Currently, if a user-supplied CA has expired, the ECK operator continually attempts (and fails) to renew certificates, resulting in silent failures and unclear operator behavior. This validation allows users to receive explicit feedback and error events when their CA is no longer valid, improving transparency and usability.
Implementation details
Related issues