⚠ Add spec.group to ClusterObjectSet revision tracking

api/v1/clusterobjectset_types.go

@@ -38,6 +38,20 @@ const (
 
 // ClusterObjectSetSpec defines the desired state of ClusterObjectSet.
 type ClusterObjectSetSpec struct {
+	// group is a required, immutable identifier that links related revisions together.
+	// Revisions sharing the same group and controller owner kind and name form an ordered sequence.
+	// Only owner references with controller set to true are considered.
+	// Revisions without a controller owner form a sequence with other such revisions in the same group.
+	// 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.
@@ -56,11 +70,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
@@ -559,15 +573,16 @@ type ObservedPhase struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:resource:scope=Cluster
 // +kubebuilder:subresource:status
+// +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"`
 
@@ -577,8 +592,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

api/v1/clusterobjectset_types_test.go

@@ -3,25 +3,66 @@ 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"
 )
 
 func TestClusterObjectSetImmutability(t *testing.T) {
 	c := newClient(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 +73,7 @@ func TestClusterObjectSetImmutability(t *testing.T) {
 		},
 		"phases may be initially empty": {
 			spec: ClusterObjectSetSpec{
+				Group:               "test-group",
 				LifecycleState:      ClusterObjectSetLifecycleStateActive,
 				Revision:            1,
 				CollisionProtection: CollisionProtectionPrevent,
@@ -49,6 +91,7 @@ func TestClusterObjectSetImmutability(t *testing.T) {
 		},
 		"phases may be initially unset": {
 			spec: ClusterObjectSetSpec{
+				Group:               "test-group",
 				LifecycleState:      ClusterObjectSetLifecycleStateActive,
 				Revision:            1,
 				CollisionProtection: CollisionProtectionPrevent,
@@ -65,6 +108,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 +130,7 @@ func TestClusterObjectSetImmutability(t *testing.T) {
 		},
 		"spec collisionProtection is immutable": {
 			spec: ClusterObjectSetSpec{
+				Group:               "test-group",
 				LifecycleState:      ClusterObjectSetLifecycleStateActive,
 				Revision:            1,
 				CollisionProtection: CollisionProtectionPrevent,
@@ -112,6 +157,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 +399,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 +412,65 @@ 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 configMap() unstructured.Unstructured {
 	return unstructured.Unstructured{
 		Object: map[string]interface{}{

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,

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)

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}}},