Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions api/v1/clusterextensionrevision_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"`
}

Expand All @@ -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
Expand All @@ -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"`
}

Expand Down
87 changes: 79 additions & 8 deletions api/v1/clusterextensionrevision_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ func TestClusterExtensionRevisionImmutability(t *testing.T) {
}{
"revision is immutable": {
spec: ClusterExtensionRevisionSpec{
Revision: 1,
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
},
updateFunc: func(cer *ClusterExtensionRevision) {
cer.Spec.Revision = 2
},
},
"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{
Expand All @@ -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{
Expand All @@ -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",
Expand Down Expand Up @@ -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,
},
"phases must have no more than 20 phases": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
Phases: make([]ClusterExtensionRevisionPhase, 21),
},
valid: false,
},
Comment on lines +138 to +145
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test case "phases must have no more than 20 phases" creates 21 ClusterExtensionRevisionPhase instances using make(), which initializes them with zero values including empty Name strings and empty Objects slices. Since Name has minLength=1 and Objects is required, these phases will fail validation for missing required fields rather than testing the maxItems constraint on phases. Consider initializing the phases with valid Name values and at least one valid Object to ensure the test properly validates the 20-phase limit.

Copilot uses AI. Check for mistakes.
"phases 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,
},
Comment on lines +146 to +158
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test case "phases entries must have no more than 50 objects" creates 51 ClusterExtensionRevisionObject instances using make(), which initializes them with zero values. Since CollisionProtection is now a required field, all 51 objects will have an empty CollisionProtection value that doesn't match the enum constraint. This test will likely fail due to the missing required field rather than testing the maxItems validation as intended. Consider initializing the objects with a valid CollisionProtection value to ensure the test properly validates the maxItems constraint.

Copilot uses AI. Check for mistakes.
"phases entry names cannot be empty": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
Phases: []ClusterExtensionRevisionPhase{
{
Name: "",
},
},
},
valid: false,
},
"phases entry names cannot start with symbols": {
spec: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
Revision: 1,
Phases: []ClusterExtensionRevisionPhase{
{
Name: "-invalid",
},
},
},
valid: false,
},
"phases 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{
Expand Down
9 changes: 8 additions & 1 deletion api/v1/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func TestValidate(t *testing.T) {
"ClusterExtensionRevision: invalid progress deadline < 10": {
args: args{
object: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
ProgressDeadlineMinutes: 9,
},
},
Expand All @@ -99,6 +100,7 @@ func TestValidate(t *testing.T) {
"ClusterExtensionRevision: valid progress deadline = 10": {
args: args{
object: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
ProgressDeadlineMinutes: 10,
},
},
Expand All @@ -107,6 +109,7 @@ func TestValidate(t *testing.T) {
"ClusterExtensionRevision: valid progress deadline = 360": {
args: args{
object: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
ProgressDeadlineMinutes: 360,
},
},
Expand All @@ -115,6 +118,7 @@ func TestValidate(t *testing.T) {
"ClusterExtensionRevision: valid progress deadline = 720": {
args: args{
object: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
ProgressDeadlineMinutes: 720,
},
},
Expand All @@ -123,14 +127,17 @@ func TestValidate(t *testing.T) {
"ClusterExtensionRevision: invalid progress deadline > 720": {
args: args{
object: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
ProgressDeadlineMinutes: 721,
},
},
want: want{valid: false},
},
"ClusterExtensionRevision: no progress deadline set": {
args: args{
object: ClusterExtensionRevisionSpec{},
object: ClusterExtensionRevisionSpec{
LifecycleState: ClusterExtensionRevisionLifecycleStateActive,
},
},
want: want{valid: true},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -191,6 +205,7 @@ spec:
- message: revision is immutable
rule: self == oldSelf
required:
- lifecycleState
- revision
type: object
status:
Expand Down
6 changes: 2 additions & 4 deletions internal/operator-controller/applier/boxcutter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
},
Expand Down
2 changes: 2 additions & 0 deletions internal/operator-controller/applier/boxcutter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
"spec": map[string]interface{}{},
},
},
CollisionProtection: ocv1.CollisionProtectionPrevent,
},
{
Object: unstructured.Unstructured{
Expand Down Expand Up @@ -259,6 +260,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
},
},
},
CollisionProtection: ocv1.CollisionProtectionPrevent,
},
},
},
Expand Down
Loading
Loading