Skip to content

✨: dedupe ClusterObjectDeployment template and ClusterObjectSet spec into shared content struct #3146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 2 commits into from
Nov 11, 2026
Merged

✨: dedupe ClusterObjectDeployment template and ClusterObjectSet spec into shared content struct #3146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c470059
✨: dedupe ClusterObjectDeployment template and ClusterObjectSet spec …
Nov 11, 2026
4cfea62
Tighten COS content immutability and neutralize shared field docs
Nov 11, 2026
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
29 changes: 4 additions & 25 deletions api/v1/clusterobjectdeployment_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,31 +124,10 @@ type ClusterObjectDeploymentTemplateMetadata struct {

// ClusterObjectDeploymentTemplateSpec contains mutable ClusterObjectSet content.
type ClusterObjectDeploymentTemplateSpec struct {
// phases is an ordered list of 1 to 20 uniquely named phases. Each phase contains
// at most 50 objects, each expressed either as an inlineManifest (an embedded
// Kubernetes object) or a secretRef (a reference to an immutable Secret holding
// a serialized manifest). Phase, object, and probe order participate in template identity.
// +required
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=20
// +listType=map
// +listMapKey=name
Phases []ClusterObjectSetPhase `json:"phases,omitempty"` // These are ordered groups of objects, not an enum describing lifecycle state.

// progressionProbes supplies custom readiness probes for objects in every phase.
// The same selector and assertion semantics as ClusterObjectSet.spec.progressionProbes apply.
// When set, must contain 1 to 20 probes.
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=20
// +listType=atomic
ProgressionProbes []ProgressionProbe `json:"progressionProbes,omitempty"`

// collisionProtection is the required default collision protection strategy for all objects
// in the revision. Individual phases or objects may override this value.
// The resolution order is: object > phase > spec.
// +required
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
// ClusterObjectSetContent holds the mutable revision content that is
// propagated to each ClusterObjectSet created from this template. Changes
// to these fields create a new revision; existing revisions remain immutable.
ClusterObjectSetContent `json:",inline"`
Comment thread
perdasilva marked this conversation as resolved.
Resolved
}

// ClusterObjectDeploymentStatus reports rollout progress.
Expand Down
6 changes: 4 additions & 2 deletions api/v1/clusterobjectdeployment_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,10 @@ func newTestClusterObjectDeployment(name string) *ClusterObjectDeployment {
Spec: ClusterObjectDeploymentSpec{
Template: ClusterObjectDeploymentTemplate{
Spec: ClusterObjectDeploymentTemplateSpec{
CollisionProtection: CollisionProtectionPrevent,
Phases: []ClusterObjectSetPhase{{Name: "deploy", Objects: []ClusterObjectSetObject{{Type: ObjectTypeInlineManifest, InlineManifest: configMap()}}}},
ClusterObjectSetContent: ClusterObjectSetContent{
CollisionProtection: CollisionProtectionPrevent,
Phases: []ClusterObjectSetPhase{{Name: "deploy", Objects: []ClusterObjectSetObject{{Type: ObjectTypeInlineManifest, InlineManifest: configMap()}}}},
},
},
},
},
Expand Down
42 changes: 19 additions & 23 deletions api/v1/clusterobjectset_types.go
View file
Open in desktop

@joelanford joelanford Nov 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you think there'd be a way for us to add a test that somehow verifies that all fields in the ClusterObjectSetContent struct are configured as immutable?

I'm thinking a unit test that parses the Go code, finds the json field names of the ClusterObjectSetContent struct, and then looks for comments on ClusterObjectSetSpec that match the pattern for the immutable CEL expressions for each of those fields.

@perdasilva perdasilva Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will handle the automatic field-completeness guard in a separate follow-up PR, keeping the field-specific behavioral regression tests here. The existing tests cover all three current fields, but do not automatically fail when a new content field is added. For the follow-up, I plan to enumerate the struct JSON fields with reflection and require an API-server-backed mutation case for each, so a future field without coverage fails the test. That would verify the generated validation behavior rather than depend on exact Go comment or CEL-expression spelling.

Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ const (
)

// ClusterObjectSetSpec defines the desired state of ClusterObjectSet.
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.phases) || has(self.phases)",message="phases cannot be removed once set"
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.progressionProbes) || has(self.progressionProbes)",message="progressionProbes cannot be removed once set"
// The content fields (phases, progressionProbes, collisionProtection) are
// immutable from creation and may only be changed by creating a new revision.
// +kubebuilder:validation:XValidation:rule="self.phases == oldSelf.phases",message="phases is immutable"
// +kubebuilder:validation:XValidation:rule="has(self.progressionProbes) == has(oldSelf.progressionProbes) && (!has(self.progressionProbes) || self.progressionProbes == oldSelf.progressionProbes)",message="progressionProbes is immutable"
// +kubebuilder:validation:XValidation:rule="self.collisionProtection == oldSelf.collisionProtection",message="collisionProtection is immutable"
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.
Expand Down Expand Up @@ -90,25 +93,23 @@ type ClusterObjectSetSpec struct {
// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="revision is immutable"
Revision int64 `json:"revision,omitempty"`

// phases is a required, immutable list of phases that group objects to be applied together.
//
// Objects are organized into phases based on their Group-Kind. Common phases include:
// - namespaces: Namespace objects
// - policies: ResourceQuota, LimitRange, NetworkPolicy objects
// - rbac: ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding objects
// - crds: CustomResourceDefinition objects
// - storage: PersistentVolume, PersistentVolumeClaim, StorageClass objects
// - deploy: Deployment, StatefulSet, DaemonSet, Service, ConfigMap, Secret objects
// - publish: Ingress, APIService, Route, Webhook objects
// ClusterObjectSetContent holds the immutable revision content shared with
// the ClusterObjectDeployment template. Immutability of these fields is
// enforced at the ClusterObjectSetSpec level, not here, so that the same
// structure can be embedded mutably in a ClusterObjectDeployment template.
ClusterObjectSetContent `json:",inline"`
}

