diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 0f41ab319..f3416bf25 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -42,14 +42,18 @@ const ( type ClusterExtensionRevisionSpec struct { // lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. // - // When set to "Active" (the default), the revision is actively managed and reconciled. + // When set to "Active", the revision is actively managed and reconciled. // When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. // The revision is removed from the owner list of all objects previously under management. // All objects that did not transition to a succeeding revision are deleted. // // Once a revision is set to "Archived", it cannot be un-archived. // - // +kubebuilder:default="Active" + // It is possible for more than one revision to be "Active" simultaneously. This will occur when + // moving from one revision to another. The old revision will not be set to "Archived" until the + // new revision has been completely rolled out. + // + // +required // +kubebuilder:validation:Enum=Active;Archived // +kubebuilder:validation:XValidation:rule="oldSelf == 'Active' || oldSelf == 'Archived' && oldSelf == self", message="cannot un-archive" LifecycleState ClusterExtensionRevisionLifecycleState `json:"lifecycleState,omitempty"` @@ -82,7 +86,10 @@ type ClusterExtensionRevisionSpec struct { // // Once set, even if empty, the phases field is immutable. // + // Each phase in the list must have a unique name. The maximum number of phases is 20. + // // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0", message="phases is immutable" + // +kubebuilder:validation:MaxItems=20 // +listType=map // +listMapKey=name // +optional @@ -125,13 +132,17 @@ type ClusterExtensionRevisionPhase struct { // // [RFC 1123]: https://tools.ietf.org/html/rfc1123 // + // +required + // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 - // +kubebuilder:validation:Pattern=`^[a-z]([-a-z0-9]*[a-z0-9])?$` + // +kubebuilder:validation:XValidation:rule=`!format.dns1123Label().validate(self).hasValue()`,message="the value must consist of only lowercase alphanumeric characters and hyphens, and must start with an alphabetic character and end with an alphanumeric character." Name string `json:"name"` // objects is a required list of all Kubernetes objects that belong to this phase. // - // All objects in this list are applied to the cluster in no particular order. + // All objects in this list are applied to the cluster in no particular order. The maximum number of objects per phase is 50. + // +required + // +kubebuilder:validation:MaxItems=50 Objects []ClusterExtensionRevisionObject `json:"objects"` } @@ -149,7 +160,9 @@ type ClusterExtensionRevisionObject struct { // collisionProtection controls whether the operator can adopt and modify objects // that already exist on the cluster. // - // When set to "Prevent" (the default), the operator only manages objects it created itself. + // Allowed values are: "Prevent", "IfNoController", and "None". + // + // When set to "Prevent", the operator only manages objects it created itself. // This prevents ownership collisions. // // When set to "IfNoController", the operator can adopt and modify pre-existing objects @@ -161,9 +174,8 @@ type ClusterExtensionRevisionObject struct { // Use this setting with extreme caution as it may cause multiple controllers to fight over // the same resource, resulting in increased load on the API server and etcd. // - // +kubebuilder:default="Prevent" + // +required // +kubebuilder:validation:Enum=Prevent;IfNoController;None - // +optional CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"` } diff --git a/api/v1/clusterextensionrevision_types_test.go b/api/v1/clusterextensionrevision_types_test.go index 9792826fb..ba2f6f99d 100644 --- a/api/v1/clusterextensionrevision_types_test.go +++ b/api/v1/clusterextensionrevision_types_test.go @@ -21,7 +21,8 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) { }{ "revision is immutable": { spec: ClusterExtensionRevisionSpec{ - Revision: 1, + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, }, updateFunc: func(cer *ClusterExtensionRevision) { cer.Spec.Revision = 2 @@ -29,8 +30,9 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) { }, "phases may be initially empty": { spec: ClusterExtensionRevisionSpec{ - Revision: 1, - Phases: []ClusterExtensionRevisionPhase{}, + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + Phases: []ClusterExtensionRevisionPhase{}, }, updateFunc: func(cer *ClusterExtensionRevision) { cer.Spec.Phases = []ClusterExtensionRevisionPhase{ @@ -44,7 +46,8 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) { }, "phases may be initially unset": { spec: ClusterExtensionRevisionSpec{ - Revision: 1, + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, }, updateFunc: func(cer *ClusterExtensionRevision) { cer.Spec.Phases = []ClusterExtensionRevisionPhase{ @@ -58,7 +61,8 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) { }, "phases are immutable if not empty": { spec: ClusterExtensionRevisionSpec{ - Revision: 1, + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, Phases: []ClusterExtensionRevisionPhase{ { Name: "foo", @@ -107,20 +111,87 @@ func TestClusterExtensionRevisionValidity(t *testing.T) { }{ "revision cannot be negative": { spec: ClusterExtensionRevisionSpec{ - Revision: -1, + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: -1, }, valid: false, }, "revision cannot be zero": { - spec: ClusterExtensionRevisionSpec{}, + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + }, valid: false, }, "revision must be positive": { spec: ClusterExtensionRevisionSpec{ - Revision: 1, + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, }, valid: true, }, + "lifecycleState must be set": { + spec: ClusterExtensionRevisionSpec{ + Revision: 1, + }, + valid: false, + }, + "inlinePhases must have no more than 20 phases": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + Phases: make([]ClusterExtensionRevisionPhase, 21), + }, + valid: false, + }, + "inlinePhases entries must have no more than 50 objects": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "too-many-objects", + Objects: make([]ClusterExtensionRevisionObject, 51), + }, + }, + }, + valid: false, + }, + "inlinePhases entry names cannot be empty": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "", + }, + }, + }, + valid: false, + }, + "inlinePhases entry names cannot start with symbols": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "-invalid", + }, + }, + }, + valid: false, + }, + "inlinePhases entry names cannot start with numeric characters": { + spec: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + Revision: 1, + Phases: []ClusterExtensionRevisionPhase{ + { + Name: "1-invalid", + }, + }, + }, + valid: false, + }, } { t.Run(name, func(t *testing.T) { cer := &ClusterExtensionRevision{ diff --git a/api/v1/validation_test.go b/api/v1/validation_test.go index bbd755c3c..16c9447a5 100644 --- a/api/v1/validation_test.go +++ b/api/v1/validation_test.go @@ -91,6 +91,7 @@ func TestValidate(t *testing.T) { "ClusterExtensionRevision: invalid progress deadline < 10": { args: args{ object: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, ProgressDeadlineMinutes: 9, }, }, @@ -99,6 +100,7 @@ func TestValidate(t *testing.T) { "ClusterExtensionRevision: valid progress deadline = 10": { args: args{ object: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, ProgressDeadlineMinutes: 10, }, }, @@ -107,6 +109,7 @@ func TestValidate(t *testing.T) { "ClusterExtensionRevision: valid progress deadline = 360": { args: args{ object: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, ProgressDeadlineMinutes: 360, }, }, @@ -115,6 +118,7 @@ func TestValidate(t *testing.T) { "ClusterExtensionRevision: valid progress deadline = 720": { args: args{ object: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, ProgressDeadlineMinutes: 720, }, }, @@ -123,6 +127,7 @@ func TestValidate(t *testing.T) { "ClusterExtensionRevision: invalid progress deadline > 720": { args: args{ object: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, ProgressDeadlineMinutes: 721, }, }, @@ -130,7 +135,9 @@ func TestValidate(t *testing.T) { }, "ClusterExtensionRevision: no progress deadline set": { args: args{ - object: ClusterExtensionRevisionSpec{}, + object: ClusterExtensionRevisionSpec{ + LifecycleState: ClusterExtensionRevisionLifecycleStateActive, + }, }, want: want{valid: true}, }, diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml index 31e267b30..2b431eab9 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -57,16 +57,19 @@ spec: description: spec defines the desired state of the ClusterExtensionRevision. properties: lifecycleState: - default: Active description: |- lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - When set to "Active" (the default), the revision is actively managed and reconciled. + When set to "Active", the revision is actively managed and reconciled. When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. The revision is removed from the owner list of all objects previously under management. All objects that did not transition to a succeeding revision are deleted. Once a revision is set to "Archived", it cannot be un-archived. + + It is possible for more than one revision to be "Active" simultaneously. This will occur when + moving from one revision to another. The old revision will not be set to "Archived" until the + new revision has been completely rolled out. enum: - Active - Archived @@ -92,6 +95,8 @@ spec: The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. Once set, even if empty, the phases field is immutable. + + Each phase in the list must have a unique name. The maximum number of phases is 20. items: description: |- ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered @@ -109,25 +114,31 @@ spec: [RFC 1123]: https://tools.ietf.org/html/rfc1123 maxLength: 63 - pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + minLength: 1 type: string + x-kubernetes-validations: + - message: the value must consist of only lowercase alphanumeric + characters and hyphens, and must start with an alphabetic + character and end with an alphanumeric character. + rule: '!format.dns1123Label().validate(self).hasValue()' objects: description: |- objects is a required list of all Kubernetes objects that belong to this phase. - All objects in this list are applied to the cluster in no particular order. + All objects in this list are applied to the cluster in no particular order. The maximum number of objects per phase is 50. items: description: |- ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part of a phase, along with its collision protection settings. properties: collisionProtection: - default: Prevent description: |- collisionProtection controls whether the operator can adopt and modify objects that already exist on the cluster. - When set to "Prevent" (the default), the operator only manages objects it created itself. + Allowed values are: "Prevent", "IfNoController", and "None". + + When set to "Prevent", the operator only manages objects it created itself. This prevents ownership collisions. When set to "IfNoController", the operator can adopt and modify pre-existing objects @@ -152,13 +163,16 @@ spec: x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true required: + - collisionProtection - object type: object + maxItems: 50 type: array required: - name - objects type: object + maxItems: 20 type: array x-kubernetes-list-map-keys: - name @@ -191,6 +205,7 @@ spec: - message: revision is immutable rule: self == oldSelf required: + - lifecycleState - revision type: object status: diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 5f10716c7..c20fdc06a 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -147,7 +147,8 @@ func (r *SimpleRevisionGenerator) GenerateRevision( sanitizedUnstructured(ctx, &unstr) objs = append(objs, ocv1.ClusterExtensionRevisionObject{ - Object: unstr, + Object: unstr, + CollisionProtection: ocv1.CollisionProtectionPrevent, }) } return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil @@ -215,9 +216,6 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision( }, }, Spec: ocv1.ClusterExtensionRevisionSpec{ - // Explicitly set LifecycleState to Active. While the CRD has a default, - // being explicit here ensures all code paths are clear and doesn't rely - // on API server defaulting behavior. LifecycleState: ocv1.ClusterExtensionRevisionLifecycleStateActive, Phases: PhaseSort(objects), }, diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index f7c1298ce..6921743ae 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -231,6 +231,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { "spec": map[string]interface{}{}, }, }, + CollisionProtection: ocv1.CollisionProtectionPrevent, }, { Object: unstructured.Unstructured{ @@ -259,6 +260,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { }, }, }, + CollisionProtection: ocv1.CollisionProtectionPrevent, }, }, }, diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 6536baf93..eb72fb01f 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -669,16 +669,19 @@ spec: description: spec defines the desired state of the ClusterExtensionRevision. properties: lifecycleState: - default: Active description: |- lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - When set to "Active" (the default), the revision is actively managed and reconciled. + When set to "Active", the revision is actively managed and reconciled. When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. The revision is removed from the owner list of all objects previously under management. All objects that did not transition to a succeeding revision are deleted. Once a revision is set to "Archived", it cannot be un-archived. + + It is possible for more than one revision to be "Active" simultaneously. This will occur when + moving from one revision to another. The old revision will not be set to "Archived" until the + new revision has been completely rolled out. enum: - Active - Archived @@ -704,6 +707,8 @@ spec: The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. Once set, even if empty, the phases field is immutable. + + Each phase in the list must have a unique name. The maximum number of phases is 20. items: description: |- ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered @@ -721,25 +726,31 @@ spec: [RFC 1123]: https://tools.ietf.org/html/rfc1123 maxLength: 63 - pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + minLength: 1 type: string + x-kubernetes-validations: + - message: the value must consist of only lowercase alphanumeric + characters and hyphens, and must start with an alphabetic + character and end with an alphanumeric character. + rule: '!format.dns1123Label().validate(self).hasValue()' objects: description: |- objects is a required list of all Kubernetes objects that belong to this phase. - All objects in this list are applied to the cluster in no particular order. + All objects in this list are applied to the cluster in no particular order. The maximum number of objects per phase is 50. items: description: |- ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part of a phase, along with its collision protection settings. properties: collisionProtection: - default: Prevent description: |- collisionProtection controls whether the operator can adopt and modify objects that already exist on the cluster. - When set to "Prevent" (the default), the operator only manages objects it created itself. + Allowed values are: "Prevent", "IfNoController", and "None". + + When set to "Prevent", the operator only manages objects it created itself. This prevents ownership collisions. When set to "IfNoController", the operator can adopt and modify pre-existing objects @@ -764,13 +775,16 @@ spec: x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true required: + - collisionProtection - object type: object + maxItems: 50 type: array required: - name - objects type: object + maxItems: 20 type: array x-kubernetes-list-map-keys: - name @@ -803,6 +817,7 @@ spec: - message: revision is immutable rule: self == oldSelf required: + - lifecycleState - revision type: object status: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index 0b750139d..6cb9b1848 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -630,16 +630,19 @@ spec: description: spec defines the desired state of the ClusterExtensionRevision. properties: lifecycleState: - default: Active description: |- lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - When set to "Active" (the default), the revision is actively managed and reconciled. + When set to "Active", the revision is actively managed and reconciled. When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. The revision is removed from the owner list of all objects previously under management. All objects that did not transition to a succeeding revision are deleted. Once a revision is set to "Archived", it cannot be un-archived. + + It is possible for more than one revision to be "Active" simultaneously. This will occur when + moving from one revision to another. The old revision will not be set to "Archived" until the + new revision has been completely rolled out. enum: - Active - Archived @@ -665,6 +668,8 @@ spec: The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. Once set, even if empty, the phases field is immutable. + + Each phase in the list must have a unique name. The maximum number of phases is 20. items: description: |- ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered @@ -682,25 +687,31 @@ spec: [RFC 1123]: https://tools.ietf.org/html/rfc1123 maxLength: 63 - pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ + minLength: 1 type: string + x-kubernetes-validations: + - message: the value must consist of only lowercase alphanumeric + characters and hyphens, and must start with an alphabetic + character and end with an alphanumeric character. + rule: '!format.dns1123Label().validate(self).hasValue()' objects: description: |- objects is a required list of all Kubernetes objects that belong to this phase. - All objects in this list are applied to the cluster in no particular order. + All objects in this list are applied to the cluster in no particular order. The maximum number of objects per phase is 50. items: description: |- ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part of a phase, along with its collision protection settings. properties: collisionProtection: - default: Prevent description: |- collisionProtection controls whether the operator can adopt and modify objects that already exist on the cluster. - When set to "Prevent" (the default), the operator only manages objects it created itself. + Allowed values are: "Prevent", "IfNoController", and "None". + + When set to "Prevent", the operator only manages objects it created itself. This prevents ownership collisions. When set to "IfNoController", the operator can adopt and modify pre-existing objects @@ -725,13 +736,16 @@ spec: x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true required: + - collisionProtection - object type: object + maxItems: 50 type: array required: - name - objects type: object + maxItems: 20 type: array x-kubernetes-list-map-keys: - name @@ -764,6 +778,7 @@ spec: - message: revision is immutable rule: self == oldSelf required: + - lifecycleState - revision type: object status: