diff --git a/Makefile b/Makefile index e9237b0ac5..b13b25608e 100644 --- a/Makefile +++ b/Makefile @@ -685,6 +685,10 @@ crd-ref-docs: $(CRD_REF_DOCS) #EXHELP Generate the API Reference Documents. $(CRD_REF_DOCS) --source-path=$(ROOT_DIR)/api/ \ --config=$(API_REFERENCE_DIR)/crd-ref-docs-gen-config.yaml \ --renderer=markdown --output-path=$(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME); + # crd-ref-docs renders doc-comment text verbatim, including internal generator + # directives; strip them from the published reference (the per-channel contracts remain in prose). + sed -E 's#]*>##g' $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME) > $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME).tmp + mv $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME).tmp $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME) VENVDIR := $(abspath docs/.venv) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6f7912ae9b..7e399ce09b 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -49,21 +49,34 @@ const ( // ClusterExtensionSpec defines the desired state of ClusterExtension type ClusterExtensionSpec struct { - // 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. + // namespace selects the namespace that namespace-scoped resources for the extension + // are applied to. // - // The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + // + // In the standard configuration, namespace is required and must reference an existing + // namespace on the cluster. + // + // + // 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. + // + // + // 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 // + // + // + // + // // +kubebuilder:validation:MaxLength:=63 - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="namespace is immutable" - // +kubebuilder:validation:XValidation:rule="self.matches(\"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\")",message="namespace must be a valid DNS1123 label" - // +required + // +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 Namespace string `json:"namespace"` // serviceAccount is a deprecated field and is completely ignored. diff --git a/applyconfigurations/api/v1/clusterextensionspec.go b/applyconfigurations/api/v1/clusterextensionspec.go index 47d810a74a..2417043750 100644 --- a/applyconfigurations/api/v1/clusterextensionspec.go +++ b/applyconfigurations/api/v1/clusterextensionspec.go @@ -22,15 +22,28 @@ package v1 // // ClusterExtensionSpec defines the desired state of ClusterExtension type ClusterExtensionSpecApplyConfiguration struct { - // 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. + // namespace selects the namespace that namespace-scoped resources for the extension + // are applied to. // - // The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + // + // In the standard configuration, namespace is required and must reference an existing + // namespace on the cluster. + // + // + // 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. + // + // + // 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 Namespace *string `json:"namespace,omitempty"` // serviceAccount is a deprecated field and is completely ignored. diff --git a/applyconfigurations/api/v1/clusterextensionstatus.go b/applyconfigurations/api/v1/clusterextensionstatus.go index d11ad931dd..d05f981ca3 100644 --- a/applyconfigurations/api/v1/clusterextensionstatus.go +++ b/applyconfigurations/api/v1/clusterextensionstatus.go @@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -// Code generated by controller-gen-v0.20. DO NOT EDIT. +// Code generated by controller-gen-v0.21. DO NOT EDIT. package v1 diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 2fcea83ef0..4decf0f6bf 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -507,6 +507,7 @@ func run() error { IsWebhookSupportEnabled: certProvider != nil, IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), IsDeploymentConfigEnabled: features.OperatorControllerFeatureGate.Enabled(features.DeploymentConfig), + IsBoxcutterRuntimeEnabled: features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime), } var cerCfg reconcilerConfigurator if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { @@ -659,6 +660,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl controllers.RetrieveRevisionStates(revisionStatesGetter), controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), controllers.UnpackBundle(c.imagePuller, c.imageCache), + controllers.ValidateInstallNamespace(coreClient), controllers.ApplyBundleWithBoxcutter(appl.Apply), } @@ -746,6 +748,7 @@ 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), } diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 1d686238ca..166e07e380 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -29,10 +29,10 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `type` _[ProbeType](#probetype)_ | type is a required field which specifies the type of probe to use.
The allowed probe types are "ConditionEqual", "FieldsEqual", and "FieldValue".
When set to "ConditionEqual", the probe checks objects that have reached a condition of specified type and status.
When set to "FieldsEqual", the probe checks that the values found at two provided field paths are matching.
When set to "FieldValue", the probe checks that the value found at the provided field path matches what was specified.
| | Enum: [ConditionEqual FieldsEqual FieldValue]
Required: \{\}
| -| `conditionEqual` _[ConditionEqualProbe](#conditionequalprobe)_ | conditionEqual contains the expected condition type and status.
| | Optional: \{\}
| -| `fieldsEqual` _[FieldsEqualProbe](#fieldsequalprobe)_ | fieldsEqual contains the two field paths whose values are expected to match.
| | Optional: \{\}
| -| `fieldValue` _[FieldValueProbe](#fieldvalueprobe)_ | fieldValue contains the expected field path and value found within.
| | Optional: \{\}
| +| `type` _[ProbeType](#probetype)_ | type is a required field which specifies the type of probe to use.
The allowed probe types are "ConditionEqual", "FieldsEqual", and "FieldValue".
When set to "ConditionEqual", the probe checks objects that have reached a condition of specified type and status.
When set to "FieldsEqual", the probe checks that the values found at two provided field paths are matching.
When set to "FieldValue", the probe checks that the value found at the provided field path matches what was specified.
| | Enum: [ConditionEqual FieldsEqual FieldValue]
Required: \{\}
| +| `conditionEqual` _[ConditionEqualProbe](#conditionequalprobe)_ | conditionEqual contains the expected condition type and status.
| | Optional: \{\}
| +| `fieldsEqual` _[FieldsEqualProbe](#fieldsequalprobe)_ | fieldsEqual contains the two field paths whose values are expected to match.
| | Optional: \{\}
| +| `fieldValue` _[FieldValueProbe](#fieldvalueprobe)_ | fieldValue contains the expected field path and value found within.
| | Optional: \{\}
| #### AvailabilityMode @@ -67,7 +67,7 @@ _Appears in:_ | --- | --- | --- | --- | | `name` _string_ | name is required and follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.),
start and end with an alphanumeric character, and be no longer than 253 characters. | | Required: \{\}
| | `version` _string_ | version is required and references the version that this bundle represents.
It follows the semantic versioning standard as defined in https://semver.org/. | | Required: \{\}
| -| `release` _string_ | release is an optional field that identifies a specific release of this bundle's version.
A release represents a re-publication of the same version, typically used to deliver
packaging or metadata changes without changing the version number. When multiple
releases exist for the same version, higher releases are preferred. An unset release
is less preferred than all other release values.
The value consists of dot-separated identifiers, where each identifier is either a
numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
"3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
compared as integers, alphanumeric identifiers are compared lexically, and numeric
identifiers always sort before alphanumeric identifiers.
For bundles with explicit pkg.Release metadata, this field contains that release value.
For registry+v1 bundles lacking an explicit release value, this field contains the release
extracted from version's build metadata (e.g., '2' from '1.0.0+2').
This field is omitted when the bundle's release value is unset.
| | MaxLength: 20
Optional: \{\}
| +| `release` _string_ | release is an optional field that identifies a specific release of this bundle's version.
A release represents a re-publication of the same version, typically used to deliver
packaging or metadata changes without changing the version number. When multiple
releases exist for the same version, higher releases are preferred. An unset release
is less preferred than all other release values.
The value consists of dot-separated identifiers, where each identifier is either a
numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
"3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
compared as integers, alphanumeric identifiers are compared lexically, and numeric
identifiers always sort before alphanumeric identifiers.
For bundles with explicit pkg.Release metadata, this field contains that release value.
For registry+v1 bundles lacking an explicit release value, this field contains the release
extracted from version's build metadata (e.g., '2' from '1.0.0+2').
This field is omitted when the bundle's release value is unset.
| | MaxLength: 20
Optional: \{\}
| #### CRDUpgradeSafetyEnforcement @@ -358,12 +358,12 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `namespace` _string_ | 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 | | MaxLength: 63
Required: \{\}
| +| `namespace` _string_ | namespace selects the namespace that namespace-scoped resources for the extension
are applied to.

In the standard configuration, namespace is required and must reference an existing
namespace on the cluster.


In the experimental configuration (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.

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


| | MaxLength: 63
Optional: \{\}
| | `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a deprecated field and is completely ignored.
OLMv1 is a single-tenant system where users with ClusterExtension write access are
effectively delegated cluster-admin trust. The operator-controller runs with
cluster-admin privileges and uses its own service account for all cluster interactions.
Deprecated: serviceAccount is no longer used and will be removed in a future release. | | MinProperties: 1
Optional: \{\}
| | `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.
Set the sourceType field to perform the selection.
Catalog is currently the only implemented sourceType.
Setting sourceType to "Catalog" requires the catalog field to also be defined.
Below is a minimal example of a source definition (in yaml):
source:
sourceType: Catalog
catalog:
packageName: example-package | | Required: \{\}
| | `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is optional and configures installation options for the ClusterExtension,
such as the pre-flight check configuration. | | Optional: \{\}
| -| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is optional and specifies bundle-specific configuration.
Configuration is bundle-specific and a bundle may provide a configuration schema.
When not specified, the default configuration of the resolved bundle is used.
config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide
a configuration schema the bundle is deemed to not be configurable. More information on how
to configure bundles can be found in the OLM documentation associated with your current OLM version.
| | Optional: \{\}
| -| `progressDeadlineMinutes` _integer_ | progressDeadlineMinutes is an optional field that defines the maximum period
of time in minutes after which an installation should be considered failed and
require manual intervention. This functionality is disabled when no value
is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
| | Maximum: 720
Minimum: 10
Optional: \{\}
| +| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is optional and specifies bundle-specific configuration.
Configuration is bundle-specific and a bundle may provide a configuration schema.
When not specified, the default configuration of the resolved bundle is used.
config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide
a configuration schema the bundle is deemed to not be configurable. More information on how
to configure bundles can be found in the OLM documentation associated with your current OLM version.
| | Optional: \{\}
| +| `progressDeadlineMinutes` _integer_ | progressDeadlineMinutes is an optional field that defines the maximum period
of time in minutes after which an installation should be considered failed and
require manual intervention. This functionality is disabled when no value
is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
| | Maximum: 720
Minimum: 10
Optional: \{\}
| #### ClusterExtensionStatus @@ -379,9 +379,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions represents the current state of the ClusterExtension.
The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether the bundle has been installed for this ClusterExtension:
- When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
- When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.

When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterObjectSets in active roll out.

When the ClusterExtension is sourced from a catalog, it surfaces deprecation conditions based on catalog metadata.
These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- BundleDeprecated is True if the installed bundle is marked deprecated, False if not deprecated, or Unknown if no bundle is installed yet or if catalog data is unavailable.
- ChannelDeprecated is True if any requested channel is marked deprecated, False if not deprecated, or Unknown if catalog data is unavailable.
- PackageDeprecated is True if the requested package is marked deprecated, False if not deprecated, or Unknown if catalog data is unavailable.
- Deprecated is a rollup condition that is True when any deprecation exists, False when none exist, or Unknown when catalog data is unavailable. | | Optional: \{\}
| +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions represents the current state of the ClusterExtension.
The set of condition types which apply to all spec.source variations are Installed and Progressing.
The Installed condition represents whether the bundle has been installed for this ClusterExtension:
- When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.
- When Installed is False and the Reason is Failed, the bundle has failed to install.
The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.
When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.

When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterObjectSets in active roll out.

When the ClusterExtension is sourced from a catalog, it surfaces deprecation conditions based on catalog metadata.
These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- BundleDeprecated is True if the installed bundle is marked deprecated, False if not deprecated, or Unknown if no bundle is installed yet or if catalog data is unavailable.
- ChannelDeprecated is True if any requested channel is marked deprecated, False if not deprecated, or Unknown if catalog data is unavailable.
- PackageDeprecated is True if the requested package is marked deprecated, False if not deprecated, or Unknown if catalog data is unavailable.
- Deprecated is a rollup condition that is True when any deprecation exists, False when none exist, or Unknown when catalog data is unavailable. | | Optional: \{\}
| | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | Optional: \{\}
| -| `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | activeRevisions holds a list of currently active (non-archived) ClusterObjectSets,
including both installed and rolling out revisions.
| | Optional: \{\}
| +| `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | activeRevisions holds a list of currently active (non-archived) ClusterObjectSets,
including both installed and rolling out revisions.
| | Optional: \{\}
| @@ -399,8 +399,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `type` _string_ | type sets the expected condition type, i.e. "Ready".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| -| `status` _string_ | status sets the expected condition status.
Allowed values are "True" and "False".
| | Enum: [True False]
Required: \{\}
| +| `type` _string_ | type sets the expected condition type, i.e. "Ready".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `status` _string_ | status sets the expected condition status.
Allowed values are "True" and "False".
| | Enum: [True False]
Required: \{\}
| #### FieldValueProbe @@ -416,8 +416,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `fieldPath` _string_ | fieldPath sets the field path for the field to check, i.e. "status.phase". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| -| `value` _string_ | value sets the expected value found at fieldPath, i.e. "Bound".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `fieldPath` _string_ | fieldPath sets the field path for the field to check, i.e. "status.phase". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `value` _string_ | value sets the expected value found at fieldPath, i.e. "Bound".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| #### FieldsEqualProbe @@ -433,8 +433,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `fieldA` _string_ | fieldA sets the field path for the first field, i.e. "spec.replicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| -| `fieldB` _string_ | fieldB sets the field path for the second field, i.e. "status.readyReplicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `fieldA` _string_ | fieldA sets the field path for the first field, i.e. "spec.replicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `fieldB` _string_ | fieldB sets the field path for the second field, i.e. "status.readyReplicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| #### ImageSource @@ -470,9 +470,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `type` _[SelectorType](#selectortype)_ | type is a required field which specifies the type of selector to use.
The allowed selector types are "GroupKind" and "Label".
When set to "GroupKind", all objects which match the specified group and kind will be selected.
When set to "Label", all objects which match the specified labels and/or expressions will be selected.
| | Enum: [GroupKind Label]
Required: \{\}
| -| `groupKind` _[GroupKind](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#groupkind-v1-meta)_ | groupKind specifies the group and kind of objects to select.
Required when type is "GroupKind".
Uses the Kubernetes format specified here:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#GroupKind
| | Optional: \{\}
| -| `label` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#labelselector-v1-meta)_ | label is the label selector definition.
Required when type is "Label".
A probe using a Label selector will be executed against every object matching the labels or expressions; you must use care
when using this type of selector. For example, if multiple Kind objects are selected via labels then the probe is
likely to fail because the values of different Kind objects rarely share the same schema.
The LabelSelector field uses the following Kubernetes format:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#LabelSelector
Requires exactly one of matchLabels or matchExpressions.
| | Optional: \{\}
| +| `type` _[SelectorType](#selectortype)_ | type is a required field which specifies the type of selector to use.
The allowed selector types are "GroupKind" and "Label".
When set to "GroupKind", all objects which match the specified group and kind will be selected.
When set to "Label", all objects which match the specified labels and/or expressions will be selected.
| | Enum: [GroupKind Label]
Required: \{\}
| +| `groupKind` _[GroupKind](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#groupkind-v1-meta)_ | groupKind specifies the group and kind of objects to select.
Required when type is "GroupKind".
Uses the Kubernetes format specified here:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#GroupKind
| | Optional: \{\}
| +| `label` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#labelselector-v1-meta)_ | label is the label selector definition.
Required when type is "Label".
A probe using a Label selector will be executed against every object matching the labels or expressions; you must use care
when using this type of selector. For example, if multiple Kind objects are selected via labels then the probe is
likely to fail because the values of different Kind objects rarely share the same schema.
The LabelSelector field uses the following Kubernetes format:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#LabelSelector
Requires exactly one of matchLabels or matchExpressions.
| | Optional: \{\}
| diff --git a/docs/draft/concepts/managed-namespaces.md b/docs/draft/concepts/managed-namespaces.md new file mode 100644 index 0000000000..d2bdc1f0bd --- /dev/null +++ b/docs/draft/concepts/managed-namespaces.md @@ -0,0 +1,53 @@ +# Managed Namespaces + +## What is a managed namespace? + +> **Note:** Managed namespaces (omitting `spec.namespace`) are available only in the +> experimental feature set, which enables the `BoxcutterRuntime` feature gate. In the +> standard feature set, `spec.namespace` is required. + +For registry+v1 bundles, when you create a ClusterExtension without specifying `spec.namespace`, operator-controller automatically creates and manages a namespace for the operator. The namespace name comes from the bundle's metadata or defaults to `-system`. + +When you specify `spec.namespace`, the namespace must already exist on the cluster and operator-controller installs into it without managing its lifecycle. + +The mode is locked at creation time: you cannot switch between managed and user-provided after the ClusterExtension is created. + +Managed mode requires the `BoxcutterRuntime` feature gate. Without it, omitting `spec.namespace` results in a terminal error, so you must set `spec.namespace` to an existing namespace instead. + +> **Note:** The behavior described in this document applies to the registry+v1 bundle format. Other bundle formats are likely to handle namespace management differently — for example, by including namespace objects directly in their manifests. This points toward namespace configuration being bundle-format-specific rather than a top-level ClusterExtension concern. + +## Namespace resolution + +For registry+v1 bundles in managed mode, the namespace name is resolved from CSV annotations in this order: + +1. `operatorframework.io/suggested-namespace-template`: the `metadata.name` field from the JSON template +2. `operatorframework.io/suggested-namespace`: a plain string with the preferred name +3. `-system`: convention fallback + +## What belongs in a managed namespace + +- The operator's own workloads (deployments, services, configmaps) +- The operator's RBAC resources (service accounts, roles, role bindings) +- CRDs and webhooks installed by the operator + +## What does NOT belong in a managed namespace + +- User application workloads +- Shared services used by multiple operators +- Persistent data that should survive operator uninstallation + +## Deletion behavior + +Deleting a ClusterExtension with a managed namespace **deletes the entire namespace and everything in it.** If you have created resources in the managed namespace that are not part of the operator, they will be lost. + +If you need the namespace to persist beyond the operator's lifecycle, use `spec.namespace` to point at an existing namespace you manage yourself. + +## PSA labels + +If the bundle declares PSA requirements via `operatorframework.io/suggested-namespace-template`, those labels are applied to the managed namespace automatically. This ensures the namespace has the correct Pod Security Admission level for the operator's workloads without manual configuration. + +## Drift protection + +Managed namespaces are reconciled by the ClusterObjectSet controller. If someone manually modifies or removes labels that the controller owns (e.g., PSA labels from the template), they are automatically restored. + +Labels or annotations added by other actors that don't conflict with controller-owned fields are preserved. diff --git a/docs/howto/namespace-configuration-for-authors.md b/docs/howto/namespace-configuration-for-authors.md new file mode 100644 index 0000000000..ff71370bb9 --- /dev/null +++ b/docs/howto/namespace-configuration-for-authors.md @@ -0,0 +1,63 @@ +# Namespace Configuration for Bundle Authors + +> **Note:** Managed namespaces (omitting `spec.namespace`) are available only in the +> experimental feature set, which enables the `BoxcutterRuntime` feature gate. In the +> standard feature set, `spec.namespace` is required, and the annotations described +> below are not consulted. + +Bundle authors can specify their preferred namespace configuration through CSV annotations. These annotations are used by operator-controller when the cluster admin does not provide an explicit `spec.namespace`, which requires the experimental feature set. + +## Annotations + +### `operatorframework.io/suggested-namespace-template` + +Full namespace template with metadata. Use this when your operator needs specific labels or annotations on its namespace (e.g., PSA labels). + +```yaml +apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + name: my-operator.v1.0.0 + annotations: + operatorframework.io/suggested-namespace-template: | + { + "apiVersion": "v1", + "kind": "Namespace", + "metadata": { + "name": "my-operator-system", + "labels": { + "pod-security.kubernetes.io/enforce": "privileged", + "pod-security.kubernetes.io/audit": "privileged", + "pod-security.kubernetes.io/warn": "privileged" + } + } + } +``` + +### `operatorframework.io/suggested-namespace` + +Simple namespace name without metadata. Use this when you want a specific name but don't need labels or annotations. + +```yaml +annotations: + operatorframework.io/suggested-namespace: my-operator-system +``` + +### No annotation + +If neither annotation is present, operator-controller uses `-system` as the namespace name. + +## Priority + +If both annotations are present, `suggested-namespace-template` takes priority. + +## Guidelines + +- Always include PSA labels if your operator runs privileged containers. +- Use a descriptive, unique namespace name that includes your package name to avoid collisions. +- Do not assume the namespace name will be exactly what you suggest as cluster admins can override it by setting `spec.namespace`. +- The namespace name from the template is used only when `spec.namespace` is omitted. When set, the admin's choice takes precedence and no namespace object is created. + +## Consistency across bundle formats + +The `operatorframework.io/suggested-namespace-template` and `operatorframework.io/suggested-namespace` annotations are the canonical way to declare namespace preferences. Future bundle formats should use the same annotation keys to avoid divergence across the ecosystem. diff --git a/hack/tools/crd-generator/main.go b/hack/tools/crd-generator/main.go index edc254494e..61ff38a941 100644 --- a/hack/tools/crd-generator/main.go +++ b/hack/tools/crd-generator/main.go @@ -260,8 +260,8 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche numValid++ jsonProps.XValidations = append(jsonProps.XValidations, apiextensionsv1.ValidationRule{ - Message: celMatch[1], - Rule: celMatch[2], + Rule: celMatch[1], + Message: celMatch[2], }) } optReqRe := regexp.MustCompile(validationPrefix + "(Optional|Required)>") diff --git a/hack/tools/crd-generator/main_test.go b/hack/tools/crd-generator/main_test.go index aebef0b336..869d379ba0 100644 --- a/hack/tools/crd-generator/main_test.go +++ b/hack/tools/crd-generator/main_test.go @@ -13,6 +13,49 @@ import ( const controllerToolsVersion = "v0.21.0" +// TestOpconTweaksXValidation verifies that a channel-specific XValidation opcon +// tag maps the captured rule and message into the correct ValidationRule fields. +func TestOpconTweaksXValidation(t *testing.T) { + tests := []struct { + name string + channel string + description string + expectRule string + expectMessage string + expectRuleCount int + }{ + { + name: "experimental xvalidation applied in experimental channel", + channel: ExperimentalChannel, + description: `Field description.` + "\n" + ``, + expectRule: "oldSelf != '' || self == ''", + expectMessage: "mode is locked at creation time", + expectRuleCount: 1, + }, + { + name: "experimental xvalidation ignored in standard channel", + channel: StandardChannel, + description: `Field description.` + "\n" + ``, + expectRuleCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + jsonProps := apiextensionsv1.JSONSchemaProps{ + Description: tt.description, + Type: "string", + } + out, _ := opconTweaks(tt.channel, "namespace", jsonProps) + require.Len(t, out.XValidations, tt.expectRuleCount) + if tt.expectRuleCount > 0 { + require.Equal(t, tt.expectRule, out.XValidations[0].Rule) + require.Equal(t, tt.expectMessage, out.XValidations[0].Message) + } + }) + } +} + func TestRunGenerator(t *testing.T) { here, err := os.Getwd() require.NoError(t, err) diff --git a/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml b/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml index 73505ecd50..59d93f5c66 100644 --- a/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.21.0 olm.operatorframework.io/generator: experimental name: clusterextensions.olm.operatorframework.io spec: @@ -128,8 +128,8 @@ spec: x-kubernetes-validations: - message: namespace must be a valid DNS1123 label rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - message: self == oldSelf - rule: namespace really is immutable + - message: namespace really is immutable + rule: self == oldSelf serviceAccount: description: |- serviceAccount is a reference to a ServiceAccount used to perform all interactions diff --git a/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml b/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml index 90c33c902a..e1ccc6e67c 100644 --- a/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.21.0 olm.operatorframework.io/generator: standard name: clusterextensions.olm.operatorframework.io spec: @@ -128,8 +128,8 @@ spec: x-kubernetes-validations: - message: namespace must be a valid DNS1123 label rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - message: self == oldSelf - rule: namespace is immutable + - message: namespace is immutable + rule: self == oldSelf serviceAccount: description: |- serviceAccount is a reference to a ServiceAccount used to perform all interactions diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 3082a69946..f5c6b7487e 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -147,12 +147,15 @@ spec: rule: has(self.preflight) namespace: description: |- - 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. + namespace selects the namespace that namespace-scoped resources for the extension + are applied to. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + 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. + + 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. @@ -160,10 +163,13 @@ spec: maxLength: 63 type: string x-kubernetes-validations: - - message: namespace is immutable - rule: self == oldSelf - message: namespace must be a valid DNS1123 label - rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + rule: self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + - message: namespace is immutable once set + rule: oldSelf == '' || self == oldSelf + - message: namespace cannot be set after creation; mode is locked + at creation time + rule: oldSelf != '' || self == '' progressDeadlineMinutes: description: |- progressDeadlineMinutes is an optional field that defines the maximum period @@ -493,7 +499,6 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object status: diff --git a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml index 954dea621e..c6563c1838 100644 --- a/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml @@ -109,12 +109,13 @@ spec: rule: has(self.preflight) namespace: description: |- - 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. + namespace selects the namespace that namespace-scoped resources for the extension + are applied to. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + In the standard configuration, namespace is required and must reference an existing + namespace on the cluster. + + 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. @@ -122,10 +123,12 @@ spec: maxLength: 63 type: string x-kubernetes-validations: - - message: namespace is immutable - rule: self == oldSelf - message: namespace must be a valid DNS1123 label - rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + rule: self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + - message: namespace is immutable once set + rule: oldSelf == '' || self == oldSelf + - message: namespace is required + rule: self != '' serviceAccount: description: |- serviceAccount is a deprecated field and is completely ignored. @@ -445,8 +448,8 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source + - namespace type: object status: description: status is an optional field that defines the observed state diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index a52fa21c7e..42f96275c3 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -114,7 +114,8 @@ func (r *SimpleRevisionGenerator) GenerateRevision( bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, ) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { - // extract plain manifests + // extract plain manifests; the renderer decides whether a system-managed + // Namespace object is part of the returned object set. plain, err := r.ManifestProvider.Get(bundleFS, ext) if err != nil { return nil, err @@ -125,7 +126,7 @@ func (r *SimpleRevisionGenerator) GenerateRevision( } // add bundle properties of interest to revision annotations - bundleAnnotations, err := getBundleAnnotations(bundleFS) + bundleAnnotations, err := GetBundleAnnotations(bundleFS) if err != nil { return nil, fmt.Errorf("error getting bundle annotations: %w", err) } @@ -178,6 +179,7 @@ func (r *SimpleRevisionGenerator) GenerateRevision( objs = append(objs, *ocv1ac.ClusterObjectSetObject(). WithObject(unstr)) } + rev := r.buildClusterObjectSet(objs, ext, revisionAnnotations) rev.Spec.WithCollisionProtection(ocv1.CollisionProtectionPrevent) return rev, nil @@ -273,6 +275,11 @@ type boxcutterStorageMigratorClient interface { // Migrate creates a ClusterObjectSet from an existing Helm release if no revisions exist yet. // The migration is idempotent and skipped if revisions already exist or no Helm release is found. func (m *BoxcutterStorageMigrator) Migrate(ctx context.Context, ext *ocv1.ClusterExtension, objectLabels map[string]string) error { + // Managed namespace mode (spec.namespace empty) means this is a new-style extension + // that never had a Helm release, so there's nothing to migrate. + if ext.Spec.Namespace == "" { + return nil + } existingRevisionList := ocv1.ClusterObjectSetList{} if err := m.Client.List(ctx, &existingRevisionList, client.MatchingLabels{ labels.OwnerNameKey: ext.Name, diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 25963c9a01..35f7a73430 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -92,66 +92,30 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) rev, err := g.GenerateRevisionFromHelmRelease(t.Context(), helmRelease, ext, objectLabels) require.NoError(t, err) - expected := ocv1ac.ClusterObjectSet("test-123-1"). - WithAnnotations(map[string]string{ - "olm.operatorframework.io/bundle-name": "my-bundle", - "olm.operatorframework.io/bundle-reference": "bundle-ref", - "olm.operatorframework.io/bundle-version": "1.2.0", - "olm.operatorframework.io/package-name": "my-package", - }). - WithLabels(map[string]string{ - labels.OwnerKindKey: ocv1.ClusterExtensionKind, - labels.OwnerNameKey: "test-123", - }). - WithSpec(ocv1ac.ClusterObjectSetSpec(). - WithLifecycleState(ocv1.ClusterObjectSetLifecycleStateActive). - WithCollisionProtection(ocv1.CollisionProtectionNone). - WithRevision(1). - WithPhases( - ocv1ac.ClusterObjectSetPhase(). - WithName("configuration"). - WithObjects( - ocv1ac.ClusterObjectSetObject(). - WithObject(unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "my-label": "my-value", - }, - "annotations": map[string]interface{}{ - "olm.operatorframework.io/bundle-version": "1.2.0", - "olm.operatorframework.io/package-name": "my-package", - }, - }, - }, - }), - ocv1ac.ClusterObjectSetObject(). - WithObject(unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Secret", - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "my-label": "my-value", - }, - "annotations": map[string]interface{}{ - "olm.operatorframework.io/bundle-version": "1.2.0", - "olm.operatorframework.io/package-name": "my-package", - }, - }, - }, - }), - )), - ) - assert.Equal(t, expected.Name, rev.Name) - assert.Equal(t, expected.Labels, rev.Labels) - assert.Equal(t, expected.Annotations, rev.Annotations) - assert.Equal(t, expected.Spec.LifecycleState, rev.Spec.LifecycleState) - assert.Equal(t, expected.Spec.CollisionProtection, rev.Spec.CollisionProtection) - assert.Equal(t, expected.Spec.Revision, rev.Spec.Revision) - assert.Equal(t, expected.Spec.Phases, rev.Spec.Phases) + assert.Equal(t, "test-123-1", *rev.Name) + assert.Equal(t, map[string]string{ + labels.OwnerKindKey: ocv1.ClusterExtensionKind, + labels.OwnerNameKey: "test-123", + }, rev.Labels) + assert.Equal(t, map[string]string{ + "olm.operatorframework.io/bundle-name": "my-bundle", + "olm.operatorframework.io/bundle-reference": "bundle-ref", + "olm.operatorframework.io/bundle-version": "1.2.0", + "olm.operatorframework.io/package-name": "my-package", + }, rev.Annotations) + assert.Equal(t, ptr.To(ocv1.ClusterObjectSetLifecycleStateActive), rev.Spec.LifecycleState) + assert.Equal(t, ptr.To(ocv1.CollisionProtectionNone), rev.Spec.CollisionProtection) + assert.Equal(t, ptr.To(int64(1)), rev.Spec.Revision) + + // The Helm-release migration path never injects a namespace (the release's + // namespace already exists), so only the configuration phase is present. + require.Len(t, rev.Spec.Phases, 1) + + configPhase := rev.Spec.Phases[0] + assert.Equal(t, "configuration", *configPhase.Name) + require.Len(t, configPhase.Objects, 2) + assert.Equal(t, "ConfigMap", configPhase.Objects[0].Object.GetKind()) + assert.Equal(t, "Secret", configPhase.Objects[1].Object.GetKind()) } func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { @@ -414,10 +378,17 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t t.Log("by checking the rendered objects contain the given object labels") for _, phase := range rev.Spec.Phases { for _, revObj := range phase.Objects { - require.Equal(t, map[string]string{ - "app": "test-obj", - "some": "value", - }, revObj.Object.GetLabels()) + // Namespace objects only have objectLabels, not bundle object labels + if revObj.Object.GetKind() == "Namespace" { + require.Equal(t, map[string]string{ + "some": "value", + }, revObj.Object.GetLabels()) + } else { + require.Equal(t, map[string]string{ + "app": "test-obj", + "some": "value", + }, revObj.Object.GetLabels()) + } } } t.Log("by checking the generated revision contain the given annotations") @@ -1141,7 +1112,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) brb := newStorageMigratorGenerator(t) @@ -1214,7 +1185,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } // GenerateRevisionFromHelmRelease should not be called when revisions already exist ctrl := gomock.NewController(t) @@ -1269,7 +1240,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) brb := mockapplier.NewMockClusterObjectSetGenerator(ctrl) @@ -1342,7 +1313,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) brb := mockapplier.NewMockClusterObjectSetGenerator(ctrl) @@ -1425,7 +1396,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) brb := mockapplier.NewMockClusterObjectSetGenerator(ctrl) @@ -1482,7 +1453,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } expectedRelease := &release.Release{ Name: "test123", @@ -1579,7 +1550,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) // GenerateRevisionFromHelmRelease should NOT be called when no deployed release exists @@ -1626,7 +1597,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } ctrl := gomock.NewController(t) brb := mockapplier.NewMockClusterObjectSetGenerator(ctrl) @@ -1651,3 +1622,167 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, err) }) } + +func Test_SimpleRevisionGenerator_GenerateRevision_NamespacePhaseCollisionProtection(t *testing.T) { + ctrl := gomock.NewController(t) + r := mockapplier.NewMockManifestProvider(ctrl) + // The renderer (mocked here) is responsible for emitting the Namespace object; + // this test verifies the generator organizes it into the namespaces phase. + r.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]client.Object{ + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + }, + }, + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, + }, + }, nil).AnyTimes() + + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + ManifestProvider: r, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-extension", + }, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "test-namespace", + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test + Name: "test-sa", + }, + }, + } + + rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{}) + require.NoError(t, err) + require.NotNil(t, rev) + + t.Log("by checking the spec-level collision protection is set to Prevent") + require.Equal(t, ptr.To(ocv1.CollisionProtectionPrevent), rev.Spec.CollisionProtection) + + // Find the namespaces phase + var namespacesPhase *ocv1ac.ClusterObjectSetPhaseApplyConfiguration + for i := range rev.Spec.Phases { + if *rev.Spec.Phases[i].Name == string(applier.PhaseNamespaces) { + namespacesPhase = &rev.Spec.Phases[i] + break + } + } + + require.NotNil(t, namespacesPhase, "namespaces phase should exist") + + t.Log("by checking the namespaces phase inherits Prevent collision protection from spec (no explicit override)") + require.Nil(t, namespacesPhase.CollisionProtection, "namespaces phase should inherit collision protection from spec") + + // Verify all phases inherit from spec (no explicit collision protection) + for i := range rev.Spec.Phases { + t.Logf("by checking phase %s does not have explicit collision protection", *rev.Spec.Phases[i].Name) + require.Nil(t, rev.Spec.Phases[i].CollisionProtection, "all phases should inherit collision protection from spec") + } +} + +func Test_GenerateRevision_NamespacePhaseIsFirst(t *testing.T) { + ctrl := gomock.NewController(t) + r := mockapplier.NewMockManifestProvider(ctrl) + // The renderer (mocked here) emits the Namespace object; this test verifies the + // generator places the namespaces phase first for proper deletion ordering. + r.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]client.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, + }, + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + }, + }, + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Namespace: "test-ns", + }, + }, + }, nil).AnyTimes() + + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + ManifestProvider: r, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-extension", + }, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "test-namespace", + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test + Name: "test-sa", + }, + }, + } + + rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{}) + require.NoError(t, err) + + t.Log("by checking that phases are present") + require.NotEmpty(t, rev.Spec.Phases, "revision should have at least one phase") + + t.Log("by checking that the first phase is the namespaces phase") + firstPhase := rev.Spec.Phases[0] + require.Equal(t, "namespaces", *firstPhase.Name, "first phase should be namespaces for proper deletion ordering") + + t.Log("by checking that the namespaces phase contains exactly one namespace object") + require.Len(t, firstPhase.Objects, 1, "namespaces phase should contain exactly one object") + + t.Log("by checking that the namespace object has the correct name") + nsObj := firstPhase.Objects[0].Object + require.Equal(t, "Namespace", nsObj.GetKind()) + require.Equal(t, "test-namespace", nsObj.GetName(), "namespace name should match ext.Spec.Namespace") +} + +func Test_GenerateRevision_COSHasOwnerLabels(t *testing.T) { + ctrl := gomock.NewController(t) + r := mockapplier.NewMockManifestProvider(ctrl) + r.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]client.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + }, + }, + }, nil).AnyTimes() + + b := applier.SimpleRevisionGenerator{ + Scheme: k8scheme.Scheme, + ManifestProvider: r, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-extension", + }, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "test-namespace", + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test + Name: "test-sa", + }, + }, + } + + rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{}) + require.NoError(t, err) + + t.Log("by checking that the COS has owner-kind label") + require.NotNil(t, rev.Labels, "COS should have labels") + require.Equal(t, ocv1.ClusterExtensionKind, rev.Labels[labels.OwnerKindKey], + "COS should have owner-kind label set to ClusterExtension") + + t.Log("by checking that the COS has owner-name label") + require.Equal(t, "test-extension", rev.Labels[labels.OwnerNameKey], + "COS should have owner-name label matching the ClusterExtension name") +} diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index e82d17ba46..ef1a1ef751 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -22,7 +22,8 @@ import ( // ManifestProvider returns the manifests that should be applied by OLM given a bundle and its associated ClusterExtension type ManifestProvider interface { - // Get returns a set of resource manifests in bundle that take into account the configuration in ext + // Get returns a set of resource manifests in bundle that take into account the + // configuration in ext. Get(bundle fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) } @@ -34,6 +35,7 @@ type RegistryV1ManifestProvider struct { IsWebhookSupportEnabled bool IsSingleOwnNamespaceEnabled bool IsDeploymentConfigEnabled bool + IsBoxcutterRuntimeEnabled bool } func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { @@ -67,10 +69,23 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens return nil, fmt.Errorf("unsupported bundle: bundle must support at least one of [AllNamespaces SingleNamespace OwnNamespace] install modes") } + if ext.Spec.Namespace == "" && !r.IsBoxcutterRuntimeEnabled { + return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("spec.namespace is required unless the BoxcutterRuntime feature gate is enabled")) + } + opts := []render.Option{ render.WithCertificateProvider(r.CertificateProvider), } + // When the user set spec.namespace, render into that (already-existing) namespace and do + // not emit a Namespace object. Otherwise default to the bundle's system-managed namespace + // and have the renderer emit the Namespace object for it. + if ext.Spec.Namespace != "" { + opts = append(opts, render.WithInstallNamespace(ext.Spec.Namespace)) + } else { + opts = append(opts, render.RenderInstallNamespace()) + } + // Always validate inline config when present so that disabled features produce // a clear error rather than being silently ignored. When IsSingleOwnNamespaceEnabled // is true we also call this with no config to validate required fields (e.g. @@ -82,7 +97,8 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens } opts = append(opts, configOpts...) } - return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...) + + return r.BundleRenderer.Render(rv1, opts...) } // extractBundleConfigOptions extracts and validates configuration options from a ClusterExtension. @@ -187,7 +203,8 @@ func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte { return nil } -func getBundleAnnotations(bundleFS fs.FS) (map[string]string, error) { +// GetBundleAnnotations returns the annotations from the bundle's CSV metadata. +func GetBundleAnnotations(bundleFS fs.FS) (map[string]string, error) { // The need to get the underlying bundle in order to extract its annotations // will go away once we have a bundle interface that can surface the annotations independently of the // underlying bundle format... diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index 6fb9760417..13f51b6cfc 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -2,6 +2,7 @@ package applier_test import ( "errors" + "io/fs" "testing" "testing/fstest" @@ -139,17 +140,7 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) { provider := applier.RegistryV1ManifestProvider{ BundleRenderer: registryv1.Renderer, } - bundleFS := bundlefs.Builder().WithPackageName("test"). - WithCSV(bundlecsv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). - WithBundleResource("service.yaml", &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Service", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-service", - }, - }).Build() + bundleFS := newAllNamespacesBundleFS(t) ext := &ocv1.ClusterExtension{ Spec: ocv1.ClusterExtensionSpec{ Namespace: "install-namespace", @@ -174,6 +165,115 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) { require.Equal(t, []client.Object{exp}, objs) }) + + t.Run("emits a system-managed Namespace object when spec.namespace is empty", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsBoxcutterRuntimeEnabled: true, + } + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(bundlecsv.Builder(). + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces). + WithAnnotations(map[string]string{ + render.AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"managed-ns","labels":{"pod-security.kubernetes.io/enforce":"privileged"},"annotations":{"example.com/note":"hello"}}}`, + }).Build()). + WithBundleResource("service.yaml", &corev1.Service{ + TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Service"}, + ObjectMeta: metav1.ObjectMeta{Name: "test-service"}, + }).Build() + // No spec.namespace -> system-managed: the renderer resolves the name from + // bundle annotations and emits the Namespace object. + ext := &ocv1.ClusterExtension{} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + require.NotEmpty(t, objs) + + t.Log("by checking the Namespace object is emitted first") + ns := objs[0] + require.Equal(t, "Namespace", ns.GetObjectKind().GroupVersionKind().Kind) + require.Equal(t, "managed-ns", ns.GetName()) + + t.Log("by checking template labels and annotations are applied") + require.Equal(t, "privileged", ns.GetLabels()["pod-security.kubernetes.io/enforce"]) + require.Equal(t, "hello", ns.GetAnnotations()["example.com/note"]) + }) + + t.Run("does not emit a Namespace object when spec.namespace is set", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Namespace: "install-namespace"}} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + for _, o := range objs { + require.NotEqual(t, "Namespace", o.GetObjectKind().GroupVersionKind().Kind, "no Namespace should be emitted when Ensure is false") + } + }) +} + +func Test_RegistryV1ManifestProvider_BoxcutterRuntimeGate(t *testing.T) { + t.Run("rejects empty spec.namespace when the BoxcutterRuntime feature gate is disabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsBoxcutterRuntimeEnabled: false, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{} + + _, err := provider.Get(bundleFS, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "spec.namespace is required unless the BoxcutterRuntime feature gate is enabled") + require.ErrorIs(t, err, reconcile.TerminalError(nil), "namespace gate error should be terminal") + }) + + t.Run("allows empty spec.namespace and renders a managed Namespace when the BoxcutterRuntime feature gate is enabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsBoxcutterRuntimeEnabled: true, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + require.Contains(t, collectKinds(objs), "Namespace") + }) + + t.Run("ignores the BoxcutterRuntime feature gate when spec.namespace is set", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsBoxcutterRuntimeEnabled: false, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Namespace: "install-namespace"}} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + require.NotContains(t, collectKinds(objs), "Namespace") + }) +} + +// newAllNamespacesBundleFS returns a minimal registry+v1 bundle FS that supports the +// AllNamespaces install mode and includes a single Service resource named "test-service". +func newAllNamespacesBundleFS(t *testing.T) fs.FS { + t.Helper() + return bundlefs.Builder().WithPackageName("test"). + WithCSV(bundlecsv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). + WithBundleResource("service.yaml", &corev1.Service{ + TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Service"}, + ObjectMeta: metav1.ObjectMeta{Name: "test-service"}, + }).Build() +} + +func collectKinds(objs []client.Object) []string { + kinds := make([]string, 0, len(objs)) + for _, o := range objs { + kinds = append(kinds, o.GetObjectKind().GroupVersionKind().Kind) + } + return kinds } func Test_RegistryV1ManifestProvider_APIServiceSupport(t *testing.T) { diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index 14cfea8fc9..4f2d9f325c 100644 --- a/internal/operator-controller/controllers/clusterextension_admission_test.go +++ b/internal/operator-controller/controllers/clusterextension_admission_test.go @@ -285,8 +285,8 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) { errMsg string }{ {"just alphanumeric", "justalphanumberic1", ""}, - {"hyphen-separated", "hyphenated-name", ""}, - {"no install namespace", "", regexMismatchError}, + {"hypen-separated", "hyphenated-name", ""}, + {"no install namespace (managed mode)", "", ""}, {"dot-separated", "dotted.name", regexMismatchError}, {"longest valid install namespace", strings.Repeat("x", 63), ""}, {"too long install namespace name", strings.Repeat("x", 64), tooLongError}, @@ -325,9 +325,87 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) { } } -// TestClusterExtensionAdmissionServiceAccount validates the deprecated spec.serviceAccount field: -// - CRD-level validation (format, length) still works -// - ValidatingAdmissionPolicy emits a deprecation warning for valid non-empty values +func TestClusterExtensionAdmissionNamespaceImmutability(t *testing.T) { + baseSpec := func(ns string) ocv1.ClusterExtensionSpec { + return ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: "package", + }, + }, + Namespace: ns, + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test + Name: "default", + }, + } + } + + testCases := []struct { + name string + initialNS string + updatedNS string + expectErr bool + errContains string + }{ + { + name: "set to same value - allowed", + initialNS: "my-ns", + updatedNS: "my-ns", + expectErr: false, + }, + { + name: "set to different value - rejected", + initialNS: "my-ns", + updatedNS: "other-ns", + expectErr: true, + errContains: "namespace is immutable once set", + }, + { + name: "empty to set - rejected", + initialNS: "", + updatedNS: "my-ns", + expectErr: true, + errContains: "namespace cannot be set after creation", + }, + { + name: "empty to empty - allowed", + initialNS: "", + updatedNS: "", + expectErr: false, + }, + { + name: "set to empty - rejected", + initialNS: "my-ns", + updatedNS: "", + expectErr: true, + errContains: "namespace is immutable once set", + }, + } + + t.Parallel() + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cl := newClient(t) + ctx := context.Background() + + ext := buildClusterExtension(baseSpec(tc.initialNS)) + require.NoError(t, cl.Create(ctx, ext)) + + ext.Spec.Namespace = tc.updatedNS + err := cl.Update(ctx, ext) + if !tc.expectErr { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Contains(t, err.Error(), tc.errContains) + } + }) + } +} + func TestClusterExtensionAdmissionServiceAccount(t *testing.T) { tooLongError := "spec.serviceAccount.name: Too long: may not be more than 253" regexMismatchError := "name must be a valid DNS1123 subdomain" diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 2637457752..6748ecc115 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -15,11 +15,14 @@ import ( "go.uber.org/mock/gomock" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/client-go/kubernetes/fake" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -979,6 +982,94 @@ func TestValidateClusterExtension(t *testing.T) { } } +func TestValidateInstallNamespace(t *testing.T) { + tests := []struct { + name string + specNamespace string + namespaceObjects []runtime.Object + expectError bool + errorMessageIncludes string + }{ + { + name: "user-provided namespace exists", + specNamespace: "existing-ns", + namespaceObjects: []runtime.Object{ + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-ns", + }, + }, + }, + expectError: false, + }, + { + name: "user-provided namespace not found", + specNamespace: "missing-ns", + namespaceObjects: nil, + expectError: true, + errorMessageIncludes: `namespace "missing-ns" not found`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + objects := make([]runtime.Object, 0, len(tt.namespaceObjects)) + objects = append(objects, tt.namespaceObjects...) + fakeClient := fake.NewClientset(objects...) + + cl := newClient(t) + reconciler := &controllers.ClusterExtensionReconciler{ + Client: cl, + ReconcileSteps: controllers.ReconcileSteps{ + controllers.HandleFinalizers(crfinalizer.NewFinalizers()), + controllers.ValidateInstallNamespace(fakeClient.CoreV1()), + }, + } + + extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} + + clusterExtension := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, + Spec: ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{ + PackageName: "test-package", + }, + }, + Namespace: tt.specNamespace, + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test + Name: "test-sa", + }, + }, + } + + require.NoError(t, cl.Create(ctx, clusterExtension)) + + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + if tt.expectError { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errorMessageIncludes) + + require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) + progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(t, progressingCond) + // A missing namespace is retryable (not terminal): the user can create it and + // the next reconcile succeeds, so Progressing stays True with Reason=Retrying. + require.Equal(t, metav1.ConditionTrue, progressingCond.Status) + require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason) + require.Contains(t, progressingCond.Message, tt.errorMessageIncludes) + } else { + require.NoError(t, err) + } + require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) + }) + } +} + func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) { // This test calls Reconcile twice: first with a successful applier, // then with a failing applier. We use gomock.InOrder to sequence the calls. diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index b07a5072f4..8fa87422a9 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -21,8 +21,10 @@ import ( "errors" "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/finalizer" @@ -402,6 +404,37 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { } } +// ValidateInstallNamespace validates a user-provided spec.namespace: it must +// reference an existing namespace. When spec.namespace is omitted the install +// namespace is system-managed and resolved+created by the bundle renderer, so +// there is nothing to validate here — the emitted Namespace object is treated +// like any other rendered object (conflicts are handled by collision protection). +// +// A missing namespace is a recoverable condition (the user can create it), so it +// is surfaced as a retryable error rather than a terminal one: the next reconcile +// succeeds once the namespace exists. +func ValidateInstallNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + l := log.FromContext(ctx) + + if ext.Spec.Namespace == "" { + return nil, nil + } + + l.V(1).Info("validating user-provided namespace exists", "namespace", ext.Spec.Namespace) + _, err := nsClient.Namespaces().Get(ctx, ext.Spec.Namespace, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + nsErr := fmt.Errorf("namespace %q not found; spec.namespace must reference an existing namespace", ext.Spec.Namespace) + setStatusProgressing(ext, nsErr) + return nil, nsErr + } + if err != nil { + return nil, fmt.Errorf("error checking namespace %q: %w", ext.Spec.Namespace, err) + } + return nil, nil + } +} + func ApplyBundle(a Applier) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) diff --git a/internal/operator-controller/controllers/clusterobjectset_controller.go b/internal/operator-controller/controllers/clusterobjectset_controller.go index e42e3c6144..d9c798b5e0 100644 --- a/internal/operator-controller/controllers/clusterobjectset_controller.go +++ b/internal/operator-controller/controllers/clusterobjectset_controller.go @@ -200,23 +200,25 @@ func (c *ClusterObjectSetReconciler) reconcile(ctx context.Context, cos *ocv1.Cl return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } - for i, pres := range rres.GetPhases() { + for _, pres := range rres.GetPhases() { if verr := pres.GetValidationError(); verr != nil { - l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", i) - setRetryingConditions(l, cos, fmt.Sprintf("phase %d validation error: %s", i, verr), isDeadlineExceeded) + phaseName := pres.GetName() + l.Error(fmt.Errorf("%w", verr), "phase preflight validation failed, retrying after 10s", "phase", phaseName) + setRetryingConditions(l, cos, fmt.Sprintf("phase %q validation error: %s", phaseName, verr), isDeadlineExceeded) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } var collidingObjs []string for _, ores := range pres.GetObjects() { if ores.Action() == machinery.ActionCollision { - collidingObjs = append(collidingObjs, ores.String()) + collidingObjs = append(collidingObjs, collisionMessage(ores)) } } if len(collidingObjs) > 0 { - l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", i, "collisions", collidingObjs) - setRetryingConditions(l, cos, fmt.Sprintf("revision object collisions in phase %d\n%s", i, strings.Join(collidingObjs, "\n\n")), isDeadlineExceeded) + phaseName := pres.GetName() + l.Error(fmt.Errorf("object collision detected"), "object collision, retrying after 10s", "phase", phaseName, "collisions", collidingObjs) + setRetryingConditions(l, cos, fmt.Sprintf("revision object collisions in phase %q\n%s", phaseName, strings.Join(collidingObjs, "\n\n")), isDeadlineExceeded) return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } } @@ -580,6 +582,30 @@ func (c *ClusterObjectSetReconciler) resolveObjectRef(ctx context.Context, ref o return obj, nil } +func collisionMessage(ores machinery.ObjectResult) string { + obj := ores.Object() + gvk := obj.GetObjectKind().GroupVersionKind() + name := obj.GetName() + + if collision, ok := ores.(machinery.ObjectResultCollision); ok { + if owner, hasOwner := collision.ConflictingOwner(); hasOwner { + ownerName := owner.Name + if gvk.Kind == "Namespace" { + return fmt.Sprintf("namespace %q is already managed by %s %q", name, owner.Kind, ownerName) + } + return fmt.Sprintf("%s %q is already managed by %s %q", gvk.Kind, name, owner.Kind, ownerName) + } + } + + if gvk.Kind == "Namespace" { + return fmt.Sprintf("namespace %q already exists and cannot be adopted", name) + } + if ns := obj.GetNamespace(); ns != "" { + return fmt.Sprintf("%s.%s %s/%s collision: %s", gvk.Kind, gvk.GroupVersion(), ns, name, ores.String()) + } + return fmt.Sprintf("%s.%s %s collision: %s", gvk.Kind, gvk.GroupVersion(), name, ores.String()) +} + // EffectiveCollisionProtection resolves the collision protection value using // the inheritance hierarchy: object > phase > spec > default ("Prevent"). func EffectiveCollisionProtection(cp ...ocv1.CollisionProtection) ocv1.CollisionProtection { diff --git a/internal/operator-controller/rukpak/render/namespace.go b/internal/operator-controller/rukpak/render/namespace.go new file mode 100644 index 0000000000..6001731217 --- /dev/null +++ b/internal/operator-controller/rukpak/render/namespace.go @@ -0,0 +1,170 @@ +package render + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash" +) + +const ( + // AnnotationSuggestedNamespaceTemplate is a CSV annotation carrying a JSON + // Namespace template whose metadata seeds the system-managed namespace. + AnnotationSuggestedNamespaceTemplate = "operatorframework.io/suggested-namespace-template" + // AnnotationSuggestedNamespace is a CSV annotation carrying the preferred + // namespace name for the operator. + AnnotationSuggestedNamespace = "operatorframework.io/suggested-namespace" +) + +var dns1123LabelRegexp = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + +// resolveSystemManagedNamespace derives the name of the namespace OLM should +// create and manage for a bundle, using the precedence: +// +// suggested-namespace-template name → suggested-namespace → -system +// +// It returns the resolved name and the parsed template (if any) so the caller can +// seed labels/annotations (e.g. PSA) on the emitted Namespace object. +func resolveSystemManagedNamespace(rv1 *bundle.RegistryV1) (string, *corev1.Namespace, error) { + csvAnnotations := rv1.CSV.GetAnnotations() + + template, err := parseNamespaceTemplate(csvAnnotations) + if err != nil { + return "", nil, err + } + + var name string + switch { + case template != nil && template.Name != "": + name = template.Name + case csvAnnotations[AnnotationSuggestedNamespace] != "": + name = csvAnnotations[AnnotationSuggestedNamespace] + default: + // The auto-derived default must always be a valid namespace, even for package names + // with disallowed characters (e.g. dots) or names that are too long. + name = defaultInstallNamespace(rv1.PackageName) + } + + if err := validateNamespaceName(name); err != nil { + return "", nil, err + } + + return name, template, nil +} + +func parseNamespaceTemplate(csvAnnotations map[string]string) (*corev1.Namespace, error) { + templateJSON, exists := csvAnnotations[AnnotationSuggestedNamespaceTemplate] + if !exists || templateJSON == "" { + return nil, nil + } + + var ns corev1.Namespace + if err := json.Unmarshal([]byte(templateJSON), &ns); err != nil { + return nil, fmt.Errorf("failed to parse namespace template: %w", err) + } + + return &ns, nil +} + +const maxNamespaceNameLength = 63 + +func validateNamespaceName(name string) error { + if name == "" { + return fmt.Errorf("resolved namespace name is empty") + } + if len(name) > maxNamespaceNameLength { + return fmt.Errorf("resolved namespace name %q exceeds %d characters", name, maxNamespaceNameLength) + } + if !dns1123LabelRegexp.MatchString(name) { + return fmt.Errorf("resolved namespace name %q is not a valid DNS1123 label", name) + } + return nil +} + +// defaultInstallNamespace derives a deterministic, DNS1123-label-valid namespace name for a +// package when the bundle does not suggest one. It normalizes disallowed characters (e.g. dots) +// and enforces the namespace length limit. When the normalized name must be truncated, a short +// hash of the original package name is appended to preserve deterministic collision resistance. +func defaultInstallNamespace(packageName string) string { + const suffix = "system" + + base := sanitizeDNS1123Label(packageName) + + // Fast path: an already-valid, short base keeps the historical "-system" name. + if base != "" && len(base)+1+len(suffix) <= maxNamespaceNameLength { + return base + "-" + suffix + } + + // Otherwise keep the name deterministic and collision-resistant: append a short hash of the + // original package name and truncate the base to fit within the length limit. + hash := hashutil.DeepHashObject(packageName)[:8] + maxBase := maxNamespaceNameLength - len(suffix) - len(hash) - 2 // account for two '-' separators + if len(base) > maxBase { + base = base[:maxBase] + } + base = strings.Trim(base, "-") + if base == "" { + return hash + "-" + suffix + } + return base + "-" + hash + "-" + suffix +} + +// sanitizeDNS1123Label lowercases s, replaces each run of disallowed characters with a single +// hyphen, and trims leading/trailing hyphens so the result is a valid DNS1123 label (or empty). +func sanitizeDNS1123Label(s string) string { + var b strings.Builder + lastHyphen := false + for _, r := range strings.ToLower(s) { + switch { + case (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9'): + b.WriteRune(r) + lastHyphen = false + case !lastHyphen: + b.WriteByte('-') + lastHyphen = true + } + } + return strings.Trim(b.String(), "-") +} + +// BuildNamespaceObject returns the Namespace object to include in the rendered set, +// seeding labels and annotations from the optional template. Empty spec/status are +// stripped to avoid apply drift. +func BuildNamespaceObject(name string, template *corev1.Namespace) (client.Object, error) { + ns := corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Namespace", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + + if template != nil { + if len(template.Labels) > 0 { + ns.Labels = template.Labels + } + if len(template.Annotations) > 0 { + ns.Annotations = template.Annotations + } + } + + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&ns) + if err != nil { + return nil, fmt.Errorf("failed to convert namespace to unstructured: %w", err) + } + delete(unstructuredObj, "status") + delete(unstructuredObj, "spec") + + return &unstructured.Unstructured{Object: unstructuredObj}, nil +} diff --git a/internal/operator-controller/rukpak/render/namespace_test.go b/internal/operator-controller/rukpak/render/namespace_test.go new file mode 100644 index 0000000000..4f01bd0e49 --- /dev/null +++ b/internal/operator-controller/rukpak/render/namespace_test.go @@ -0,0 +1,362 @@ +package render + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/testing/bundle/csv" +) + +func rv1WithAnnotations(pkg string, annotations map[string]string) *bundle.RegistryV1 { + return &bundle.RegistryV1{ + PackageName: pkg, + CSV: csv.Builder().WithName("test-csv").WithAnnotations(annotations).Build(), + } +} + +func TestParseNamespaceTemplate(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected *corev1.Namespace + expectError bool + }{ + { + name: "nil annotations", + annotations: nil, + expected: nil, + }, + { + name: "empty map", + annotations: map[string]string{}, + expected: nil, + }, + { + name: "annotation absent", + annotations: map[string]string{"some.other/annotation": "value"}, + expected: nil, + }, + { + name: "empty string value", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: ""}, + expected: nil, + }, + { + name: "valid template with PSA labels", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"labels":{"pod-security.kubernetes.io/enforce":"restricted"}}}`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"pod-security.kubernetes.io/enforce": "restricted"}, + }, + }, + }, + { + name: "valid template with annotations", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"annotations":{"openshift.io/description":"Operator namespace"}}}`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"openshift.io/description": "Operator namespace"}, + }, + }, + }, + { + name: "invalid JSON", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata": invalid json}`}, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := parseNamespaceTemplate(tt.annotations) + if tt.expectError { + require.Error(t, err) + assert.Nil(t, result) + return + } + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestResolveSystemManagedNamespace(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + packageName string + wantName string + wantTemplate bool + }{ + { + name: "suggested-namespace-template with name", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"from-template","labels":{"pod-security.kubernetes.io/enforce":"privileged"}}}`}, + packageName: "my-operator", + wantName: "from-template", + wantTemplate: true, + }, + { + name: "suggested-namespace without template", + annotations: map[string]string{AnnotationSuggestedNamespace: "my-custom-ns"}, + packageName: "my-operator", + wantName: "my-custom-ns", + }, + { + name: "template takes priority over suggested-namespace", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"from-template"}}`, + AnnotationSuggestedNamespace: "from-annotation", + }, + packageName: "my-operator", + wantName: "from-template", + wantTemplate: true, + }, + { + name: "fallback to packageName-system", + annotations: map[string]string{}, + packageName: "my-operator", + wantName: "my-operator-system", + }, + { + name: "nil annotations fallback", + annotations: nil, + packageName: "my-operator", + wantName: "my-operator-system", + }, + { + name: "template without name falls back to suggested-namespace", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"labels":{"foo":"bar"}}}`, + AnnotationSuggestedNamespace: "from-annotation", + }, + packageName: "my-operator", + wantName: "from-annotation", + wantTemplate: true, + }, + { + name: "template without name and no suggested-namespace falls back to convention", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata":{"labels":{"foo":"bar"}}}`}, + packageName: "my-operator", + wantName: "my-operator-system", + wantTemplate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + name, template, err := resolveSystemManagedNamespace(rv1WithAnnotations(tt.packageName, tt.annotations)) + require.NoError(t, err) + require.Equal(t, tt.wantName, name) + if tt.wantTemplate { + require.NotNil(t, template) + } else { + require.Nil(t, template) + } + }) + } +} + +func TestResolveSystemManagedNamespace_InvalidTemplate(t *testing.T) { + _, _, err := resolveSystemManagedNamespace(rv1WithAnnotations("pkg", map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{invalid json`, + })) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse namespace template") +} + +func TestResolveSystemManagedNamespace_Validation(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + packageName string + expectErr bool + errContains string + }{ + { + name: "rejects uppercase characters in suggested-namespace", + annotations: map[string]string{AnnotationSuggestedNamespace: "Invalid-NS"}, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + { + name: "rejects name exceeding 63 characters", + annotations: map[string]string{AnnotationSuggestedNamespace: "a234567890123456789012345678901234567890123456789012345678901234"}, + packageName: "pkg", + expectErr: true, + errContains: "exceeds 63 characters", + }, + { + name: "rejects name with dots", + annotations: map[string]string{AnnotationSuggestedNamespace: "my.namespace"}, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + { + name: "accepts valid fallback name", + annotations: nil, + packageName: "my-package", + }, + { + name: "accepts valid suggested-namespace", + annotations: map[string]string{AnnotationSuggestedNamespace: "valid-ns-123"}, + packageName: "pkg", + }, + { + name: "rejects invalid name from template", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"INVALID"}}`}, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := resolveSystemManagedNamespace(rv1WithAnnotations(tt.packageName, tt.annotations)) + if tt.expectErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestBuildNamespaceObject(t *testing.T) { + tests := []struct { + name string + nsName string + template *corev1.Namespace + validate func(t *testing.T, obj map[string]interface{}) + }{ + { + name: "with template labels", + nsName: "my-ns", + template: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"pod-security.kubernetes.io/enforce": "restricted"}, + }, + }, + validate: func(t *testing.T, obj map[string]interface{}) { + assert.Equal(t, "v1", obj["apiVersion"]) + assert.Equal(t, "Namespace", obj["kind"]) + metadata := obj["metadata"].(map[string]interface{}) + assert.Equal(t, "my-ns", metadata["name"]) + labels := metadata["labels"].(map[string]interface{}) + assert.Equal(t, "restricted", labels["pod-security.kubernetes.io/enforce"]) + }, + }, + { + name: "nil template", + nsName: "my-ns", + template: nil, + validate: func(t *testing.T, obj map[string]interface{}) { + metadata := obj["metadata"].(map[string]interface{}) + assert.Equal(t, "my-ns", metadata["name"]) + _, hasLabels := metadata["labels"] + assert.False(t, hasLabels) + }, + }, + { + name: "template name is overridden", + nsName: "override", + template: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "template-name"}, + }, + validate: func(t *testing.T, obj map[string]interface{}) { + metadata := obj["metadata"].(map[string]interface{}) + assert.Equal(t, "override", metadata["name"]) + }, + }, + { + name: "strips empty spec and status", + nsName: "my-ns", + validate: func(t *testing.T, obj map[string]interface{}) { + _, hasSpec := obj["spec"] + _, hasStatus := obj["status"] + assert.False(t, hasSpec) + assert.False(t, hasStatus) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := BuildNamespaceObject(tt.nsName, tt.template) + require.NoError(t, err) + tt.validate(t, result.(*unstructured.Unstructured).Object) + }) + } +} + +func TestDefaultInstallNamespace(t *testing.T) { + tests := []struct { + name string + packageName string + want string // exact expected name; empty means only assert validity + }{ + { + name: "valid short name keeps -system", + packageName: "argocd-operator", + want: "argocd-operator-system", + }, + { + name: "dotted package name is normalized", + packageName: "my.operator", + want: "my-operator-system", + }, + { + name: "uppercase and underscores are normalized", + packageName: "My_Operator", + want: "my-operator-system", + }, + { + name: "overlong package name is truncated to a valid label", + packageName: strings.Repeat("a", 80), + // no exact expectation; validated below + }, + { + name: "package name with no valid characters still yields a valid namespace", + packageName: "...", + // no exact expectation; validated below + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := defaultInstallNamespace(tt.packageName) + + // The default must always be a valid, length-bounded namespace name. + require.NoError(t, validateNamespaceName(got)) + + // It must be deterministic. + require.Equal(t, got, defaultInstallNamespace(tt.packageName)) + + if tt.want != "" { + require.Equal(t, tt.want, got) + } + }) + } + + t.Run("distinct overlong names that share a prefix do not collide", func(t *testing.T) { + a := defaultInstallNamespace(strings.Repeat("a", 70) + "-one") + b := defaultInstallNamespace(strings.Repeat("a", 70) + "-two") + require.NoError(t, validateNamespaceName(a)) + require.NoError(t, validateNamespaceName(b)) + require.NotEqual(t, a, b) + }) +} diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go index 454d4944fd..26c0742f76 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go @@ -56,6 +56,22 @@ var certVolumeConfigs = []certVolumeConfig{ }, } +// BundleInstallNamespaceGenerator emits the install Namespace object when the caller requested +// it via render.RenderInstallNamespace (opts.GenerateInstallNamespace). The install namespace +// name and template are resolved during Render setup; this generator stamps the name into the +// optional template. When rendering the namespace was not requested (e.g. the caller supplied an +// existing namespace), it is a no-op. +func BundleInstallNamespaceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + if !opts.GenerateInstallNamespace { + return nil, nil + } + obj, err := render.BuildNamespaceObject(opts.InstallNamespace, opts.InstallNamespaceTemplate) + if err != nil { + return nil, err + } + return []client.Object{obj}, nil +} + // BundleCSVDeploymentGenerator generates all deployments defined in rv1's cluster service version (CSV). The generated // resource aim to have parity with OLMv0 generated Deployment resources: // - olm.targetNamespaces annotation is set with the opts.TargetNamespace value diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go index 931e4429d3..1f8a3dc8e4 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go @@ -62,6 +62,45 @@ func Test_ResourceGenerators_Errors(t *testing.T) { require.Contains(t, err.Error(), "generator error") } +func Test_BundleInstallNamespaceGenerator(t *testing.T) { + t.Run("is a no-op when rendering the install namespace was not requested", func(t *testing.T) { + objs, err := generators.BundleInstallNamespaceGenerator(&bundle.RegistryV1{}, render.Options{ + InstallNamespace: "install-namespace", + }) + require.NoError(t, err) + require.Empty(t, objs) + }) + + t.Run("emits a Namespace object for the install namespace when requested", func(t *testing.T) { + objs, err := generators.BundleInstallNamespaceGenerator(&bundle.RegistryV1{}, render.Options{ + InstallNamespace: "install-namespace", + GenerateInstallNamespace: true, + }) + require.NoError(t, err) + require.Len(t, objs, 1) + require.Equal(t, "install-namespace", objs[0].GetName()) + require.Equal(t, "Namespace", objs[0].GetObjectKind().GroupVersionKind().Kind) + }) + + t.Run("seeds labels and annotations from the template", func(t *testing.T) { + objs, err := generators.BundleInstallNamespaceGenerator(&bundle.RegistryV1{}, render.Options{ + InstallNamespace: "install-namespace", + GenerateInstallNamespace: true, + InstallNamespaceTemplate: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"pod-security.kubernetes.io/enforce": "privileged"}, + Annotations: map[string]string{"example.com/foo": "bar"}, + }, + }, + }) + require.NoError(t, err) + require.Len(t, objs, 1) + require.Equal(t, "install-namespace", objs[0].GetName()) + require.Equal(t, map[string]string{"pod-security.kubernetes.io/enforce": "privileged"}, objs[0].GetLabels()) + require.Equal(t, map[string]string{"example.com/foo": "bar"}, objs[0].GetAnnotations()) + }) +} + func Test_BundleCSVDeploymentGenerator_Succeeds(t *testing.T) { for _, tc := range []struct { name string diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1.go b/internal/operator-controller/rukpak/render/registryv1/registryv1.go index 87ab11ba43..63cad34f9e 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1.go +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1.go @@ -38,6 +38,7 @@ var ResourceGenerators = []render.ResourceGenerator{ // NOTE: if you update this list, Test_ResourceGeneratorsHasAllGenerators will fail until // you bring the same changes over to that test. This helps ensure all validation rules are executed // while giving us the flexibility to test each generator individually + generators.BundleInstallNamespaceGenerator, generators.BundleCSVServiceAccountGenerator, generators.BundleCSVPermissionsGenerator, generators.BundleCSVClusterPermissionsGenerator, diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go index f84a2305ed..630ba47f6b 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go @@ -46,6 +46,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) { func Test_ResourceGeneratorsHasAllGenerators(t *testing.T) { expectedGenerators := []render.ResourceGenerator{ + generators.BundleInstallNamespaceGenerator, generators.BundleCSVServiceAccountGenerator, generators.BundleCSVPermissionsGenerator, generators.BundleCSVClusterPermissionsGenerator, @@ -84,7 +85,7 @@ func Test_Renderer_Success(t *testing.T) { }, } - objs, err := registryv1.Renderer.Render(someBundle, "install-namespace") + objs, err := registryv1.Renderer.Render(someBundle, render.WithInstallNamespace("install-namespace")) t.Log("Check renderer returns objects and no errors") require.NoError(t, err) require.NotEmpty(t, objs) @@ -98,6 +99,38 @@ func Test_Renderer_Success(t *testing.T) { require.Equal(t, "install-namespace", objs[0].GetNamespace()) } +func Test_Renderer_RenderInstallNamespace(t *testing.T) { + someBundle := bundle.RegistryV1{ + PackageName: "my-package", + CSV: csv.Builder(). + WithName("test-bundle"). + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), + Others: []unstructured.Unstructured{ + *ToUnstructuredT(t, &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + }, + }), + }, + } + + objs, err := registryv1.Renderer.Render(someBundle, render.RenderInstallNamespace()) + require.NoError(t, err) + + t.Log("Check the install namespace defaults to -system and a Namespace object is emitted") + require.Len(t, objs, 2) + require.Equal(t, "Namespace", objs[0].GetObjectKind().GroupVersionKind().Kind) + require.Equal(t, "my-package-system", objs[0].GetName()) + + t.Log("Check namespace-scoped resources are rendered into the defaulted install namespace") + require.Equal(t, "my-service", objs[1].GetName()) + require.Equal(t, "my-package-system", objs[1].GetNamespace()) +} + func Test_Renderer_Failure_UnsupportedKind(t *testing.T) { someBundle := bundle.RegistryV1{ PackageName: "my-package", @@ -117,7 +150,7 @@ func Test_Renderer_Failure_UnsupportedKind(t *testing.T) { }, } - objs, err := registryv1.Renderer.Render(someBundle, "install-namespace") + objs, err := registryv1.Renderer.Render(someBundle, render.WithInstallNamespace("install-namespace")) t.Log("Check renderer returns objects and no errors") require.Error(t, err) require.Contains(t, err.Error(), "unsupported resource") diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 86eb2ff492..0df897cccc 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -66,6 +66,15 @@ type Options struct { // DeploymentConfig contains optional customizations to apply to CSV deployments. // If nil, no customizations are applied. DeploymentConfig *config.DeploymentConfig + + // GenerateInstallNamespace, when true, has the renderer emit a Namespace object for the + // install namespace (see RenderInstallNamespace). When false, the install namespace is + // assumed to already exist and no Namespace object is rendered. + GenerateInstallNamespace bool + // InstallNamespaceTemplate seeds labels/annotations (e.g. PSA) on the emitted Namespace + // object. It is defaulted from the bundle's suggested-namespace-template annotation during + // Render setup and only consulted when GenerateInstallNamespace is true. + InstallNamespaceTemplate *corev1.Namespace } func (o *Options) apply(opts ...Option) *Options { @@ -90,6 +99,26 @@ func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) { type Option func(*Options) +// WithInstallNamespace overrides the install namespace that namespace-scoped resources are +// rendered into. When unset, the renderer defaults to the bundle's system-managed namespace +// (resolved from CSV annotations, else "-system"). This only sets the namespace +// name; it does not cause a Namespace object to be emitted (see RenderInstallNamespace). +func WithInstallNamespace(ns string) Option { + return func(o *Options) { + o.InstallNamespace = ns + } +} + +// RenderInstallNamespace instructs the renderer to also emit a Namespace object for the +// install namespace (seeded from the bundle's suggested-namespace-template annotation, if any). +// Without this option the install namespace is assumed to already exist and no Namespace object +// is rendered. +func RenderInstallNamespace() Option { + return func(o *Options) { + o.GenerateInstallNamespace = true + } +} + // WithTargetNamespaces sets the target namespaces to be used when rendering the bundle // The value will only be used if len(namespaces) > 0. Otherwise, the default value for the bundle // derived from its install mode support will be used (if such a value can be defined). @@ -126,31 +155,39 @@ type BundleRenderer struct { ResourceGenerators []ResourceGenerator } -func (r BundleRenderer) Render(rv1 bundle.RegistryV1, installNamespace string, opts ...Option) ([]client.Object, error) { +func (r BundleRenderer) Render(rv1 bundle.RegistryV1, opts ...Option) ([]client.Object, error) { // validate bundle if err := r.BundleValidator.Validate(&rv1); err != nil { return nil, err } - // generate bundle objects - genOpts, errs := (&Options{ + genOpts := (&Options{ // default options - InstallNamespace: installNamespace, TargetNamespaces: defaultTargetNamespacesForBundle(&rv1), UniqueNameGenerator: DefaultUniqueNameGenerator, CertificateProvider: nil, - }).apply(opts...).validate(&rv1) - - if len(errs) > 0 { - return nil, fmt.Errorf("invalid option(s): %w", errors.Join(errs...)) + }).apply(opts...) + + // Default the install namespace (and its Namespace template) from the bundle's + // system-managed namespace metadata. The derived name is only needed when the caller did + // not supply an install namespace; the template is only needed when we emit a Namespace + // object. The BundleInstallNamespaceGenerator emits the Namespace object when requested. + if genOpts.InstallNamespace == "" || genOpts.GenerateInstallNamespace { + name, template, err := resolveSystemManagedNamespace(&rv1) + if err != nil { + return nil, err + } + if genOpts.InstallNamespace == "" { + genOpts.InstallNamespace = name + } + genOpts.InstallNamespaceTemplate = template } - objs, err := ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts) - if err != nil { - return nil, err + if _, errs := genOpts.validate(&rv1); len(errs) > 0 { + return nil, fmt.Errorf("invalid option(s): %w", errors.Join(errs...)) } - return objs, nil + return ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts) } func DefaultUniqueNameGenerator(base string, o interface{}) string { diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go index fb24b7d3b1..30b71bbc16 100644 --- a/internal/operator-controller/rukpak/render/render_test.go +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -26,7 +26,7 @@ func Test_BundleRenderer_NoConfig(t *testing.T) { objs, err := renderer.Render( bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, "", nil) + }, render.WithInstallNamespace("install-namespace"), nil) require.NoError(t, err) require.Empty(t, objs) } @@ -39,7 +39,7 @@ func Test_BundleRenderer_ValidatesBundle(t *testing.T) { }, }, } - objs, err := renderer.Render(bundle.RegistryV1{}, "") + objs, err := renderer.Render(bundle.RegistryV1{}, render.WithInstallNamespace("install-namespace")) require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "this bundle is invalid") @@ -61,7 +61,7 @@ func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) { }, } - _, _ = renderer.Render(bundle.RegistryV1{}, expectedInstallNamespace) + _, _ = renderer.Render(bundle.RegistryV1{}, render.WithInstallNamespace(expectedInstallNamespace)) } func Test_BundleRenderer_DefaultTargetNamespaces(t *testing.T) { @@ -160,7 +160,7 @@ func Test_BundleRenderer_DefaultTargetNamespaces(t *testing.T) { CSV: csv.Builder(). WithName("test"). WithInstallModeSupportFor(tc.supportedInstallModes...).Build(), - }, "some-namespace") + }, render.WithInstallNamespace("some-namespace")) if tc.expectedErrMsg != "" { require.Error(t, err) require.Contains(t, err.Error(), tc.expectedErrMsg) @@ -283,8 +283,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { renderer := render.BundleRenderer{} _, err := renderer.Render( bundle.RegistryV1{CSV: tc.csv}, - tc.installNamespace, - tc.opts..., + append([]render.Option{render.WithInstallNamespace(tc.installNamespace)}, tc.opts...)..., ) if tc.err == nil { require.NoError(t, err) @@ -298,7 +297,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { func Test_BundleRenderer_AppliesUserOptions(t *testing.T) { isOptionApplied := false - _, _ = render.BundleRenderer{}.Render(bundle.RegistryV1{}, "install-namespace", func(options *render.Options) { + _, _ = render.BundleRenderer{}.Render(bundle.RegistryV1{}, render.WithInstallNamespace("install-namespace"), func(options *render.Options) { isOptionApplied = true }) require.True(t, isOptionApplied) @@ -345,7 +344,7 @@ func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { objs, err := renderer.Render( bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, "") + }, render.WithInstallNamespace("install-namespace")) require.NoError(t, err) require.Equal(t, []client.Object{&corev1.Namespace{}, &corev1.Service{}, &appsv1.Deployment{}}, objs) } @@ -364,7 +363,7 @@ func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) { objs, err := renderer.Render( bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, "") + }, render.WithInstallNamespace("install-namespace")) require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "generator error") @@ -408,7 +407,7 @@ func Test_WithDeploymentConfig(t *testing.T) { bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), }, - "test-namespace", + render.WithInstallNamespace("test-namespace"), render.WithDeploymentConfig(expectedConfig), ) @@ -431,7 +430,7 @@ func Test_WithDeploymentConfig(t *testing.T) { bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), }, - "test-namespace", + render.WithInstallNamespace("test-namespace"), ) require.NoError(t, err) @@ -453,7 +452,7 @@ func Test_WithDeploymentConfig(t *testing.T) { bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), }, - "test-namespace", + render.WithInstallNamespace("test-namespace"), render.WithDeploymentConfig(nil), ) diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 6d9346b4ae..00e7b87898 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -761,12 +761,15 @@ spec: rule: has(self.preflight) namespace: description: |- - 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. + namespace selects the namespace that namespace-scoped resources for the extension + are applied to. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + 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. + + 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. @@ -774,10 +777,13 @@ spec: maxLength: 63 type: string x-kubernetes-validations: - - message: namespace is immutable - rule: self == oldSelf - message: namespace must be a valid DNS1123 label - rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + rule: self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + - message: namespace is immutable once set + rule: oldSelf == '' || self == oldSelf + - message: namespace cannot be set after creation; mode is locked + at creation time + rule: oldSelf != '' || self == '' progressDeadlineMinutes: description: |- progressDeadlineMinutes is an optional field that defines the maximum period @@ -1107,7 +1113,6 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object status: diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index f8c3add53b..b4df446a1d 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -722,12 +722,15 @@ spec: rule: has(self.preflight) namespace: description: |- - 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. + namespace selects the namespace that namespace-scoped resources for the extension + are applied to. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + 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. + + 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. @@ -735,10 +738,13 @@ spec: maxLength: 63 type: string x-kubernetes-validations: - - message: namespace is immutable - rule: self == oldSelf - message: namespace must be a valid DNS1123 label - rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + rule: self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + - message: namespace is immutable once set + rule: oldSelf == '' || self == oldSelf + - message: namespace cannot be set after creation; mode is locked + at creation time + rule: oldSelf != '' || self == '' progressDeadlineMinutes: description: |- progressDeadlineMinutes is an optional field that defines the maximum period @@ -1068,7 +1074,6 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object status: diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 28dca6563d..9b92e24de7 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -723,12 +723,13 @@ spec: rule: has(self.preflight) namespace: description: |- - 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. + namespace selects the namespace that namespace-scoped resources for the extension + are applied to. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + In the standard configuration, namespace is required and must reference an existing + namespace on the cluster. + + 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. @@ -736,10 +737,12 @@ spec: maxLength: 63 type: string x-kubernetes-validations: - - message: namespace is immutable - rule: self == oldSelf - message: namespace must be a valid DNS1123 label - rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + rule: self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + - message: namespace is immutable once set + rule: oldSelf == '' || self == oldSelf + - message: namespace is required + rule: self != '' serviceAccount: description: |- serviceAccount is a deprecated field and is completely ignored. @@ -1059,8 +1062,8 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source + - namespace type: object status: description: status is an optional field that defines the observed state diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 71c7677772..c9d4faf1c5 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -684,12 +684,13 @@ spec: rule: has(self.preflight) namespace: description: |- - 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. + namespace selects the namespace that namespace-scoped resources for the extension + are applied to. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + In the standard configuration, namespace is required and must reference an existing + namespace on the cluster. + + 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. @@ -697,10 +698,12 @@ spec: maxLength: 63 type: string x-kubernetes-validations: - - message: namespace is immutable - rule: self == oldSelf - message: namespace must be a valid DNS1123 label - rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + rule: self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + - message: namespace is immutable once set + rule: oldSelf == '' || self == oldSelf + - message: namespace is required + rule: self != '' serviceAccount: description: |- serviceAccount is a deprecated field and is completely ignored. @@ -1020,8 +1023,8 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source + - namespace type: object status: description: status is an optional field that defines the observed state diff --git a/test/e2e/features/namespace.feature b/test/e2e/features/namespace.feature new file mode 100644 index 0000000000..6da7e7838e --- /dev/null +++ b/test/e2e/features/namespace.feature @@ -0,0 +1,62 @@ +Feature: Namespace PSA Management + + As an OLM user, when I install an operator that declares PSA requirements + via the suggested-namespace-template CSV annotation, operator-controller + should create a managed namespace with PSA labels applied. + + Background: + Given OLM is available + And an image registry is available + + @BoxcutterRuntime + Scenario: Managed namespace with PSA template applies labels + Given a catalog "test" with packages: + | package | version | channel | replaces | contents | + | test | 1.0.0 | stable | | CRD, Deployment, NSTemplate(privileged) | + When ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + source: + sourceType: Catalog + catalog: + packageName: ${PACKAGE:test} + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": ${CATALOG:test} + """ + Then ClusterExtension is rolled out + And ClusterExtension is available + And namespace "${PACKAGE:test}-system" has labels + | key | value | + | pod-security.kubernetes.io/enforce | privileged | + | pod-security.kubernetes.io/audit | privileged | + | pod-security.kubernetes.io/warn | privileged | + + Scenario: User-provided namespace does not get PSA labels + Given namespace "${TEST_NAMESPACE}" is available + And a catalog "test" with packages: + | package | version | channel | replaces | contents | + | test | 1.0.0 | stable | | CRD, Deployment, ConfigMap | + When ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + namespace: ${TEST_NAMESPACE} + source: + sourceType: Catalog + catalog: + packageName: ${PACKAGE:test} + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": ${CATALOG:test} + """ + Then ClusterExtension is rolled out + And ClusterExtension is available + And namespace "${TEST_NAMESPACE}" does not have label "pod-security.kubernetes.io/enforce" diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 31abf4bc0b..d3edd5169c 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -185,6 +185,9 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)catalog "([^"]+)" is labeled with "([^"]+)"$`, CatalogIsLabeledWith) sc.Step(`^(?i)ValidatingAdmissionPolicy "([^"]+)" is active$`, ValidatingAdmissionPolicyIsActive) + sc.Step(`^(?i)namespace "([^"]+)" has labels$`, NamespaceHasLabels) + sc.Step(`^(?i)namespace "([^"]+)" does not have label "([^"]+)"$`, NamespaceDoesNotHaveLabel) + sc.Step(`^(?i)operator "([^"]+)" target namespace is "([^"]+)"$`, OperatorTargetNamespace) sc.Step(`^(?i)Prometheus metrics are returned in the response$`, PrometheusMetricsAreReturned) @@ -1968,6 +1971,10 @@ func parseContents(contents string) ([]catalog.BundleOption, error) { dir := part[len("StaticBundleDir(") : len(part)-1] absDir := filepath.Join(projectRootDir(), dir) opts = append(opts, catalog.StaticBundleDir(absDir)) + case strings.HasPrefix(part, "NSTemplate(") && strings.HasSuffix(part, ")"): + // NSTemplate(privileged) or NSTemplate(baseline) or NSTemplate(restricted) + level := part[len("NSTemplate(") : len(part)-1] + opts = append(opts, catalog.WithNSTemplate(level)) } } return opts, nil @@ -2459,6 +2466,53 @@ func ResourceHasLabels(ctx context.Context, resourceName string, table *godog.Ta return nil } +// NamespaceHasLabels waits for a namespace (cluster-scoped) to have all labels specified in the data table. +func NamespaceHasLabels(ctx context.Context, nsName string, table *godog.Table) error { + sc := scenarioCtx(ctx) + nsName = substituteScenarioVars(nsName, sc) + + expected, err := parseKeyValueTable(table, sc) + if err != nil { + return fmt.Errorf("invalid labels table: %w", err) + } + + waitFor(ctx, func() bool { + out, err := k8sClient(ctx, "get", "namespace", nsName, "-o", "json") + if err != nil { + return false + } + var obj unstructured.Unstructured + if err := json.Unmarshal([]byte(out), &obj); err != nil { + return false + } + if key, got, ok := matchLabels(obj.GetLabels(), expected); !ok { + logger.V(1).Info("Namespace label not yet present or value mismatch", "namespace", nsName, "key", key, "expected", expected[key], "actual", got) + return false + } + return true + }) + return nil +} + +// NamespaceDoesNotHaveLabel verifies a namespace does not have the specified label. +func NamespaceDoesNotHaveLabel(ctx context.Context, nsName string, labelKey string) error { + sc := scenarioCtx(ctx) + nsName = substituteScenarioVars(nsName, sc) + + out, err := k8sClient(ctx, "get", "namespace", nsName, "-o", "json") + if err != nil { + return fmt.Errorf("failed to get namespace %q: %w", nsName, err) + } + var obj unstructured.Unstructured + if err := json.Unmarshal([]byte(out), &obj); err != nil { + return fmt.Errorf("failed to unmarshal namespace: %w", err) + } + if v, ok := obj.GetLabels()[labelKey]; ok { + return fmt.Errorf("namespace %q has unexpected label %s=%s", nsName, labelKey, v) + } + return nil +} + // nestedString traverses a nested map[string]interface{} by the given keys // and returns the leaf value as a string. func nestedString(obj map[string]interface{}, keys ...string) (string, bool) { diff --git a/test/internal/catalog/bundle.go b/test/internal/catalog/bundle.go index 7bb80b5bce..491846c654 100644 --- a/test/internal/catalog/bundle.go +++ b/test/internal/catalog/bundle.go @@ -41,6 +41,7 @@ type bundleConfig struct { largeCRDFieldCount int // if > 0, generate a CRD with this many fields staticBundleDir string // if set, read bundle from this directory (no parameterization) clusterRegistryOverride string // if set, use this host in the FBC image ref instead of the default + csvAnnotations map[string]string } // bundleSpec is the resolved bundle: version + file map ready for crane.Image(). @@ -109,6 +110,22 @@ func WithBundleProperty(propertyType, value string) BundleOption { } } +// WithCSVAnnotation adds an annotation to the bundle's CSV. +func WithCSVAnnotation(key, value string) BundleOption { + return func(c *bundleConfig) { + if c.csvAnnotations == nil { + c.csvAnnotations = make(map[string]string) + } + c.csvAnnotations[key] = value + } +} + +// WithNSTemplate adds a suggested namespace template annotation to the CSV with the specified PSA level. +func WithNSTemplate(psaLevel string) BundleOption { + template := fmt.Sprintf(`{"apiVersion":"v1","kind":"Namespace","metadata":{"labels":{"pod-security.kubernetes.io/enforce":"%s","pod-security.kubernetes.io/audit":"%s","pod-security.kubernetes.io/warn":"%s"}}}`, psaLevel, psaLevel, psaLevel) + return WithCSVAnnotation("operatorframework.io/suggested-namespace-template", template) +} + // BadImage produces a bundle with CRD and deployment but uses "wrong/image" as // the container image, causing ImagePullBackOff at runtime. func BadImage() BundleOption { @@ -164,6 +181,10 @@ func buildBundle(scenarioID, packageName, version string, opts []BundleOption) ( WithName(fmt.Sprintf("%s.v%s", packageName, version)). WithInstallModeSupportFor(installModes...) + if len(cfg.csvAnnotations) > 0 { + csvBuilder = csvBuilder.WithAnnotations(cfg.csvAnnotations) + } + if cfg.hasCRD { csvBuilder = csvBuilder.WithOwnedCRDs(v1alpha1.CRDDescription{ Name: crdName, diff --git a/test/regression/convert/generate-manifests.go b/test/regression/convert/generate-manifests.go index a3e3197e6e..73b0d21795 100644 --- a/test/regression/convert/generate-manifests.go +++ b/test/regression/convert/generate-manifests.go @@ -275,11 +275,14 @@ func generateManifests(outputPath, bundleDir, installNamespace, watchNamespace s } // Convert RegistryV1 to plain manifests - opts := []render.Option{render.WithTargetNamespaces(watchNamespace)} + opts := []render.Option{ + render.WithInstallNamespace(installNamespace), + render.WithTargetNamespaces(watchNamespace), + } if deploymentConfig != nil { opts = append(opts, render.WithDeploymentConfig(deploymentConfig)) } - objs, err := registryv1.Renderer.Render(regv1, installNamespace, opts...) + objs, err := registryv1.Renderer.Render(regv1, opts...) if err != nil { return fmt.Errorf("error converting registry+v1 bundle: %w", err) }