✨ add revisionHistoryLimit to ClusterObjectDeployment

api/v1/clusterobjectdeployment_types.go

@@ -40,6 +40,22 @@ const (
 
 // ClusterObjectDeploymentSpec defines the mutable desired state of a deployment.
 type ClusterObjectDeploymentSpec struct {
+	// revisionHistoryLimit is the maximum number of archived (superseded)
+	// ClusterObjectSet revisions to retain, so that rollout history can be
+	// inspected and earlier revisions rolled back to. Active revisions are never
+	// removed and do not count against this limit. When the number of archived
+	// revisions exceeds this value, the oldest archived revisions are deleted
+	// until the limit is met.
+	//
+	// A value of 0 retains no archived revisions. The minimum is 0, the maximum
+	// is 50, and the default is 5.
+	//
+	// +kubebuilder:validation:Minimum=0
+	// +kubebuilder:validation:Maximum=50
+	// +kubebuilder:default=5
+	// +optional
+	RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty"`
+
 	// template is the desired content of newly created ClusterObjectSets.
 	// Changes to template create a new revision; existing revisions remain immutable.
 	// +required

api/v1/validation_test.go

@@ -2,12 +2,14 @@ package v1
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -183,6 +185,59 @@ func TestValidate(t *testing.T) {
 	}
 }
 
+// newRevisionHistoryLimitCOD returns a minimal valid ClusterObjectDeployment with
+// the given revisionHistoryLimit (nil to leave it unset).
+func newRevisionHistoryLimitCOD(name string, limit *int32) *ClusterObjectDeployment {
+	return &ClusterObjectDeployment{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+		Spec: ClusterObjectDeploymentSpec{
+			RevisionHistoryLimit: limit,
+			Template: ClusterObjectDeploymentTemplate{
+				Spec: ClusterObjectDeploymentTemplateSpec{
+					CollisionProtection: CollisionProtectionPrevent,
+					Phases: []ClusterObjectSetPhase{
+						{Name: "deploy", Objects: []ClusterObjectSetObject{{Object: configMap()}}},
+					},
+				},
+			},
+		},
+	}
+}
+
+func TestClusterObjectDeploymentRevisionHistoryLimitBounds(t *testing.T) {
+	c := newClient(t)
+
+	for name, tc := range map[string]struct {
+		limit *int32
+		valid bool
+	}{
+		"below minimum is rejected": {limit: ptr.To(int32(-1)), valid: false},
+		"minimum of zero is valid":  {limit: ptr.To(int32(0)), valid: true},
+		"maximum of fifty is valid": {limit: ptr.To(int32(50)), valid: true},
+		"above maximum is rejected": {limit: ptr.To(int32(51)), valid: false},
+	} {
+		t.Run(name, func(t *testing.T) {
+			cod := newRevisionHistoryLimitCOD("cod-rhl-"+strings.ReplaceAll(name, " ", "-"), tc.limit)
+			err := c.Create(t.Context(), cod)
+			if tc.valid {
+				require.NoError(t, err)
+			} else {
+				require.True(t, errors.IsInvalid(err), "expected create to fail as invalid, but got: %v", err)
+			}
+		})
+	}
+}
+
+func TestClusterObjectDeploymentRevisionHistoryLimitDefault(t *testing.T) {
+	c := newClient(t)
+
+	cod := newRevisionHistoryLimitCOD("cod-rhl-default", nil)
+	require.NoError(t, c.Create(t.Context(), cod))
+
+	require.NotNil(t, cod.Spec.RevisionHistoryLimit, "revisionHistoryLimit should be defaulted on create when unset")
+	require.Equal(t, int32(5), *cod.Spec.RevisionHistoryLimit, "unset revisionHistoryLimit should default to 5")
+}
+
 func TestClusterObjectSetCompletedAtImmutable(t *testing.T) {
 	c := newClient(t)
 

docs/api-reference/olmv1-api-reference.md

@@ -442,6 +442,7 @@ _Appears in:_
 
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
+| `revisionHistoryLimit` _integer_ | revisionHistoryLimit is the maximum number of archived (superseded)<br />ClusterObjectSet revisions to retain, so that rollout history can be<br />inspected and earlier revisions rolled back to. Active revisions are never<br />removed and do not count against this limit. When the number of archived<br />revisions exceeds this value, the oldest archived revisions are deleted<br />until the limit is met.<br />A value of 0 retains no archived revisions. The minimum is 0, the maximum<br />is 50, and the default is 5. | 5 | Maximum: 50 <br />Minimum: 0 <br />Optional: \{\} <br /> |
 | `template` _[ClusterObjectDeploymentTemplate](#clusterobjectdeploymenttemplate)_ | template is the desired content of newly created ClusterObjectSets.<br />Changes to template create a new revision; existing revisions remain immutable. |  | Required: \{\} <br /> |
 
 

helm/olmv1/base/object-controller/crd/experimental/olm.operatorframework.io_clusterobjectdeployments.yaml

@@ -54,6 +54,22 @@ spec:
           spec:
             description: spec defines the desired deployment template.
             properties:
+              revisionHistoryLimit:
+                default: 5
+                description: |-
+                  revisionHistoryLimit is the maximum number of archived (superseded)
+                  ClusterObjectSet revisions to retain, so that rollout history can be
+                  inspected and earlier revisions rolled back to. Active revisions are never
+                  removed and do not count against this limit. When the number of archived
+                  revisions exceeds this value, the oldest archived revisions are deleted
+                  until the limit is met.
+
+                  A value of 0 retains no archived revisions. The minimum is 0, the maximum
+                  is 50, and the default is 5.
+                format: int32
+                maximum: 50
+                minimum: 0
+                type: integer
               template:
                 description: |-
                   template is the desired content of newly created ClusterObjectSets.

internal/object-controller/controllers/clusterobjectdeployment_controller.go

@@ -10,6 +10,7 @@ import (
 	"slices"
 
 	"k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -27,6 +28,11 @@ import (
 	hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash"
 )
 
+// defaultRevisionHistoryLimit is the number of archived revisions retained when
+// spec.revisionHistoryLimit is unset. It mirrors the CRD default and is applied
+// defensively for callers (e.g. tests) that bypass API-server defaulting.
+const defaultRevisionHistoryLimit = 5
+
 // ClusterObjectDeploymentReconciler reconciles ClusterObjectDeployment objects by
 // managing their owned ClusterObjectSet revisions.
 type ClusterObjectDeploymentReconciler struct {
@@ -68,16 +74,30 @@ func (c *ClusterObjectDeploymentReconciler) Reconcile(ctx context.Context, req c
 	return ctrl.Result{}, reconcileErr
 }
 
-// reconcile creates a new ClusterObjectSet revision when the template has changed
-// (or no active revision exists) and reflects the latest revision's rollout
-// progress in the deployment's Progressing condition. Requeues are driven by the
+// reconcile garbage-collects excess archived revisions and reconciles the
+// deployment's rollout (see reconcileRollout). Requeues are driven by the
 // controller's watch on owned ClusterObjectSets.
 func (c *ClusterObjectDeploymentReconciler) reconcile(ctx context.Context, cod *ocv1.ClusterObjectDeployment) error {
 	revisions, err := c.listRevisions(ctx, cod.Name)
 	if err != nil {
 		return err
 	}
 
+	// Garbage collection is best-effort housekeeping and must not block rollout or
+	// status reconciliation: a persistently failing prune (e.g. a delete denied by
+	// an admission policy) would otherwise indefinitely stall deployment of an
+	// updated template and leave status stale even with a healthy active revision.
+	// Preserve the error, continue reconciling, and return it joined with the
+	// rollout outcome so the failure is still surfaced and retried.
+	gcErr := c.garbageCollectArchivedRevisions(ctx, cod, revisions)
+
+	return errors.Join(gcErr, c.reconcileRollout(ctx, cod, revisions))
+}
+
+// reconcileRollout creates a new ClusterObjectSet revision when the template has
+// changed (or no active revision exists) and reflects the latest revision's
+// rollout progress in the deployment's Available and Progressing conditions.
+func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet) error {
 	hash := templateHash(cod.Spec.Template)
 
 	setAvailable(cod, revisions)
@@ -113,6 +133,74 @@ func (c *ClusterObjectDeploymentReconciler) listRevisions(ctx context.Context, g
 	return list.Items, nil
 }
 
+// garbageCollectArchivedRevisions deletes the oldest archived ClusterObjectSet
+// revisions in excess of spec.revisionHistoryLimit. Active revisions are never
+// deleted and do not count against the limit. A nil limit falls back to
+// defaultRevisionHistoryLimit; a limit of 0 retains no archived revisions.
+//
+// Only archived revisions that have finished tearing down are candidates for
+// pruning (see archivedTeardownComplete). Deleting a revision mid-teardown would
+// send it down the ClusterObjectSet controller's deletion path, which removes the
+// teardown finalizer without calling RevisionEngine.Teardown and so bypasses
+// ordered reverse-phase cleanup (e.g. a controller could be removed before the
+// resources it finalizes have terminated). A revision still tearing down is
+// skipped and pruned on a later reconcile, which the controller's watch on owned
+// ClusterObjectSets triggers once teardown completes.
+//
+// revisions must be sorted ascending by revision number (oldest first), as
+// returned by listRevisions.
+//
+// NOTE: The candidate revisions come from listRevisions, which selects by
+// spec.group (equal to the deployment name) rather than by this deployment's
+// controller owner reference. This is deliberate and consistent with the rest of
+// OLMv1: sibling membership of a revision family is defined by spec.group, not by
+// ownership (see createRevision's naming/grouping and the ClusterObjectSet
+// controller's own sibling archival, both keyed on spec.group). A consequence is
+// that a ClusterObjectSet sharing this group but created by a different owner
+// would be counted toward the limit and could be deleted here. That is tolerated
+// because the deployment name is the group and is treated as an exclusive
+// namespace for its revisions — nothing else should create ClusterObjectSets in
+// it. If siblinghood is ever redefined in terms of ownership, this function and
+// listRevisions must be updated together to filter by owner reference and UID.
+func (c *ClusterObjectDeploymentReconciler) garbageCollectArchivedRevisions(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet) error {
+	limit := defaultRevisionHistoryLimit
+	if cod.Spec.RevisionHistoryLimit != nil {
+		limit = int(*cod.Spec.RevisionHistoryLimit)
+	}
+
+	archived := make([]*ocv1.ClusterObjectSet, 0, len(revisions))
+	for i := range revisions {
+		rev := &revisions[i]
+		if rev.DeletionTimestamp.IsZero() &&
+			rev.Spec.LifecycleState == ocv1.ClusterObjectSetLifecycleStateArchived &&
+			archivedTeardownComplete(rev) {
+			archived = append(archived, rev)
+		}
+	}
+
+	// Delete the oldest archived revisions beyond the limit. Attempt every
+	// excess revision even if one fails, so a single revision that cannot be
+	// deleted (e.g. a denying webhook or a conflict) does not block pruning of
+	// the others on every reconcile.
+	var errs []error
+	for i := 0; i < len(archived)-limit; i++ {
+		if err := c.Client.Delete(ctx, archived[i]); err != nil && !apierrors.IsNotFound(err) {
+			errs = append(errs, fmt.Errorf("deleting archived revision %q: %w", archived[i].Name, err))
+		}
+	}
+	return errors.Join(errs...)
+}
+
+// archivedTeardownComplete reports whether an archived revision has finished
+// tearing down its managed objects, as signalled by its Ready condition
+// (Ready=False/Archived). The ClusterObjectSet controller sets this reason only
+// after RevisionEngine.Teardown completes; while teardown is still in progress it
+// reports Ready=False/Archiving.
+func archivedTeardownComplete(rev *ocv1.ClusterObjectSet) bool {
+	cond := apimeta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
+	return cond != nil && cond.Reason == ocv1.ClusterObjectSetReasonArchived
+}
+
 // createRevision builds and creates the next ClusterObjectSet revision owned by
 // the deployment. Any create error, including AlreadyExists, is returned.
 //