Skip to content

Commit cf3e638

Browse files
Per G. da Silvaclaude
andcommitted
🐛 Prevent removal of ClusterObjectSet completedAt once set
The field-level CEL transition rule on status.completedAt is skipped when the field is absent from an update, so a status write that omits it would clear the timestamp and let a later reconciliation record a different one, breaking the set-once guarantee. Add a parent-level transition rule on the status that rejects removal once the field is set, and cover the removal case with a validation test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
1 parent 325e9df commit cf3e638

6 files changed

Lines changed: 44 additions & 0 deletions

File tree

‎api/v1/clusterobjectset_types.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,12 @@ const (
485485
)
486486

487487
// ClusterObjectSetStatus defines the observed state of a ClusterObjectSet.
488+
//
489+
// The completedAt removal guard lives here at the parent level because a
490+
// field-level transition rule is skipped when the field is absent from an
491+
// update, which would otherwise allow the timestamp to be cleared and re-set.
492+
//
493+
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.completedAt) || has(self.completedAt)",message="completedAt cannot be removed once set"
488494
type ClusterObjectSetStatus struct {
489495
// conditions is an optional list of status conditions describing the state of the
490496
// ClusterObjectSet.

‎api/v1/validation_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,28 @@ func TestClusterObjectSetCompletedAtImmutable(t *testing.T) {
211211
err := c.Status().Update(t.Context(), cos)
212212
require.True(t, errors.IsInvalid(err), "expected update to fail as invalid, but got: %v", err)
213213
}
214+
215+
func TestClusterObjectSetCompletedAtCannotBeRemoved(t *testing.T) {
216+
c := newClient(t)
217+
218+
cos := &ClusterObjectSet{
219+
ObjectMeta: metav1.ObjectMeta{Name: "cos-completedat-noremove"},
220+
Spec: ClusterObjectSetSpec{
221+
Revision: 1,
222+
CollisionProtection: CollisionProtectionPrevent,
223+
LifecycleState: ClusterObjectSetLifecycleStateActive,
224+
},
225+
}
226+
require.NoError(t, c.Create(t.Context(), cos))
227+
228+
// Set completedAt once.
229+
cos.Status.CompletedAt = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC))
230+
require.NoError(t, c.Status().Update(t.Context(), cos))
231+
232+
// Removing completedAt once set must be rejected. The field-level transition
233+
// rule is skipped when the field is absent from the update, so a parent-level
234+
// rule must reject its removal.
235+
cos.Status.CompletedAt = metav1.Time{}
236+
err := c.Status().Update(t.Context(), cos)
237+
require.True(t, errors.IsInvalid(err), "expected removal to fail as invalid, but got: %v", err)
238+
}

‎applyconfigurations/api/v1/clusterobjectsetstatus.go‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,9 @@ spec:
672672
- message: observedPhases is immutable
673673
rule: self == oldSelf || oldSelf.size() == 0
674674
type: object
675+
x-kubernetes-validations:
676+
- message: completedAt cannot be removed once set
677+
rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
675678
type: object
676679
served: true
677680
storage: true

‎manifests/experimental-e2e.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2028,6 +2028,9 @@ spec:
20282028
- message: observedPhases is immutable
20292029
rule: self == oldSelf || oldSelf.size() == 0
20302030
type: object
2031+
x-kubernetes-validations:
2032+
- message: completedAt cannot be removed once set
2033+
rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
20312034
type: object
20322035
served: true
20332036
storage: true

‎manifests/experimental.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,6 +1989,9 @@ spec:
19891989
- message: observedPhases is immutable
19901990
rule: self == oldSelf || oldSelf.size() == 0
19911991
type: object
1992+
x-kubernetes-validations:
1993+
- message: completedAt cannot be removed once set
1994+
rule: '!has(oldSelf.completedAt) || has(self.completedAt)'
19921995
type: object
19931996
served: true
19941997
storage: true

0 commit comments

Comments
 (0)