From 5f1c59c2fc22595bca52216b64c05d19881ed3b1 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Fri, 15 May 2026 23:19:53 -0400 Subject: [PATCH] allow overriding the Secret data keys for the custom transport CA The transport CA secret parser hardcodes `ca.crt`/`ca.key` (with a legacy `tls.crt`/`tls.key` fallback) and rejects secrets that contain both sets of keys as a configuration error. cert-manager always emits `tls.crt`, `tls.key`, *and* `ca.crt` (the issuing CA bundle) in its output Secret, and does not allow renaming these keys (`secretTemplate` only affects metadata, `additionalOutputFormats` only adds extra keys). The "both exist" check thus makes it impossible to point ECK directly at a cert-manager `Certificate` that mints the transport CA. Add an optional `caSecretKeys` field on `TransportTLSOptions` with `caCertKey` and `caKeyKey` overrides. When either is set, the parser reads only the named keys and bypasses the legacy/modern auto-detection and the "both exist" conflict check. When unset, behavior is unchanged. The new `ParseCustomCASecretWithKeys` is invoked from `ReconcileOrRetrieveCA`; the original `ParseCustomCASecret` remains as a no-override wrapper so other callers stay untouched. --- config/crds/v1/all-crds.yaml | 17 ++++ ...search.k8s.elastic.co_elasticsearches.yaml | 17 ++++ .../eck-operator-crds/templates/all-crds.yaml | 17 ++++ .../elasticsearch/v1/elasticsearch_types.go | 16 ++++ .../elasticsearch/v1/zz_generated.deepcopy.go | 20 ++++ .../common/certificates/ca_secret.go | 20 ++++ .../common/certificates/ca_secret_test.go | 93 +++++++++++++++++++ .../certificates/transport/ca.go | 6 +- 8 files changed, 205 insertions(+), 1 deletion(-) diff --git a/config/crds/v1/all-crds.yaml b/config/crds/v1/all-crds.yaml index 1bc5f19f531..4d23010719e 100644 --- a/config/crds/v1/all-crds.yaml +++ b/config/crds/v1/all-crds.yaml @@ -6576,6 +6576,23 @@ spec: description: TLS defines options for configuring TLS on the transport layer. properties: + caSecretKeys: + description: |- + CASecretKeys optionally overrides which keys ECK reads from the Secret referenced by Certificate. + Useful when the Secret was produced by cert-manager, which emits tls.crt/tls.key alongside ca.crt. + When either field is set, the legacy tls.* vs modern ca.* auto-detection (and its "both exist" + conflict check) is skipped entirely. + properties: + caCertKey: + description: CACertKey is the Secret data key holding + the CA certificate (PEM). Defaults to "ca.crt" when + empty. + type: string + caKeyKey: + description: CAKeyKey is the Secret data key holding the + CA private key (PEM). Defaults to "ca.key" when empty. + type: string + type: object certificate: description: |- Certificate is a reference to a Kubernetes secret that contains the CA certificate diff --git a/config/crds/v1/resources/elasticsearch.k8s.elastic.co_elasticsearches.yaml b/config/crds/v1/resources/elasticsearch.k8s.elastic.co_elasticsearches.yaml index 49b5c1488da..963a55c7dff 100644 --- a/config/crds/v1/resources/elasticsearch.k8s.elastic.co_elasticsearches.yaml +++ b/config/crds/v1/resources/elasticsearch.k8s.elastic.co_elasticsearches.yaml @@ -10845,6 +10845,23 @@ spec: description: TLS defines options for configuring TLS on the transport layer. properties: + caSecretKeys: + description: |- + CASecretKeys optionally overrides which keys ECK reads from the Secret referenced by Certificate. + Useful when the Secret was produced by cert-manager, which emits tls.crt/tls.key alongside ca.crt. + When either field is set, the legacy tls.* vs modern ca.* auto-detection (and its "both exist" + conflict check) is skipped entirely. + properties: + caCertKey: + description: CACertKey is the Secret data key holding + the CA certificate (PEM). Defaults to "ca.crt" when + empty. + type: string + caKeyKey: + description: CAKeyKey is the Secret data key holding the + CA private key (PEM). Defaults to "ca.key" when empty. + type: string + type: object certificate: description: |- Certificate is a reference to a Kubernetes secret that contains the CA certificate diff --git a/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml b/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml index af65fb0aa75..744d3ef988c 100644 --- a/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml +++ b/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml @@ -6625,6 +6625,23 @@ spec: description: TLS defines options for configuring TLS on the transport layer. properties: + caSecretKeys: + description: |- + CASecretKeys optionally overrides which keys ECK reads from the Secret referenced by Certificate. + Useful when the Secret was produced by cert-manager, which emits tls.crt/tls.key alongside ca.crt. + When either field is set, the legacy tls.* vs modern ca.* auto-detection (and its "both exist" + conflict check) is skipped entirely. + properties: + caCertKey: + description: CACertKey is the Secret data key holding + the CA certificate (PEM). Defaults to "ca.crt" when + empty. + type: string + caKeyKey: + description: CAKeyKey is the Secret data key holding the + CA private key (PEM). Defaults to "ca.key" when empty. + type: string + type: object certificate: description: |- Certificate is a reference to a Kubernetes secret that contains the CA certificate diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_types.go b/pkg/apis/elasticsearch/v1/elasticsearch_types.go index 7b977458087..1cbdaf0c0d8 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_types.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_types.go @@ -235,6 +235,12 @@ type TransportTLSOptions struct { // - `ca.crt`: The CA certificate in PEM format. // - `ca.key`: The private key for the CA certificate in PEM format. Certificate commonv1.SecretRef `json:"certificate,omitempty"` + // CASecretKeys optionally overrides which keys ECK reads from the Secret referenced by Certificate. + // Useful when the Secret was produced by cert-manager, which emits tls.crt/tls.key alongside ca.crt. + // When either field is set, the legacy tls.* vs modern ca.* auto-detection (and its "both exist" + // conflict check) is skipped entirely. + // +kubebuilder:validation:Optional + CASecretKeys *CASecretKeys `json:"caSecretKeys,omitempty"` // CertificateAuthorities is a reference to a config map that contains one or more x509 certificates for // trusted authorities in PEM format. The certificates need to be in a file called `ca.crt`. CertificateAuthorities commonv1.ConfigMapRef `json:"certificateAuthorities,omitempty"` @@ -242,6 +248,16 @@ type TransportTLSOptions struct { SelfSignedCertificates *SelfSignedTransportCertificates `json:"selfSignedCertificates,omitempty"` } +// CASecretKeys overrides the Secret data keys ECK reads when consuming a user-provided transport CA. +type CASecretKeys struct { + // CACertKey is the Secret data key holding the CA certificate (PEM). Defaults to "ca.crt" when empty. + // +kubebuilder:validation:Optional + CACertKey string `json:"caCertKey,omitempty"` + // CAKeyKey is the Secret data key holding the CA private key (PEM). Defaults to "ca.key" when empty. + // +kubebuilder:validation:Optional + CAKeyKey string `json:"caKeyKey,omitempty"` +} + func (tto TransportTLSOptions) SelfSignedEnabled() bool { return tto.SelfSignedCertificates == nil || !tto.SelfSignedCertificates.Disabled } diff --git a/pkg/apis/elasticsearch/v1/zz_generated.deepcopy.go b/pkg/apis/elasticsearch/v1/zz_generated.deepcopy.go index b95693ab815..6bdeeeec6b1 100644 --- a/pkg/apis/elasticsearch/v1/zz_generated.deepcopy.go +++ b/pkg/apis/elasticsearch/v1/zz_generated.deepcopy.go @@ -71,6 +71,21 @@ func (in AutoscaledNodeSets) DeepCopy() AutoscaledNodeSets { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CASecretKeys) DeepCopyInto(out *CASecretKeys) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CASecretKeys. +func (in *CASecretKeys) DeepCopy() *CASecretKeys { + if in == nil { + return nil + } + out := new(CASecretKeys) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ChangeBudget) DeepCopyInto(out *ChangeBudget) { *out = *in @@ -735,6 +750,11 @@ func (in *TransportTLSOptions) DeepCopyInto(out *TransportTLSOptions) { copy(*out, *in) } out.Certificate = in.Certificate + if in.CASecretKeys != nil { + in, out := &in.CASecretKeys, &out.CASecretKeys + *out = new(CASecretKeys) + **out = **in + } out.CertificateAuthorities = in.CertificateAuthorities if in.SelfSignedCertificates != nil { in, out := &in.SelfSignedCertificates, &out.SelfSignedCertificates diff --git a/pkg/controller/common/certificates/ca_secret.go b/pkg/controller/common/certificates/ca_secret.go index 08d025cdb1a..b23c45062ce 100644 --- a/pkg/controller/common/certificates/ca_secret.go +++ b/pkg/controller/common/certificates/ca_secret.go @@ -19,6 +19,26 @@ import ( // It does not check that the public key matches the private key. // Legacy tls.* keys are still supported while the expected default keys are ca.crt and ca.key. func ParseCustomCASecret(s corev1.Secret) (*CA, error) { + return ParseCustomCASecretWithKeys(s, "", "") +} + +// ParseCustomCASecretWithKeys behaves like ParseCustomCASecret but lets the caller specify which +// Secret data keys hold the CA certificate and private key. When both overrides are empty, the +// legacy tls.* vs modern ca.* auto-detection runs unchanged. When either override is non-empty, +// only the named keys are read and the "both exist" conflict check is bypassed — required for +// consuming Secrets produced by cert-manager (which always emits tls.crt, tls.key, and ca.crt). +func ParseCustomCASecretWithKeys(s corev1.Secret, certKeyOverride, keyKeyOverride string) (*CA, error) { + if certKeyOverride != "" || keyKeyOverride != "" { + crtFileName := certKeyOverride + if crtFileName == "" { + crtFileName = CAFileName + } + keyFileName := keyKeyOverride + if keyFileName == "" { + keyFileName = CAKeyFileName + } + return parseCAFromSecret(s, keyFileName, crtFileName) + } keyFileName := CAKeyFileName crtFileName := CAFileName // For backwards compatibility we support both tls.* and the newer ca.* keys in the secret diff --git a/pkg/controller/common/certificates/ca_secret_test.go b/pkg/controller/common/certificates/ca_secret_test.go index 06a53179a35..30bb94f9097 100644 --- a/pkg/controller/common/certificates/ca_secret_test.go +++ b/pkg/controller/common/certificates/ca_secret_test.go @@ -137,6 +137,99 @@ func TestParseCustomCASecret(t *testing.T) { } } +func TestParseCustomCASecretWithKeys(t *testing.T) { + ca := loadFileBytes("ca.crt") + key := loadFileBytes("tls.key") + + tests := []struct { + name string + s corev1.Secret + certKeyOverride string + keyKeyOverride string + wantErr bool + }{ + { + name: "Empty overrides → identical to ParseCustomCASecret (ca.* path)", + s: corev1.Secret{ + Data: map[string][]byte{ + "ca.crt": ca, + "ca.key": key, + }, + }, + wantErr: false, + }, + { + name: "Both overrides set → reads tls.* even when ca.crt also present (cert-manager shape)", + s: corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": ca, + "tls.key": key, + "ca.crt": ca, // cert-manager emits this as the issuing CA bundle + }, + }, + certKeyOverride: "tls.crt", + keyKeyOverride: "tls.key", + wantErr: false, + }, + { + name: "Only cert override set → key falls back to ca.key default", + s: corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": ca, + "ca.key": key, + }, + }, + certKeyOverride: "tls.crt", + wantErr: false, + }, + { + name: "Only key override set → cert falls back to ca.crt default", + s: corev1.Secret{ + Data: map[string][]byte{ + "ca.crt": ca, + "tls.key": key, + }, + }, + keyKeyOverride: "tls.key", + wantErr: false, + }, + { + name: "Override pointing at missing key returns error", + s: corev1.Secret{ + Data: map[string][]byte{ + "ca.crt": ca, + "ca.key": key, + }, + }, + certKeyOverride: "does-not-exist.crt", + keyKeyOverride: "does-not-exist.key", + wantErr: true, + }, + { + name: "Override skips the both-exist conflict check", + s: corev1.Secret{ + Data: map[string][]byte{ + "tls.crt": ca, + "tls.key": key, + "ca.crt": ca, + "ca.key": key, + }, + }, + certKeyOverride: "tls.crt", + keyKeyOverride: "tls.key", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := ParseCustomCASecretWithKeys(tt.s, tt.certKeyOverride, tt.keyKeyOverride) + if (err != nil) != tt.wantErr { + t.Errorf("ParseCustomCASecretWithKeys() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + func TestValidateCustomCA(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/elasticsearch/certificates/transport/ca.go b/pkg/controller/elasticsearch/certificates/transport/ca.go index 49b7973f342..4db187e15cf 100644 --- a/pkg/controller/elasticsearch/certificates/transport/ca.go +++ b/pkg/controller/elasticsearch/certificates/transport/ca.go @@ -76,7 +76,11 @@ func ReconcileOrRetrieveCA( // 2. Assuming from here on the user wants to use custom certs and has configured a secret with them. // Try to parse the provided secret to get to the CA and to report any validation errors to the user. - ca, err := certificates.ParseCustomCASecret(*customCASecret) + var certKeyOverride, keyKeyOverride string + if ks := es.Spec.Transport.TLS.CASecretKeys; ks != nil { + certKeyOverride, keyKeyOverride = ks.CACertKey, ks.CAKeyKey + } + ca, err := certificates.ParseCustomCASecretWithKeys(*customCASecret, certKeyOverride, keyKeyOverride) if err != nil { // Surface validation/parsing errors to the user via an event otherwise they might be hard to spot // validation at admission would also be an alternative but seems quite costly and secret contents might change