Skip to content

Commit 693708d

Browse files
committed
fix(api): enforce EtcdSnapshot spec immutability
Signed-off-by: immanuwell <pchpr.00@list.ru>
1 parent a371f0c commit 693708d

3 files changed

Lines changed: 81 additions & 0 deletions

File tree

api/v1alpha2/etcdsnapshot_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ const SnapshotReady = "Ready"
103103
// EtcdSnapshotSpec defines a one-shot etcd snapshot of a cluster to a
104104
// destination. Snapshots are immutable: change the destination by creating a
105105
// new EtcdSnapshot.
106+
//
107+
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec is immutable; create a new EtcdSnapshot instead"
106108
type EtcdSnapshotSpec struct {
107109
// ClusterRef names the EtcdCluster (same namespace) to snapshot.
108110
ClusterRef corev1.LocalObjectReference `json:"clusterRef"`

api/v1alpha2/snapshot_cel_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,82 @@ func TestCEL_SnapshotLocationExactlyOne(t *testing.T) {
9191
})
9292
}
9393

94+
func TestCEL_EtcdSnapshotSpecImmutable(t *testing.T) {
95+
skipIfNoEnvtest(t)
96+
ctx := context.Background()
97+
98+
newSnapshot := func(name string) *lll.EtcdSnapshot {
99+
return &lll.EtcdSnapshot{
100+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"},
101+
Spec: lll.EtcdSnapshotSpec{
102+
ClusterRef: corev1.LocalObjectReference{Name: "c1"},
103+
Destination: s3Source(),
104+
},
105+
}
106+
}
107+
108+
for _, tc := range []struct {
109+
name string
110+
mutate func(*lll.EtcdSnapshot)
111+
}{
112+
{
113+
name: "cluster-ref",
114+
mutate: func(snapshot *lll.EtcdSnapshot) {
115+
snapshot.Spec.ClusterRef.Name = "c2"
116+
},
117+
},
118+
{
119+
name: "destination",
120+
mutate: func(snapshot *lll.EtcdSnapshot) {
121+
snapshot.Spec.Destination.S3.Key = "other/snapshot.db"
122+
},
123+
},
124+
} {
125+
t.Run(tc.name, func(t *testing.T) {
126+
snapshot := newSnapshot("immutable-" + tc.name)
127+
if err := k8s.Create(ctx, snapshot); err != nil {
128+
t.Fatalf("Create: %v", err)
129+
}
130+
t.Cleanup(func() { _ = k8s.Delete(ctx, snapshot) })
131+
132+
live := &lll.EtcdSnapshot{}
133+
if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(snapshot), live); err != nil {
134+
t.Fatalf("Get: %v", err)
135+
}
136+
tc.mutate(live)
137+
err := k8s.Update(ctx, live)
138+
if err == nil {
139+
t.Fatal("apiserver accepted an EtcdSnapshot spec update; expected rejection")
140+
}
141+
if !strings.Contains(err.Error(), "spec is immutable") {
142+
t.Fatalf("error did not mention spec immutability: %v", err)
143+
}
144+
})
145+
}
146+
147+
t.Run("metadata and status remain mutable", func(t *testing.T) {
148+
snapshot := newSnapshot("immutable-non-spec-updates")
149+
if err := k8s.Create(ctx, snapshot); err != nil {
150+
t.Fatalf("Create: %v", err)
151+
}
152+
t.Cleanup(func() { _ = k8s.Delete(ctx, snapshot) })
153+
154+
live := &lll.EtcdSnapshot{}
155+
if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(snapshot), live); err != nil {
156+
t.Fatalf("Get: %v", err)
157+
}
158+
live.Labels = map[string]string{"example.com/owner": "test"}
159+
if err := k8s.Update(ctx, live); err != nil {
160+
t.Fatalf("metadata update rejected unexpectedly: %v", err)
161+
}
162+
163+
live.Status.Phase = lll.EtcdSnapshotStatusPhasePending
164+
if err := k8s.Status().Update(ctx, live); err != nil {
165+
t.Fatalf("status update rejected unexpectedly: %v", err)
166+
}
167+
})
168+
}
169+
94170
// A restore S3 source addresses one exact object; an empty key must be
95171
// rejected by the apiserver rather than failing opaquely in the seed.
96172
func TestCEL_RestoreS3KeyRequired(t *testing.T) {

charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdsnapshots.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ spec:
146146
- clusterRef
147147
- destination
148148
type: object
149+
x-kubernetes-validations:
150+
- message: spec is immutable; create a new EtcdSnapshot instead
151+
rule: self == oldSelf
149152
status:
150153
description: EtcdSnapshotStatus is the observed state of an EtcdSnapshot.
151154
properties:

0 commit comments

Comments
 (0)