From 1d3c81e22bfef47b9edf3fe9e000e13346443ba0 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Tue, 20 Oct 2026 11:10:53 +0000 Subject: [PATCH 1/5] feat(api): add spec.extraArgs for additional etcd flags EtcdCluster gains spec.extraArgs, a map of etcd flag names (without the leading dashes) to values, rendered as --key=value on each member's command line. It is latched through status.observed and mirrored onto EtcdMember like spec.options. CEL on the field rejects keys that do not look like flag names, every flag the operator manages (identity, data placement, listeners and URLs, bootstrap and discovery, --config-file, TLS) and the flags typed by spec.options, naming the offending flag in the message. spec.options was kept closed because a free-form map let users override the operator's own flags. The denylist closes that hole, and a typed set cannot cover flags of an image that wraps etcd, or etcd flags no field exists for yet. The EtcdOptions comment now says so. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- api/v1alpha2/etcdcluster_types.go | 57 ++++++++++++-- api/v1alpha2/etcdmember_types.go | 8 ++ api/v1alpha2/zz_generated.deepcopy.go | 21 +++++ ...cd-operator.cozystack.io_etcdclusters.yaml | 78 ++++++++++++++++++- ...tcd-operator.cozystack.io_etcdmembers.yaml | 10 +++ 5 files changed, 165 insertions(+), 9 deletions(-) diff --git a/api/v1alpha2/etcdcluster_types.go b/api/v1alpha2/etcdcluster_types.go index 8bdd5253..a57d32f2 100644 --- a/api/v1alpha2/etcdcluster_types.go +++ b/api/v1alpha2/etcdcluster_types.go @@ -253,11 +253,16 @@ const ( ) // EtcdOptions carries the etcd server tuning flags the operator passes to -// each member's `etcd` command line. Deliberately a closed, typed set — the -// legacy aenix operator exposed a free-form `spec.options` map[string]string, -// which let users inject arbitrary (and operator-conflicting) flags; this -// port types exactly the keys Cozystack's etcd package actually used. New -// flags land here as new typed fields, not as an escape hatch. +// each member's `etcd` command line, typed and validated. The legacy aenix +// operator exposed a free-form `spec.options` map[string]string, which let +// users inject flags that conflict with the ones the operator itself manages +// (listen URLs, initial-cluster wiring, TLS paths); this port types exactly +// the keys Cozystack's etcd package actually used. A tuning knob worth +// validating lands here as a new typed field. +// +// Flags outside this set go through spec.extraArgs, which rejects every flag +// the operator manages — including the ones typed here, so each flag has one +// source. See EtcdClusterSpec.ExtraArgs. // // Like spec.resources, options are latched through status.observed and take // effect on newly-created members (scale-up, replacement) only; the operator @@ -550,8 +555,7 @@ type EtcdClusterSpec struct { // Options carries etcd server tuning flags (backend quota, // auto-compaction, raft snapshot count) passed to each member's - // command line. A closed typed set — see EtcdOptions for why there - // is deliberately no free-form flag map. + // command line, typed and validated. See EtcdOptions. // // Updates take effect on newly-created members (scale-up, // replacement); the operator does not roll existing Pods to apply a @@ -559,6 +563,38 @@ type EtcdClusterSpec struct { // +optional Options *EtcdOptions `json:"options,omitempty"` + // ExtraArgs passes additional flags to each member's etcd command line. + // Keys are flag names without the leading dashes; each entry renders as + // --key=value, with the value passed as is. Entries are appended after + // the operator's own flags, sorted by key so the rendered Pod is + // deterministic. + // + // The operator owns a set of flags and rejects them here: member + // identity and data placement (--name, --data-dir), the listen, + // advertise and initial-cluster wiring, the TLS material it mounts from + // spec.tls, alternative bootstrap mechanisms (--discovery*, + // --config-file), and the flags typed by spec.options. The apiserver + // rejects such a key on admission, naming the flag; the member + // controller refuses to render one as a second guard. That denylist is + // what made the original free-form map unsafe — it let a user break the + // operator's own wiring — so with it in place a free-form map is safe + // to offer. It is still needed next to spec.options: a typed set only + // grows with the operator's releases, while a cluster may run an image + // that wraps etcd and takes flags of its own, or need an etcd flag no + // field covers yet. Prefer a typed field where one exists; this map is + // not validated beyond the denylist, and a flag etcd does not know + // makes the member fail to start. + // + // Like spec.options, extraArgs are latched through status.observed and + // take effect on newly-created members (scale-up, replacement) only; + // the operator does not roll existing Pods to apply a change in place. + // +kubebuilder:validation:MaxProperties=64 + // +kubebuilder:validation:XValidation:rule="self.all(k, k.matches('^[a-z0-9]+(-[a-z0-9]+)*$'))",messageExpression="'spec.extraArgs keys are etcd flag names without the leading dashes (for example log-level); invalid key: ' + self.filter(k, !k.matches('^[a-z0-9]+(-[a-z0-9]+)*$'))[0]" + // +kubebuilder:validation:XValidation:rule="self.all(k, !(k in ['name', 'data-dir', 'wal-dir', 'listen-peer-urls', 'listen-client-urls', 'listen-client-http-urls', 'listen-metrics-urls', 'advertise-client-urls', 'initial-advertise-peer-urls', 'initial-cluster', 'initial-cluster-token', 'initial-cluster-state', 'force-new-cluster', 'discovery', 'discovery-srv', 'discovery-endpoints', 'config-file', 'auto-tls', 'cert-file', 'key-file', 'client-cert-file', 'client-key-file', 'client-cert-auth', 'trusted-ca-file', 'peer-auto-tls', 'peer-cert-file', 'peer-key-file', 'peer-client-cert-file', 'peer-client-key-file', 'peer-trusted-ca-file', 'peer-client-cert-auth']))",messageExpression="'spec.extraArgs: --' + self.filter(k, k in ['name', 'data-dir', 'wal-dir', 'listen-peer-urls', 'listen-client-urls', 'listen-client-http-urls', 'listen-metrics-urls', 'advertise-client-urls', 'initial-advertise-peer-urls', 'initial-cluster', 'initial-cluster-token', 'initial-cluster-state', 'force-new-cluster', 'discovery', 'discovery-srv', 'discovery-endpoints', 'config-file', 'auto-tls', 'cert-file', 'key-file', 'client-cert-file', 'client-key-file', 'client-cert-auth', 'trusted-ca-file', 'peer-auto-tls', 'peer-cert-file', 'peer-key-file', 'peer-client-cert-file', 'peer-client-key-file', 'peer-trusted-ca-file', 'peer-client-cert-auth'])[0] + ' is managed by the operator and cannot be set here'" + // +kubebuilder:validation:XValidation:rule="self.all(k, !(k in ['quota-backend-bytes', 'auto-compaction-mode', 'auto-compaction-retention', 'snapshot-count']))",messageExpression="'spec.extraArgs: --' + self.filter(k, k in ['quota-backend-bytes', 'auto-compaction-mode', 'auto-compaction-retention', 'snapshot-count'])[0] + ' is a typed option; set spec.options.' + {'quota-backend-bytes': 'quotaBackendBytes', 'auto-compaction-mode': 'autoCompactionMode', 'auto-compaction-retention': 'autoCompactionRetention', 'snapshot-count': 'snapshotCount'}[self.filter(k, k in ['quota-backend-bytes', 'auto-compaction-mode', 'auto-compaction-retention', 'snapshot-count'])[0]] + ' instead'" + // +optional + ExtraArgs map[string]string `json:"extraArgs,omitempty"` + // ImagePullSecrets is a list of Secret references in the cluster's // namespace used to pull the etcd (and restore initContainer) image from // a private registry — e.g. an air-gapped mirror behind credentials. @@ -640,6 +676,13 @@ type ObservedClusterSpec struct { // Pods. Latched with the rest of the target spec. // +optional ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` + + // ExtraArgs is the locked target set of additional etcd flags for + // member Pods. Latched with the rest of the target spec so a mid-flight + // edit only applies to members created once the current target is + // reached. + // +optional + ExtraArgs map[string]string `json:"extraArgs,omitempty"` } // EtcdClusterStatus defines the observed state of an etcd cluster. diff --git a/api/v1alpha2/etcdmember_types.go b/api/v1alpha2/etcdmember_types.go index 46599fd4..873a2847 100644 --- a/api/v1alpha2/etcdmember_types.go +++ b/api/v1alpha2/etcdmember_types.go @@ -122,6 +122,14 @@ type EtcdMemberSpec struct { // +optional Options *EtcdOptions `json:"options,omitempty"` + // ExtraArgs mirrors EtcdCluster.spec.extraArgs at the time this member + // was created. The member controller renders each entry as --key=value + // after its own flags, sorted by key, and refuses to create the Pod if + // a key collides with a flag it manages. Existing members are not + // re-templated when the cluster spec changes. + // +optional + ExtraArgs map[string]string `json:"extraArgs,omitempty"` + // ImagePullSecrets mirrors EtcdCluster.spec.imagePullSecrets at the time // this member was created. Passed straight to the Pod's // spec.imagePullSecrets at build time. diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index 7f59359e..4bf4f3b6 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -297,6 +297,13 @@ func (in *EtcdClusterSpec) DeepCopyInto(out *EtcdClusterSpec) { *out = new(EtcdOptions) (*in).DeepCopyInto(*out) } + if in.ExtraArgs != nil { + in, out := &in.ExtraArgs, &out.ExtraArgs + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.ImagePullSecrets != nil { in, out := &in.ImagePullSecrets, &out.ImagePullSecrets *out = make([]v1.LocalObjectReference, len(*in)) @@ -714,6 +721,13 @@ func (in *EtcdMemberSpec) DeepCopyInto(out *EtcdMemberSpec) { *out = new(EtcdOptions) (*in).DeepCopyInto(*out) } + if in.ExtraArgs != nil { + in, out := &in.ExtraArgs, &out.ExtraArgs + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.ImagePullSecrets != nil { in, out := &in.ImagePullSecrets, &out.ImagePullSecrets *out = make([]v1.LocalObjectReference, len(*in)) @@ -982,6 +996,13 @@ func (in *ObservedClusterSpec) DeepCopyInto(out *ObservedClusterSpec) { *out = make([]v1.LocalObjectReference, len(*in)) copy(*out, *in) } + if in.ExtraArgs != nil { + in, out := &in.ExtraArgs, &out.ExtraArgs + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObservedClusterSpec. diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml index ed4d78ad..727920c5 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml +++ b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml @@ -1166,6 +1166,72 @@ spec: rule: '!has(self.source.pvc) || (has(self.source.pvc.subPath) && size(self.source.pvc.subPath) > 0)' type: object + extraArgs: + additionalProperties: + type: string + description: |- + ExtraArgs passes additional flags to each member's etcd command line. + Keys are flag names without the leading dashes; each entry renders as + --key=value, with the value passed as is. Entries are appended after + the operator's own flags, sorted by key so the rendered Pod is + deterministic. + + The operator owns a set of flags and rejects them here: member + identity and data placement (--name, --data-dir), the listen, + advertise and initial-cluster wiring, the TLS material it mounts from + spec.tls, alternative bootstrap mechanisms (--discovery*, + --config-file), and the flags typed by spec.options. The apiserver + rejects such a key on admission, naming the flag; the member + controller refuses to render one as a second guard. That denylist is + what made the original free-form map unsafe — it let a user break the + operator's own wiring — so with it in place a free-form map is safe + to offer. It is still needed next to spec.options: a typed set only + grows with the operator's releases, while a cluster may run an image + that wraps etcd and takes flags of its own, or need an etcd flag no + field covers yet. Prefer a typed field where one exists; this map is + not validated beyond the denylist, and a flag etcd does not know + makes the member fail to start. + + Like spec.options, extraArgs are latched through status.observed and + take effect on newly-created members (scale-up, replacement) only; + the operator does not roll existing Pods to apply a change in place. + maxProperties: 64 + type: object + x-kubernetes-validations: + - messageExpression: '''spec.extraArgs keys are etcd flag names without + the leading dashes (for example log-level); invalid key: '' + + self.filter(k, !k.matches(''^[a-z0-9]+(-[a-z0-9]+)*$''))[0]' + rule: self.all(k, k.matches('^[a-z0-9]+(-[a-z0-9]+)*$')) + - messageExpression: '''spec.extraArgs: --'' + self.filter(k, k in + [''name'', ''data-dir'', ''wal-dir'', ''listen-peer-urls'', ''listen-client-urls'', + ''listen-client-http-urls'', ''listen-metrics-urls'', ''advertise-client-urls'', + ''initial-advertise-peer-urls'', ''initial-cluster'', ''initial-cluster-token'', + ''initial-cluster-state'', ''force-new-cluster'', ''discovery'', + ''discovery-srv'', ''discovery-endpoints'', ''config-file'', ''auto-tls'', + ''cert-file'', ''key-file'', ''client-cert-file'', ''client-key-file'', + ''client-cert-auth'', ''trusted-ca-file'', ''peer-auto-tls'', + ''peer-cert-file'', ''peer-key-file'', ''peer-client-cert-file'', + ''peer-client-key-file'', ''peer-trusted-ca-file'', ''peer-client-cert-auth''])[0] + + '' is managed by the operator and cannot be set here''' + rule: self.all(k, !(k in ['name', 'data-dir', 'wal-dir', 'listen-peer-urls', + 'listen-client-urls', 'listen-client-http-urls', 'listen-metrics-urls', + 'advertise-client-urls', 'initial-advertise-peer-urls', 'initial-cluster', + 'initial-cluster-token', 'initial-cluster-state', 'force-new-cluster', + 'discovery', 'discovery-srv', 'discovery-endpoints', 'config-file', + 'auto-tls', 'cert-file', 'key-file', 'client-cert-file', 'client-key-file', + 'client-cert-auth', 'trusted-ca-file', 'peer-auto-tls', 'peer-cert-file', + 'peer-key-file', 'peer-client-cert-file', 'peer-client-key-file', + 'peer-trusted-ca-file', 'peer-client-cert-auth'])) + - messageExpression: '''spec.extraArgs: --'' + self.filter(k, k in + [''quota-backend-bytes'', ''auto-compaction-mode'', ''auto-compaction-retention'', + ''snapshot-count''])[0] + '' is a typed option; set spec.options.'' + + {''quota-backend-bytes'': ''quotaBackendBytes'', ''auto-compaction-mode'': + ''autoCompactionMode'', ''auto-compaction-retention'': ''autoCompactionRetention'', + ''snapshot-count'': ''snapshotCount''}[self.filter(k, k in [''quota-backend-bytes'', + ''auto-compaction-mode'', ''auto-compaction-retention'', ''snapshot-count''])[0]] + + '' instead''' + rule: self.all(k, !(k in ['quota-backend-bytes', 'auto-compaction-mode', + 'auto-compaction-retention', 'snapshot-count'])) imagePullSecrets: description: |- ImagePullSecrets is a list of Secret references in the cluster's @@ -1197,8 +1263,7 @@ spec: description: |- Options carries etcd server tuning flags (backend quota, auto-compaction, raft snapshot count) passed to each member's - command line. A closed typed set — see EtcdOptions for why there - is deliberately no free-form flag map. + command line, typed and validated. See EtcdOptions. Updates take effect on newly-created members (scale-up, replacement); the operator does not roll existing Pods to apply a @@ -2883,6 +2948,15 @@ spec: x-kubernetes-list-type: atomic type: object type: object + extraArgs: + additionalProperties: + type: string + description: |- + ExtraArgs is the locked target set of additional etcd flags for + member Pods. Latched with the rest of the target spec so a mid-flight + edit only applies to members created once the current target is + reached. + type: object imagePullSecrets: description: |- ImagePullSecrets is the locked target pull-secret list for member diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml index fbad5192..2b2f3e3f 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml +++ b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml @@ -1023,6 +1023,16 @@ spec: the same ClusterID and member ID. While dormant, the member does not count toward the EtcdCluster's `current` replica accounting. type: boolean + extraArgs: + additionalProperties: + type: string + description: |- + ExtraArgs mirrors EtcdCluster.spec.extraArgs at the time this member + was created. The member controller renders each entry as --key=value + after its own flags, sorted by key, and refuses to create the Pod if + a key collides with a flag it manages. Existing members are not + re-templated when the cluster spec changes. + type: object imagePullSecrets: description: |- ImagePullSecrets mirrors EtcdCluster.spec.imagePullSecrets at the time From 150e6b293be760a7420991639081eba86bb64d84 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Tue, 20 Oct 2026 11:10:53 +0000 Subject: [PATCH 2/5] feat(controllers): render extraArgs behind an operator-flag denylist The member controller appends extraArgs after its own flags, sorted by key so the Pod does not change between reconciles. The denylist lives next to buildPod, and ensurePod refuses to create a Pod whose member carries a denied key, as a second guard behind admission. Tests check that every flag buildPod renders is on the denylist, that the CRD's CEL rule rejects every denylisted key (envtest), the render guard, ordering, and propagation through status.observed to new members. Assisted-by: LLM Signed-off-by: Andrei Kvapil --- controllers/etcdcluster_controller.go | 6 +- controllers/etcdcluster_controller_test.go | 39 ++-- controllers/etcdmember_controller.go | 93 ++++++++++ controllers/extra_args_envtest_test.go | 105 +++++++++++ controllers/extra_args_test.go | 202 +++++++++++++++++++++ internal/migrate/adopt.go | 2 + 6 files changed, 434 insertions(+), 13 deletions(-) create mode 100644 controllers/extra_args_envtest_test.go create mode 100644 controllers/extra_args_test.go diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index 49464ae3..a16e6034 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -450,6 +450,7 @@ func (r *EtcdClusterReconciler) bootstrap( Affinity: cluster.Status.Observed.Affinity, TopologySpreadConstraints: cluster.Status.Observed.TopologySpreadConstraints, Options: cluster.Status.Observed.Options, + ExtraArgs: cluster.Status.Observed.ExtraArgs, ImagePullSecrets: cluster.Status.Observed.ImagePullSecrets, Bootstrap: true, ClusterToken: cluster.Status.ClusterToken, @@ -855,6 +856,7 @@ func (r *EtcdClusterReconciler) scaleUp( Affinity: cluster.Status.Observed.Affinity, TopologySpreadConstraints: cluster.Status.Observed.TopologySpreadConstraints, Options: cluster.Status.Observed.Options, + ExtraArgs: cluster.Status.Observed.ExtraArgs, ImagePullSecrets: cluster.Status.Observed.ImagePullSecrets, Bootstrap: false, ClusterToken: cluster.Status.ClusterToken, @@ -2141,6 +2143,7 @@ func snapshotSpecIntoObserved(cluster *lll.EtcdCluster) { AdditionalMetadata: cluster.Spec.AdditionalMetadata, Options: cluster.Spec.Options, ImagePullSecrets: cluster.Spec.ImagePullSecrets, + ExtraArgs: cluster.Spec.ExtraArgs, } } @@ -2162,7 +2165,8 @@ func specEqualsObserved(cluster *lll.EtcdCluster) bool { equality.Semantic.DeepEqual(o.TopologySpreadConstraints, cluster.Spec.TopologySpreadConstraints) && equality.Semantic.DeepEqual(o.AdditionalMetadata, cluster.Spec.AdditionalMetadata) && equality.Semantic.DeepEqual(o.Options, cluster.Spec.Options) && - equality.Semantic.DeepEqual(o.ImagePullSecrets, cluster.Spec.ImagePullSecrets) + equality.Semantic.DeepEqual(o.ImagePullSecrets, cluster.Spec.ImagePullSecrets) && + equality.Semantic.DeepEqual(o.ExtraArgs, cluster.Spec.ExtraArgs) } // observedAdditionalMetadata returns the latched additionalMetadata target diff --git a/controllers/etcdcluster_controller_test.go b/controllers/etcdcluster_controller_test.go index 74ac7caa..43820d3a 100644 --- a/controllers/etcdcluster_controller_test.go +++ b/controllers/etcdcluster_controller_test.go @@ -3174,7 +3174,8 @@ func TestSpecEqualsObserved_OptionsMatter(t *testing.T) { } // TestBootstrap_PropagatesOptionsToSeed verifies the wiring of -// spec.options onto the seed EtcdMember at cluster creation. The member +// spec.options and spec.extraArgs onto the seed EtcdMember at cluster +// creation. The member // controller reads its own Spec.Options at buildPod time, so any // propagation gap shows up as the seed's etcd running with built-in // defaults instead of the user's tuning (e.g. no backend quota). @@ -3188,13 +3189,15 @@ func TestBootstrap_PropagatesOptionsToSeed(t *testing.T) { AutoCompactionRetention: "5m", SnapshotCount: &snapCount, } + extra := map[string]string{"log-level": "debug"} cluster := &lll.EtcdCluster{ ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, Spec: lll.EtcdClusterSpec{ - Replicas: ptrInt32(3), - Version: "3.5.17", - Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, - Options: opts, + Replicas: ptrInt32(3), + Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + Options: opts, + ExtraArgs: extra, }, } c, _ := newTestClient(t, cluster) @@ -3209,6 +3212,9 @@ func TestBootstrap_PropagatesOptionsToSeed(t *testing.T) { if !equality.Semantic.DeepEqual(got.Status.Observed.Options, opts) { t.Errorf("Observed.Options = %+v; want %+v", got.Status.Observed.Options, opts) } + if !equality.Semantic.DeepEqual(got.Status.Observed.ExtraArgs, extra) { + t.Errorf("Observed.ExtraArgs = %v; want %v", got.Status.Observed.ExtraArgs, extra) + } members := &lll.EtcdMemberList{} if err := c.List(ctx, members, client.InNamespace("ns")); err != nil { @@ -3220,6 +3226,9 @@ func TestBootstrap_PropagatesOptionsToSeed(t *testing.T) { if !equality.Semantic.DeepEqual(members.Items[0].Spec.Options, opts) { t.Fatalf("seed Spec.Options = %+v; want %+v", members.Items[0].Spec.Options, opts) } + if !equality.Semantic.DeepEqual(members.Items[0].Spec.ExtraArgs, extra) { + t.Fatalf("seed Spec.ExtraArgs = %v; want %v", members.Items[0].Spec.ExtraArgs, extra) + } } // TestBootstrap_NativeServicesAndNoReservedAnnotations pins the post-review @@ -3471,9 +3480,10 @@ func TestScaleUp_PropagatesResourcesToNewMember(t *testing.T) { } // TestScaleUp_PropagatesOptionsToNewMember covers the second member- -// creation site for spec.options: scaleUp's fresh-CR path. Bootstrap is -// covered by TestBootstrap_PropagatesOptionsToSeed; this one makes sure -// a member added later runs with the same tuning as the seed. +// creation site for spec.options and spec.extraArgs: scaleUp's fresh-CR +// path. Bootstrap is covered by TestBootstrap_PropagatesOptionsToSeed; +// this one makes sure a member added later runs with the same flags as +// the seed. func TestScaleUp_PropagatesOptionsToNewMember(t *testing.T) { ctx := context.Background() now := metav1.Now() @@ -3483,6 +3493,7 @@ func TestScaleUp_PropagatesOptionsToNewMember(t *testing.T) { AutoCompactionMode: lll.AutoCompactionModePeriodic, AutoCompactionRetention: "5m", } + extra := map[string]string{"log-level": "debug"} // Cluster targets 2 replicas; 1 ready voter already exists. scaleUp // must create a fresh second member carrying Observed.Options. @@ -3498,10 +3509,11 @@ func TestScaleUp_PropagatesOptionsToNewMember(t *testing.T) { ClusterToken: "ns-test-x", ClusterID: "deadbeef", Observed: &lll.ObservedClusterSpec{ - Replicas: 2, - Version: "3.5.17", - Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, - Options: opts, + Replicas: 2, + Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + Options: opts, + ExtraArgs: extra, }, ProgressDeadline: &metav1.Time{Time: metav1.Now().Add(time.Hour)}, }, @@ -3539,6 +3551,9 @@ func TestScaleUp_PropagatesOptionsToNewMember(t *testing.T) { if !equality.Semantic.DeepEqual(fresh.Spec.Options, opts) { t.Fatalf("scaled-up member Spec.Options = %+v; want %+v", fresh.Spec.Options, opts) } + if !equality.Semantic.DeepEqual(fresh.Spec.ExtraArgs, extra) { + t.Fatalf("scaled-up member Spec.ExtraArgs = %v; want %v", fresh.Spec.ExtraArgs, extra) + } } // TestUpdateStatus_SetsScaleSelector covers the /scale subresource diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index f3e87112..7ca1d698 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -20,6 +20,7 @@ import ( "context" "crypto/tls" "fmt" + "slices" "strconv" "time" @@ -542,6 +543,10 @@ func (r *EtcdMemberReconciler) ensurePod(ctx context.Context, member *lll.EtcdMe clusterFormed = cluster.Status.ClusterID != "" } + if err := checkExtraArgs(member.Spec.ExtraArgs); err != nil { + return fmt.Errorf("member %q: refusing to create the Pod: %w", member.Name, err) + } + pod = r.buildPod(member, clusterFormed) if err := controllerutil.SetControllerReference(member, pod, r.Scheme); err != nil { return err @@ -681,6 +686,91 @@ func optionFlags(o *lll.EtcdOptions) []string { return flags } +// deniedExtraArgs is every etcd flag spec.extraArgs may not set, with the +// reason it is reserved. It covers each flag buildPod and optionFlags render, +// plus the flags that would quietly override them (--config-file makes etcd +// ignore the command line; --discovery* replaces --initial-cluster; the client +// cert flags replace the identity mounted from spec.tls). A flag added to +// buildPod or optionFlags goes here too, and into the matching CEL rule on +// EtcdClusterSpec.ExtraArgs, which rejects the key at admission: +// TestDeniedExtraArgsCoverRenderedFlags fails until it is listed here, and +// TestExtraArgsAdmission_Envtest until the CEL rule rejects it. +var deniedExtraArgs = map[string]string{ + "name": "member identity is managed by the operator", + "data-dir": "data placement is managed by the operator", + "wal-dir": "data placement is managed by the operator", + "listen-peer-urls": "listeners are managed by the operator", + "listen-client-urls": "listeners are managed by the operator", + "listen-client-http-urls": "listeners are managed by the operator", + "listen-metrics-urls": "listeners are managed by the operator", + "advertise-client-urls": "member URLs are managed by the operator", + "initial-advertise-peer-urls": "member URLs are managed by the operator", + "initial-cluster": "cluster bootstrap is managed by the operator", + "initial-cluster-token": "cluster bootstrap is managed by the operator", + "initial-cluster-state": "cluster bootstrap is managed by the operator", + "force-new-cluster": "membership is managed by the operator", + "discovery": "cluster bootstrap is managed by the operator", + "discovery-srv": "cluster bootstrap is managed by the operator", + "discovery-endpoints": "cluster bootstrap is managed by the operator", + "config-file": "a config file makes etcd ignore the operator's flags", + "auto-tls": "client TLS is configured through spec.tls.client", + "cert-file": "client TLS is configured through spec.tls.client", + "key-file": "client TLS is configured through spec.tls.client", + "client-cert-file": "client TLS is configured through spec.tls.client", + "client-key-file": "client TLS is configured through spec.tls.client", + "client-cert-auth": "client TLS is configured through spec.tls.client", + "trusted-ca-file": "client TLS is configured through spec.tls.client", + "peer-auto-tls": "peer TLS is configured through spec.tls.peer", + "peer-cert-file": "peer TLS is configured through spec.tls.peer", + "peer-key-file": "peer TLS is configured through spec.tls.peer", + "peer-client-cert-file": "peer TLS is configured through spec.tls.peer", + "peer-client-key-file": "peer TLS is configured through spec.tls.peer", + "peer-trusted-ca-file": "peer TLS is configured through spec.tls.peer", + "peer-client-cert-auth": "peer TLS is configured through spec.tls.peer", + "quota-backend-bytes": "set it through spec.options.quotaBackendBytes", + "auto-compaction-mode": "set it through spec.options.autoCompactionMode", + "auto-compaction-retention": "set it through spec.options.autoCompactionRetention", + "snapshot-count": "set it through spec.options.snapshotCount", +} + +// checkExtraArgs reports the first extraArgs key (in sorted order) that is +// on the denylist. Admission already rejects such a key on the EtcdCluster, +// so this only fires for an EtcdMember written around the cluster +// controller; ensurePod refuses to create the Pod rather than letting a +// user flag override the operator's own. +func checkExtraArgs(args map[string]string) error { + for _, k := range sortedKeys(args) { + if reason, denied := deniedExtraArgs[k]; denied { + return fmt.Errorf("extraArgs: --%s cannot be set: %s", k, reason) + } + } + return nil +} + +// extraArgFlags renders the member's extraArgs (mirrored from +// EtcdCluster.spec.extraArgs) as --key=value flags, sorted by key so the +// rendered Pod does not depend on map iteration order. Denied keys never +// render, even if checkExtraArgs was bypassed. +func extraArgFlags(args map[string]string) []string { + var flags []string + for _, k := range sortedKeys(args) { + if _, denied := deniedExtraArgs[k]; denied { + continue + } + flags = append(flags, "--"+k+"="+args[k]) + } + return flags +} + +func sortedKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + slices.Sort(keys) + return keys +} + // buildPod renders a member's Pod. clusterFormed reports whether the parent // cluster has latched status.clusterID; see the --initial-cluster-state // derivation below for why it is a parameter rather than read from the member. @@ -834,6 +924,9 @@ func (r *EtcdMemberReconciler) buildPod(member *lll.EtcdMember, clusterFormed bo }) } + // User flags go last, after every flag the operator renders. + cmd = append(cmd, extraArgFlags(member.Spec.ExtraArgs)...) + etcdImage := resolveEtcdImage(member, r.EtcdImageRepository) // Restore initContainers: when the seed carries a restore spec, populate diff --git a/controllers/extra_args_envtest_test.go b/controllers/extra_args_envtest_test.go new file mode 100644 index 00000000..7c2bfda5 --- /dev/null +++ b/controllers/extra_args_envtest_test.go @@ -0,0 +1,105 @@ +/* +Copyright 2023 Timofey Larkin. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package controllers + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +// The admission-side copy of deniedExtraArgs is a CEL rule on +// EtcdClusterSpec.ExtraArgs, written out literally in a kubebuilder marker. +// This drives every denylisted key through a real apiserver with the +// generated CRDs installed, so the two lists cannot drift. `make test` sets +// KUBEBUILDER_ASSETS; without it the test skips. +func TestExtraArgsAdmission_Envtest(t *testing.T) { + if os.Getenv("KUBEBUILDER_ASSETS") == "" { + t.Skip("KUBEBUILDER_ASSETS not set; envtest harness not initialized") + } + env := &envtest.Environment{ + Scheme: testScheme(t), + CRDDirectoryPaths: []string{filepath.Join("..", "charts", "etcd-operator", "crd-bases")}, + ErrorIfCRDPathMissing: true, + } + cfg, err := env.Start() + if err != nil { + t.Fatalf("envtest start: %v", err) + } + t.Cleanup(func() { _ = env.Stop() }) + k8s, err := client.New(cfg, client.Options{Scheme: testScheme(t)}) + if err != nil { + t.Fatalf("client: %v", err) + } + ctx := context.Background() + + cluster := func(name string, extra map[string]string) *lll.EtcdCluster { + return &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: lll.EtcdClusterSpec{ + Replicas: ptrInt32(3), + Version: "3.6.11", + Storage: lll.StorageSpec{Size: resource.MustParse("128Mi")}, + ExtraArgs: extra, + }, + } + } + reject := func(t *testing.T, extra map[string]string, wantMsg string) { + t.Helper() + c := cluster("x", extra) + err := k8s.Create(ctx, c) + if err == nil { + _ = k8s.Delete(ctx, c) + t.Fatalf("apiserver accepted extraArgs %v; expected rejection", extra) + } + if !strings.Contains(err.Error(), wantMsg) { + t.Fatalf("rejection for %v does not mention %q: %v", extra, wantMsg, err) + } + } + + t.Run("allowed flags", func(t *testing.T) { + c := cluster("extra-ok", map[string]string{ + "log-level": "debug", + "wrapper-mode": "dry-run", + "experimental-initial-corrupt-check": "true", + "empty-value": "", + }) + if err := k8s.Create(ctx, c); err != nil { + t.Fatalf("apiserver rejected allowed extraArgs: %v", err) + } + _ = k8s.Delete(ctx, c) + }) + + for flag, reason := range deniedExtraArgs { + t.Run("denied "+flag, func(t *testing.T) { + want := "--" + flag + " is managed by the operator" + if field, typed := strings.CutPrefix(reason, "set it through "); typed { + want = "--" + flag + " is a typed option; set " + field + " instead" + } + reject(t, map[string]string{"log-level": "debug", flag: "x"}, want) + }) + } + + for _, key := range []string{"--log-level", "Log-Level", "log_level", "log-level=debug", "-log", ""} { + t.Run("malformed key "+key, func(t *testing.T) { + reject(t, map[string]string{key: "x"}, "spec.extraArgs keys are etcd flag names without the leading dashes") + }) + } +} diff --git a/controllers/extra_args_test.go b/controllers/extra_args_test.go new file mode 100644 index 00000000..5c3da42a --- /dev/null +++ b/controllers/extra_args_test.go @@ -0,0 +1,202 @@ +/* +Copyright 2023 Timofey Larkin. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package controllers + +import ( + "context" + "reflect" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +// TestDeniedExtraArgsCoverRenderedFlags renders a member Pod through every +// flag-emitting branch of buildPod (plaintext and TLS, mTLS, peer TLS and +// peer auto-TLS, all typed options) and requires each flag the operator +// renders to be on the extraArgs denylist. A flag added to buildPod without +// a denylist entry fails here. +func TestDeniedExtraArgsCoverRenderedFlags(t *testing.T) { + r := &EtcdMemberReconciler{} + quota := int64(1 << 30) + snapCount := int64(10000) + base := func() lll.EtcdMemberSpec { + return lll.EtcdMemberSpec{ + ClusterName: "test", Version: "3.6.11", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + InitialCluster: "m=http://m.test.ns.svc:2380", ClusterToken: "tok", + Options: &lll.EtcdOptions{ + QuotaBackendBytes: "a, + AutoCompactionMode: lll.AutoCompactionModePeriodic, + AutoCompactionRetention: "5m", + SnapshotCount: &snapCount, + }, + } + } + plain := base() + tlsFull := base() + tlsFull.TLS = &lll.EtcdMemberTLS{ + ClientServerSecretRef: &corev1.LocalObjectReference{Name: "srv"}, + ClientMTLS: true, + PeerSecretRef: &corev1.LocalObjectReference{Name: "peer"}, + } + autoTLS := base() + autoTLS.TLS = &lll.EtcdMemberTLS{PeerAutoTLS: true} + + rendered := map[string]bool{} + for _, spec := range []lll.EtcdMemberSpec{plain, tlsFull, autoTLS} { + pod := r.buildPod(&lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "m", Namespace: "ns"}, + Spec: spec, + }, false) + for _, arg := range pod.Spec.Containers[0].Command[1:] { + name, _, _ := strings.Cut(strings.TrimPrefix(arg, "--"), "=") + rendered[name] = true + } + } + // Sanity: the branches above really did render the TLS and options flags. + for _, want := range []string{"peer-auto-tls", "peer-client-cert-auth", "client-cert-auth", "snapshot-count"} { + if !rendered[want] { + t.Fatalf("test fixture did not exercise --%s; rendered %v", want, rendered) + } + } + for name := range rendered { + if _, ok := deniedExtraArgs[name]; !ok { + t.Errorf("buildPod renders --%s but deniedExtraArgs does not list it; add it there and to the CEL rule on EtcdClusterSpec.ExtraArgs", name) + } + } +} + +// TestBuildPod_RendersExtraArgs pins the rendering contract: each entry is +// --key=value with the value untouched, entries come after the operator's +// own flags, sorted by key, and the result does not depend on map order. +func TestBuildPod_RendersExtraArgs(t *testing.T) { + r := &EtcdMemberReconciler{} + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "m", Namespace: "ns"}, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", Version: "3.6.11", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + TLS: &lll.EtcdMemberTLS{PeerSecretRef: &corev1.LocalObjectReference{Name: "peer"}}, + ExtraArgs: map[string]string{ + "wrapper-mode": "dry-run", + "log-level": "debug", + "experimental-foo": "", + "log-outputs": "a=b, c", + }, + }, + } + cmd := r.buildPod(member, false).Spec.Containers[0].Command + want := []string{ + "--experimental-foo=", + "--log-level=debug", + "--log-outputs=a=b, c", + "--wrapper-mode=dry-run", + } + if len(cmd) < len(want) { + t.Fatalf("command too short: %v", cmd) + } + if got := cmd[len(cmd)-len(want):]; !reflect.DeepEqual(got, want) { + t.Fatalf("extraArgs tail = %v; want %v (full command %v)", got, want, cmd) + } + for i := 0; i < 20; i++ { + if again := r.buildPod(member, false).Spec.Containers[0].Command; !reflect.DeepEqual(again, cmd) { + t.Fatalf("command not deterministic:\n%v\n%v", cmd, again) + } + } +} + +// TestExtraArgs_DenylistRejectsEveryFlag runs every denylisted key through +// the render-time guard: checkExtraArgs must name the flag, and +// extraArgFlags must never emit it. +func TestExtraArgs_DenylistRejectsEveryFlag(t *testing.T) { + for flag := range deniedExtraArgs { + t.Run(flag, func(t *testing.T) { + args := map[string]string{"log-level": "debug", flag: "x"} + err := checkExtraArgs(args) + if err == nil || !strings.Contains(err.Error(), "--"+flag+" ") { + t.Fatalf("checkExtraArgs(%v) = %v; want an error naming --%s", args, err, flag) + } + if got := extraArgFlags(args); !reflect.DeepEqual(got, []string{"--log-level=debug"}) { + t.Fatalf("extraArgFlags(%v) = %v; want only the allowed flag", args, got) + } + }) + } + if err := checkExtraArgs(map[string]string{"log-level": "debug", "wrapper-mode": "enforce"}); err != nil { + t.Fatalf("checkExtraArgs rejected allowed flags: %v", err) + } + if err := checkExtraArgs(nil); err != nil { + t.Fatalf("checkExtraArgs(nil) = %v", err) + } +} + +// TestEnsurePod_RefusesDeniedExtraArgs covers the guard's call site: a +// member carrying a denied key (only reachable by writing the EtcdMember +// directly) gets no Pod, and the error says which flag. +func TestEnsurePod_RefusesDeniedExtraArgs(t *testing.T) { + ctx := context.Background() + member := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: "test-0", Namespace: "ns", UID: types.UID("uid"), Labels: memberLabels("test", "test-0")}, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", Version: "3.6.11", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + InitialCluster: "x", ClusterToken: "test", + ExtraArgs: map[string]string{"data-dir": "/tmp"}, + }, + } + c, _ := newTestClient(t, member) + r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)} + + err := r.ensurePod(ctx, member) + if err == nil || !strings.Contains(err.Error(), "--data-dir") { + t.Fatalf("ensurePod = %v; want a refusal naming --data-dir", err) + } + pods := &corev1.PodList{} + if err := c.List(ctx, pods); err != nil { + t.Fatal(err) + } + if len(pods.Items) != 0 { + t.Fatalf("ensurePod created %d Pod(s) despite a denied extraArgs key", len(pods.Items)) + } +} + +// spec.extraArgs is latched through status.observed like spec.options, so +// an edit must register as "spec ≠ observed". +func TestSpecEqualsObserved_ExtraArgsMatter(t *testing.T) { + cluster := &lll.EtcdCluster{ + Spec: lll.EtcdClusterSpec{ + Replicas: ptrInt32(3), + Version: "3.6.11", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + ExtraArgs: map[string]string{"log-level": "debug"}, + }, + Status: lll.EtcdClusterStatus{ + Observed: &lll.ObservedClusterSpec{ + Replicas: 3, + Version: "3.6.11", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + }, + }, + } + if specEqualsObserved(cluster) { + t.Fatalf("specEqualsObserved must return false when extraArgs differ") + } + snapshotSpecIntoObserved(cluster) + if !reflect.DeepEqual(cluster.Status.Observed.ExtraArgs, cluster.Spec.ExtraArgs) { + t.Fatalf("snapshotSpecIntoObserved did not latch extraArgs: %v", cluster.Status.Observed.ExtraArgs) + } + if !specEqualsObserved(cluster) { + t.Fatalf("specEqualsObserved must return true once extraArgs are latched") + } +} diff --git a/internal/migrate/adopt.go b/internal/migrate/adopt.go index 7410ecee..538901ac 100644 --- a/internal/migrate/adopt.go +++ b/internal/migrate/adopt.go @@ -279,6 +279,7 @@ func BuildAdoption(name, namespace string, spec legacy.EtcdClusterSpec, facts Cl Affinity: cluster.Spec.Affinity, TopologySpreadConstraints: cluster.Spec.TopologySpreadConstraints, Options: cluster.Spec.Options, + ExtraArgs: cluster.Spec.ExtraArgs, ImagePullSecrets: cluster.Spec.ImagePullSecrets, Bootstrap: false, InitialCluster: initialCluster, @@ -312,6 +313,7 @@ func BuildAdoption(name, namespace string, spec legacy.EtcdClusterSpec, facts Cl TopologySpreadConstraints: cluster.Spec.TopologySpreadConstraints, AdditionalMetadata: cluster.Spec.AdditionalMetadata, Options: cluster.Spec.Options, + ExtraArgs: cluster.Spec.ExtraArgs, ImagePullSecrets: cluster.Spec.ImagePullSecrets, }, } From 7e4dc2d61763f4a7ac135b144adf3500cc20cc38 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Tue, 20 Oct 2026 11:10:53 +0000 Subject: [PATCH 3/5] docs: describe spec.extraArgs and its denylist Assisted-by: LLM Signed-off-by: Andrei Kvapil --- docs/concepts.md | 33 ++++++++++++++++++++++++++++++++- docs/migration.md | 11 ++++++----- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/docs/concepts.md b/docs/concepts.md index 54eefabc..aa5817a0 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -348,10 +348,41 @@ spec: Every field is optional; an unset field emits no flag, leaving etcd's built-in default in force. -The set is deliberately **closed and typed**. The legacy aenix operator exposed `spec.options` as a free-form `map[string]string`, which let users inject arbitrary flags — including ones that conflict with flags the operator itself manages (listen URLs, initial-cluster wiring, TLS paths). This operator types exactly the keys Cozystack's etcd package actually used; a new tuning knob lands as a new typed field with validation, not via an escape hatch. +The typed set covers the knobs worth validating. The legacy aenix operator exposed `spec.options` as a free-form `map[string]string`, which let users inject flags that conflict with the ones the operator itself manages (listen URLs, initial-cluster wiring, TLS paths); this operator types exactly the keys Cozystack's etcd package actually used. Any other flag goes through [`spec.extraArgs`](#extra-etcd-flags). Like `spec.resources`, options are latched through `status.observed` and apply **to newly-created members only** (scale-up, replacement) — the operator does not roll existing Pods when they change. To apply a tuning change to an existing cluster, delete one Pod at a time and let the operator recreate it with the new flags. A transient mix of old- and new-flag members is harmless: these are per-member settings (backend quota, compaction cadence, raft snapshot interval), the same heterogeneity any manual rolling flag change passes through. +## Extra etcd flags + +`spec.extraArgs` passes flags the typed `spec.options` does not cover — an etcd flag no field exists for yet, or flags for an image that wraps etcd and reads options of its own: + +```yaml +spec: + extraArgs: + log-level: debug # --log-level=debug + experimental-initial-corrupt-check: "true" +``` + +Keys are flag names without the leading dashes (lowercase words joined by `-`); each entry renders as `--key=value` with the value passed as is. The entries come after the operator's own flags, sorted by key, so the rendered Pod does not change between reconciles. Values are strings: quote `"true"` and numbers in YAML. + +Flags the operator manages are rejected at admission, with a message naming the flag: + +| Flags | Managed through | +|---|---| +| `name`, `data-dir`, `wal-dir` | member identity and data placement | +| `listen-peer-urls`, `listen-client-urls`, `listen-client-http-urls`, `listen-metrics-urls`, `advertise-client-urls`, `initial-advertise-peer-urls` | listeners and member URLs | +| `initial-cluster`, `initial-cluster-token`, `initial-cluster-state`, `force-new-cluster`, `discovery`, `discovery-srv`, `discovery-endpoints` | cluster bootstrap and membership | +| `config-file` | would make etcd ignore the operator's command line | +| `auto-tls`, `cert-file`, `key-file`, `client-cert-file`, `client-key-file`, `client-cert-auth`, `trusted-ca-file` | `spec.tls.client` | +| `peer-auto-tls`, `peer-cert-file`, `peer-key-file`, `peer-client-cert-file`, `peer-client-key-file`, `peer-trusted-ca-file`, `peer-client-cert-auth` | `spec.tls.peer` | +| `quota-backend-bytes`, `auto-compaction-mode`, `auto-compaction-retention`, `snapshot-count` | `spec.options` | + +The member controller applies the same denylist when it builds a Pod and refuses to create one that carries a denied key. + +Anything else passes through unchecked. A flag etcd does not recognise, or a malformed value, makes the member fail to start — the operator does not know etcd's flag set. Prefer a typed field where one exists. + +`extraArgs` follow the same rule as `spec.options`: latched through `status.observed`, applied **to newly-created members only**. Delete one Pod at a time to roll a change onto existing members. + ## Member DNS identity (adoption annotations) The operator's headless Service (per-member DNS for peer discovery) is always named after the cluster, and every URL the operator constructs for a member — peer/client dial endpoints, `--initial-cluster`, the Pod's `spec.subdomain` — derives from that member's resolved Service name. There is **no cluster-level override**: a natively-created member always resolves under the cluster's own name. diff --git a/docs/migration.md b/docs/migration.md index d5e2bcb9..55f8b878 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -382,9 +382,9 @@ its name. The legacy operator's `spec.options` was a free-form `map[string]string` passed through as etcd flags. This operator keeps the `spec.options` path but types it -as a closed struct covering exactly the keys Cozystack's etcd package set — -arbitrary flag injection is no longer possible (see -[concepts: etcd tuning options](concepts.md#etcd-tuning-options) for why). +as a struct covering exactly the keys Cozystack's etcd package set; other +flags move to `spec.extraArgs`, which rejects the flags the operator manages +(see [concepts: etcd tuning options](concepts.md#etcd-tuning-options) for why). The key mapping, using Cozystack's actual legacy values: @@ -404,8 +404,9 @@ The key mapping, using Cozystack's actual legacy values: Note the value types: `quotaBackendBytes` and `snapshotCount` are integers, not quoted strings. `etcd-migrate` performs this mapping automatically. Any other key the legacy map accepted has no typed equivalent — the tool drops it with a -warning; if you relied on one, file an issue: the flag gets a typed field, not -a pass-through. +warning. If you relied on one, set it under `spec.extraArgs` after the +migration (see [concepts: extra etcd flags](concepts.md#extra-etcd-flags)); +flags the operator manages itself are rejected there. ## Authentication: root credentials are BYO and required From b99cb1583abd826cdf9b7345b15999cb52ae3bde Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 20 Oct 2026 22:26:49 +0500 Subject: [PATCH 4/5] fix(controllers): sync latched Pod-template fields onto existing members The docs say that a Pod deleted to roll an options change comes back with the new flags, but members kept the template from their creation, so a recreated Pod still got the old flags. That made extraArgs unusable on existing clusters short of replacing members. The cluster controller now copies options, extraArgs and resources from status.observed onto existing members, dormant ones included, before any scale step. Running Pods are not touched. Storage, version, scheduling and metadata keep their creation-time values. Assisted-by: LLM Signed-off-by: Kirill Ilin --- controllers/etcdcluster_controller.go | 36 +++++ controllers/member_template_test.go | 192 ++++++++++++++++++++++++++ 2 files changed, 228 insertions(+) create mode 100644 controllers/member_template_test.go diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index a16e6034..96a19a1c 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "maps" "math" "sort" "strings" @@ -271,6 +272,12 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) } } + // Before any scale step: waking a dormant member creates its Pod from + // the member's own template, so the template must already be current. + if err := r.syncMemberTemplates(ctx, cluster, active); err != nil { + return ctrl.Result{}, err + } + // If a half-baked scale-up CR is present (Spec.InitialCluster empty), // route through scaleUp regardless of replica accounting. Two reasons: // @@ -399,6 +406,35 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.updateStatus(ctx, cluster, active, pending) } +// ── Member templates ───────────────────────────────────────────────────── + +// syncMemberTemplates copies the latched Pod-template fields (options, +// extraArgs, resources) onto existing members. Running Pods are left alone; +// a Pod recreated for any reason, including a deliberate delete to roll a +// change, picks the new template up. Storage, version, scheduling and +// metadata stay as they were at member creation: they are bound to the +// member's volume or are upgrades, not per-Pod settings. +func (r *EtcdClusterReconciler) syncMemberTemplates(ctx context.Context, cluster *lll.EtcdCluster, members []lll.EtcdMember) error { + o := cluster.Status.Observed + for i := range members { + m := &members[i] + if equality.Semantic.DeepEqual(m.Spec.Options, o.Options) && + equality.Semantic.DeepEqual(m.Spec.ExtraArgs, o.ExtraArgs) && + equality.Semantic.DeepEqual(m.Spec.Resources, o.Resources) { + continue + } + patch := client.MergeFrom(m.DeepCopy()) + m.Spec.Options = o.Options.DeepCopy() + m.Spec.ExtraArgs = maps.Clone(o.ExtraArgs) + m.Spec.Resources = *o.Resources.DeepCopy() + if err := r.Patch(ctx, m, patch); err != nil { + return fmt.Errorf("sync template of member %s: %w", m.Name, err) + } + log.FromContext(ctx).Info("synced member template with the cluster spec; applies when the Pod is recreated", "member", m.Name) + } + return nil +} + // ── Bootstrap ──────────────────────────────────────────────────────────── // bootstrap brings up the single seed member for a fresh cluster. Its diff --git a/controllers/member_template_test.go b/controllers/member_template_test.go new file mode 100644 index 00000000..ba3b4b82 --- /dev/null +++ b/controllers/member_template_test.go @@ -0,0 +1,192 @@ +package controllers + +import ( + "context" + "fmt" + "slices" + "testing" + "time" + + "go.etcd.io/etcd/api/v3/etcdserverpb" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +// steadyCluster returns a converged three-member cluster whose latched +// target is observed, with members still carrying an older template. +func steadyCluster(t *testing.T, observed *lll.ObservedClusterSpec) []client.Object { + t.Helper() + now := metav1.Now() + spec := lll.EtcdClusterSpec{ + Replicas: ptrInt32(observed.Replicas), + Version: observed.Version, + Storage: observed.Storage, + Resources: observed.Resources, + Options: observed.Options, + ExtraArgs: observed.ExtraArgs, + } + objs := []client.Object{&lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Spec: spec, + Status: lll.EtcdClusterStatus{ + ClusterToken: "ns-test-x", + ClusterID: "deadbeef", + Observed: observed, + ProgressDeadline: &metav1.Time{Time: metav1.Now().Add(time.Hour)}, + }, + }} + for i := range 3 { + objs = append(objs, &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("test-%d", i), Namespace: "ns", Labels: memberLabels("test", fmt.Sprintf("test-%d", i))}, + Spec: lll.EtcdMemberSpec{ + ClusterName: "test", + Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + InitialCluster: "x", + Options: &lll.EtcdOptions{SnapshotCount: ptrInt64(10000)}, + ExtraArgs: map[string]string{"log-level": "info"}, + }, + Status: lll.EtcdMemberStatus{ + PodName: fmt.Sprintf("test-%d", i), + MemberID: "abc", + Conditions: []metav1.Condition{{Type: lll.MemberReady, Status: metav1.ConditionTrue, Reason: "PodReady", LastTransitionTime: now}}, + }, + }) + } + return objs +} + +func steadyEtcd() *fakeEtcd { + return newFakeEtcd(0xdeadbeef, + &etcdserverpb.Member{ID: 0xa01, Name: "test-0", PeerURLs: []string{peerURL("http", "test-0", "test", "ns")}}, + &etcdserverpb.Member{ID: 0xa02, Name: "test-1", PeerURLs: []string{peerURL("http", "test-1", "test", "ns")}}, + &etcdserverpb.Member{ID: 0xa03, Name: "test-2", PeerURLs: []string{peerURL("http", "test-2", "test", "ns")}}, + ) +} + +func reconcileTest(t *testing.T, c client.Client) { + t.Helper() + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryReturning(steadyEtcd())} + if _, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile: %v", err) + } +} + +// TestReconcile_SyncsMemberTemplatesAtSteadyState: once a spec change is +// latched, existing members carry the new Pod template, so a Pod deleted to +// roll the change comes back with it. +func TestReconcile_SyncsMemberTemplatesAtSteadyState(t *testing.T) { + observed := &lll.ObservedClusterSpec{ + Replicas: 3, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + Resources: corev1.ResourceRequirements{Limits: corev1.ResourceList{corev1.ResourceMemory: quickQty(t, "2Gi")}}, + Options: &lll.EtcdOptions{SnapshotCount: ptrInt64(5000)}, + ExtraArgs: map[string]string{"log-level": "debug", "threek-mode": "dry-run"}, + } + c, _ := newTestClient(t, steadyCluster(t, observed)...) + reconcileTest(t, c) + + for i := range 3 { + m := mustGet(t, c, fmt.Sprintf("test-%d", i), "ns", &lll.EtcdMember{}) + if !equality.Semantic.DeepEqual(m.Spec.Options, observed.Options) { + t.Errorf("%s options = %+v, want %+v", m.Name, m.Spec.Options, observed.Options) + } + if !equality.Semantic.DeepEqual(m.Spec.ExtraArgs, observed.ExtraArgs) { + t.Errorf("%s extraArgs = %v, want %v", m.Name, m.Spec.ExtraArgs, observed.ExtraArgs) + } + if !equality.Semantic.DeepEqual(m.Spec.Resources, observed.Resources) { + t.Errorf("%s resources = %+v, want %+v", m.Name, m.Spec.Resources, observed.Resources) + } + + pod := (&EtcdMemberReconciler{}).buildPod(m, true) + cmd := pod.Spec.Containers[0].Command + for _, flag := range []string{"--snapshot-count=5000", "--log-level=debug", "--threek-mode=dry-run"} { + if !slices.Contains(cmd, flag) { + t.Errorf("%s: a recreated Pod lacks %s: %v", m.Name, flag, cmd) + } + } + } +} + +// TestReconcile_SyncLeavesOtherMemberFieldsAlone: only the Pod-template +// fields follow the cluster. Storage is bound to the member's volume and the +// version is an upgrade, so neither changes under an existing member. +func TestReconcile_SyncLeavesOtherMemberFieldsAlone(t *testing.T) { + observed := &lll.ObservedClusterSpec{ + Replicas: 3, Version: "3.6.1", Storage: lll.StorageSpec{Size: quickQty(t, "8Gi")}, + ExtraArgs: map[string]string{"threek-mode": "dry-run"}, + } + c, _ := newTestClient(t, steadyCluster(t, observed)...) + reconcileTest(t, c) + + m := mustGet(t, c, "test-0", "ns", &lll.EtcdMember{}) + if m.Spec.Version != "3.5.17" { + t.Errorf("version = %s, want the member's own 3.5.17", m.Spec.Version) + } + if got := m.Spec.Storage.Size.String(); got != "1Gi" { + t.Errorf("storage size = %s, want the member's own 1Gi", got) + } +} + +// TestReconcile_NoMemberWritesWhenTemplatesMatch: a converged cluster whose +// members already carry the latched template is not rewritten every pass. +func TestReconcile_NoMemberWritesWhenTemplatesMatch(t *testing.T) { + observed := &lll.ObservedClusterSpec{ + Replicas: 3, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + Options: &lll.EtcdOptions{SnapshotCount: ptrInt64(10000)}, + ExtraArgs: map[string]string{"log-level": "info"}, + } + writes := 0 + countMemberWrites := func(obj client.Object) { + if _, ok := obj.(*lll.EtcdMember); ok { + writes++ + } + } + c := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects(steadyCluster(t, observed)...). + WithStatusSubresource(&lll.EtcdCluster{}, &lll.EtcdMember{}). + WithInterceptorFuncs(interceptor.Funcs{ + Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + countMemberWrites(obj) + return c.Patch(ctx, obj, patch, opts...) + }, + Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + countMemberWrites(obj) + return c.Update(ctx, obj, opts...) + }, + }). + Build() + reconcileTest(t, c) + if writes != 0 { + t.Errorf("members written %d times without a template change", writes) + } +} + +// TestReconcile_SyncsDormantMemberTemplate: a paused cluster's dormant member +// is woken by scale-up, which creates its Pod before the cluster converges +// again, so the dormant member must already carry the latched template. +func TestReconcile_SyncsDormantMemberTemplate(t *testing.T) { + observed := &lll.ObservedClusterSpec{ + Replicas: 0, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + ExtraArgs: map[string]string{"threek-mode": "dry-run"}, + } + objs := steadyCluster(t, observed)[:2] + dormant := objs[1].(*lll.EtcdMember) + dormant.Spec.Dormant = true + dormant.Status = lll.EtcdMemberStatus{MemberID: "abc"} + c, _ := newTestClient(t, objs...) + reconcileTest(t, c) + + m := mustGet(t, c, dormant.Name, "ns", &lll.EtcdMember{}) + if !equality.Semantic.DeepEqual(m.Spec.ExtraArgs, observed.ExtraArgs) { + t.Errorf("dormant member extraArgs = %v, want %v", m.Spec.ExtraArgs, observed.ExtraArgs) + } +} From 98532ffa5820dcc7158e975a07153e1d269e56f8 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 20 Oct 2026 22:26:50 +0500 Subject: [PATCH 5/5] docs: describe how latched options reach existing members Assisted-by: LLM Signed-off-by: Kirill Ilin --- api/v1alpha2/etcdmember_types.go | 29 ++++++++++--------- ...tcd-operator.cozystack.io_etcdmembers.yaml | 29 ++++++++++--------- docs/concepts.md | 4 +-- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/api/v1alpha2/etcdmember_types.go b/api/v1alpha2/etcdmember_types.go index 873a2847..bc8b6ebf 100644 --- a/api/v1alpha2/etcdmember_types.go +++ b/api/v1alpha2/etcdmember_types.go @@ -88,12 +88,11 @@ type EtcdMemberSpec struct { // immutable per-member spec. Storage StorageSpec `json:"storage"` - // Resources mirrors EtcdCluster.spec.resources at the time this - // member was created. The cluster controller copies it onto each - // member at creation. The member controller passes the value + // Resources mirrors the latched EtcdCluster.spec.resources. The + // cluster controller copies it onto each member at creation and keeps + // it in sync afterwards. The member controller passes the value // straight to the etcd container's resources field at Pod-build - // time; existing members are not re-templated when the cluster - // spec changes. + // time, so a running Pod picks a change up only when it is recreated. // +optional Resources corev1.ResourceRequirements `json:"resources,omitempty"` @@ -115,18 +114,20 @@ type EtcdMemberSpec struct { // +optional TopologySpreadConstraints []corev1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"` - // Options mirrors EtcdCluster.spec.options at the time this member - // was created. The member controller renders the set fields as etcd - // command-line flags at Pod-build time; existing members are not - // re-templated when the cluster spec changes. + // Options mirrors the latched EtcdCluster.spec.options. The cluster + // controller copies it onto each member at creation and keeps it in + // sync afterwards. The member controller renders the set fields as + // etcd command-line flags at Pod-build time, so a running Pod picks a + // change up only when it is recreated. // +optional Options *EtcdOptions `json:"options,omitempty"` - // ExtraArgs mirrors EtcdCluster.spec.extraArgs at the time this member - // was created. The member controller renders each entry as --key=value - // after its own flags, sorted by key, and refuses to create the Pod if - // a key collides with a flag it manages. Existing members are not - // re-templated when the cluster spec changes. + // ExtraArgs mirrors the latched EtcdCluster.spec.extraArgs. The + // cluster controller copies it onto each member at creation and keeps + // it in sync afterwards. The member controller renders each entry as + // --key=value after its own flags, sorted by key, and refuses to + // create the Pod if a key collides with a flag it manages. A running + // Pod picks a change up only when it is recreated. // +optional ExtraArgs map[string]string `json:"extraArgs,omitempty"` diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml index 2b2f3e3f..d1412872 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml +++ b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yaml @@ -1027,11 +1027,12 @@ spec: additionalProperties: type: string description: |- - ExtraArgs mirrors EtcdCluster.spec.extraArgs at the time this member - was created. The member controller renders each entry as --key=value - after its own flags, sorted by key, and refuses to create the Pod if - a key collides with a flag it manages. Existing members are not - re-templated when the cluster spec changes. + ExtraArgs mirrors the latched EtcdCluster.spec.extraArgs. The + cluster controller copies it onto each member at creation and keeps + it in sync afterwards. The member controller renders each entry as + --key=value after its own flags, sorted by key, and refuses to + create the Pod if a key collides with a flag it manages. A running + Pod picks a change up only when it is recreated. type: object imagePullSecrets: description: |- @@ -1062,10 +1063,11 @@ spec: type: string options: description: |- - Options mirrors EtcdCluster.spec.options at the time this member - was created. The member controller renders the set fields as etcd - command-line flags at Pod-build time; existing members are not - re-templated when the cluster spec changes. + Options mirrors the latched EtcdCluster.spec.options. The cluster + controller copies it onto each member at creation and keeps it in + sync afterwards. The member controller renders the set fields as + etcd command-line flags at Pod-build time, so a running Pod picks a + change up only when it is recreated. properties: autoCompactionMode: description: |- @@ -1120,12 +1122,11 @@ spec: type: integer resources: description: |- - Resources mirrors EtcdCluster.spec.resources at the time this - member was created. The cluster controller copies it onto each - member at creation. The member controller passes the value + Resources mirrors the latched EtcdCluster.spec.resources. The + cluster controller copies it onto each member at creation and keeps + it in sync afterwards. The member controller passes the value straight to the etcd container's resources field at Pod-build - time; existing members are not re-templated when the cluster - spec changes. + time, so a running Pod picks a change up only when it is recreated. properties: claims: description: |- diff --git a/docs/concepts.md b/docs/concepts.md index aa5817a0..74402396 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -350,7 +350,7 @@ Every field is optional; an unset field emits no flag, leaving etcd's built-in d The typed set covers the knobs worth validating. The legacy aenix operator exposed `spec.options` as a free-form `map[string]string`, which let users inject flags that conflict with the ones the operator itself manages (listen URLs, initial-cluster wiring, TLS paths); this operator types exactly the keys Cozystack's etcd package actually used. Any other flag goes through [`spec.extraArgs`](#extra-etcd-flags). -Like `spec.resources`, options are latched through `status.observed` and apply **to newly-created members only** (scale-up, replacement) — the operator does not roll existing Pods when they change. To apply a tuning change to an existing cluster, delete one Pod at a time and let the operator recreate it with the new flags. A transient mix of old- and new-flag members is harmless: these are per-member settings (backend quota, compaction cadence, raft snapshot interval), the same heterogeneity any manual rolling flag change passes through. +Like `spec.resources`, options are latched through `status.observed`. New members get them at creation, and the operator copies them onto existing `EtcdMember` objects without touching their running Pods — it does not roll Pods. To apply a tuning change to an existing cluster, delete one Pod at a time and let the operator recreate it with the new flags. A transient mix of old- and new-flag members is harmless: these are per-member settings (backend quota, compaction cadence, raft snapshot interval), the same heterogeneity any manual rolling flag change passes through. ## Extra etcd flags @@ -381,7 +381,7 @@ The member controller applies the same denylist when it builds a Pod and refuses Anything else passes through unchecked. A flag etcd does not recognise, or a malformed value, makes the member fail to start — the operator does not know etcd's flag set. Prefer a typed field where one exists. -`extraArgs` follow the same rule as `spec.options`: latched through `status.observed`, applied **to newly-created members only**. Delete one Pod at a time to roll a change onto existing members. +`extraArgs` follow the same rule as `spec.options`: latched through `status.observed` and copied onto every member, applied to a running member only when its Pod is recreated. Delete one Pod at a time to roll a change onto existing members. ## Member DNS identity (adoption annotations)