diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index 4c8028a07a..f8e9059ebd 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -517,6 +517,18 @@ type ClusterObjectSetStatus struct { // +optional Conditions []metav1.Condition `json:"conditions,omitempty"` + // resolvedContentHash is a SHA-256 hash of all resolved phase object + // content (inline objects and objectRef-resolved objects, in phase order). + // Set once on the first successful resolution and never changed. Used to + // detect content substitution. + // + // +optional + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=256 + // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0",message="resolvedContentHash is immutable" + // +kubebuilder:validation:XValidation:rule=`self.matches('^[a-z0-9]+:[a-f0-9]+$')`,message="digest must be in the format ':'" + ResolvedContentHash string `json:"resolvedContentHash,omitempty"` + // objectCounts reports aggregate object counts across all phases. // Values are sums of the per-phase counts in observedPhases. // +optional diff --git a/applyconfigurations/api/v1/clusterobjectsetstatus.go b/applyconfigurations/api/v1/clusterobjectsetstatus.go index 906da18177..a28aa26210 100644 --- a/applyconfigurations/api/v1/clusterobjectsetstatus.go +++ b/applyconfigurations/api/v1/clusterobjectsetstatus.go @@ -51,6 +51,11 @@ type ClusterObjectSetStatusApplyConfiguration struct { // // Rollout completion is recorded separately by status.completedAt. Conditions []metav1.ConditionApplyConfiguration `json:"conditions,omitempty"` + // resolvedContentHash is a SHA-256 hash of all resolved phase object + // content (inline objects and objectRef-resolved objects, in phase order). + // Set once on the first successful resolution and never changed. Used to + // detect content substitution. + ResolvedContentHash *string `json:"resolvedContentHash,omitempty"` // objectCounts reports aggregate object counts across all phases. // Values are sums of the per-phase counts in observedPhases. ObjectCounts *ObjectCountsApplyConfiguration `json:"objectCounts,omitempty"` @@ -85,6 +90,14 @@ func (b *ClusterObjectSetStatusApplyConfiguration) WithConditions(values ...*met return b } +// WithResolvedContentHash sets the ResolvedContentHash field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ResolvedContentHash field is set to the value of the last call. +func (b *ClusterObjectSetStatusApplyConfiguration) WithResolvedContentHash(value string) *ClusterObjectSetStatusApplyConfiguration { + b.ResolvedContentHash = &value + return b +} + // WithObjectCounts sets the ObjectCounts field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the ObjectCounts field is set to the value of the last call. diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index 12d01d336f..86fb4ee9d7 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -350,6 +350,9 @@ var schemaYAML = typed.YAMLObject(`types: elementRelationship: associative keys: - name + - name: resolvedContentHash + type: + scalar: string - name: com.github.operator-framework.operator-controller.api.v1.CollisionProtection scalar: string - name: com.github.operator-framework.operator-controller.api.v1.ConditionEqualProbe diff --git a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml index dec8cd06eb..815457ebc2 100644 --- a/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml +++ b/helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectsets.yaml @@ -749,6 +749,20 @@ spec: x-kubernetes-list-map-keys: - name x-kubernetes-list-type: map + resolvedContentHash: + description: |- + resolvedContentHash is a SHA-256 hash of all resolved phase object + content (inline objects and objectRef-resolved objects, in phase order). + Set once on the first successful resolution and never changed. Used to + detect content substitution. + maxLength: 256 + minLength: 1 + type: string + x-kubernetes-validations: + - message: resolvedContentHash is immutable + rule: self == oldSelf || oldSelf.size() == 0 + - message: digest must be in the format ':' + rule: self.matches('^[a-z0-9]+:[a-f0-9]+$') type: object x-kubernetes-validations: - message: completedAt cannot be removed once set diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index 9a066a6b84..5e1e79da71 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -155,7 +155,7 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl return ctrl.Result{}, fmt.Errorf("verifying referenced Secrets: %v", err) } - phases, currentPhases, opts, err := c.buildBoxcutterPhases(ctx, cos) + phases, currentPhases, resolvedContentHash, opts, err := c.buildBoxcutterPhases(ctx, cos) if err != nil { var contentErr *referencedContentError if errors.As(err, &contentErr) { @@ -188,6 +188,12 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl return ctrl.Result{RequeueAfter: validationRetryInterval}, nil } + if len(cos.Spec.Phases) > 0 && cos.Status.ResolvedContentHash != "" && resolvedContentHash != cos.Status.ResolvedContentHash { + l.Error(fmt.Errorf("resolved content hash mismatch"), "resolved content changed since first reconciliation, marking revision not ready") + markAsNotReady(cos, ocv1.ClusterObjectSetReasonPreconditionsNotMet, fmt.Sprintf("resolved content hash mismatch: expected %s, got %s", cos.Status.ResolvedContentHash, resolvedContentHash)) + return ctrl.Result{RequeueAfter: validationRetryInterval}, nil + } + revisionEngine, err := c.RevisionEngineFactory.CreateRevisionEngine(ctx, cos) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to create revision engine: %v", err) @@ -216,6 +222,10 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl return ctrl.Result{}, fmt.Errorf("establish watch: %v", err) } + if len(cos.Spec.Phases) > 0 && cos.Status.ResolvedContentHash == "" { + cos.Status.ResolvedContentHash = resolvedContentHash + } + rres, err := revisionEngine.Reconcile(ctx, revision, opts...) if err != nil { if rres != nil { @@ -567,10 +577,10 @@ type configValidationError struct{ error } // so the revision can recover without a controller restart. type referencedContentError struct{ error } -func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, cos *ocv1.ClusterObjectSet) ([]boxcutter.Phase, []ocv1.ObservedPhase, []boxcutter.RevisionReconcileOption, error) { +func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, cos *ocv1.ClusterObjectSet) ([]boxcutter.Phase, []ocv1.ObservedPhase, string, []boxcutter.RevisionReconcileOption, error) { siblings, err := c.listSiblingRevisions(ctx, cos) if err != nil { - return nil, nil, nil, fmt.Errorf("listing sibling revisions: %w", err) + return nil, nil, "", nil, fmt.Errorf("listing sibling revisions: %w", err) } siblingObjs := make([]client.Object, len(siblings)) @@ -580,7 +590,7 @@ func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, c progressionProbes, err := buildProgressionProbes(cos.Spec.ProgressionProbes) if err != nil { - return nil, nil, nil, err + return nil, nil, "", nil, err } opts := []boxcutter.RevisionReconcileOption{ @@ -592,6 +602,7 @@ func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, c phases := make([]boxcutter.Phase, 0, len(cos.Spec.Phases)) observedPhases := make([]ocv1.ObservedPhase, 0, len(cos.Spec.Phases)) + aggregateHash := sha256.New() for _, specPhase := range cos.Spec.Phases { objs := make([]client.Object, 0, len(specPhase.Objects)) for _, specObj := range specPhase.Objects { @@ -602,20 +613,23 @@ func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, c case specObj.Ref.Name != "": resolved, err := c.resolveObjectRef(ctx, specObj.Ref) if err != nil { - return nil, nil, nil, fmt.Errorf("resolving ref in phase %q: %w", specPhase.Name, err) + return nil, nil, "", nil, fmt.Errorf("resolving ref in phase %q: %w", specPhase.Name, err) } obj = resolved default: - return nil, nil, nil, &configValidationError{fmt.Errorf("an object in phase %q has neither an inline object nor a Secret reference", specPhase.Name)} + return nil, nil, "", nil, &configValidationError{fmt.Errorf("an object in phase %q has neither an inline object nor a Secret reference", specPhase.Name)} } - objs = append(objs, obj) } - // Compute digest from the user-provided objects before controller mutations. + // computePhaseDigest marshals the full phase content (name + objects) and returns + // a sha256 digest. Feed that digest into aggregateHash to cover all phases. digest, err := computePhaseDigest(specPhase.Name, objs) if err != nil { - return nil, nil, nil, fmt.Errorf("computing phase digest: %w", err) + return nil, nil, "", nil, fmt.Errorf("computing phase digest: %w", err) + } + if _, err := aggregateHash.Write([]byte(digest)); err != nil { + return nil, nil, "", nil, fmt.Errorf("hashing phase %q digest: %w", specPhase.Name, err) } observedPhases = append(observedPhases, ocv1.ObservedPhase{Name: specPhase.Name, Digest: digest}) @@ -637,7 +651,7 @@ func (c *ClusterObjectSetReconciler) buildBoxcutterPhases(ctx context.Context, c phases = append(phases, boxcutter.NewPhase(specPhase.Name, objs)) } - return phases, observedPhases, opts, nil + return phases, observedPhases, fmt.Sprintf("sha256:%x", aggregateHash.Sum(nil)), opts, nil } // resolveObjectRef fetches the referenced Secret, reads the value at the specified key, diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index fabb88b1b6..583e64a5c7 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -1054,6 +1054,54 @@ func Test_ClusterObjectSetReconciler_Reconcile_ReferencedContentValidationRecove require.Equal(t, ocv1.ClusterObjectSetReasonAllObjectsReady, cond.Reason) } +func Test_ClusterObjectSetReconciler_Reconcile_HashMismatch(t *testing.T) { + const clusterObjectSetName = "test-ext-1" + + testScheme := newScheme(t) + mockCtrl := gomock.NewController(t) + + ext := newTestClusterExtension() + rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) + // Pre-set a hash in status that won't match the hash computed from the + // inline phase content, triggering a mismatch error. + rev1.Status.ResolvedContentHash = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + testClient := fake.NewClientBuilder(). + WithScheme(testScheme). + WithStatusSubresource(&ocv1.ClusterObjectSet{}). + WithObjects(ext, rev1). + Build() + + mockEngine := newMockRevisionEngineWithReconcile(mockCtrl, + func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) { + return newMockRevisionResult(mockCtrl, revisionResultConfig{}), nil + }, nil, + ) + + result, err := (&controllers.ClusterObjectSetReconciler{ + Client: testClient, + RevisionEngineFactory: newMockRevisionEngineFactoryWithEngine(mockCtrl, mockEngine, nil), + TrackingCache: newMockTrackingCache(mockCtrl, testClient, nil), + }).Reconcile(t.Context(), ctrl.Request{ + NamespacedName: types.NamespacedName{Name: clusterObjectSetName}, + }) + + require.Equal(t, ctrl.Result{RequeueAfter: 10 * time.Second}, result) + require.NoError(t, err) + + // The controller must surface a ValidationFailure condition so the Ready + // status reflects the mismatch rather than being stale from the last + // successful reconcile. + updated := &ocv1.ClusterObjectSet{} + require.NoError(t, testClient.Get(t.Context(), types.NamespacedName{Name: clusterObjectSetName}, updated)) + cond := meta.FindStatusCondition(updated.Status.Conditions, ocv1.ClusterObjectSetTypeReady) + require.NotNil(t, cond) + require.Equal(t, metav1.ConditionFalse, cond.Status) + require.Equal(t, ocv1.ClusterObjectSetReasonPreconditionsNotMet, cond.Reason) + require.Contains(t, cond.Message, "resolved content hash mismatch") + require.Contains(t, cond.Message, "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") +} + func Test_ClusterObjectSetReconciler_Reconcile_BuildPhasesErrorClassification(t *testing.T) { const clusterObjectSetName = "test-ext-1" diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 062ec31cb7..f60af84cc2 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -2160,6 +2160,20 @@ spec: x-kubernetes-list-map-keys: - name x-kubernetes-list-type: map + resolvedContentHash: + description: |- + resolvedContentHash is a SHA-256 hash of all resolved phase object + content (inline objects and objectRef-resolved objects, in phase order). + Set once on the first successful resolution and never changed. Used to + detect content substitution. + maxLength: 256 + minLength: 1 + type: string + x-kubernetes-validations: + - message: resolvedContentHash is immutable + rule: self == oldSelf || oldSelf.size() == 0 + - message: digest must be in the format ':' + rule: self.matches('^[a-z0-9]+:[a-f0-9]+$') type: object x-kubernetes-validations: - message: completedAt cannot be removed once set diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index cd9328ce7d..83bc851163 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -2121,6 +2121,20 @@ spec: x-kubernetes-list-map-keys: - name x-kubernetes-list-type: map + resolvedContentHash: + description: |- + resolvedContentHash is a SHA-256 hash of all resolved phase object + content (inline objects and objectRef-resolved objects, in phase order). + Set once on the first successful resolution and never changed. Used to + detect content substitution. + maxLength: 256 + minLength: 1 + type: string + x-kubernetes-validations: + - message: resolvedContentHash is immutable + rule: self == oldSelf || oldSelf.size() == 0 + - message: digest must be in the format ':' + rule: self.matches('^[a-z0-9]+:[a-f0-9]+$') type: object x-kubernetes-validations: - message: completedAt cannot be removed once set