// ClusterObjectSetContent holds the revision content shared between
// ClusterObjectSet.spec and a ClusterObjectDeployment's template spec.
type ClusterObjectSetContent struct {
// phases is a required list of phases that group objects to be applied together.
//
// All objects in a phase are applied in no particular order.
// The revision progresses to the next phase only after all objects in the current phase pass their readiness probes.
//
// Once set, 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:MinItems=1
// +kubebuilder:validation:MaxItems=20
// +listType=map
Expand All @@ -117,7 +118,7 @@ type ClusterObjectSetSpec struct {
Phases []ClusterObjectSetPhase `json:"phases,omitempty"`

// progressionProbes is an optional field which provides the ability to define custom readiness probes
// for objects defined within spec.phases. As documented in that field, most kubernetes-native objects
// for objects defined within phases. As documented in that field, most kubernetes-native objects
// within the phases already have some kind of readiness check built-in, but this field allows for checks
// which are tailored to the objects being rolled out - particularly custom resources.
//
Expand All @@ -128,10 +129,6 @@ type ClusterObjectSetSpec struct {
//
// The maximum number of probes is 20.
//
// Once set, the progressionProbes field is immutable, as it is
// part of the immutable content of a revision.
//
// +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0", message="progressionProbes is immutable"
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=20
// +listType=atomic
Expand All @@ -140,13 +137,12 @@ type ClusterObjectSetSpec struct {
ProgressionProbes []ProgressionProbe `json:"progressionProbes,omitempty"`

// collisionProtection is a required field that specifies the default collision protection
// strategy for all objects in this revision. Individual phases or objects can override this value.
// strategy for all objects in this content. Individual phases or objects can override this value.
//
// This value is used as the default for any phase or object that does not explicitly specify
// its own collisionProtection. The resolution order is: object > phase > spec.
// its own collisionProtection. The resolution order is: object > phase > content.
//
// +required
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="collisionProtection is immutable"
CollisionProtection CollisionProtection `json:"collisionProtection,omitempty"`
}

Expand Down
Loading
Loading