address PR review feedback

Author: nader-ziada

api/v1/clusterextension_types.go

@@ -49,34 +49,43 @@ const (
 
 // ClusterExtensionSpec defines the desired state of ClusterExtension
 type ClusterExtensionSpec struct {
-	// namespace selects the namespace that namespace-scoped resources for the extension
-	// are applied to.
-	//
 	// <opcon:standard:description>
-	// In the standard configuration, namespace is required and must reference an existing
-	// namespace on the cluster.
+	// namespace specifies a Kubernetes namespace.
+	// It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster.
+	// Some extensions may contain namespace-scoped resources to be applied in other namespaces.
+	// This namespace must exist.
+	//
+	// The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123].
+	// It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
+	// and be no longer than 63 characters.
+	//
+	// [RFC 1123]: https://tools.ietf.org/html/rfc1123
 	// </opcon:standard:description>
 	// <opcon:experimental:description>
-	// BoxcutterRuntime feature set, namespace is optional.
-	// When set, it must reference an existing namespace. When omitted, operator-controller
-	// resolves and creates a managed namespace from bundle metadata. The mode (set vs omitted)
-	// is locked at creation time and cannot be changed.
-	// </opcon:experimental:description>
+	// namespace selects the namespace that namespace-scoped resources for the extension
+	// are applied to.
+	//
+	// namespace is optional. When set, it must reference an existing namespace on the cluster.
+	// When omitted, operator-controller resolves and creates a managed namespace from the
+	// bundle's metadata. Whether namespace is set or omitted is fixed at creation time and
+	// cannot be changed afterwards.
 	//
 	// The namespace field follows the DNS label standard as defined in [RFC 1123].
 	// It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
 	// and be no longer than 63 characters.
 	//
 	// [RFC 1123]: https://tools.ietf.org/html/rfc1123
+	// </opcon:experimental:description>
 	//
-	// <opcon:standard:validation:Required>
-	// <opcon:standard:validation:XValidation:rule="self != ''",message="namespace is required">
+	// <opcon:standard:validation:XValidation:rule="self == oldSelf",message="namespace is immutable">
+	// <opcon:standard:validation:XValidation:rule="self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")",message="namespace must be a valid DNS1123 label">
+	// <opcon:experimental:validation:XValidation:rule="self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")",message="namespace must be a valid DNS1123 label">
+	// <opcon:experimental:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="namespace is immutable once set">
 	// <opcon:experimental:validation:XValidation:rule="oldSelf != '' || self == ''",message="namespace cannot be set after creation; mode is locked at creation time">
+	// <opcon:experimental:validation:Optional>
 	//
 	// +kubebuilder:validation:MaxLength:=63
-	// +kubebuilder:validation:XValidation:rule="self == '' || self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\")",message="namespace must be a valid DNS1123 label"
-	// +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="namespace is immutable once set"
-	// +optional
+	// +required
 	Namespace string `json:"namespace"`
 
 	// serviceAccount is a deprecated field and is completely ignored.
@@ -599,6 +608,8 @@ type ClusterExtension struct {
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
 	// spec is an optional field that defines the desired state of the ClusterExtension.
+	//
+	// <opcon:experimental:validation:XValidation:rule="has(oldSelf.namespace) == has(self.namespace)",message="namespace presence is immutable; it cannot be added or removed after creation">
 	// +optional
 	Spec ClusterExtensionSpec `json:"spec,omitempty"`
 

applyconfigurations/api/v1/clusterextension.go

@@ -502,12 +502,12 @@ func run() error {
 
 	certProvider := getCertificateProvider()
 	regv1ManifestProvider := &applier.RegistryV1ManifestProvider{
-		BundleRenderer:              registryv1.Renderer,
-		CertificateProvider:         certProvider,
-		IsWebhookSupportEnabled:     certProvider != nil,
-		IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport),
-		IsDeploymentConfigEnabled:   features.OperatorControllerFeatureGate.Enabled(features.DeploymentConfig),
-		IsBoxcutterRuntimeEnabled:   features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime),
+		BundleRenderer:               registryv1.Renderer,
+		CertificateProvider:          certProvider,
+		IsWebhookSupportEnabled:      certProvider != nil,
+		IsSingleOwnNamespaceEnabled:  features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport),
+		IsDeploymentConfigEnabled:    features.OperatorControllerFeatureGate.Enabled(features.DeploymentConfig),
+		IsNamespaceManagementEnabled: features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime),
 	}
 	var cerCfg reconcilerConfigurator
 	if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {
@@ -748,7 +748,6 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
 		controllers.UnpackBundle(c.imagePuller, c.imageCache),
-		controllers.ValidateInstallNamespace(coreClient),
 		controllers.ApplyBundle(appl),
 	}
 

