diff --git a/api/v1alpha1/crashlooppolicy_envtest_test.go b/api/v1alpha1/crashlooppolicy_envtest_test.go index e680dfc..9c2557e 100644 --- a/api/v1alpha1/crashlooppolicy_envtest_test.go +++ b/api/v1alpha1/crashlooppolicy_envtest_test.go @@ -309,3 +309,49 @@ func TestSampleManifestsAreValid(t *testing.T) { t.Fatalf("no sample manifests found in %s", dir) } } + +// TestStatusAcceptsAnyWorkloadKind guards against reintroducing an enum on the +// status field. The API server validates status writes, so a constrained value +// there would reject the whole update, not just the offending entry, and every +// condition and counter would silently stop being written. +func TestStatusAcceptsAnyWorkloadKind(t *testing.T) { + ctx := context.Background() + p := newPolicy("status-kind-test") + if err := k8sClient.Create(ctx, p); err != nil { + t.Fatalf("failed to create policy: %v", err) + } + t.Cleanup(func() { _ = k8sClient.Delete(ctx, p) }) + + p.Status.Phase = CrashLoopPolicyPhaseActive + p.Status.ActiveScaledDown = []ScaledDownWorkloadRef{ + {Kind: "Deployment", Namespace: "default", Name: "known"}, + {Kind: "SomeFutureKind", Namespace: "default", Name: "unknown"}, + } + if err := k8sClient.Status().Update(ctx, p); err != nil { + t.Fatalf("status write rejected for an unlisted kind: %v", err) + } + + fetched := &CrashLoopPolicy{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: "status-kind-test"}, fetched); err != nil { + t.Fatalf("failed to get policy: %v", err) + } + // The whole update must have landed, not just the recognised entry. + if len(fetched.Status.ActiveScaledDown) != 2 { + t.Errorf("expected both entries to persist, got %d", len(fetched.Status.ActiveScaledDown)) + } + if fetched.Status.Phase != CrashLoopPolicyPhaseActive { + t.Errorf("expected the rest of the status update to land, phase is %q", fetched.Status.Phase) + } +} + +// TestSpecStillRejectsUnknownTarget is the other half: the enum on the spec is +// the one that should stay, so a user typo is caught at write time. +func TestSpecStillRejectsUnknownTarget(t *testing.T) { + ctx := context.Background() + p := newPolicy("spec-target-test") + p.Spec.Targets = []string{"SomeFutureKind"} + if err := k8sClient.Create(ctx, p); err == nil { + t.Error("expected an unknown target kind to be rejected on the spec") + _ = k8sClient.Delete(ctx, p) + } +} diff --git a/api/v1alpha1/crashlooppolicy_types.go b/api/v1alpha1/crashlooppolicy_types.go index 13cd56c..15cf0f4 100644 --- a/api/v1alpha1/crashlooppolicy_types.go +++ b/api/v1alpha1/crashlooppolicy_types.go @@ -74,8 +74,11 @@ type CrashLoopPolicySpec struct { // ScaledDownWorkloadRef identifies a workload currently held at zero replicas // (or suspended, for CronJobs) by this policy. type ScaledDownWorkloadRef struct { - // Kind of the workload: Deployment, StatefulSet or CronJob. - // +kubebuilder:validation:Enum=Deployment;StatefulSet;CronJob + // Kind of the workload, for example Deployment, StatefulSet or CronJob. + // Deliberately not constrained by an enum: this is a status field, and the + // API server validates status writes, so a value outside the list would + // reject the entire status update rather than just this entry. Validate + // what users write, describe what the controller reports. Kind string `json:"kind"` // Namespace of the workload. diff --git a/charts/crashloop-operator/crds/crashloop-operator.lauger.de_crashlooppolicies.yaml b/charts/crashloop-operator/crds/crashloop-operator.lauger.de_crashlooppolicies.yaml index e3d698c..38c0786 100644 --- a/charts/crashloop-operator/crds/crashloop-operator.lauger.de_crashlooppolicies.yaml +++ b/charts/crashloop-operator/crds/crashloop-operator.lauger.de_crashlooppolicies.yaml @@ -237,12 +237,12 @@ spec: (or suspended, for CronJobs) by this policy. properties: kind: - description: 'Kind of the workload: Deployment, StatefulSet - or CronJob.' - enum: - - Deployment - - StatefulSet - - CronJob + description: |- + Kind of the workload, for example Deployment, StatefulSet or CronJob. + Deliberately not constrained by an enum: this is a status field, and the + API server validates status writes, so a value outside the list would + reject the entire status update rather than just this entry. Validate + what users write, describe what the controller reports. type: string name: description: Name of the workload. diff --git a/config/crd/bases/crashloop-operator.lauger.de_crashlooppolicies.yaml b/config/crd/bases/crashloop-operator.lauger.de_crashlooppolicies.yaml index e3d698c..38c0786 100644 --- a/config/crd/bases/crashloop-operator.lauger.de_crashlooppolicies.yaml +++ b/config/crd/bases/crashloop-operator.lauger.de_crashlooppolicies.yaml @@ -237,12 +237,12 @@ spec: (or suspended, for CronJobs) by this policy. properties: kind: - description: 'Kind of the workload: Deployment, StatefulSet - or CronJob.' - enum: - - Deployment - - StatefulSet - - CronJob + description: |- + Kind of the workload, for example Deployment, StatefulSet or CronJob. + Deliberately not constrained by an enum: this is a status field, and the + API server validates status writes, so a value outside the list would + reject the entire status update rather than just this entry. Validate + what users write, describe what the controller reports. type: string name: description: Name of the workload.