diff --git a/api/v1/clusterobjectset_types.go b/api/v1/clusterobjectset_types.go index 57cab880ff..ab9b5f6eae 100644 --- a/api/v1/clusterobjectset_types.go +++ b/api/v1/clusterobjectset_types.go @@ -39,6 +39,18 @@ const ( // ClusterObjectSetSpec defines the desired state of ClusterObjectSet. type ClusterObjectSetSpec struct { + // group is a required, immutable identifier that links related revisions together. + // All revisions sharing the same group form an ordered sequence, regardless of owner references. + // The value must be 1 to 52 characters long, start with a lowercase letter, + // contain only lowercase letters, digits or hyphens, and end with a letter or digit. + // + // +required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=52 + // +kubebuilder:validation:XValidation:rule=`self.matches('^[a-z]([a-z0-9-]*[a-z0-9])?$')`,message="group must start with a lowercase letter, contain only lowercase letters, digits or hyphens, and end with a letter or digit" + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="group is immutable" + Group string `json:"group,omitempty"` + // lifecycleState specifies the lifecycle state of the ClusterObjectSet. // // When set to "Active", the revision is actively managed and reconciled. @@ -57,11 +69,11 @@ type ClusterObjectSetSpec struct { // +kubebuilder:validation:XValidation:rule="oldSelf == 'Active' || oldSelf == 'Archived' && oldSelf == self", message="cannot un-archive" LifecycleState ClusterObjectSetLifecycleState `json:"lifecycleState,omitempty"` - // revision is a required, immutable sequence number representing a specific revision - // of the parent ClusterExtension. + // revision is a required, immutable sequence number identifying a specific + // ClusterObjectSet within a sequence of related revisions. // // The revision field must be a positive integer. - // Each ClusterObjectSet belonging to the same parent ClusterExtension must have a unique revision number. + // Each ClusterObjectSet in the same revision sequence must have a unique revision number. // The revision number must always be the previous revision number plus one, or 1 for the first revision. // // +required @@ -561,15 +573,17 @@ type ObservedPhase struct { // +kubebuilder:object:root=true // +kubebuilder:resource:scope=Cluster // +kubebuilder:subresource:status +// +kubebuilder:selectablefield:JSONPath=`.spec.group` +// +kubebuilder:printcolumn:name="Group",type=string,JSONPath=`.spec.group` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Ready')].status` // +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp` -// ClusterObjectSet represents an immutable snapshot of Kubernetes objects -// for a specific version of a ClusterExtension. Each revision contains objects -// organized into phases that roll out sequentially. The same object can only be managed by a single revision -// at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded -// or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for -// posterity. +// ClusterObjectSet represents an immutable snapshot of Kubernetes objects to +// apply and manage on the cluster. Each revision contains objects organized into +// phases that roll out sequentially. The same object can only be managed by a +// single revision at a time. Ownership of objects is transitioned from one revision +// to the next as new revisions are rolled out. Once the latest revision has rolled +// out successfully, previous active revisions are archived for posterity. type ClusterObjectSet struct { metav1.TypeMeta `json:",inline"` @@ -579,8 +593,8 @@ type ClusterObjectSet struct { metav1.ObjectMeta `json:"metadata,omitempty"` // spec defines the desired state of the ClusterObjectSet. - // +optional - Spec ClusterObjectSetSpec `json:"spec,omitempty"` + // +required + Spec ClusterObjectSetSpec `json:"spec,omitzero"` // status is optional and defines the observed state of the ClusterObjectSet. // +optional diff --git a/api/v1/clusterobjectset_types_test.go b/api/v1/clusterobjectset_types_test.go index 6770fbb129..0a864cec48 100644 --- a/api/v1/clusterobjectset_types_test.go +++ b/api/v1/clusterobjectset_types_test.go @@ -3,12 +3,15 @@ package v1 import ( "context" "fmt" + "strings" "testing" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" ) func TestClusterObjectSetImmutability(t *testing.T) { @@ -16,12 +19,51 @@ func TestClusterObjectSetImmutability(t *testing.T) { ctx := context.Background() i := 0 for name, tc := range map[string]struct { - spec ClusterObjectSetSpec - updateFunc func(*ClusterObjectSet) - allowed bool + spec ClusterObjectSetSpec + updateFunc func(*ClusterObjectSet) + allowed bool + expectedError string }{ + "group is immutable": { + spec: ClusterObjectSetSpec{ + Group: "test-group", + LifecycleState: ClusterObjectSetLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + }, + updateFunc: func(cos *ClusterObjectSet) { + cos.Spec.Group = "another-group" + }, + expectedError: "group is immutable", + }, + "group cannot be cleared": { + spec: ClusterObjectSetSpec{ + Group: "test-group", + LifecycleState: ClusterObjectSetLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + }, + updateFunc: func(cos *ClusterObjectSet) { + cos.Spec.Group = "" + }, + expectedError: "spec.group: Required value", + }, + "unchanged group permits lifecycle update": { + spec: ClusterObjectSetSpec{ + Group: "test-group", + LifecycleState: ClusterObjectSetLifecycleStateActive, + Revision: 1, + CollisionProtection: CollisionProtectionPrevent, + }, + updateFunc: func(cos *ClusterObjectSet) { + cos.Spec.Group = "test-group" + cos.Spec.LifecycleState = ClusterObjectSetLifecycleStateArchived + }, + allowed: true, + }, "revision is immutable": { spec: ClusterObjectSetSpec{ + Group: "test-group", LifecycleState: ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: CollisionProtectionPrevent, @@ -32,6 +74,7 @@ func TestClusterObjectSetImmutability(t *testing.T) { }, "phases may be initially empty": { spec: ClusterObjectSetSpec{ + Group: "test-group", LifecycleState: ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: CollisionProtectionPrevent, @@ -49,6 +92,7 @@ func TestClusterObjectSetImmutability(t *testing.T) { }, "phases may be initially unset": { spec: ClusterObjectSetSpec{ + Group: "test-group", LifecycleState: ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: CollisionProtectionPrevent, @@ -65,6 +109,7 @@ func TestClusterObjectSetImmutability(t *testing.T) { }, "phases are immutable if not empty": { spec: ClusterObjectSetSpec{ + Group: "test-group", LifecycleState: ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: CollisionProtectionPrevent, @@ -86,6 +131,7 @@ func TestClusterObjectSetImmutability(t *testing.T) { }, "spec collisionProtection is immutable": { spec: ClusterObjectSetSpec{ + Group: "test-group", LifecycleState: ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: CollisionProtectionPrevent, @@ -112,6 +158,9 @@ func TestClusterObjectSetImmutability(t *testing.T) { if !tc.allowed && !errors.IsInvalid(err) { t.Fatal("expected update to fail due to invalid payload, but got:", err) } + if tc.expectedError != "" { + require.ErrorContains(t, err, tc.expectedError) + } }) } } @@ -351,6 +400,7 @@ func TestClusterObjectSetValidity(t *testing.T) { }, Spec: tc.spec, } + cos.Spec.Group = "test-group" i = i + 1 err := c.Create(ctx, cos) if tc.valid && err != nil { @@ -363,6 +413,109 @@ func TestClusterObjectSetValidity(t *testing.T) { } } +func TestClusterObjectSetSpecValidation(t *testing.T) { + c := newClient(t) + t.Run("missing spec", func(t *testing.T) { + cos := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": GroupVersion.String(), + "kind": ClusterObjectSetKind, + "metadata": map[string]any{"generateName": "spec-validation-"}, + }} + err := c.Create(t.Context(), cos) + require.True(t, errors.IsInvalid(err), "%v", err) + require.ErrorContains(t, err, "spec: Required") + }) +} + +func TestClusterObjectSetGroupValidation(t *testing.T) { + c := newClient(t) + for _, tc := range []struct { + name string + group *string + valid bool + }{ + {name: "missing group", valid: false}, + {name: "empty group", group: ptr.To(""), valid: false}, + {name: "one character", group: ptr.To("a"), valid: true}, + {name: "lowercase hyphens and digits", group: ptr.To("my-group-1"), valid: true}, + {name: "maximum length", group: ptr.To(strings.Repeat("a", 52)), valid: true}, + {name: "over maximum length", group: ptr.To(strings.Repeat("a", 53)), valid: false}, + {name: "starts with digit", group: ptr.To("1group"), valid: false}, + {name: "starts with hyphen", group: ptr.To("-group"), valid: false}, + {name: "ends with hyphen", group: ptr.To("group-"), valid: false}, + {name: "uppercase", group: ptr.To("Group"), valid: false}, + {name: "underscore", group: ptr.To("my_group"), valid: false}, + {name: "dot", group: ptr.To("my.group"), valid: false}, + } { + t.Run(tc.name, func(t *testing.T) { + cos := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": GroupVersion.String(), + "kind": ClusterObjectSetKind, + "metadata": map[string]any{"generateName": "group-validation-"}, + }} + spec := map[string]any{ + "revision": int64(1), "lifecycleState": string(ClusterObjectSetLifecycleStateActive), + "collisionProtection": string(CollisionProtectionPrevent), + } + if tc.group != nil { + spec["group"] = *tc.group + } + cos.Object["spec"] = spec + err := c.Create(t.Context(), cos) + if tc.valid { + require.NoError(t, err) + } else { + require.True(t, errors.IsInvalid(err), "%v", err) + require.ErrorContains(t, err, "spec.group") + } + }) + } +} + +func TestClusterObjectSetGroupFieldSelector(t *testing.T) { + // Use an uncached client so filtering is performed by the API server. + c := newClient(t) + for _, fixture := range []struct { + name string + group string + revision int64 + }{ + {name: "field-selector-a-1", group: "field-selector-a", revision: 1}, + {name: "field-selector-a-2", group: "field-selector-a", revision: 2}, + {name: "field-selector-b-1", group: "field-selector-b", revision: 1}, + } { + cos := &ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: fixture.name}, + Spec: ClusterObjectSetSpec{ + Group: fixture.group, + Revision: fixture.revision, + LifecycleState: ClusterObjectSetLifecycleStateActive, + CollisionProtection: CollisionProtectionPrevent, + }, + } + require.NoError(t, c.Create(t.Context(), cos)) + } + + for _, tc := range []struct { + group string + want []string + }{ + {group: "field-selector-a", want: []string{"field-selector-a-1", "field-selector-a-2"}}, + {group: "field-selector-b", want: []string{"field-selector-b-1"}}, + {group: "field-selector-missing"}, + } { + t.Run(tc.group, func(t *testing.T) { + var list ClusterObjectSetList + require.NoError(t, c.List(t.Context(), &list, client.MatchingFields{"spec.group": tc.group})) + names := make([]string, 0, len(list.Items)) + for _, cos := range list.Items { + names = append(names, cos.Name) + } + require.ElementsMatch(t, tc.want, names) + }) + } +} + func configMap() unstructured.Unstructured { return unstructured.Unstructured{ Object: map[string]interface{}{ diff --git a/api/v1/validation_test.go b/api/v1/validation_test.go index 2c77c17663..60453733b6 100644 --- a/api/v1/validation_test.go +++ b/api/v1/validation_test.go @@ -34,6 +34,7 @@ func TestValidate(t *testing.T) { return s } defaultRevisionSpec := func(s *ClusterObjectSetSpec) *ClusterObjectSetSpec { + s.Group = "test-group" s.Revision = 1 s.CollisionProtection = CollisionProtectionPrevent return s @@ -188,6 +189,7 @@ func TestClusterObjectSetCompletedAtImmutable(t *testing.T) { cos := &ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{Name: "cos-completedat-immutable"}, Spec: ClusterObjectSetSpec{ + Group: "test-group", Revision: 1, CollisionProtection: CollisionProtectionPrevent, LifecycleState: ClusterObjectSetLifecycleStateActive, @@ -218,6 +220,7 @@ func TestClusterObjectSetCompletedAtCannotBeRemoved(t *testing.T) { cos := &ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{Name: "cos-completedat-noremove"}, Spec: ClusterObjectSetSpec{ + Group: "test-group", Revision: 1, CollisionProtection: CollisionProtectionPrevent, LifecycleState: ClusterObjectSetLifecycleStateActive, diff --git a/applyconfigurations/api/v1/clusterobjectset.go b/applyconfigurations/api/v1/clusterobjectset.go index 040648390d..57a89c5464 100644 --- a/applyconfigurations/api/v1/clusterobjectset.go +++ b/applyconfigurations/api/v1/clusterobjectset.go @@ -30,12 +30,12 @@ import ( // ClusterObjectSetApplyConfiguration represents a declarative configuration of the ClusterObjectSet type for use // with apply. // -// ClusterObjectSet represents an immutable snapshot of Kubernetes objects -// for a specific version of a ClusterExtension. Each revision contains objects -// organized into phases that roll out sequentially. The same object can only be managed by a single revision -// at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded -// or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for -// posterity. +// ClusterObjectSet represents an immutable snapshot of Kubernetes objects to +// apply and manage on the cluster. Each revision contains objects organized into +// phases that roll out sequentially. The same object can only be managed by a +// single revision at a time. Ownership of objects is transitioned from one revision +// to the next as new revisions are rolled out. Once the latest revision has rolled +// out successfully, previous active revisions are archived for posterity. type ClusterObjectSetApplyConfiguration struct { metav1.TypeMetaApplyConfiguration `json:",inline"` // metadata is the standard object's metadata. diff --git a/applyconfigurations/api/v1/clusterobjectsetspec.go b/applyconfigurations/api/v1/clusterobjectsetspec.go index afba3d87d4..4e12c1bbc8 100644 --- a/applyconfigurations/api/v1/clusterobjectsetspec.go +++ b/applyconfigurations/api/v1/clusterobjectsetspec.go @@ -27,6 +27,11 @@ import ( // // ClusterObjectSetSpec defines the desired state of ClusterObjectSet. type ClusterObjectSetSpecApplyConfiguration struct { + // group is a required, immutable identifier that links related revisions together. + // All revisions sharing the same group form an ordered sequence, regardless of owner references. + // The value must be 1 to 52 characters long, start with a lowercase letter, + // contain only lowercase letters, digits or hyphens, and end with a letter or digit. + Group *string `json:"group,omitempty"` // lifecycleState specifies the lifecycle state of the ClusterObjectSet. // // When set to "Active", the revision is actively managed and reconciled. @@ -40,11 +45,11 @@ type ClusterObjectSetSpecApplyConfiguration struct { // moving from one revision to another. The old revision will not be set to "Archived" until the // new revision has been completely rolled out. LifecycleState *apiv1.ClusterObjectSetLifecycleState `json:"lifecycleState,omitempty"` - // revision is a required, immutable sequence number representing a specific revision - // of the parent ClusterExtension. + // revision is a required, immutable sequence number identifying a specific + // ClusterObjectSet within a sequence of related revisions. // // The revision field must be a positive integer. - // Each ClusterObjectSet belonging to the same parent ClusterExtension must have a unique revision number. + // Each ClusterObjectSet in the same revision sequence must have a unique revision number. // The revision number must always be the previous revision number plus one, or 1 for the first revision. Revision *int64 `json:"revision,omitempty"` // phases is an optional, immutable list of phases that group objects to be applied together. @@ -102,6 +107,14 @@ func ClusterObjectSetSpec() *ClusterObjectSetSpecApplyConfiguration { return &ClusterObjectSetSpecApplyConfiguration{} } +// WithGroup sets the Group 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 Group field is set to the value of the last call. +func (b *ClusterObjectSetSpecApplyConfiguration) WithGroup(value string) *ClusterObjectSetSpecApplyConfiguration { + b.Group = &value + return b +} + // WithLifecycleState sets the LifecycleState 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 LifecycleState field is set to the value of the last call. diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index 1da3b4c1a6..bca0675a8e 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -302,6 +302,9 @@ var schemaYAML = typed.YAMLObject(`types: - name: collisionProtection type: namedType: com.github.operator-framework.operator-controller.api.v1.CollisionProtection + - name: group + type: + scalar: string - name: lifecycleState type: namedType: com.github.operator-framework.operator-controller.api.v1.ClusterObjectSetLifecycleState diff --git a/cmd/object-controller/main.go b/cmd/object-controller/main.go index a6ceec21ee..f6616d6cbf 100644 --- a/cmd/object-controller/main.go +++ b/cmd/object-controller/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "context" "crypto/tls" "flag" "fmt" @@ -181,10 +182,12 @@ func newManager(cfg *config, restConfig *rest.Config) (manager.Manager, error) { if err != nil { return nil, fmt.Errorf("creating discovery client: %w", err) } - // Keep the field owner prefix unchanged so existing objects can be reconciled after migration. + if err := controllers.SetupIndexes(context.Background(), mgr.GetFieldIndexer()); err != nil { + return nil, fmt.Errorf("indexing ClusterObjectSet group: %w", err) + } factory, err := controllers.NewDefaultRevisionEngineFactory( mgr.GetScheme(), trackingCache, memory.NewMemCacheClient(discoveryClient), - mgr.GetRESTMapper(), "olm.operatorframework.io", mgr.GetConfig(), + mgr.GetRESTMapper(), mgr.GetConfig(), ) if err != nil { return nil, fmt.Errorf("creating revision engine factory: %w", err) diff --git a/cmd/object-controller/main_test.go b/cmd/object-controller/main_test.go index 9453967b35..b82f730028 100644 --- a/cmd/object-controller/main_test.go +++ b/cmd/object-controller/main_test.go @@ -116,6 +116,7 @@ func TestStandaloneController(t *testing.T) { cos := &ocv1.ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{Name: name}, Spec: ocv1.ClusterObjectSetSpec{ + Group: name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: ocv1.CollisionProtectionPrevent, Phases: []ocv1.ClusterObjectSetPhase{{Name: "deploy", Objects: []ocv1.ClusterObjectSetObject{obj}}}, diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index dfae2b6a97..28547cec4d 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -45,19 +45,17 @@ import ( crcache "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/certwatcher" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" - crhandler "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "sigs.k8s.io/controller-runtime/pkg/predicate" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" + clusterobjctrl "github.com/operator-framework/operator-controller/internal/object-controller/controllers" "github.com/operator-framework/operator-controller/internal/operator-controller/action" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/cache" @@ -419,6 +417,13 @@ func run() error { } cl := mgr.GetClient() + // TODO(COD): Remove this registration when COD replaces this manager's + // direct ClusterExtension-to-COS queries. + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + if err := clusterobjctrl.SetupIndexes(context.Background(), mgr.GetFieldIndexer()); err != nil { + return fmt.Errorf("indexing ClusterObjectSet group: %w", err) + } + } catalogsCachePath := filepath.Join(cfg.cachePath, "catalogs") if err := os.MkdirAll(catalogsCachePath, 0700); err != nil { @@ -457,10 +462,7 @@ func run() error { } var trackingCache managedcache.TrackingCache - var ctrlBuilderOpts []controllers.ControllerBuilderOption - if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { - ctrlBuilderOpts = append(ctrlBuilderOpts, controllers.WithOwns(&ocv1.ClusterObjectSet{})) - } else { + if !features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { trackingCache, err = managedcache.NewTrackingCache( ctrl.Log.WithName("trackingCache"), mgr.GetConfig(), @@ -472,21 +474,16 @@ func run() error { if err := mgr.Add(trackingCache); err != nil { return fmt.Errorf("unable to add tracking cache to manager: %w", err) } - ctrlBuilderOpts = append(ctrlBuilderOpts, controllers.WithWatchesRawSource( - trackingCache.Source( - crhandler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &ocv1.ClusterExtension{}), - predicate.ResourceVersionChangedPredicate{}, - predicate.Funcs{ - CreateFunc: func(event.TypedCreateEvent[client.Object]) bool { return false }, - }, - ), - )) } ceReconciler := &controllers.ClusterExtensionReconciler{ Client: cl, } - _, err = ceReconciler.SetupWithManager(mgr, ctrlBuilderOpts...) + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { + _, err = ceReconciler.SetupWithManagerForBoxcutter(mgr) + } else { + _, err = ceReconciler.SetupWithManagerForHelm(mgr, trackingCache) + } if err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension") return err diff --git a/docs/draft/concepts/clusterobjectsets.md b/docs/draft/concepts/clusterobjectsets.md index 86158d145f..5952c6561b 100644 --- a/docs/draft/concepts/clusterobjectsets.md +++ b/docs/draft/concepts/clusterobjectsets.md @@ -6,10 +6,11 @@ This document explains what ClusterObjectSets are, how they enable safe phased r A `ClusterObjectSet` is a cluster-scoped Kubernetes API that represents a versioned set of Kubernetes resources organized into ordered phases. It provides a declarative way to roll out a group of related resources sequentially, with built-in readiness checks between phases. -The revision content — the `revision` number, `collisionProtection` strategy, and `phases` (including all objects within them) — is immutable once set. This guarantees that the record of what was deployed at a given revision cannot change after creation. Other fields like `lifecycleState` can change over the object's lifecycle (e.g. transitioning from `Active` to `Archived`), and optional fields like `progressDeadlineMinutes` and `progressionProbes` can be configured independently. +The revision content — the `group` identifier, `revision` number, `collisionProtection` strategy, and `phases` (including all objects within them) — is immutable once set. This guarantees that the record of what was deployed at a given revision cannot change after creation. Other fields like `lifecycleState` can change over the object's lifecycle (e.g. transitioning from `Active` to `Archived`), and optional fields like `progressDeadlineMinutes` and `progressionProbes` can be configured independently. Each ClusterObjectSet has: +- A **group** — a required, immutable identifier linking revisions into one sequence, regardless of their owner references - A **revision number** — an immutable, sequential integer that identifies the version - A **lifecycle state** — either `Active` (being reconciled) or `Archived` (inactive); transitions from Active to Archived but not back - A list of **phases** — immutable, ordered groups of Kubernetes objects that are applied sequentially @@ -18,6 +19,13 @@ Each ClusterObjectSet has: ClusterObjectSets can be used by any controller or system that needs to manage the rollout of a set of Kubernetes resources in a controlled, phased manner. Within OLM, the operator-controller uses ClusterObjectSets as the mechanism to deploy and upgrade ClusterExtensions. +The `group` value must be 1–52 characters long, start with a lowercase letter, +contain only lowercase letters, digits or hyphens, and end with a letter or digit. +Revisions in the same group can transfer managed objects between them even when +they have different controller owners or no owner references. They share the +server-side apply field manager `cos-group/`. Different groups remain +subject to normal object collision protection. + ### Controller deployment The experimental `object-controller` runs in its own Deployment with its own service @@ -85,7 +93,7 @@ When a ClusterObjectSet is `Active`, the controller actively reconciles it using When transitioning from one revision to the next: -1. A new ClusterObjectSet is created with the next sequential revision number +1. A new ClusterObjectSet is created with the same group and the next sequential revision number 2. Both old and new revisions remain `Active` during the transition 3. Objects transition ownership from the old revision to the new one 4. Once the new revision reports `Succeeded`, the old revision can be archived @@ -256,6 +264,10 @@ When a ClusterExtension is upgraded to a new version: A `ClusterExtension` owns one or more `ClusterObjectSet` resources. The ClusterExtension's `.status.activeRevisions` field lists all currently active (non-archived) revisions along with their status conditions. +Operator-controller sets `spec.group` to the ClusterExtension's name and retains +the ClusterExtension controller owner reference for garbage collection. Revision +lookup uses the group; labels and owner references do not determine membership. + When created by operator-controller, ClusterObjectSets carry labels that identify the parent: ```yaml @@ -279,6 +291,7 @@ kind: ClusterObjectSet metadata: name: my-app-rev1 spec: + group: my-app revision: 1 lifecycleState: Active collisionProtection: Prevent @@ -386,6 +399,7 @@ kind: ClusterObjectSet metadata: name: my-app-rev1 spec: + group: my-app revision: 1 lifecycleState: Active collisionProtection: Prevent @@ -422,8 +436,8 @@ Multiple objects can reference different keys within the same Secret, allowing e # List all ClusterObjectSets kubectl get clusterobjectsets -# List revisions for a specific extension -kubectl get clusterobjectsets -l olm.operatorframework.io/owner-name=my-extension +# List revisions in a specific extension's group +kubectl get clusterobjectsets --field-selector spec.group=my-extension # View full details for a specific revision kubectl get clusterobjectset -o yaml @@ -435,7 +449,7 @@ kubectl -n olmv1-system logs deployment/object-controller-controller-manager Example output: ``` -NAME READY AGE -my-extension-abc12 Unknown 2d -my-extension-def34 True 1h +NAME GROUP READY AGE +my-extension-abc12 my-extension Unknown 2d +my-extension-def34 my-extension True 1h ``` diff --git a/docs/draft/concepts/large-bundle-support.md b/docs/draft/concepts/large-bundle-support.md index 554e2a5b8f..f78231c934 100644 --- a/docs/draft/concepts/large-bundle-support.md +++ b/docs/draft/concepts/large-bundle-support.md @@ -207,6 +207,7 @@ kind: ClusterObjectSet metadata: name: my-extension-1 spec: + group: my-extension revision: 1 lifecycleState: Active collisionProtection: Prevent @@ -298,6 +299,7 @@ kind: ClusterObjectSet metadata: name: my-extension-1 spec: + group: my-extension revision: 1 lifecycleState: Active collisionProtection: Prevent 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 0df374be97..aae0408370 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 @@ -16,6 +16,9 @@ spec: scope: Cluster versions: - additionalPrinterColumns: + - jsonPath: .spec.group + name: Group + type: string - jsonPath: .status.conditions[?(@.type=='Ready')].status name: Ready type: string @@ -26,12 +29,12 @@ spec: schema: openAPIV3Schema: description: |- - ClusterObjectSet represents an immutable snapshot of Kubernetes objects - for a specific version of a ClusterExtension. Each revision contains objects - organized into phases that roll out sequentially. The same object can only be managed by a single revision - at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded - or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for - posterity. + ClusterObjectSet represents an immutable snapshot of Kubernetes objects to + apply and manage on the cluster. Each revision contains objects organized into + phases that roll out sequentially. The same object can only be managed by a + single revision at a time. Ownership of objects is transitioned from one revision + to the next as new revisions are rolled out. Once the latest revision has rolled + out successfully, previous active revisions are archived for posterity. properties: apiVersion: description: |- @@ -70,6 +73,22 @@ spec: x-kubernetes-validations: - message: collisionProtection is immutable rule: self == oldSelf + group: + description: |- + group is a required, immutable identifier that links related revisions together. + All revisions sharing the same group form an ordered sequence, regardless of owner references. + The value must be 1 to 52 characters long, start with a lowercase letter, + contain only lowercase letters, digits or hyphens, and end with a letter or digit. + maxLength: 52 + minLength: 1 + type: string + x-kubernetes-validations: + - message: group must start with a lowercase letter, contain only + lowercase letters, digits or hyphens, and end with a letter or + digit + rule: self.matches('^[a-z]([a-z0-9-]*[a-z0-9])?$') + - message: group is immutable + rule: self == oldSelf lifecycleState: description: |- lifecycleState specifies the lifecycle state of the ClusterObjectSet. @@ -518,11 +537,11 @@ spec: x-kubernetes-list-type: atomic revision: description: |- - revision is a required, immutable sequence number representing a specific revision - of the parent ClusterExtension. + revision is a required, immutable sequence number identifying a specific + ClusterObjectSet within a sequence of related revisions. The revision field must be a positive integer. - Each ClusterObjectSet belonging to the same parent ClusterExtension must have a unique revision number. + Each ClusterObjectSet in the same revision sequence must have a unique revision number. The revision number must always be the previous revision number plus one, or 1 for the first revision. format: int64 minimum: 1 @@ -532,6 +551,7 @@ spec: rule: self == oldSelf required: - collisionProtection + - group - lifecycleState - revision type: object @@ -673,7 +693,11 @@ spec: x-kubernetes-validations: - message: completedAt cannot be removed once set rule: '!has(oldSelf.completedAt) || has(self.completedAt)' + required: + - spec type: object + selectableFields: + - jsonPath: .spec.group served: true storage: true subresources: diff --git a/internal/object-controller/controllers/clusterobjectset_controller.go b/internal/object-controller/controllers/clusterobjectset_controller.go index ed08eed75c..4dac32b8dd 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller.go +++ b/internal/object-controller/controllers/clusterobjectset_controller.go @@ -412,6 +412,17 @@ type Sourcoser interface { Source(handler handler.EventHandler, predicates ...predicate.Predicate) source.Source } +// SetupIndexes registers the cache indexes used to look up ClusterObjectSet revisions. +func SetupIndexes(ctx context.Context, indexer client.FieldIndexer) error { + return indexer.IndexField(ctx, &ocv1.ClusterObjectSet{}, ".spec.group", func(obj client.Object) []string { + cos, ok := obj.(*ocv1.ClusterObjectSet) + if !ok || cos.Spec.Group == "" { + return nil + } + return []string{cos.Spec.Group} + }) +} + func (c *ClusterObjectSetReconciler) SetupWithManager(mgr ctrl.Manager) error { c.Clock = clock.RealClock{} return ctrl.NewControllerManagedBy(mgr). @@ -495,14 +506,15 @@ func (c *ClusterObjectSetReconciler) removeFinalizer(ctx context.Context, obj cl return nil } -// listSiblingRevisions returns all active revisions belonging to the same owner, excluding the current one. +// listSiblingRevisions returns active revisions in the same group, +// excluding the current one. // This includes both lower and higher revision numbers, enabling boxcutter to properly classify // sibling owners and avoid reporting false collisions during revision handover. func (c *ClusterObjectSetReconciler) listSiblingRevisions(ctx context.Context, cos *ocv1.ClusterObjectSet) ([]*ocv1.ClusterObjectSet, error) { return c.listOtherActiveRevisions(ctx, cos, func(*ocv1.ClusterObjectSet) bool { return true }) } -// listPreviousRevisions returns active revisions belonging to the same owner with lower revision numbers. +// listPreviousRevisions returns active revisions in the same group with lower revision numbers. func (c *ClusterObjectSetReconciler) listPreviousRevisions(ctx context.Context, cos *ocv1.ClusterObjectSet) ([]*ocv1.ClusterObjectSet, error) { return c.listOtherActiveRevisions(ctx, cos, func(r *ocv1.ClusterObjectSet) bool { return r.Spec.Revision < cos.Spec.Revision @@ -514,15 +526,8 @@ func (c *ClusterObjectSetReconciler) listOtherActiveRevisions( cos *ocv1.ClusterObjectSet, predicate func(*ocv1.ClusterObjectSet) bool, ) ([]*ocv1.ClusterObjectSet, error) { - ownerLabel, ok := cos.Labels[labels.OwnerNameKey] - if !ok { - return nil, nil - } - revList := &ocv1.ClusterObjectSetList{} - if err := c.TrackingCache.List(ctx, revList, client.MatchingLabels{ - labels.OwnerNameKey: ownerLabel, - }); err != nil { + if err := c.Client.List(ctx, revList, client.MatchingFields{".spec.group": cos.Spec.Group}); err != nil { return nil, fmt.Errorf("listing revisions: %w", err) } diff --git a/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go b/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go index 4c6d5595d1..be790cb02b 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_internal_test.go @@ -24,6 +24,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/shared/labels" + "github.com/operator-framework/operator-controller/test" ) func Test_ClusterObjectSetReconciler_listSiblingRevisions(t *testing.T) { @@ -85,7 +86,7 @@ func Test_ClusterObjectSetReconciler_listSiblingRevisions(t *testing.T) { expectedRevs: []string{"rev-1"}, }, { - name: "should only include revisions matching owner label", + name: "should only include revisions in the same group", existingObjs: func() []client.Object { ext := newTestClusterExtensionInternal() ext2 := newTestClusterExtensionInternal() @@ -94,7 +95,7 @@ func Test_ClusterObjectSetReconciler_listSiblingRevisions(t *testing.T) { rev1 := newTestClusterObjectSetInternal(t, "rev-1") rev2 := newTestClusterObjectSetInternal(t, "rev-2") - rev2.Labels[labels.OwnerNameKey] = "test-ext-2" + rev2.Spec.Group = "test-ext-2" rev3 := newTestClusterObjectSetInternal(t, "rev-3") require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) require.NoError(t, controllerutil.SetControllerReference(ext2, rev2, testScheme)) @@ -105,16 +106,18 @@ func Test_ClusterObjectSetReconciler_listSiblingRevisions(t *testing.T) { expectedRevs: []string{"rev-1"}, }, { - name: "should return empty list when owner label missing", + name: "should include revisions when owner label is missing", existingObjs: func() []client.Object { ext := newTestClusterExtensionInternal() rev1 := newTestClusterObjectSetInternal(t, "rev-1") - delete(rev1.Labels, labels.OwnerNameKey) + rev2 := newTestClusterObjectSetInternal(t, "rev-2") + delete(rev2.Labels, labels.OwnerNameKey) require.NoError(t, controllerutil.SetControllerReference(ext, rev1, testScheme)) - return []client.Object{ext, rev1} + require.NoError(t, controllerutil.SetControllerReference(ext, rev2, testScheme)) + return []client.Object{ext, rev1, rev2} }, currentRev: "rev-1", - expectedRevs: []string{}, + expectedRevs: []string{"rev-2"}, }, } { t.Run(tc.name, func(t *testing.T) { @@ -178,6 +181,50 @@ func Test_ClusterObjectSetReconciler_listPreviousRevisions(t *testing.T) { } } +func Test_ClusterObjectSetReconciler_OwnerChangesPreserveRevisionChain(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(scheme)) + ext := newTestClusterExtensionInternal() + owner := *metav1.NewControllerRef(ext, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind)) + foreignOwner := owner + foreignOwner.Kind = "OtherController" + foreignOwner.Name = "other-owner" + foreignOwner.UID = "other-uid" + previous := newTestClusterObjectSetInternal(t, "rev-1") + current := newTestClusterObjectSetInternal(t, "rev-2") + current.OwnerReferences = []metav1.OwnerReference{owner} + otherGroup := newTestClusterObjectSetInternal(t, "other-1") + otherGroup.Spec.Group = "other-group" + otherGroup.OwnerReferences = []metav1.OwnerReference{owner} + cl := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(scheme), SetupIndexes). + WithObjects(previous, current, otherGroup).Build() + r := &ClusterObjectSetReconciler{Client: cl} + + for _, tc := range []struct { + name string + owners []metav1.OwnerReference + }{ + {name: "same owner", owners: []metav1.OwnerReference{owner}}, + {name: "different owner", owners: []metav1.OwnerReference{foreignOwner}}, + {name: "owner removed"}, + {name: "owner added again", owners: []metav1.OwnerReference{owner}}, + } { + t.Run(tc.name, func(t *testing.T) { + require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(previous), previous)) + previous.OwnerReferences = tc.owners + require.NoError(t, cl.Update(t.Context(), previous)) + siblings, err := r.listSiblingRevisions(t.Context(), current) + require.NoError(t, err) + require.Len(t, siblings, 1) + require.Equal(t, previous.Name, siblings[0].Name) + predecessors, err := r.listPreviousRevisions(t.Context(), current) + require.NoError(t, err) + require.Len(t, predecessors, 1) + require.Equal(t, previous.Name, predecessors[0].Name) + }) + } +} + func callRevisionLister( t *testing.T, testScheme *runtime.Scheme, @@ -187,8 +234,7 @@ func callRevisionLister( ) []string { t.Helper() mockCtrl := gomock.NewController(t) - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), SetupIndexes). WithObjects(existingObjs...). Build() @@ -245,6 +291,7 @@ func newTestClusterObjectSetInternal(t *testing.T, name string) *ocv1.ClusterObj }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: "test-ext", Revision: revNum, Phases: []ocv1.ClusterObjectSetPhase{ { diff --git a/internal/object-controller/controllers/clusterobjectset_controller_test.go b/internal/object-controller/controllers/clusterobjectset_controller_test.go index 3d030bd9ae..42d18ff60f 100644 --- a/internal/object-controller/controllers/clusterobjectset_controller_test.go +++ b/internal/object-controller/controllers/clusterobjectset_controller_test.go @@ -33,6 +33,7 @@ import ( "github.com/operator-framework/operator-controller/internal/shared/labels" mockcontrollers "github.com/operator-framework/operator-controller/internal/testutil/mock/controllers" mockmachinery "github.com/operator-framework/operator-controller/internal/testutil/mock/machinery" + "github.com/operator-framework/operator-controller/test" ) const clusterObjectSetName = "test-ext-1" @@ -451,8 +452,13 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing ext := newTestClusterExtension() prevRev1 := newTestClusterObjectSet(t, "test-ext-1", ext, testScheme) prevRev2 := newTestClusterObjectSet(t, "test-ext-2", ext, testScheme) + prevRev1.OwnerReferences = nil + prevRev2.OwnerReferences[0].Name = "other-owner" + prevRev2.OwnerReferences[0].UID = "other-uid" rev := newTestClusterObjectSet(t, "test-ext-3", ext, testScheme) - return []client.Object{ext, prevRev1, prevRev2, rev} + otherGroup := newTestClusterObjectSet(t, "other-group-1", ext, testScheme) + otherGroup.Spec.Group = "other-group" + return []client.Object{ext, prevRev1, prevRev2, rev, otherGroup} }, validate: func(t *testing.T, c client.Client) { rev := &ocv1.ClusterObjectSet{} @@ -473,6 +479,8 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing }, rev) require.NoError(t, err) require.Equal(t, ocv1.ClusterObjectSetLifecycleStateActive, rev.Spec.LifecycleState) + require.NoError(t, c.Get(t.Context(), client.ObjectKey{Name: "other-group-1"}, rev)) + require.Equal(t, ocv1.ClusterObjectSetLifecycleStateActive, rev.Spec.LifecycleState) }, }, } { @@ -480,8 +488,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing mockCtrl := gomock.NewController(t) // create extension and cluster extension - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(tc.existingObjs()...). Build() @@ -584,8 +591,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_CompletedAt(t *testing.T) { t.Run(tc.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(tc.existingObjs()...). Build() @@ -698,8 +704,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ValidationError_Retries(t *testin rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme) // create extension and cluster extension - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(ext, rev1). Build() @@ -797,8 +802,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_VerifyReferencedSecrets(t *testin }, }} - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(append([]client.Object{ext, rev1}, tc.secrets...)...). WithInterceptorFuncs(interceptor.Funcs{ @@ -864,8 +868,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ReferencedContentValidationRecove Data: map[string][]byte{"configmap": []byte("not-json")}, } - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(ext, rev1, badSecret). Build() @@ -956,8 +959,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_BuildPhasesErrorClassification(t tc.setupCOS(rev1) } - builder := fake.NewClientBuilder(). - WithScheme(testScheme). + builder := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(ext, rev1) if tc.listFails { @@ -1249,8 +1251,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T) mockCtrl := gomock.NewController(t) // create extension and cluster extension - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(tc.existingObjs()...). Build() @@ -1482,8 +1483,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) { mockCtrl := gomock.NewController(t) // create extension and cluster extension - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(tc.existingObjs()...). Build() @@ -1552,6 +1552,7 @@ func newTestClusterObjectSet(t *testing.T, revisionName string, ext *ocv1.Cluste }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: revNum, Phases: []ocv1.ClusterObjectSetPhase{ @@ -1963,8 +1964,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi } { t.Run(tc.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithStatusSubresource(&ocv1.ClusterObjectSet{}). WithObjects(tc.existingObjs()...). Build() @@ -2065,8 +2065,7 @@ func Test_ClusterObjectSetReconciler_getScopedClient_Errors(t *testing.T) { ext := newTestClusterExtension() rev := newTestClusterObjectSet(t, "test-rev", ext, testScheme) - testClient := fake.NewClientBuilder(). - WithScheme(testScheme). + testClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithObjects(ext, rev). Build() diff --git a/internal/object-controller/controllers/indexes_test.go b/internal/object-controller/controllers/indexes_test.go new file mode 100644 index 0000000000..98516c9fa8 --- /dev/null +++ b/internal/object-controller/controllers/indexes_test.go @@ -0,0 +1,64 @@ +//go:build !standard + +package controllers_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/object-controller/controllers" + "github.com/operator-framework/operator-controller/test" +) + +func TestSetupIndexes_GroupLookup(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(scheme)) + cl := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(scheme), controllers.SetupIndexes). + WithObjects( + &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "matching-group", + Labels: map[string]string{ + "olm.operatorframework.io/owner-name": "other-group", + }, + }, + Spec: ocv1.ClusterObjectSetSpec{Group: "my-group"}, + }, + &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-group", + Labels: map[string]string{ + "olm.operatorframework.io/owner-name": "my-group", + }, + }, + Spec: ocv1.ClusterObjectSetSpec{Group: "other-group"}, + }, + &ocv1.ClusterObjectSet{ObjectMeta: metav1.ObjectMeta{Name: "missing-group"}}, + ). + Build() + + for _, tc := range []struct { + group string + want []string + }{ + {group: "my-group", want: []string{"matching-group"}}, + {group: "other-group", want: []string{"other-group"}}, + {group: ""}, + } { + t.Run(tc.group, func(t *testing.T) { + list := &ocv1.ClusterObjectSetList{} + require.NoError(t, cl.List(t.Context(), list, client.MatchingFields{".spec.group": tc.group})) + names := make([]string, 0, len(list.Items)) + for _, cos := range list.Items { + names = append(names, cos.Name) + } + require.ElementsMatch(t, tc.want, names) + }) + } +} diff --git a/internal/object-controller/controllers/resolve_ref_test.go b/internal/object-controller/controllers/resolve_ref_test.go index eba5c81a8b..c2b505af2e 100644 --- a/internal/object-controller/controllers/resolve_ref_test.go +++ b/internal/object-controller/controllers/resolve_ref_test.go @@ -26,6 +26,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/object-controller/controllers" "github.com/operator-framework/operator-controller/internal/shared/labels" + "github.com/operator-framework/operator-controller/test" ) func newSchemeWithCoreV1(t *testing.T) *apimachineryruntime.Scheme { @@ -68,8 +69,7 @@ func TestResolveObjectRef_PlainJSON(t *testing.T) { Key: "my-key", }) - fakeClient := fake.NewClientBuilder(). - WithScheme(testScheme). + fakeClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithObjects(secret, cos). WithStatusSubresource(&ocv1.ClusterObjectSet{}). Build() @@ -131,8 +131,7 @@ func TestResolveObjectRef_GzipCompressed(t *testing.T) { Key: "my-key", }) - fakeClient := fake.NewClientBuilder(). - WithScheme(testScheme). + fakeClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithObjects(secret, cos). WithStatusSubresource(&ocv1.ClusterObjectSet{}). Build() @@ -165,8 +164,7 @@ func TestResolveObjectRef_SecretNotFound(t *testing.T) { Key: "my-key", }) - fakeClient := fake.NewClientBuilder(). - WithScheme(testScheme). + fakeClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithObjects(cos). WithStatusSubresource(&ocv1.ClusterObjectSet{}). Build() @@ -206,8 +204,7 @@ func TestResolveObjectRef_KeyNotFound(t *testing.T) { Key: "missing-key", }) - fakeClient := fake.NewClientBuilder(). - WithScheme(testScheme). + fakeClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithObjects(secret, cos). WithStatusSubresource(&ocv1.ClusterObjectSet{}). Build() @@ -256,8 +253,7 @@ func TestResolveObjectRef_InvalidJSON(t *testing.T) { Key: "my-key", }) - fakeClient := fake.NewClientBuilder(). - WithScheme(testScheme). + fakeClient := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), controllers.SetupIndexes). WithObjects(secret, cos). WithStatusSubresource(&ocv1.ClusterObjectSet{}). Build() @@ -295,6 +291,7 @@ func newRefTestCOS(name string, ref ocv1.ObjectSourceRef) *ocv1.ClusterObjectSet }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: "ref-test", LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 1, CollisionProtection: ocv1.CollisionProtectionPrevent, diff --git a/internal/object-controller/controllers/revision_engine_factory.go b/internal/object-controller/controllers/revision_engine_factory.go index d361644ca0..6b915beda8 100644 --- a/internal/object-controller/controllers/revision_engine_factory.go +++ b/internal/object-controller/controllers/revision_engine_factory.go @@ -37,23 +37,25 @@ type RevisionEngineFactory interface { // defaultRevisionEngineFactory creates boxcutter RevisionEngines. type defaultRevisionEngineFactory struct { - Scheme *runtime.Scheme - TrackingCache managedcache.TrackingCache - DiscoveryClient discovery.CachedDiscoveryInterface - RESTMapper meta.RESTMapper - FieldOwnerPrefix string - Client client.Client + Scheme *runtime.Scheme + TrackingCache managedcache.TrackingCache + DiscoveryClient discovery.CachedDiscoveryInterface + RESTMapper meta.RESTMapper + Client client.Client } // CreateRevisionEngine constructs a boxcutter RevisionEngine for the given ClusterObjectSet. func (f *defaultRevisionEngineFactory) CreateRevisionEngine(_ context.Context, rev *ocv1.ClusterObjectSet) (RevisionEngine, error) { + fieldOwner := "cos-group/" + rev.Spec.Group + systemPrefix := ocv1.GroupVersion.Group + managedBy := ocv1.GroupVersion.Group return machinery.NewRevisionEngine( machinery.NewPhaseEngine( machinery.NewObjectEngine( f.Scheme, f.TrackingCache, f.Client, - machinery.NewComparator(f.DiscoveryClient, f.Scheme, f.FieldOwnerPrefix), - f.FieldOwnerPrefix, f.FieldOwnerPrefix, - f.FieldOwnerPrefix, // managedBy + machinery.NewComparator(f.DiscoveryClient, f.Scheme, fieldOwner), + fieldOwner, systemPrefix, + managedBy, f.Client, ), validation.NewClusterPhaseValidator(f.RESTMapper, f.Client), @@ -68,7 +70,6 @@ func NewDefaultRevisionEngineFactory( trackingCache managedcache.TrackingCache, discoveryClient discovery.CachedDiscoveryInterface, restMapper meta.RESTMapper, - fieldOwnerPrefix string, baseConfig *rest.Config, ) (RevisionEngineFactory, error) { if baseConfig == nil { @@ -79,11 +80,10 @@ func NewDefaultRevisionEngineFactory( return nil, fmt.Errorf("failed to create client: %w", err) } return &defaultRevisionEngineFactory{ - Scheme: scheme, - TrackingCache: trackingCache, - DiscoveryClient: discoveryClient, - RESTMapper: restMapper, - FieldOwnerPrefix: fieldOwnerPrefix, - Client: c, + Scheme: scheme, + TrackingCache: trackingCache, + DiscoveryClient: discoveryClient, + RESTMapper: restMapper, + Client: c, }, nil } diff --git a/internal/object-controller/controllers/revision_engine_factory_test.go b/internal/object-controller/controllers/revision_engine_factory_test.go new file mode 100644 index 0000000000..527d4680f4 --- /dev/null +++ b/internal/object-controller/controllers/revision_engine_factory_test.go @@ -0,0 +1,173 @@ +//go:build !standard + +/* +Copyright 2026. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers_test + +import ( + "context" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/discovery" + "k8s.io/client-go/discovery/cached/memory" + "pkg.package-operator.run/boxcutter" + "pkg.package-operator.run/boxcutter/managedcache" + "pkg.package-operator.run/boxcutter/ownerhandling" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/object-controller/controllers" +) + +func TestDefaultRevisionEngineFactory_FieldOwnership(t *testing.T) { + scheme := newScheme(t) + require.NoError(t, corev1.AddToScheme(scheme)) + cl, err := client.New(config, client.Options{Scheme: scheme}) + require.NoError(t, err) + + // A core resource serves as the owner to isolate the factory's SSA and + // metadata configuration from the COS controller's lifecycle behavior. + owner := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{GenerateName: "factory-field-owner-"}} + require.NoError(t, cl.Create(t.Context(), owner)) + objectKey := client.ObjectKey{Namespace: owner.Name, Name: "payload"} + t.Cleanup(func() { + require.NoError(t, client.IgnoreNotFound(cl.Delete(context.Background(), &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: objectKey.Namespace, Name: objectKey.Name}, + }))) + require.NoError(t, cl.Delete(context.Background(), owner)) + }) + + trackingCache, err := managedcache.NewTrackingCache(logr.Discard(), config, cache.Options{ + Scheme: scheme, + Mapper: cl.RESTMapper(), + }) + require.NoError(t, err) + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + cacheDone := make(chan error, 1) + go func() { cacheDone <- trackingCache.Start(ctx) }() + t.Cleanup(func() { + cancel() + select { + case err := <-cacheDone: + require.NoError(t, err) + case <-time.After(5 * time.Second): + t.Fatal("tracking cache did not stop") + } + }) + require.NoError(t, trackingCache.Watch(ctx, owner, sets.New(corev1.SchemeGroupVersion.WithKind("ConfigMap")))) + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + require.NoError(t, err) + factory, err := controllers.NewDefaultRevisionEngineFactory( + scheme, trackingCache, memory.NewMemCacheClient(discoveryClient), cl.RESTMapper(), config, + ) + require.NoError(t, err) + cos := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: "factory-test-1"}, + Spec: ocv1.ClusterObjectSetSpec{Group: "factory-test", Revision: 1}, + } + engine, err := factory.CreateRevisionEngine(ctx, cos) + require.NoError(t, err) + + obj := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "ConfigMap"}, + ObjectMeta: metav1.ObjectMeta{Namespace: objectKey.Namespace, Name: objectKey.Name}, + Data: map[string]string{"value": "revision-1"}, + } + revision := boxcutter.NewRevisionWithOwner(cos.Name, cos.Spec.Revision, + []boxcutter.Phase{boxcutter.NewPhase("deploy", []client.Object{obj})}, + owner, ownerhandling.NewNative(scheme), + ) + result, err := engine.Reconcile(ctx, revision) + require.NoError(t, err) + require.NotNil(t, result) + require.Nil(t, result.GetValidationError(), "%v", result.GetValidationError()) + + actual := &corev1.ConfigMap{} + require.NoError(t, cl.Get(ctx, objectKey, actual)) + require.Equal(t, "1", actual.Annotations["olm.operatorframework.io/revision"]) + require.Equal(t, "olm.operatorframework.io", actual.Labels["app.kubernetes.io/managed-by"]) + + var applyManagers []string + for _, fields := range actual.ManagedFields { + if fields.Operation == metav1.ManagedFieldsOperationApply { + applyManagers = append(applyManagers, fields.Manager) + } + } + require.Contains(t, applyManagers, "cos-group/factory-test") + + // A subsequent revision updates the same field using the same SSA manager. + cos.Name = "factory-test-2" + cos.Spec.Revision = 2 + engine, err = factory.CreateRevisionEngine(ctx, cos) + require.NoError(t, err) + obj.Data["value"] = "revision-2" + objectMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + require.NoError(t, err) + desired := &unstructured.Unstructured{Object: objectMap} + revision = boxcutter.NewRevisionWithOwner(cos.Name, cos.Spec.Revision, + []boxcutter.Phase{boxcutter.NewPhase("deploy", []client.Object{desired})}, + owner, ownerhandling.NewNative(scheme), + ) + require.Eventually(t, func() bool { + if _, err := engine.Reconcile(ctx, revision); err != nil { + return false + } + return cl.Get(ctx, objectKey, actual) == nil && actual.Data["value"] == "revision-2" + }, 5*time.Second, 10*time.Millisecond) + applyManagers = nil + for _, fields := range actual.ManagedFields { + if fields.Operation == metav1.ManagedFieldsOperationApply { + applyManagers = append(applyManagers, fields.Manager) + } + } + require.Equal(t, []string{"cos-group/factory-test"}, applyManagers) + require.Equal(t, "2", actual.Annotations["olm.operatorframework.io/revision"]) + + // Another group gets its own SSA manager. + cos.Name = "factory-other-1" + cos.Spec.Group = "factory-other" + cos.Spec.Revision = 1 + engine, err = factory.CreateRevisionEngine(ctx, cos) + require.NoError(t, err) + other := desired.DeepCopy() + other.SetName("other-payload") + t.Cleanup(func() { require.NoError(t, client.IgnoreNotFound(cl.Delete(context.Background(), other))) }) + revision = boxcutter.NewRevisionWithOwner(cos.Name, cos.Spec.Revision, + []boxcutter.Phase{boxcutter.NewPhase("deploy", []client.Object{other})}, + owner, ownerhandling.NewNative(scheme), + ) + _, err = engine.Reconcile(ctx, revision) + require.NoError(t, err) + require.NoError(t, cl.Get(ctx, client.ObjectKeyFromObject(other), actual)) + applyManagers = nil + for _, fields := range actual.ManagedFields { + if fields.Operation == metav1.ManagedFieldsOperationApply { + applyManagers = append(applyManagers, fields.Manager) + } + } + require.Equal(t, []string{"cos-group/factory-other"}, applyManagers) +} diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 87155bb592..4b45ebcff8 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -1,13 +1,11 @@ package applier import ( - "cmp" "context" "errors" "fmt" "io/fs" "maps" - "slices" "strings" "github.com/cert-manager/cert-manager/pkg/apis/certmanager" @@ -32,6 +30,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" ocv1ac "github.com/operator-framework/operator-controller/applyconfigurations/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/revisions" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/shared/labels" "github.com/operator-framework/operator-controller/internal/shared/util/cache" @@ -236,6 +235,7 @@ func (r *SimpleRevisionGenerator) buildClusterObjectSet( phases := PhaseSort(objects) spec := ocv1ac.ClusterObjectSetSpec(). + WithGroup(ext.Name). WithLifecycleState(ocv1.ClusterObjectSetLifecycleStateActive). WithPhases(phases...). WithProgressionProbes(defaultProgressionProbes...) @@ -254,6 +254,8 @@ func (r *SimpleRevisionGenerator) buildClusterObjectSet( // BoxcutterStorageMigrator migrates ClusterExtensions from Helm-based storage to // ClusterObjectSet storage, enabling upgrades from older operator-controller versions. +// TODO(COD): Adapt Helm storage migration for ClusterObjectDeployment-backed +// installations and remove direct COS discovery when that migration path is in place. type BoxcutterStorageMigrator struct { ActionClientGetter helmclient.ActionClientGetter RevisionGenerator ClusterObjectSetGenerator @@ -280,14 +282,12 @@ func (m *BoxcutterStorageMigrator) Migrate(ctx context.Context, ext *ocv1.Cluste if ext.Spec.Namespace == "" { return nil } - existingRevisionList := ocv1.ClusterObjectSetList{} - if err := m.Client.List(ctx, &existingRevisionList, client.MatchingLabels{ - labels.OwnerNameKey: ext.Name, - }); err != nil { + existingRevisions, err := revisions.ListForGroup(ctx, m.Client, ext.Name) + if err != nil { return fmt.Errorf("listing ClusterObjectSets before attempting migration: %w", err) } - if len(existingRevisionList.Items) != 0 { - return m.ensureMigratedRevisionStatus(ctx, existingRevisionList.Items) + if len(existingRevisions) != 0 { + return m.ensureMigratedRevisionStatus(ctx, existingRevisions) } ac, err := m.ActionClientGetter.ActionClientFor(ctx, ext) @@ -449,6 +449,8 @@ func (m *BoxcutterStorageMigrator) ensureRevisionStatus(ctx context.Context, nam return nil } +// TODO(COD): Replace direct COS creation, revision discovery and retention with +// ClusterObjectDeployment reconciliation. type Boxcutter struct { Client client.Client Scheme *runtime.Scheme @@ -464,9 +466,9 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust // cluster access is unavailable (since the ClusterObjectSet controller also requires // API access to maintain resources). The revision list is also needed to determine if fallback // is possible when contentFS is nil (at least one revision must exist). - existingRevisions, err := bc.getExistingRevisions(ctx, ext.GetName()) + existingRevisions, err := revisions.ListForGroup(ctx, bc.Client, ext.Name) if err != nil { - return false, "", err + return false, "", fmt.Errorf("listing revisions: %w", err) } // If contentFS is nil, we're maintaining the current state without catalog access. @@ -644,20 +646,6 @@ func (bc *Boxcutter) garbageCollectOldRevisions(ctx context.Context, revisionLis return nil } -// getExistingRevisions returns the list of ClusterObjectSets for a ClusterExtension with name extName in revision order (oldest to newest) -func (bc *Boxcutter) getExistingRevisions(ctx context.Context, extName string) ([]ocv1.ClusterObjectSet, error) { - existingRevisionList := &ocv1.ClusterObjectSetList{} - if err := bc.Client.List(ctx, existingRevisionList, client.MatchingLabels{ - labels.OwnerNameKey: extName, - }); err != nil { - return nil, fmt.Errorf("listing revisions: %w", err) - } - slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterObjectSet) int { - return cmp.Compare(a.Spec.Revision, b.Spec.Revision) - }) - return existingRevisionList.Items, nil -} - func latestRevisionNumber(prevRevisions []ocv1.ClusterObjectSet) int64 { if len(prevRevisions) == 0 { return 0 diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index f42ed763f1..9ba2346313 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -29,12 +29,14 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" ocv1ac "github.com/operator-framework/operator-controller/applyconfigurations/api/v1" + coscontrollers "github.com/operator-framework/operator-controller/internal/object-controller/controllers" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/shared/labels" bundlecsv "github.com/operator-framework/operator-controller/internal/testing/bundle/csv" bundlefs "github.com/operator-framework/operator-controller/internal/testing/bundle/fs" mockapplier "github.com/operator-framework/operator-controller/internal/testutil/mock/applier" mockctrlclient "github.com/operator-framework/operator-controller/internal/testutil/mock/ctrlclient" + "github.com/operator-framework/operator-controller/test" ) var ( @@ -103,6 +105,7 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) labels.OwnerNameKey: "test-123", }). WithSpec(ocv1ac.ClusterObjectSetSpec(). + WithGroup(ext.Name). WithLifecycleState(ocv1.ClusterObjectSetLifecycleStateActive). WithCollisionProtection(ocv1.CollisionProtectionNone). WithRevision(1). @@ -148,6 +151,7 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) assert.Equal(t, expected.Labels, rev.Labels) assert.Equal(t, expected.Annotations, rev.Annotations) assert.Equal(t, expected.Spec.LifecycleState, rev.Spec.LifecycleState) + assert.Equal(t, expected.Spec.Group, rev.Spec.Group) assert.Equal(t, expected.Spec.CollisionProtection, rev.Spec.CollisionProtection) assert.Equal(t, expected.Spec.Revision, rev.Spec.Revision) assert.Equal(t, expected.Spec.Phases, rev.Spec.Phases) @@ -205,6 +209,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { require.NoError(t, err) t.Log("by checking the olm.operatorframework.io/owner-name and owner-kind labels are set") + require.Equal(t, ptr.To(ext.Name), rev.Spec.Group) require.Equal(t, map[string]string{ labels.OwnerKindKey: ocv1.ClusterExtensionKind, labels.OwnerNameKey: "test-extension", @@ -522,6 +527,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, Revision: 1, Phases: []ocv1.ClusterObjectSetPhase{ { @@ -544,6 +550,9 @@ func TestBoxcutter_Apply(t *testing.T) { }, } + revisionWithoutOwnerLabel := defaultDesiredRevision.DeepCopy() + revisionWithoutOwnerLabel.Labels = nil + allowedRevisionValue := func(revNum int64) *interceptor.Funcs { return &interceptor.Funcs{ Apply: func(ctx context.Context, client client.WithWatch, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { @@ -584,6 +593,7 @@ func TestBoxcutter_Apply(t *testing.T) { labels.OwnerNameKey: ext.Name, }). WithSpec(ocv1ac.ClusterObjectSetSpec(). + WithGroup(ext.Name). WithPhases( ocv1ac.ClusterObjectSetPhase(). WithName(string(applier.PhaseDeploy)). @@ -607,7 +617,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, validate: func(t *testing.T, c client.Client) { revList := &ocv1.ClusterObjectSetList{} - err := c.List(t.Context(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name}) + err := c.List(t.Context(), revList, client.MatchingFields{".spec.group": ext.Name}) require.NoError(t, err) require.Len(t, revList.Items, 1) @@ -617,6 +627,10 @@ func TestBoxcutter_Apply(t *testing.T) { assert.Len(t, rev.OwnerReferences, 1) assert.Equal(t, ext.Name, rev.OwnerReferences[0].Name) assert.Equal(t, ext.UID, rev.OwnerReferences[0].UID) + assert.Equal(t, ocv1.GroupVersion.String(), rev.OwnerReferences[0].APIVersion) + assert.Equal(t, ocv1.ClusterExtensionKind, rev.OwnerReferences[0].Kind) + assert.Equal(t, ptr.To(true), rev.OwnerReferences[0].Controller) + assert.Equal(t, ptr.To(true), rev.OwnerReferences[0].BlockOwnerDeletion) }, }, { @@ -632,6 +646,7 @@ func TestBoxcutter_Apply(t *testing.T) { labels.OwnerNameKey: ext.Name, }). WithSpec(ocv1ac.ClusterObjectSetSpec(). + WithGroup(ext.Name). WithPhases( ocv1ac.ClusterObjectSetPhase(). WithName(string(applier.PhaseDeploy)). @@ -654,11 +669,11 @@ func TestBoxcutter_Apply(t *testing.T) { return m }, existingObjs: []client.Object{ - defaultDesiredRevision, + revisionWithoutOwnerLabel, }, validate: func(t *testing.T, c client.Client) { revList := &ocv1.ClusterObjectSetList{} - err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name}) + err := c.List(context.Background(), revList, client.MatchingFields{".spec.group": ext.Name}) require.NoError(t, err) // No new revision should be created require.Len(t, revList.Items, 1) @@ -678,6 +693,7 @@ func TestBoxcutter_Apply(t *testing.T) { labels.OwnerNameKey: ext.Name, }). WithSpec(ocv1ac.ClusterObjectSetSpec(). + WithGroup(ext.Name). WithPhases( ocv1ac.ClusterObjectSetPhase(). WithName(string(applier.PhaseDeploy)). @@ -705,7 +721,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, validate: func(t *testing.T, c client.Client) { revList := &ocv1.ClusterObjectSetList{} - err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name}) + err := c.List(context.Background(), revList, client.MatchingFields{".spec.group": ext.Name}) require.NoError(t, err) require.Len(t, revList.Items, 2) @@ -736,13 +752,13 @@ func TestBoxcutter_Apply(t *testing.T) { validate: func(t *testing.T, c client.Client) { // Ensure no revisions were created revList := &ocv1.ClusterObjectSetList{} - err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name}) + err := c.List(context.Background(), revList, client.MatchingFields{".spec.group": ext.Name}) require.NoError(t, err) assert.Empty(t, revList.Items) }, }, { - name: "keep at most 5 past revisions", + name: "keep at most 5 past revisions across controller owners", mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) @@ -753,7 +769,7 @@ func TestBoxcutter_Apply(t *testing.T) { WithLabels(map[string]string{ labels.OwnerNameKey: ext.Name, }). - WithSpec(ocv1ac.ClusterObjectSetSpec()), nil + WithSpec(ocv1ac.ClusterObjectSetSpec().WithGroup(ext.Name)), nil }).AnyTimes() m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m @@ -762,11 +778,16 @@ func TestBoxcutter_Apply(t *testing.T) { &ocv1.ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{ Name: "rev-1", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: ocv1.GroupVersion.String(), Kind: ocv1.ClusterExtensionKind, + Name: "other-owner", UID: "other-uid", Controller: ptr.To(true), + }}, Labels: map[string]string{ labels.OwnerNameKey: ext.Name, }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 1, }, @@ -779,6 +800,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 2, }, @@ -791,6 +813,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 3, }, @@ -803,6 +826,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 4, }, @@ -815,6 +839,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 5, }, @@ -822,11 +847,16 @@ func TestBoxcutter_Apply(t *testing.T) { &ocv1.ClusterObjectSet{ ObjectMeta: metav1.ObjectMeta{ Name: "rev-6", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: ocv1.GroupVersion.String(), Kind: ocv1.ClusterExtensionKind, + Name: "other-owner", UID: "other-uid", Controller: ptr.To(true), + }}, Labels: map[string]string{ labels.OwnerNameKey: ext.Name, }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 6, }, @@ -841,10 +871,17 @@ func TestBoxcutter_Apply(t *testing.T) { // Verify garbage collection: should only keep the limit + 1 (current) revisions revList := &ocv1.ClusterObjectSetList{} - err = c.List(t.Context(), revList) + err = c.List(t.Context(), revList, client.MatchingFields{".spec.group": ext.Name}) require.NoError(t, err) // Should have ClusterObjectSetRetentionLimit (5) + current (1) = 6 revisions max - assert.LessOrEqual(t, len(revList.Items), applier.ClusterObjectSetRetentionLimit+1) + names := make([]string, 0, len(revList.Items)) + for _, rev := range revList.Items { + names = append(names, rev.Name) + } + require.ElementsMatch(t, []string{"rev-2", "rev-3", "rev-4", "rev-5", "rev-6", "test-ext-7"}, names) + created := &ocv1.ClusterObjectSet{} + require.NoError(t, c.Get(t.Context(), client.ObjectKey{Name: "test-ext-7"}, created)) + require.Equal(t, int64(7), created.Spec.Revision) }, }, { @@ -859,7 +896,7 @@ func TestBoxcutter_Apply(t *testing.T) { WithLabels(map[string]string{ labels.OwnerNameKey: ext.Name, }). - WithSpec(ocv1ac.ClusterObjectSetSpec()), nil + WithSpec(ocv1ac.ClusterObjectSetSpec().WithGroup(ext.Name)), nil }).AnyTimes() m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m @@ -873,6 +910,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 1, }, @@ -885,6 +923,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, // index beyond the retention limit but active; should be preserved LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 2, @@ -898,6 +937,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 3, }, @@ -910,6 +950,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, // archived but should be preserved since it is within the limit LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, Revision: 4, @@ -923,6 +964,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 5, }, @@ -935,6 +977,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 6, }, @@ -947,6 +990,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, Revision: 7, }, @@ -982,6 +1026,7 @@ func TestBoxcutter_Apply(t *testing.T) { labels.OwnerNameKey: ext.Name, }). WithSpec(ocv1ac.ClusterObjectSetSpec(). + WithGroup(ext.Name). WithPhases( ocv1ac.ClusterObjectSetPhase(). WithName(string(applier.PhaseDeploy)). @@ -1017,6 +1062,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, }, Spec: ocv1.ClusterObjectSetSpec{ + Group: ext.Name, Revision: 1, Phases: []ocv1.ClusterObjectSetPhase{ { @@ -1041,7 +1087,7 @@ func TestBoxcutter_Apply(t *testing.T) { }, validate: func(t *testing.T, c client.Client) { revList := &ocv1.ClusterObjectSetList{} - err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name}) + err := c.List(context.Background(), revList, client.MatchingFields{".spec.group": ext.Name}) require.NoError(t, err) // Should still be only 1 revision (in-place update, not new revision) require.Len(t, revList.Items, 1) @@ -1061,7 +1107,16 @@ func TestBoxcutter_Apply(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // Setup - cb := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(tc.existingObjs...) + cb := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), coscontrollers.SetupIndexes). + WithObjects(tc.existingObjs...) + // A misleading owner label must not let another group's revisions + // affect numbering or retention for this extension. + cb.WithObjects(&ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: "other-group-99", Labels: map[string]string{labels.OwnerNameKey: ext.Name}}, + Spec: ocv1.ClusterObjectSetSpec{ + Group: "other-group", Revision: 99, LifecycleState: ocv1.ClusterObjectSetLifecycleStateArchived, + }, + }) if tc.clientIterceptor != nil { cb.WithInterceptorFuncs(*tc.clientIterceptor) } @@ -1109,6 +1164,54 @@ func TestBoxcutter_Apply(t *testing.T) { tc.validate(t, fakeClient) } } + require.NoError(t, fakeClient.Get(t.Context(), client.ObjectKey{Name: "other-group-99"}, &ocv1.ClusterObjectSet{})) + }) + } +} + +func TestBoxcutter_Apply_FallbackUsesGroup(t *testing.T) { + for _, tc := range []struct { + name string + group string + ownerName string + wantError bool + }{ + {name: "ownerless", group: "test-ext"}, + {name: "different controller owner", group: "test-ext", ownerName: "other-ext"}, + {name: "different group with matching owner", group: "other-group", ownerName: "test-ext", wantError: true}, + } { + t.Run(tc.name, func(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(scheme)) + ext := &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", UID: "test-ext-uid"}} + cos := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: "revision-1"}, + Spec: ocv1.ClusterObjectSetSpec{ + Group: tc.group, Revision: 1, LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + }, + } + if tc.ownerName != "" { + owner := *metav1.NewControllerRef(ext, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind)) + owner.Name = tc.ownerName + if tc.ownerName != ext.Name { + owner.UID = "other-owner-uid" + } + cos.OwnerReferences = []metav1.OwnerReference{owner} + } + cl := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(scheme), coscontrollers.SetupIndexes). + WithObjects(cos).Build() + bc := &applier.Boxcutter{ + Client: cl, Scheme: scheme, + RevisionGenerator: mockapplier.NewMockClusterObjectSetGenerator(gomock.NewController(t)), + } + completed, _, err := bc.Apply(t.Context(), nil, ext, nil, nil) + if tc.wantError { + require.ErrorContains(t, err, "no revision installed") + require.False(t, completed) + } else { + require.NoError(t, err) + require.True(t, completed) + } }) } } @@ -1120,7 +1223,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { WithLabels(map[string]string{ labels.OwnerNameKey: ext.Name, }). - WithSpec(ocv1ac.ClusterObjectSetSpec()) + WithSpec(ocv1ac.ClusterObjectSetSpec().WithGroup(ext.Name)) } // newStorageMigratorGenerator creates a gomock ClusterObjectSetGenerator for storage migrator tests. @@ -1135,12 +1238,53 @@ func TestBoxcutterStorageMigrator(t *testing.T) { return m } + t.Run("looks up only the group regardless of owner metadata", func(t *testing.T) { + testScheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(testScheme)) + ext := &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "migration-group", UID: "migration-group-uid"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}} + other := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: "other-group-1", Labels: map[string]string{labels.OwnerNameKey: ext.Name}}, + Spec: ocv1.ClusterObjectSetSpec{Group: "other-group", Revision: 1}, + } + cl := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(testScheme), coscontrollers.SetupIndexes). + WithStatusSubresource(&ocv1.ClusterObjectSet{}). + WithObjects(other).Build() + sm := &applier.BoxcutterStorageMigrator{ + RevisionGenerator: newStorageMigratorGenerator(t), + ActionClientGetter: newMockActionGetter(gomock.NewController(t), mockActionGetterConfig{ + currentRel: &release.Release{Name: ext.Name, Info: &release.Info{Status: release.StatusDeployed}}, + }), + Client: cl, Scheme: testScheme, FieldOwner: "test-owner", + } + require.NoError(t, sm.Migrate(t.Context(), ext, nil)) + rev := &ocv1.ClusterObjectSet{} + require.NoError(t, cl.Get(t.Context(), client.ObjectKey{Name: "test-revision"}, rev)) + require.Equal(t, ext.Name, rev.Spec.Group) + delete(rev.Labels, labels.OwnerNameKey) + require.NoError(t, cl.Update(t.Context(), rev)) + // The existing revision must short-circuit migration without Helm access. + sm.ActionClientGetter = nil + sm.RevisionGenerator = nil + require.NoError(t, sm.Migrate(t.Context(), ext, nil)) + foreignOwner := *metav1.NewControllerRef(ext, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind)) + foreignOwner.Name = "other-owner" + foreignOwner.UID = "other-uid" + for _, owners := range [][]metav1.OwnerReference{{foreignOwner}, nil} { + require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(rev), rev)) + rev.OwnerReferences = owners + require.NoError(t, cl.Update(t.Context(), rev)) + require.NoError(t, sm.Migrate(t.Context(), ext, nil)) + } + require.NoError(t, cl.Get(t.Context(), client.ObjectKeyFromObject(other), other)) + require.Empty(t, other.Status.Conditions) + }) + t.Run("creates revision", func(t *testing.T) { testScheme := runtime.NewScheme() require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123", UID: "test123-uid"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) @@ -1164,12 +1308,25 @@ func TestBoxcutterStorageMigrator(t *testing.T) { } mockClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil) + DoAndReturn(func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + listOpts := (&client.ListOptions{}).ApplyOptions(opts) + require.Equal(t, ".spec.group"+"="+ext.Name, listOpts.FieldSelector.String()) + require.Nil(t, listOpts.LabelSelector) + return nil + }) mockClient.EXPECT().Apply(gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn(func(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { // Verify the migration marker label is set before apply rev := obj.(*ocv1ac.ClusterObjectSetApplyConfiguration) require.Equal(t, "true", rev.Labels[labels.MigratedFromHelmKey], "Migration marker label should be set") + require.Equal(t, ptr.To(ext.Name), rev.Spec.Group) + require.Len(t, rev.OwnerReferences, 1) + owner := rev.OwnerReferences[0] + require.Equal(t, ptr.To(ext.Name), owner.Name) + require.Equal(t, ptr.To(ext.UID), owner.UID) + require.Equal(t, ptr.To(ocv1.ClusterExtensionKind), owner.Kind) + require.Equal(t, ptr.To(true), owner.Controller) + require.Equal(t, ptr.To(true), owner.BlockOwnerDeletion) return nil }) mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index e623aedda2..466eb51a3f 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -17,12 +17,10 @@ limitations under the License. package controllers import ( - "cmp" "context" "errors" "fmt" "io/fs" - "slices" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -30,29 +28,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/revisions" "github.com/operator-framework/operator-controller/internal/shared/labels" ) +// TODO(COD): Read revision state from ClusterObjectDeployment status instead of +// discovering COS revisions directly from ClusterExtension reconciliation. type BoxcutterRevisionStatesGetter struct { Reader client.Reader } func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, ext *ocv1.ClusterExtension) (*RevisionStates, error) { - // TODO: boxcutter applier has a nearly identical bit of code for listing and sorting revisions - // only difference here is that it sorts in reverse order to start iterating with the most - // recent revisions. We should consolidate to avoid code duplication. - existingRevisionList := &ocv1.ClusterObjectSetList{} - if err := d.Reader.List(ctx, existingRevisionList, client.MatchingLabels{ - labels.OwnerNameKey: ext.Name, - }); err != nil { + existingRevisions, err := revisions.ListForGroup(ctx, d.Reader, ext.Name) + if err != nil { return nil, fmt.Errorf("listing revisions: %w", err) } - slices.SortFunc(existingRevisionList.Items, func(a, b ocv1.ClusterObjectSet) int { - return cmp.Compare(a.Spec.Revision, b.Spec.Revision) - }) rs := &RevisionStates{} - for _, rev := range existingRevisionList.Items { + for _, rev := range existingRevisions { if rev.Spec.LifecycleState == ocv1.ClusterObjectSetLifecycleStateArchived { continue } diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps_internal_test.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps_internal_test.go new file mode 100644 index 0000000000..65a3c93c94 --- /dev/null +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps_internal_test.go @@ -0,0 +1,72 @@ +/* +Copyright 2026. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + coscontrollers "github.com/operator-framework/operator-controller/internal/object-controller/controllers" + "github.com/operator-framework/operator-controller/test" +) + +func TestBoxcutterRevisionStatesGetter_ClassifiesByCompletedAt(t *testing.T) { + sch := apimachineryruntime.NewScheme() + require.NoError(t, ocv1.AddToScheme(sch)) + + const extName = "test-ext" + + newRevision := func(name string, revision int64, completed bool) *ocv1.ClusterObjectSet { + cos := &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: ocv1.ClusterObjectSetSpec{Group: extName, Revision: revision}, + } + if completed { + cos.Status.CompletedAt = metav1.Now() + } + return cos + } + + completed := newRevision("test-ext-1", 1, true) + rollingOut := newRevision("test-ext-2", 2, false) + + cl := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(sch), coscontrollers.SetupIndexes). + WithObjects(completed, rollingOut). + Build() + + getter := &BoxcutterRevisionStatesGetter{Reader: cl} + states, err := getter.GetRevisionStates(context.Background(), &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extName}, + }) + require.NoError(t, err) + + // A revision with completedAt set is Installed. + require.NotNil(t, states.Installed) + require.Equal(t, "test-ext-1", states.Installed.RevisionName) + + // A revision without completedAt is still RollingOut. + require.Len(t, states.RollingOut, 1) + require.Equal(t, "test-ext-2", states.RollingOut[0].RevisionName) +} diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go index 5ce9b7259a..39bf3c300e 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps_test.go @@ -1,73 +1,101 @@ -/* -Copyright 2026. +//go:build !standard -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 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers +package controllers_test import ( "context" "testing" + "time" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - apimachineryruntime "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" + coscontrollers "github.com/operator-framework/operator-controller/internal/object-controller/controllers" + "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" "github.com/operator-framework/operator-controller/internal/shared/labels" ) -func TestBoxcutterRevisionStatesGetter_ClassifiesByCompletedAt(t *testing.T) { - sch := apimachineryruntime.NewScheme() - require.NoError(t, ocv1.AddToScheme(sch)) - - const extName = "test-ext" - - newRevision := func(name string, revision int64, completed bool) *ocv1.ClusterObjectSet { +func TestBoxcutterRevisionStatesGetter_GroupIndex(t *testing.T) { + scheme := newScheme(t) + cl, err := client.New(config, client.Options{Scheme: scheme}) + require.NoError(t, err) + group := "cache-group" + ext := &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: group, UID: "cache-group-uid"}} + for _, tc := range []struct { + name string + group string + revision int64 + archived bool + succeeded bool + ownerName string + }{ + {name: "cache-installed", group: group, revision: 1, succeeded: true, ownerName: "other-owner"}, + {name: "cache-rolling", group: group, revision: 2}, + {name: "cache-archived", group: group, revision: 3, archived: true, succeeded: true, ownerName: group}, + {name: "cache-other-group", group: "other-group", revision: 99, succeeded: true, ownerName: group}, + } { cos := &ocv1.ClusterObjectSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Labels: map[string]string{labels.OwnerNameKey: extName}, + ObjectMeta: metav1.ObjectMeta{Name: tc.name}, + Spec: ocv1.ClusterObjectSetSpec{ + Group: tc.group, Revision: tc.revision, + LifecycleState: ocv1.ClusterObjectSetLifecycleStateActive, + CollisionProtection: ocv1.CollisionProtectionPrevent, }, - Spec: ocv1.ClusterObjectSetSpec{Revision: revision}, } - if completed { + if tc.group != group { + cos.Labels = map[string]string{labels.OwnerNameKey: group} + } + if tc.ownerName != "" { + owner := *metav1.NewControllerRef(ext, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind)) + owner.Name = tc.ownerName + if tc.ownerName != ext.Name { + owner.UID = "other-owner-uid" + } + cos.OwnerReferences = []metav1.OwnerReference{owner} + } + if tc.archived { + cos.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateArchived + } + require.NoError(t, cl.Create(t.Context(), cos)) + t.Cleanup(func() { require.NoError(t, client.IgnoreNotFound(cl.Delete(context.Background(), cos))) }) + if tc.succeeded { cos.Status.CompletedAt = metav1.Now() + require.NoError(t, cl.Status().Update(t.Context(), cos)) } - return cos } - completed := newRevision("test-ext-1", 1, true) - rollingOut := newRevision("test-ext-2", 2, false) - - cl := fake.NewClientBuilder(). - WithScheme(sch). - WithObjects(completed, rollingOut). - Build() - - getter := &BoxcutterRevisionStatesGetter{Reader: cl} - states, err := getter.GetRevisionStates(context.Background(), &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extName}, + managerCache, err := cache.New(config, cache.Options{Scheme: scheme}) + require.NoError(t, err) + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + require.NoError(t, coscontrollers.SetupIndexes(ctx, managerCache)) + done := make(chan error, 1) + go func() { done <- managerCache.Start(ctx) }() + t.Cleanup(func() { + cancel() + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(5 * time.Second): + t.Fatal("manager cache did not stop") + } }) + require.True(t, managerCache.WaitForCacheSync(ctx)) + cachedClient, err := client.New(config, client.Options{Scheme: scheme, Cache: &client.CacheOptions{Reader: managerCache}}) + require.NoError(t, err) + getter := controllers.BoxcutterRevisionStatesGetter{Reader: cachedClient} + states, err := getter.GetRevisionStates(ctx, ext) require.NoError(t, err) - - // A revision with completedAt set is Installed. require.NotNil(t, states.Installed) - require.Equal(t, "test-ext-1", states.Installed.RevisionName) - - // A revision without completedAt is still RollingOut. + require.Equal(t, "cache-installed", states.Installed.RevisionName) require.Len(t, states.RollingOut, 1) - require.Equal(t, "test-ext-2", states.RollingOut[0].RevisionName) + require.Equal(t, "cache-rolling", states.RollingOut[0].RevisionName) + + require.NoError(t, cl.Delete(ctx, &ocv1.ClusterObjectSet{ObjectMeta: metav1.ObjectMeta{Name: "cache-rolling"}})) + require.Eventually(t, func() bool { + states, err := getter.GetRevisionStates(ctx, ext) + return err == nil && len(states.RollingOut) == 0 + }, 5*time.Second, 10*time.Millisecond) } diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 70e6d6f572..2367ad88a1 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -32,6 +32,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + "pkg.package-operator.run/boxcutter/managedcache" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,7 +43,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" "github.com/operator-framework/api/pkg/operators/v1alpha1" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" @@ -420,23 +420,39 @@ func collectDeprecationMessages(entries []declcfg.DeprecationEntry) []string { return messages } -type ControllerBuilderOption func(builder *ctrl.Builder) +// SetupWithManagerForBoxcutter sets up the controller with ClusterObjectSet revision watches. +// TODO(COD): Replace this COS watch and mapper with a COD watch when ClusterExtension +// reconciliation manages ClusterObjectDeployments. +func (r *ClusterExtensionReconciler) SetupWithManagerForBoxcutter(mgr ctrl.Manager) (crcontroller.Controller, error) { + return r.newControllerBuilder(mgr). + Watches(&ocv1.ClusterObjectSet{}, + crhandler.EnqueueRequestsFromMapFunc(clusterExtensionRequestsForClusterObjectSet)). + Build(r) +} -func WithOwns(obj client.Object) ControllerBuilderOption { - return func(builder *ctrl.Builder) { - builder.Owns(obj) - } +// SetupWithManagerForHelm sets up the controller with watches for managed Helm resources. +func (r *ClusterExtensionReconciler) SetupWithManagerForHelm(mgr ctrl.Manager, trackingCache managedcache.TrackingCache) (crcontroller.Controller, error) { + return r.newControllerBuilder(mgr). + WatchesRawSource(trackingCache.Source( + crhandler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &ocv1.ClusterExtension{}), + predicate.ResourceVersionChangedPredicate{}, + predicate.Funcs{ + CreateFunc: func(event.TypedCreateEvent[client.Object]) bool { return false }, + }, + )). + Build(r) } -func WithWatchesRawSource(src source.Source) ControllerBuilderOption { - return func(b *ctrl.Builder) { - b.WatchesRawSource(src) +func clusterExtensionRequestsForClusterObjectSet(_ context.Context, obj client.Object) []reconcile.Request { + cos, ok := obj.(*ocv1.ClusterObjectSet) + if !ok || cos.Spec.Group == "" { + return nil } + return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: cos.Spec.Group}}} } -// SetupWithManager sets up the controller with the Manager. -func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ...ControllerBuilderOption) (crcontroller.Controller, error) { - ctrlBuilder := ctrl.NewControllerManagedBy(mgr). +func (r *ClusterExtensionReconciler) newControllerBuilder(mgr ctrl.Manager) *ctrl.Builder { + return ctrl.NewControllerManagedBy(mgr). For(&ocv1.ClusterExtension{}). Named("controller-operator-cluster-extension-controller"). Watches(&ocv1.ClusterCatalog{}, @@ -458,12 +474,6 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ... return true }, })) - - for _, applyOpt := range opts { - applyOpt(ctrlBuilder) - } - - return ctrlBuilder.Build(r) } func wrapErrorWithResolutionInfo(resolved ocv1.BundleMetadata, err error) error { diff --git a/internal/operator-controller/controllers/clusterextension_controller_internal_test.go b/internal/operator-controller/controllers/clusterextension_controller_internal_test.go new file mode 100644 index 0000000000..31363ea79e --- /dev/null +++ b/internal/operator-controller/controllers/clusterextension_controller_internal_test.go @@ -0,0 +1,46 @@ +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +func TestClusterExtensionRequestsForClusterObjectSet(t *testing.T) { + for _, tc := range []struct { + name string + obj client.Object + want []reconcile.Request + }{ + { + name: "group without owner", + obj: &ocv1.ClusterObjectSet{Spec: ocv1.ClusterObjectSetSpec{Group: "my-extension"}}, + want: []reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-extension"}}}, + }, + { + name: "group takes precedence over owner metadata", + obj: &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"olm.operatorframework.io/owner-name": "other"}, + OwnerReferences: []metav1.OwnerReference{{Kind: ocv1.ClusterExtensionKind, Name: "other", Controller: ptr.To(true)}}, + }, + Spec: ocv1.ClusterObjectSetSpec{Group: "my-extension"}, + }, + want: []reconcile.Request{{NamespacedName: types.NamespacedName{Name: "my-extension"}}}, + }, + {name: "empty group", obj: &ocv1.ClusterObjectSet{}}, + {name: "other resource", obj: &corev1.ConfigMap{}}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, clusterExtensionRequestsForClusterObjectSet(t.Context(), tc.obj)) + }) + } +} diff --git a/internal/operator-controller/revisions/clusterobjectsets.go b/internal/operator-controller/revisions/clusterobjectsets.go new file mode 100644 index 0000000000..04599f82de --- /dev/null +++ b/internal/operator-controller/revisions/clusterobjectsets.go @@ -0,0 +1,41 @@ +/* +Copyright 2026. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package revisions + +import ( + "cmp" + "context" + "slices" + + "sigs.k8s.io/controller-runtime/pkg/client" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +// ListForGroup returns all COS revisions in the group, oldest first. +// Owner references do not affect revision association. Lifecycle filtering is +// left to callers. +func ListForGroup(ctx context.Context, reader client.Reader, group string) ([]ocv1.ClusterObjectSet, error) { + list := &ocv1.ClusterObjectSetList{} + if err := reader.List(ctx, list, client.MatchingFields{".spec.group": group}); err != nil { + return nil, err + } + slices.SortFunc(list.Items, func(a, b ocv1.ClusterObjectSet) int { + return cmp.Compare(a.Spec.Revision, b.Spec.Revision) + }) + return list.Items, nil +} diff --git a/internal/operator-controller/revisions/clusterobjectsets_test.go b/internal/operator-controller/revisions/clusterobjectsets_test.go new file mode 100644 index 0000000000..670ac30d7b --- /dev/null +++ b/internal/operator-controller/revisions/clusterobjectsets_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2026. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package revisions_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" + coscontrollers "github.com/operator-framework/operator-controller/internal/object-controller/controllers" + "github.com/operator-framework/operator-controller/internal/operator-controller/revisions" + "github.com/operator-framework/operator-controller/internal/shared/labels" + "github.com/operator-framework/operator-controller/test" +) + +func TestListForGroup(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(scheme)) + const group = "test-group" + ext := &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", UID: "current-uid"}} + owner := *metav1.NewControllerRef(ext, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind)) + differentName, differentKind, differentIdentity := owner, owner, owner + differentName.Name = "other-ext" + differentKind.Kind = "OtherController" + differentIdentity.UID = "previous-uid" + differentIdentity.APIVersion = "olm.operatorframework.io/v2" + nonController, unspecifiedController := owner, owner + nonController.Controller = ptr.To(false) + unspecifiedController.Controller = nil + + objects := make([]client.Object, 0, 9) + for _, tc := range []struct { + name string + group string + owners []metav1.OwnerReference + revision int64 + }{ + {name: "matching-owner", group: group, owners: []metav1.OwnerReference{owner}, revision: 2}, + {name: "oldest", group: group, owners: []metav1.OwnerReference{differentIdentity}, revision: 1}, + {name: "other-group", group: "other-group", owners: []metav1.OwnerReference{owner}, revision: 99}, + {name: "other-name", group: group, owners: []metav1.OwnerReference{differentName}, revision: 100}, + {name: "other-kind", group: group, owners: []metav1.OwnerReference{differentKind}, revision: 101}, + {name: "ownerless", group: group, revision: 102}, + {name: "non-controller", group: group, owners: []metav1.OwnerReference{nonController}, revision: 103}, + {name: "unspecified-controller", group: group, owners: []metav1.OwnerReference{unspecifiedController}, revision: 104}, + {name: "foreign-controller-with-ce-owner", group: group, owners: []metav1.OwnerReference{differentKind, nonController}, revision: 105}, + } { + objects = append(objects, &ocv1.ClusterObjectSet{ + ObjectMeta: metav1.ObjectMeta{Name: tc.name, OwnerReferences: tc.owners}, + Spec: ocv1.ClusterObjectSetSpec{Group: tc.group, Revision: tc.revision}, + }) + } + // Labels cannot override group membership, regardless of owner references. + for _, object := range objects[2:] { + object.SetLabels(map[string]string{labels.OwnerNameKey: ext.Name}) + } + cl := test.WithIndexes(t, fake.NewClientBuilder().WithScheme(scheme), coscontrollers.SetupIndexes). + WithObjects(objects...).Build() + got, err := revisions.ListForGroup(t.Context(), cl, group) + require.NoError(t, err) + names := make([]string, 0, len(got)) + for _, cos := range got { + names = append(names, cos.Name) + } + require.Equal(t, []string{ + "oldest", "matching-owner", "other-name", "other-kind", "ownerless", + "non-controller", "unspecified-controller", "foreign-controller-with-ce-owner", + }, names) +} diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 4c2cce2adc..4b4edb0dc7 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -1427,6 +1427,9 @@ spec: scope: Cluster versions: - additionalPrinterColumns: + - jsonPath: .spec.group + name: Group + type: string - jsonPath: .status.conditions[?(@.type=='Ready')].status name: Ready type: string @@ -1437,12 +1440,12 @@ spec: schema: openAPIV3Schema: description: |- - ClusterObjectSet represents an immutable snapshot of Kubernetes objects - for a specific version of a ClusterExtension. Each revision contains objects - organized into phases that roll out sequentially. The same object can only be managed by a single revision - at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded - or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for - posterity. + ClusterObjectSet represents an immutable snapshot of Kubernetes objects to + apply and manage on the cluster. Each revision contains objects organized into + phases that roll out sequentially. The same object can only be managed by a + single revision at a time. Ownership of objects is transitioned from one revision + to the next as new revisions are rolled out. Once the latest revision has rolled + out successfully, previous active revisions are archived for posterity. properties: apiVersion: description: |- @@ -1481,6 +1484,22 @@ spec: x-kubernetes-validations: - message: collisionProtection is immutable rule: self == oldSelf + group: + description: |- + group is a required, immutable identifier that links related revisions together. + All revisions sharing the same group form an ordered sequence, regardless of owner references. + The value must be 1 to 52 characters long, start with a lowercase letter, + contain only lowercase letters, digits or hyphens, and end with a letter or digit. + maxLength: 52 + minLength: 1 + type: string + x-kubernetes-validations: + - message: group must start with a lowercase letter, contain only + lowercase letters, digits or hyphens, and end with a letter or + digit + rule: self.matches('^[a-z]([a-z0-9-]*[a-z0-9])?$') + - message: group is immutable + rule: self == oldSelf lifecycleState: description: |- lifecycleState specifies the lifecycle state of the ClusterObjectSet. @@ -1929,11 +1948,11 @@ spec: x-kubernetes-list-type: atomic revision: description: |- - revision is a required, immutable sequence number representing a specific revision - of the parent ClusterExtension. + revision is a required, immutable sequence number identifying a specific + ClusterObjectSet within a sequence of related revisions. The revision field must be a positive integer. - Each ClusterObjectSet belonging to the same parent ClusterExtension must have a unique revision number. + Each ClusterObjectSet in the same revision sequence must have a unique revision number. The revision number must always be the previous revision number plus one, or 1 for the first revision. format: int64 minimum: 1 @@ -1943,6 +1962,7 @@ spec: rule: self == oldSelf required: - collisionProtection + - group - lifecycleState - revision type: object @@ -2084,7 +2104,11 @@ spec: x-kubernetes-validations: - message: completedAt cannot be removed once set rule: '!has(oldSelf.completedAt) || has(self.completedAt)' + required: + - spec type: object + selectableFields: + - jsonPath: .spec.group served: true storage: true subresources: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 146a9513e8..e0c6c9cc13 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -1388,6 +1388,9 @@ spec: scope: Cluster versions: - additionalPrinterColumns: + - jsonPath: .spec.group + name: Group + type: string - jsonPath: .status.conditions[?(@.type=='Ready')].status name: Ready type: string @@ -1398,12 +1401,12 @@ spec: schema: openAPIV3Schema: description: |- - ClusterObjectSet represents an immutable snapshot of Kubernetes objects - for a specific version of a ClusterExtension. Each revision contains objects - organized into phases that roll out sequentially. The same object can only be managed by a single revision - at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded - or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for - posterity. + ClusterObjectSet represents an immutable snapshot of Kubernetes objects to + apply and manage on the cluster. Each revision contains objects organized into + phases that roll out sequentially. The same object can only be managed by a + single revision at a time. Ownership of objects is transitioned from one revision + to the next as new revisions are rolled out. Once the latest revision has rolled + out successfully, previous active revisions are archived for posterity. properties: apiVersion: description: |- @@ -1442,6 +1445,22 @@ spec: x-kubernetes-validations: - message: collisionProtection is immutable rule: self == oldSelf + group: + description: |- + group is a required, immutable identifier that links related revisions together. + All revisions sharing the same group form an ordered sequence, regardless of owner references. + The value must be 1 to 52 characters long, start with a lowercase letter, + contain only lowercase letters, digits or hyphens, and end with a letter or digit. + maxLength: 52 + minLength: 1 + type: string + x-kubernetes-validations: + - message: group must start with a lowercase letter, contain only + lowercase letters, digits or hyphens, and end with a letter or + digit + rule: self.matches('^[a-z]([a-z0-9-]*[a-z0-9])?$') + - message: group is immutable + rule: self == oldSelf lifecycleState: description: |- lifecycleState specifies the lifecycle state of the ClusterObjectSet. @@ -1890,11 +1909,11 @@ spec: x-kubernetes-list-type: atomic revision: description: |- - revision is a required, immutable sequence number representing a specific revision - of the parent ClusterExtension. + revision is a required, immutable sequence number identifying a specific + ClusterObjectSet within a sequence of related revisions. The revision field must be a positive integer. - Each ClusterObjectSet belonging to the same parent ClusterExtension must have a unique revision number. + Each ClusterObjectSet in the same revision sequence must have a unique revision number. The revision number must always be the previous revision number plus one, or 1 for the first revision. format: int64 minimum: 1 @@ -1904,6 +1923,7 @@ spec: rule: self == oldSelf required: - collisionProtection + - group - lifecycleState - revision type: object @@ -2045,7 +2065,11 @@ spec: x-kubernetes-validations: - message: completedAt cannot be removed once set rule: '!has(oldSelf.completedAt) || has(self.completedAt)' + required: + - spec type: object + selectableFields: + - jsonPath: .spec.group served: true storage: true subresources: diff --git a/test/e2e/features/group.feature b/test/e2e/features/group.feature new file mode 100644 index 0000000000..2dc31a07e9 --- /dev/null +++ b/test/e2e/features/group.feature @@ -0,0 +1,211 @@ +@BoxcutterRuntime +Feature: Track ClusterObjectSet revisions by group + + Background: + Given OLM is available + And an image registry is available + And namespace "${TEST_NAMESPACE}" is available + + Scenario Outline: Group handoff preserves objects across + Given resource is applied + """ + apiVersion: v1 + kind: Namespace + metadata: + name: ${TEST_NAMESPACE}-other + """ + And ClusterObjectSet with Namespace owner "" is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${NAME}-1 + spec: + group: ${NAME} + revision: 1 + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: config + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: ${NAME}-payload + namespace: ${TEST_NAMESPACE} + data: + revision: "1" + """ + Then ClusterObjectSet "${NAME}-1" reports Ready as True with Reason AllObjectsReady + And ClusterObjectSet "${NAME}-1" phase objects use SSA manager "cos-group/${NAME}" + And resource "configmap/${NAME}-payload" UID is remembered + When ClusterObjectSet with Namespace owner "" is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${NAME}-2 + spec: + group: ${NAME} + revision: 2 + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: config + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: ${NAME}-payload + namespace: ${TEST_NAMESPACE} + data: + revision: "2" + """ + Then ClusterObjectSet "${NAME}-2" reports Ready as True with Reason AllObjectsReady + And ClusterObjectSet "${NAME}-1" is archived + And ClusterObjectSet "${NAME}-2" phase objects use SSA manager "cos-group/${NAME}" + And resource "configmap/${NAME}-payload" matches + """ + data: + revision: "2" + """ + And resource "configmap/${NAME}-payload" keeps its UID + + # An empty owner creates an ownerless COS. Namespace owners are real, + # cluster-scoped objects, independent of CE and future COD controllers. + Examples: + | ownership | lower-owner | higher-owner | + | different controllers | ${TEST_NAMESPACE} | ${TEST_NAMESPACE}-other | + | owned to ownerless | ${TEST_NAMESPACE} | | + | ownerless to owned | | ${TEST_NAMESPACE} | + + Scenario: Different groups collide even with the same controller owner + Given ClusterObjectSet with Namespace owner "${TEST_NAMESPACE}" is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${NAME}-1 + spec: + group: ${NAME} + revision: 1 + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: config + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: ${NAME}-payload + namespace: ${TEST_NAMESPACE} + data: + revision: "1" + """ + Then ClusterObjectSet "${NAME}-1" reports Ready as True with Reason AllObjectsReady + And resource "configmap/${NAME}-payload" UID is remembered + When ClusterObjectSet with Namespace owner "${TEST_NAMESPACE}" is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${NAME}-other-2 + spec: + group: ${NAME}-other + revision: 2 + lifecycleState: Active + collisionProtection: Prevent + phases: + - name: config + objects: + - object: + apiVersion: v1 + kind: ConfigMap + metadata: + name: ${NAME}-payload + namespace: ${TEST_NAMESPACE} + data: + revision: "2" + """ + Then ClusterObjectSet "${NAME}-other-2" reports Ready as False with Reason ObjectCollision and Message includes: + """ + Cannot take ownership of 1 object(s) + """ + And ClusterObjectSet "${NAME}-1" phase objects use SSA manager "cos-group/${NAME}" + And resource "configmap/${NAME}-payload" matches + """ + data: + revision: "1" + """ + And resource "configmap/${NAME}-payload" keeps its UID + And resource "clusterobjectset/${NAME}-1" matches + """ + spec: + lifecycleState: Active + """ + + Scenario: Revision pruning is isolated by group + Given a catalog "test" with packages: + | package | version | channel | replaces | contents | + | test | 1.0.0 | beta | | CRD, Deployment, ConfigMap | + | test | 1.0.1 | beta | 1.0.0 | CRD, Deployment, ConfigMap | + | test | 1.0.2 | beta | 1.0.1 | CRD, Deployment, ConfigMap | + | test | 1.0.3 | beta | 1.0.2 | CRD, Deployment, ConfigMap | + | test | 1.0.4 | beta | 1.0.3 | CRD, Deployment, ConfigMap | + | test | 1.0.5 | beta | 1.0.4 | CRD, Deployment, ConfigMap | + | test | 1.0.6 | beta | 1.0.5 | CRD, Deployment, ConfigMap | + And ClusterObjectSet is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterObjectSet + metadata: + name: ${COS_NAME} + labels: + olm.operatorframework.io/owner-name: ${NAME} + spec: + group: ${NAME}-other + revision: 1 + lifecycleState: Archived + collisionProtection: Prevent + """ + And ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + namespace: ${TEST_NAMESPACE} + source: + sourceType: Catalog + catalog: + packageName: ${PACKAGE:test} + version: 1.0.0 + selector: + matchLabels: + olm.operatorframework.io/metadata.name: ${CATALOG:test} + """ + And ClusterExtension is rolled out + When ClusterExtension version is updated to "1.0.1" + Then ClusterExtension is rolled out + When ClusterExtension version is updated to "1.0.2" + Then ClusterExtension is rolled out + When ClusterExtension version is updated to "1.0.3" + Then ClusterExtension is rolled out + When ClusterExtension version is updated to "1.0.4" + Then ClusterExtension is rolled out + When ClusterExtension version is updated to "1.0.5" + Then ClusterExtension is rolled out + When ClusterExtension version is updated to "1.0.6" + Then ClusterExtension is rolled out + And ClusterExtension is available + And ClusterExtension reports "${NAME}-7" as active revision + And ClusterObjectSet "${NAME}-7" has group "${NAME}" + And resource "clusterobjectset/${NAME}-1" is eventually not found + # The existing retention policy keeps five prior revisions plus the current one. + And ClusterExtension "${NAME}" owns 6 ClusterObjectSets + And resource "clusterobjectset/${COS_NAME}" exists + And ClusterObjectSet "${COS_NAME}" has group "${NAME}-other" diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index 3ae8b98ccc..95d0eec364 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -465,6 +465,8 @@ Feature: Install ClusterExtension And ClusterExtension is available And ClusterObjectSet "${NAME}-1" has label "olm.operatorframework.io/owner-kind" with value "ClusterExtension" And ClusterObjectSet "${NAME}-1" has label "olm.operatorframework.io/owner-name" with value "${NAME}" + And ClusterObjectSet "${NAME}-1" has group "${NAME}" + And ClusterExtension "${NAME}" owns 1 ClusterObjectSet @BoxcutterRuntime Scenario: ClusterObjectSet objects are externalized to immutable Secrets diff --git a/test/e2e/features/revision.feature b/test/e2e/features/revision.feature index 75c93d0fa7..9cf0b15056 100644 --- a/test/e2e/features/revision.feature +++ b/test/e2e/features/revision.feature @@ -15,6 +15,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent progressionProbes: @@ -75,6 +76,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent progressionProbes: @@ -161,6 +163,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent progressionProbes: @@ -219,6 +222,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent progressionProbes: @@ -403,6 +407,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent phases: @@ -454,6 +459,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent phases: @@ -502,6 +508,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent phases: @@ -585,6 +592,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent progressDeadlineMinutes: 1 @@ -651,6 +659,7 @@ Feature: Install ClusterObjectSet metadata: name: ${COS_NAME} spec: + group: ${NAME} lifecycleState: Active collisionProtection: Prevent progressDeadlineMinutes: 1 diff --git a/test/e2e/features/uninstall.feature b/test/e2e/features/uninstall.feature index bd2fd7b5f4..973d90ab5a 100644 --- a/test/e2e/features/uninstall.feature +++ b/test/e2e/features/uninstall.feature @@ -34,3 +34,13 @@ Feature: Uninstall ClusterExtension When ClusterExtension is removed Then the ClusterExtension's constituent resources are removed + @BoxcutterRuntime + Scenario: Removing ClusterExtension cascades to grouped revisions and their content Secrets + Given ClusterObjectSet "${NAME}-1" has group "${NAME}" + And ClusterExtension "${NAME}" owns 1 ClusterObjectSet + And ClusterObjectSet "${NAME}-1" referred secrets are owned by the object set + And ClusterObjectSet "${NAME}-1" referred secrets are remembered + When ClusterExtension is removed + Then resource "clusterobjectset/${NAME}-1" is eventually not found + And the ClusterExtension's constituent resources are removed + And the remembered revision secrets are removed diff --git a/test/e2e/features/update.feature b/test/e2e/features/update.feature index 94520d0f18..d24ad0ad9a 100644 --- a/test/e2e/features/update.feature +++ b/test/e2e/features/update.feature @@ -310,6 +310,8 @@ Feature: Update ClusterExtension """ And ClusterExtension is rolled out And ClusterExtension is available + And ClusterObjectSet "${NAME}-1" has group "${NAME}" + And ClusterObjectSet "${NAME}-1" phase objects use SSA manager "cos-group/${NAME}" When ClusterExtension version is updated to "1.2.0" Then bundle "${PACKAGE:test}.1.2.0" is installed in version "1.2.0" And ClusterExtension is rolled out @@ -318,6 +320,8 @@ Feature: Update ClusterExtension And ClusterObjectSet "${NAME}-2" reports Ready as True with Reason AllObjectsReady And ClusterObjectSet "${NAME}-1" is archived And ClusterObjectSet "${NAME}-1" phase objects are not found or not owned by the revision + And ClusterObjectSet "${NAME}-2" has group "${NAME}" + And ClusterObjectSet "${NAME}-2" phase objects use SSA manager "cos-group/${NAME}" @BoxcutterRuntime Scenario: Report all active revisions on ClusterExtension diff --git a/test/e2e/steps/group_steps.go b/test/e2e/steps/group_steps.go new file mode 100644 index 0000000000..204fd42dee --- /dev/null +++ b/test/e2e/steps/group_steps.go @@ -0,0 +1,211 @@ +package steps + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/cucumber/godog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/yaml" + + ocv1 "github.com/operator-framework/operator-controller/api/v1" +) + +// An empty owner name creates an ownerless revision. Resolve a real cluster-scoped +// owner before creation so garbage collection and reconciliation see valid metadata. +func ClusterObjectSetWithNamespaceOwnerApplied(ctx context.Context, ownerName string, template *godog.DocString) error { + sc := scenarioCtx(ctx) + obj, err := toUnstructured(substituteScenarioVars(template.Content, sc)) + if err != nil { + return err + } + ownerName = substituteScenarioVars(ownerName, sc) + if ownerName != "" { + owner, err := getResource("namespace", ownerName, "") + if err != nil { + return err + } + obj.SetOwnerReferences([]metav1.OwnerReference{{ + APIVersion: "v1", Kind: "Namespace", Name: ownerName, UID: owner.GetUID(), + Controller: ptr.To(true), BlockOwnerDeletion: ptr.To(true), + }}) + } else { + obj.SetOwnerReferences(nil) + } + content, err := yaml.Marshal(obj.Object) + if err != nil { + return err + } + return ResourceIsApplied(ctx, &godog.DocString{Content: string(content)}) +} + +func RememberResourceUID(ctx context.Context, resourceName string) error { + sc := scenarioCtx(ctx) + resourceName = substituteScenarioVars(resourceName, sc) + kind, name, found := strings.Cut(resourceName, "/") + if !found { + return fmt.Errorf("resource %s is not in the format /", resourceName) + } + obj, err := getResource(kind, name, sc.namespace) + if err != nil { + return err + } + if obj.GetUID() == "" { + return fmt.Errorf("resource %s has no UID", resourceName) + } + if sc.resourceUIDs == nil { + sc.resourceUIDs = map[string]types.UID{} + } + sc.resourceUIDs[resourceName] = obj.GetUID() + return nil +} + +func ResourceKeepsUID(ctx context.Context, resourceName string) error { + sc := scenarioCtx(ctx) + resourceName = substituteScenarioVars(resourceName, sc) + uid, found := sc.resourceUIDs[resourceName] + if !found { + return fmt.Errorf("no UID remembered for resource %s", resourceName) + } + kind, name, _ := strings.Cut(resourceName, "/") + obj, err := getResource(kind, name, sc.namespace) + if err != nil { + return err + } + if obj.GetUID() != uid { + return fmt.Errorf("resource %s was replaced: UID changed from %s to %s", resourceName, uid, obj.GetUID()) + } + return nil +} + +// A local cache index does not make spec.group selectable through kubectl. +func listClusterObjectSetsInGroup(ctx context.Context, group string) ([]ocv1.ClusterObjectSet, error) { + out, err := k8sClient(ctx, "get", "clusterobjectsets", "-o", "json") + if err != nil { + return nil, err + } + var list ocv1.ClusterObjectSetList + if err := json.Unmarshal([]byte(out), &list); err != nil { + return nil, err + } + var revisions []ocv1.ClusterObjectSet + for _, rev := range list.Items { + if rev.Spec.Group == group { + revisions = append(revisions, rev) + } + } + return revisions, nil +} + +func ClusterObjectSetHasGroup(ctx context.Context, revisionName, group string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(revisionName, sc) + group = substituteScenarioVars(group, sc) + waitFor(ctx, func() bool { + obj, err := getResource("clusterobjectset", revisionName, "") + if err != nil { + return false + } + var cos ocv1.ClusterObjectSet + return runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &cos) == nil && cos.Spec.Group == group + }) + return nil +} + +// ClusterObjectSetObjectsUseSSAManager checks actual managed fields on every +// inline or externally stored phase object, including their revision owner. +func ClusterObjectSetObjectsUseSSAManager(ctx context.Context, revisionName, manager string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(revisionName, sc) + manager = substituteScenarioVars(manager, sc) + waitFor(ctx, func() bool { + obj, err := getResource("clusterobjectset", revisionName, "") + if err != nil { + return false + } + var cos ocv1.ClusterObjectSet + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &cos); err != nil { + return false + } + count := 0 + for _, phase := range cos.Spec.Phases { + for _, phaseObj := range phase.Objects { + desired := &phaseObj.Object + if phaseObj.Ref.Name != "" { + desired, err = resolveObjectRef(phaseObj.Ref) + if err != nil { + return false + } + } + // kubectl's JSON/YAML printers omit managedFields by default. + args := []string{"get", desired.GetKind(), desired.GetName(), "-o", "json", "--show-managed-fields"} + if desired.GetNamespace() != "" { + args = append(args, "-n", desired.GetNamespace()) + } + out, err := k8sClient(ctx, args...) + if err != nil { + return false + } + actual := &unstructured.Unstructured{} + if err := json.Unmarshal([]byte(out), &actual.Object); err != nil { + return false + } + owner := metav1.GetControllerOf(actual) + if owner == nil || owner.Kind != ocv1.ClusterObjectSetKind || owner.UID != cos.UID { + return false + } + found := false + for _, fields := range actual.GetManagedFields() { + if fields.Operation != metav1.ManagedFieldsOperationApply { + continue + } + if strings.HasPrefix(fields.Manager, "cos-group/") && fields.Manager != manager { + return false + } + if fields.Manager == manager { + found = true + } + } + if !found { + return false + } + count++ + } + } + return count > 0 + }) + return nil +} + +func RememberClusterObjectSetSecrets(ctx context.Context, revisionName string) error { + sc := scenarioCtx(ctx) + revisionName = substituteScenarioVars(revisionName, sc) + names, err := collectReferredSecretNames(ctx, revisionName) + if err != nil { + return err + } + for _, name := range names { + sc.revisionSecrets = append(sc.revisionSecrets, resource{kind: "secret", name: name, namespace: olmNamespace}) + } + return nil +} + +func RememberedRevisionSecretsRemoved(ctx context.Context) error { + sc := scenarioCtx(ctx) + if len(sc.revisionSecrets) == 0 { + return fmt.Errorf("no revision secrets were remembered") + } + for _, secret := range sc.revisionSecrets { + waitFor(ctx, func() bool { + out, err := k8sClient(ctx, "get", secret.kind, secret.name, "-n", secret.namespace, "--ignore-not-found", "-o", "name") + return err == nil && strings.TrimSpace(out) == "" + }) + } + return nil +} diff --git a/test/e2e/steps/hooks.go b/test/e2e/steps/hooks.go index ee93303408..3278a31e0e 100644 --- a/test/e2e/steps/hooks.go +++ b/test/e2e/steps/hooks.go @@ -17,6 +17,7 @@ import ( "golang.org/x/sync/errgroup" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/component-base/featuregate" "k8s.io/klog/v2/textlogger" "sigs.k8s.io/controller-runtime/pkg/client" @@ -56,6 +57,8 @@ type scenarioContext struct { leaderPods map[string]string // component name -> leader pod name deploymentRestores []deploymentRestore extensionObjects []client.Object + revisionSecrets []resource + resourceUIDs map[string]types.UID proxy *recordingProxy catalogAddr string catalogCleanup func() diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 330dffc41e..9840005bfe 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -129,11 +129,18 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)ClusterObjectSet "([^"]+)" reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message includes:$`, ClusterObjectSetReportsConditionWithMessageFragment) sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) transition between (\d+) and (\d+) minutes since its creation$`, ClusterExtensionReportsConditionTransitionTime) sc.Step(`^(?i)ClusterObjectSet is applied(?:\s+.*)?$`, ResourceIsApplied) + sc.Step(`^(?i)ClusterObjectSet with Namespace owner "([^"]*)" is applied$`, ClusterObjectSetWithNamespaceOwnerApplied) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" reconciliation is triggered$`, TriggerClusterObjectSetReconciliation) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" has observed phase "([^"]+)" with a non-empty digest$`, ClusterObjectSetHasObservedPhase) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" is archived$`, ClusterObjectSetIsArchived) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" contains annotation "([^"]+)" with value$`, ClusterObjectSetHasAnnotationWithValue) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" has label "([^"]+)" with value "([^"]+)"$`, ClusterObjectSetHasLabelWithValue) + sc.Step(`^(?i)ClusterObjectSet "([^"]+)" has group "([^"]+)"$`, ClusterObjectSetHasGroup) + sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects use SSA manager "([^"]+)"$`, ClusterObjectSetObjectsUseSSAManager) + sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets are remembered$`, RememberClusterObjectSetSecrets) + sc.Step(`^(?i)the remembered revision secrets are removed$`, RememberedRevisionSecretsRemoved) + sc.Step(`^(?i)resource "([^"]+)" UID is remembered$`, RememberResourceUID) + sc.Step(`^(?i)resource "([^"]+)" keeps its UID$`, ResourceKeepsUID) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects are not found or not owned by the revision$`, ClusterObjectSetObjectsNotFoundOrNotOwned) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" phase objects are managed in Kubernetes secrets$`, ClusterObjectSetPhaseObjectsManagedInSecrets) sc.Step(`^(?i)ClusterObjectSet "([^"]+)" referred secrets exist in "([^"]+)" namespace$`, ClusterObjectSetReferredSecretsExist) @@ -550,14 +557,24 @@ func ClusterExtensionOwnsClusterObjectSets(ctx context.Context, extName string, sc := scenarioCtx(ctx) extName = substituteScenarioVars(extName, sc) waitFor(ctx, func() bool { - out, err := k8sClient(ctx, "get", "clusterobjectsets", - "-l", fmt.Sprintf("olm.operatorframework.io/owner-name=%s", extName), - "-o", "jsonpath={.items[*].metadata.name}") + revisions, err := listClusterObjectSetsInGroup(ctx, extName) if err != nil { return false } - names := strings.Fields(strings.TrimSpace(out)) - return len(names) == expectedCount + if len(revisions) != expectedCount { + return false + } + ext, err := getResource("clusterextension", extName, "") + if err != nil { + return false + } + for _, rev := range revisions { + owner := metav1.GetControllerOf(&rev) + if owner == nil || owner.Kind != ocv1.ClusterExtensionKind || owner.Name != extName || owner.UID != ext.GetUID() { + return false + } + } + return true }) return nil } @@ -2298,21 +2315,13 @@ func resolveObjectRef(ref ocv1.ObjectSourceRef) (*unstructured.Unstructured, err // latestActiveRevisionForExtension returns the latest active revision for the extension called extName func latestActiveRevisionForExtension(extName string) (*ocv1.ClusterObjectSet, error) { - out, err := k8sClient(context.Background(), "get", "clusterobjectsets", "-l", fmt.Sprintf("olm.operatorframework.io/owner-name=%s", extName), "-o", "json") + revisions, err := listClusterObjectSetsInGroup(context.Background(), extName) if err != nil { return nil, fmt.Errorf("error listing revisions for extension '%s': %w", extName, err) } - if strings.TrimSpace(out) == "" { - return nil, fmt.Errorf("no revisions found for extension '%s'", extName) - } - var revisionList ocv1.ClusterObjectSetList - if err := json.Unmarshal([]byte(out), &revisionList); err != nil { - return nil, fmt.Errorf("error unmarshalling revisions for extension '%s': %w", extName, err) - } - var latest *ocv1.ClusterObjectSet - for i := range revisionList.Items { - rev := &revisionList.Items[i] + for i := range revisions { + rev := &revisions[i] if rev.Spec.LifecycleState != ocv1.ClusterObjectSetLifecycleStateActive { continue } diff --git a/test/indexes.go b/test/indexes.go new file mode 100644 index 0000000000..032d578fbe --- /dev/null +++ b/test/indexes.go @@ -0,0 +1,28 @@ +package test + +import ( + "context" + "testing" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// WithIndexes registers controller cache indexes on a fake client builder. +// The builder must already have its scheme configured. +func WithIndexes(t *testing.T, builder *fake.ClientBuilder, setup func(context.Context, client.FieldIndexer) error) *fake.ClientBuilder { + t.Helper() + if err := setup(t.Context(), fakeFieldIndexer{builder: builder}); err != nil { + t.Fatalf("setting up fake client indexes: %v", err) + } + return builder +} + +type fakeFieldIndexer struct { + builder *fake.ClientBuilder +} + +func (f fakeFieldIndexer) IndexField(_ context.Context, obj client.Object, field string, extract client.IndexerFunc) error { + f.builder.WithIndex(obj, field, extract) + return nil +}