applyconfigurations/api/v1/clusterextensionspec.go

@@ -255,7 +255,7 @@ _Appears in:_
 | `kind` _string_ | Kind is a string value representing the REST resource this object represents.<br />Servers may infer this from the endpoint the client submits requests to.<br />Cannot be updated.<br />In CamelCase.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds |  | Optional: \{\} <br /> |
 | `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.<br />Servers should convert recognized schemas to the latest internal value, and<br />may reject unrecognized values.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources |  | Optional: \{\} <br /> |
 | `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. |  | Optional: \{\} <br /> |
-| `spec` _[ClusterExtensionSpec](#clusterextensionspec)_ | spec is an optional field that defines the desired state of the ClusterExtension. |  | Optional: \{\} <br /> |
+| `spec` _[ClusterExtensionSpec](#clusterextensionspec)_ | spec is an optional field that defines the desired state of the ClusterExtension.<br /> |  | Optional: \{\} <br /> |
 | `status` _[ClusterExtensionStatus](#clusterextensionstatus)_ | status is an optional field that defines the observed state of the ClusterExtension. |  | Optional: \{\} <br /> |
 
 
@@ -358,7 +358,7 @@ _Appears in:_
 
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
-| `namespace` _string_ | namespace selects the namespace that namespace-scoped resources for the extension<br />are applied to.<br /><br />In the standard configuration, namespace is required and must reference an existing<br />namespace on the cluster.<br /><br /><br />In the experimental configuration (BoxcutterRuntime feature set), namespace is optional.<br />When set, it must reference an existing namespace. When omitted, operator-controller<br />resolves and creates a managed namespace from bundle metadata. The mode (set vs omitted)<br />is locked at creation time and cannot be changed.<br /><br />The namespace field follows the DNS label standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,<br />and be no longer than 63 characters.<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123<br /><br /><br /> |  | MaxLength: 63 <br />Optional: \{\} <br /> |
+| `namespace` _string_ | <br />namespace specifies a Kubernetes namespace.<br />It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br />The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,<br />and be no longer than 63 characters.<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123<br /><br /><br />namespace selects the namespace that namespace-scoped resources for the extension<br />are applied to.<br />namespace is optional. When set, it must reference an existing namespace on the cluster.<br />When omitted, operator-controller resolves and creates a managed namespace from the<br />bundle's metadata. Whether namespace is set or omitted is fixed at creation time and<br />cannot be changed afterwards.<br />The namespace field follows the DNS label standard as defined in [RFC 1123].<br />It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,<br />and be no longer than 63 characters.<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123<br /><br /><br /><br /><br /><br /><br /> |  | MaxLength: 63 <br />Required: \{\} <br /> |
 | `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a deprecated field and is completely ignored.<br />OLMv1 is a single-tenant system where users with ClusterExtension write access are<br />effectively delegated cluster-admin trust. The operator-controller runs with<br />cluster-admin privileges and uses its own service account for all cluster interactions.<br />Deprecated: serviceAccount is no longer used and will be removed in a future release. |  | MinProperties: 1 <br />Optional: \{\} <br /> |
 | `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.<br />Set the sourceType field to perform the selection.<br />Catalog is currently the only implemented sourceType.<br />Setting sourceType to "Catalog" requires the catalog field to also be defined.<br />Below is a minimal example of a source definition (in yaml):<br />source:<br />  sourceType: Catalog<br />  catalog:<br />    packageName: example-package |  | Required: \{\} <br /> |
 | `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is optional and configures installation options for the ClusterExtension,<br />such as the pre-flight check configuration. |  | Optional: \{\} <br /> |

cmd/operator-controller/main.go

@@ -251,7 +251,10 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche
 			}
 		}
 
-		celRe := regexp.MustCompile(validationPrefix + "XValidation:rule=\"([^\"]*)\",message=\"([^\"]*)\">")
+		// The rule is captured non-greedily so it may itself contain double quotes (e.g. a CEL
+		// self.matches("...") call); it extends to the real ",message=" delimiter. The message
+		// remains quote-free.
+		celRe := regexp.MustCompile(validationPrefix + "XValidation:rule=\"(.*?)\",message=\"([^\"]*)\">")
 		celMatches := celRe.FindAllStringSubmatch(jsonProps.Description, 64)
 		for _, celMatch := range celMatches {
 			if len(celMatch) != 3 {

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

@@ -38,6 +38,22 @@ func TestOpconTweaksXValidation(t *testing.T) {
 			description:     `Field description.` + "\n" + `<opcon:experimental:validation:XValidation:rule="oldSelf != '' || self == ''",message="mode is locked at creation time">`,
 			expectRuleCount: 0,
 		},
+		{
+			name:            "rule containing double quotes is captured in full",
+			channel:         StandardChannel,
+			description:     `Field description.` + "\n" + `<opcon:standard:validation:XValidation:rule="self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")",message="namespace must be a valid DNS1123 label">`,
+			expectRule:      `self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")`,
+			expectMessage:   "namespace must be a valid DNS1123 label",
+			expectRuleCount: 1,
+		},
+		{
+			name:            "quoted rule combined with disjunction is captured in full",
+			channel:         ExperimentalChannel,
+			description:     `Field description.` + "\n" + `<opcon:experimental:validation:XValidation:rule="self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")",message="namespace must be a valid DNS1123 label">`,
+			expectRule:      `self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")`,
+			expectMessage:   "namespace must be a valid DNS1123 label",
+			expectRuleCount: 1,
+		},
 	}
 
 	for _, tt := range tests {

…o/namespace-configuration-for-authors.md …o/namespace-configuration-for-authors.mddocs/howto/namespace-configuration-for-authors.md renamed to docs/draft/howto/namespace-configuration-for-authors.md docs/howto/namespace-configuration-for-authors.md renamed to docs/draft/howto/namespace-configuration-for-authors.md

@@ -150,10 +150,10 @@ spec:
                   namespace selects the namespace that namespace-scoped resources for the extension
                   are applied to.
 
-                  BoxcutterRuntime feature set, namespace is optional.
-                  When set, it must reference an existing namespace. When omitted, operator-controller
-                  resolves and creates a managed namespace from bundle metadata. The mode (set vs omitted)
-                  is locked at creation time and cannot be changed.
+                  namespace is optional. When set, it must reference an existing namespace on the cluster.
+                  When omitted, operator-controller resolves and creates a managed namespace from the
+                  bundle's metadata. Whether namespace is set or omitted is fixed at creation time and
+                  cannot be changed afterwards.
 
                   The namespace field follows the DNS label standard as defined in [RFC 1123].
                   It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
@@ -501,6 +501,10 @@ spec:
             required:
             - source
             type: object
+            x-kubernetes-validations:
+            - message: namespace presence is immutable; it cannot be added or removed
+                after creation
+              rule: has(oldSelf.namespace) == has(self.namespace)
           status:
             description: status is an optional field that defines the observed state
               of the ClusterExtension.