diff --git a/api/v1alpha2/defrag_cel_test.go b/api/v1alpha2/defrag_cel_test.go new file mode 100644 index 00000000..366e0002 --- /dev/null +++ b/api/v1alpha2/defrag_cel_test.go @@ -0,0 +1,173 @@ +/* +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 v1alpha2_test + +import ( + "context" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +func defrag(name string, rule *lll.DefragRule) *lll.EtcdDefrag { + return &lll.EtcdDefrag{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: lll.EtcdDefragSpec{ + ClusterRef: corev1.LocalObjectReference{Name: "c1"}, + Rule: rule, + }, + } +} + +func freeSpaceRule(q string) *lll.DefragRule { + v := resource.MustParse(q) + return &lll.DefragRule{FreeSpaceAbove: &v} +} + +// clusterRef and rule are the immutable request; ttlSecondsAfterFinished is a +// retention knob that must stay editable after the run finishes. +func TestCEL_EtcdDefragSpecImmutable(t *testing.T) { + skipIfNoEnvtest(t) + ctx := context.Background() + + t.Run("create accepted", func(t *testing.T) { + d := defrag("df-create", freeSpaceRule("256Mi")) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create rejected unexpectedly: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + }) + + t.Run("cannot mutate clusterRef", func(t *testing.T) { + d := defrag("df-clusterref", nil) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + + got := &lll.EtcdDefrag{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(d), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.ClusterRef.Name = "c2" + err := k8s.Update(ctx, got) + if err == nil { + t.Fatal("apiserver accepted mutating spec.clusterRef; expected rejection") + } + if !strings.Contains(err.Error(), "spec.clusterRef is immutable") { + t.Fatalf("error did not mention the clusterRef rule: %v", err) + } + }) + + t.Run("cannot mutate rule", func(t *testing.T) { + d := defrag("df-rule-mutate", freeSpaceRule("256Mi")) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + + got := &lll.EtcdDefrag{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(d), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.Rule = freeSpaceRule("512Mi") + err := k8s.Update(ctx, got) + if err == nil { + t.Fatal("apiserver accepted mutating spec.rule; expected rejection") + } + if !strings.Contains(err.Error(), "spec.rule is immutable") { + t.Fatalf("error did not mention the rule immutability: %v", err) + } + }) + + t.Run("cannot add rule", func(t *testing.T) { + d := defrag("df-rule-add", nil) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + + got := &lll.EtcdDefrag{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(d), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.Rule = freeSpaceRule("256Mi") + err := k8s.Update(ctx, got) + if err == nil { + t.Fatal("apiserver accepted adding spec.rule; expected rejection") + } + if !strings.Contains(err.Error(), "cannot be added to or removed") { + t.Fatalf("error did not mention the add/remove rule: %v", err) + } + }) + + t.Run("cannot remove rule", func(t *testing.T) { + d := defrag("df-rule-remove", freeSpaceRule("256Mi")) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + + got := &lll.EtcdDefrag{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(d), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Spec.Rule = nil + err := k8s.Update(ctx, got) + if err == nil { + t.Fatal("apiserver accepted removing spec.rule; expected rejection") + } + if !strings.Contains(err.Error(), "cannot be added to or removed") { + t.Fatalf("error did not mention the add/remove rule: %v", err) + } + }) + + t.Run("ttlSecondsAfterFinished stays mutable", func(t *testing.T) { + d := defrag("df-ttl", freeSpaceRule("256Mi")) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + + got := &lll.EtcdDefrag{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(d), got); err != nil { + t.Fatalf("Get: %v", err) + } + ttl := int32(3600) + got.Spec.TTLSecondsAfterFinished = &ttl + if err := k8s.Update(ctx, got); err != nil { + t.Fatalf("apiserver rejected setting spec.ttlSecondsAfterFinished; it must stay mutable: %v", err) + } + }) + + t.Run("status remains mutable", func(t *testing.T) { + d := defrag("df-status", freeSpaceRule("256Mi")) + if err := k8s.Create(ctx, d); err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = k8s.Delete(ctx, d) }) + + got := &lll.EtcdDefrag{} + if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(d), got); err != nil { + t.Fatalf("Get: %v", err) + } + got.Status.Phase = lll.EtcdDefragPhasePending + if err := k8s.Status().Update(ctx, got); err != nil { + t.Fatalf("apiserver rejected a status update; status must stay mutable: %v", err) + } + }) +} diff --git a/api/v1alpha2/etcddefrag_types.go b/api/v1alpha2/etcddefrag_types.go index e69fea64..bcff94ec 100644 --- a/api/v1alpha2/etcddefrag_types.go +++ b/api/v1alpha2/etcddefrag_types.go @@ -25,7 +25,15 @@ import ( // EtcdDefragSpec is the desired state of an EtcdDefrag: a one-shot request to // defragment an EtcdCluster's members. // +// clusterRef and rule are the request itself and are immutable: the controller +// consults them once and never re-runs, so editing them post-create would +// silently do nothing. ttlSecondsAfterFinished stays mutable — it is a +// record-retention knob, not part of the request (see Job.spec.ttlSecondsAfterFinished). +// // +kubebuilder:validation:XValidation:rule="size(self.clusterRef.name) != 0",message="spec.clusterRef.name is required" +// +kubebuilder:validation:XValidation:rule="self.clusterRef == oldSelf.clusterRef",message="spec.clusterRef is immutable; create a new EtcdDefrag to defragment a different cluster" +// +kubebuilder:validation:XValidation:rule="has(self.rule) == has(oldSelf.rule)",message="spec.rule cannot be added to or removed from an existing EtcdDefrag; create a new EtcdDefrag to change what it defragments" +// +kubebuilder:validation:XValidation:rule="!has(self.rule) || !has(oldSelf.rule) || self.rule == oldSelf.rule",message="spec.rule is immutable; create a new EtcdDefrag to change what it defragments" type EtcdDefragSpec struct { // ClusterRef names the EtcdCluster (same namespace) to defragment. ClusterRef corev1.LocalObjectReference `json:"clusterRef"` diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcddefrags.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcddefrags.yaml index abc07b7e..9f76a0ad 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcddefrags.yaml +++ b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcddefrags.yaml @@ -57,6 +57,11 @@ spec: description: |- EtcdDefragSpec is the desired state of an EtcdDefrag: a one-shot request to defragment an EtcdCluster's members. + + clusterRef and rule are the request itself and are immutable: the controller + consults them once and never re-runs, so editing them post-create would + silently do nothing. ttlSecondsAfterFinished stays mutable — it is a + record-retention knob, not part of the request (see Job.spec.ttlSecondsAfterFinished). properties: clusterRef: description: ClusterRef names the EtcdCluster (same namespace) to @@ -144,6 +149,15 @@ spec: x-kubernetes-validations: - message: spec.clusterRef.name is required rule: size(self.clusterRef.name) != 0 + - message: spec.clusterRef is immutable; create a new EtcdDefrag to defragment + a different cluster + rule: self.clusterRef == oldSelf.clusterRef + - message: spec.rule cannot be added to or removed from an existing EtcdDefrag; + create a new EtcdDefrag to change what it defragments + rule: has(self.rule) == has(oldSelf.rule) + - message: spec.rule is immutable; create a new EtcdDefrag to change what + it defragments + rule: '!has(self.rule) || !has(oldSelf.rule) || self.rule == oldSelf.rule' status: description: EtcdDefragStatus is the observed state of an EtcdDefrag. properties: