diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6f7912ae9b..1a15c6a33a 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -49,21 +49,25 @@ 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 is optional. When set, it specifies an existing namespace where + // namespace-scoped resources for the extension are applied. The namespace must + // already exist on the cluster. When omitted, operator-controller resolves and + // creates a managed namespace from bundle metadata. // - // The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + // The mode (set vs omitted) is locked at creation time and cannot be changed. + // When set, the value is immutable. + // + // 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" + // +kubebuilder:validation:XValidation:rule="oldSelf != '' || self == ''",message="namespace cannot be set after creation; mode is locked at creation time" + // +optional Namespace string `json:"namespace"` // serviceAccount is a deprecated field and is completely ignored. @@ -545,6 +549,13 @@ type ClusterExtensionStatus struct { // +optional Install *ClusterExtensionInstallStatus `json:"install,omitempty"` + // namespace is the resolved namespace where the extension is installed. + // For user-provided namespaces, this mirrors spec.namespace. + // For managed namespaces, this shows the name resolved from bundle metadata. + // + // +optional + Namespace string `json:"namespace,omitempty"` + // activeRevisions holds a list of currently active (non-archived) ClusterObjectSets, // including both installed and rolling out revisions. // +listType=map diff --git a/applyconfigurations/api/v1/clusterextensionspec.go b/applyconfigurations/api/v1/clusterextensionspec.go index 47d810a74a..0d00cc0c23 100644 --- a/applyconfigurations/api/v1/clusterextensionspec.go +++ b/applyconfigurations/api/v1/clusterextensionspec.go @@ -22,12 +22,15 @@ 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 is optional. When set, it specifies an existing namespace where + // namespace-scoped resources for the extension are applied. The namespace must + // already exist on the cluster. When omitted, operator-controller resolves and + // creates a managed namespace from bundle metadata. // - // The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + // The mode (set vs omitted) is locked at creation time and cannot be changed. + // When set, the value is immutable. + // + // 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. // diff --git a/applyconfigurations/api/v1/clusterextensionstatus.go b/applyconfigurations/api/v1/clusterextensionstatus.go index d11ad931dd..42c3db8766 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 @@ -51,6 +51,10 @@ type ClusterExtensionStatusApplyConfiguration struct { Conditions []metav1.ConditionApplyConfiguration `json:"conditions,omitempty"` // install is a representation of the current installation status for this ClusterExtension. Install *ClusterExtensionInstallStatusApplyConfiguration `json:"install,omitempty"` + // namespace is the resolved namespace where the extension is installed. + // For user-provided namespaces, this mirrors spec.namespace. + // For managed namespaces, this shows the name resolved from bundle metadata. + Namespace *string `json:"namespace,omitempty"` // activeRevisions holds a list of currently active (non-archived) ClusterObjectSets, // including both installed and rolling out revisions. // @@ -84,6 +88,14 @@ func (b *ClusterExtensionStatusApplyConfiguration) WithInstall(value *ClusterExt return b } +// WithNamespace sets the Namespace field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Namespace field is set to the value of the last call. +func (b *ClusterExtensionStatusApplyConfiguration) WithNamespace(value string) *ClusterExtensionStatusApplyConfiguration { + b.Namespace = &value + return b +} + // WithActiveRevisions adds the given value to the ActiveRevisions field in the declarative configuration // and returns the receiver, so that objects can be build by chaining "With" function invocations. // If called multiple times, values provided by each call will be appended to the ActiveRevisions field. diff --git a/applyconfigurations/internal/internal.go b/applyconfigurations/internal/internal.go index dde5aaf513..ddbccdd501 100644 --- a/applyconfigurations/internal/internal.go +++ b/applyconfigurations/internal/internal.go @@ -248,6 +248,9 @@ var schemaYAML = typed.YAMLObject(`types: - name: install type: namedType: com.github.operator-framework.operator-controller.api.v1.ClusterExtensionInstallStatus + - name: namespace + type: + scalar: string - name: com.github.operator-framework.operator-controller.api.v1.ClusterObjectSet map: fields: diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index b017927a38..784dee4e05 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -659,6 +659,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.ResolveNamespace(coreClient), controllers.ApplyBundleWithBoxcutter(appl.Apply), } @@ -746,6 +747,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.ResolveNamespace(coreClient), controllers.ApplyBundle(appl), } diff --git a/docs/concepts/managed-namespaces.md b/docs/concepts/managed-namespaces.md new file mode 100644 index 0000000000..eef2bbf9f1 --- /dev/null +++ b/docs/concepts/managed-namespaces.md @@ -0,0 +1,45 @@ +# Managed Namespaces + +## What is a managed namespace? + +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. + +## Namespace resolution + +In managed mode, the namespace name is resolved from bundle CSV annotations in this order: + +1. `operatorframework.io/suggested-namespace-template`: the `metadata.name` field from the JSON template +2. `operators.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..1b4a987326 --- /dev/null +++ b/docs/howto/namespace-configuration-for-authors.md @@ -0,0 +1,58 @@ +# Namespace Configuration for Bundle Authors + +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`. + +## 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" + } + } + } +``` + +### `operators.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: + operators.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 `operators.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/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..61a4351a95 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 is optional. When set, it specifies an existing namespace where + namespace-scoped resources for the extension are applied. The namespace must + already exist on the cluster. When omitted, operator-controller resolves and + creates a managed namespace from bundle metadata. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + The mode (set vs omitted) is locked at creation time and cannot be changed. + When set, the value is immutable. + + 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: @@ -725,6 +730,12 @@ spec: required: - bundle type: object + namespace: + description: |- + namespace is the resolved namespace where the extension is installed. + For user-provided namespaces, this mirrors spec.namespace. + For managed namespaces, this shows the name resolved from bundle metadata. + type: string type: object type: object served: true 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..d75b29147e 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,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 is optional. When set, it specifies an existing namespace where + namespace-scoped resources for the extension are applied. The namespace must + already exist on the cluster. When omitted, operator-controller resolves and + creates a managed namespace from bundle metadata. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + The mode (set vs omitted) is locked at creation time and cannot be changed. + When set, the value is immutable. + + 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 +125,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 == '' serviceAccount: description: |- serviceAccount is a deprecated field and is completely ignored. @@ -445,7 +451,6 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object status: @@ -569,6 +574,12 @@ spec: required: - bundle type: object + namespace: + description: |- + namespace is the resolved namespace where the extension is installed. + For user-provided namespaces, this mirrors spec.namespace. + For managed namespaces, this shows the name resolved from bundle metadata. + type: string type: object type: object served: true diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index a52fa21c7e..afa4edbf79 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -42,12 +42,19 @@ const ( ClusterObjectSetRetentionLimit = 5 ) +type NamespaceConfig struct { + Name string + Managed bool + Template *corev1.Namespace +} + type ClusterObjectSetGenerator interface { - GenerateRevision(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) + GenerateRevision(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) GenerateRevisionFromHelmRelease( ctx context.Context, helmRelease *release.Release, ext *ocv1.ClusterExtension, objectLabels map[string]string, + nsConfig *NamespaceConfig, ) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) } @@ -60,6 +67,7 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( ctx context.Context, helmRelease *release.Release, ext *ocv1.ClusterExtension, objectLabels map[string]string, + nsConfig *NamespaceConfig, ) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { docs := splitManifestDocuments(helmRelease.Manifest) objs := make([]ocv1ac.ClusterObjectSetObjectApplyConfiguration, 0, len(docs)) @@ -93,6 +101,15 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( WithObject(obj)) } + if nsConfig != nil && nsConfig.Managed { + nsObj, err := BuildNamespaceObject(nsConfig.Name, nsConfig.Template) + if err != nil { + return nil, err + } + nsObj.SetLabels(mergeStringMaps(nsObj.GetLabels(), objectLabels)) + objs = append(objs, *ocv1ac.ClusterObjectSetObject().WithObject(nsObj)) + } + revisionAnnotations := map[string]string{ labels.BundleNameKey: helmRelease.Labels[labels.BundleNameKey], labels.PackageNameKey: helmRelease.Labels[labels.PackageNameKey], @@ -113,7 +130,15 @@ func (r *SimpleRevisionGenerator) GenerateRevision( ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, + nsConfig *NamespaceConfig, ) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + // For managed namespaces, the resolved name must be visible to the renderer + // so that namespaced bundle resources get the correct default namespace. + if nsConfig != nil && nsConfig.Managed && ext.Spec.Namespace == "" { + ext = ext.DeepCopy() + ext.Spec.Namespace = nsConfig.Name + } + // extract plain manifests plain, err := r.ManifestProvider.Get(bundleFS, ext) if err != nil { @@ -125,7 +150,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 +203,16 @@ func (r *SimpleRevisionGenerator) GenerateRevision( objs = append(objs, *ocv1ac.ClusterObjectSetObject(). WithObject(unstr)) } + + if nsConfig != nil && nsConfig.Managed { + nsObj, err := BuildNamespaceObject(nsConfig.Name, nsConfig.Template) + if err != nil { + return nil, err + } + nsObj.SetLabels(mergeStringMaps(nsObj.GetLabels(), objectLabels)) + objs = append(objs, *ocv1ac.ClusterObjectSetObject().WithObject(nsObj)) + } + rev := r.buildClusterObjectSet(objs, ext, revisionAnnotations) rev.Spec.WithCollisionProtection(ocv1.CollisionProtectionPrevent) return rev, nil @@ -273,6 +308,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, @@ -313,7 +353,7 @@ func (m *BoxcutterStorageMigrator) Migrate(ctx context.Context, ext *ocv1.Cluste } } - rev, err := m.RevisionGenerator.GenerateRevisionFromHelmRelease(ctx, helmRelease, ext, objectLabels) + rev, err := m.RevisionGenerator.GenerateRevisionFromHelmRelease(ctx, helmRelease, ext, objectLabels, nil) if err != nil { return err } @@ -454,7 +494,7 @@ type Boxcutter struct { SystemNamespace string } -func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error) { +func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *NamespaceConfig) (bool, string, error) { // List existing revisions first to validate cluster connectivity before checking contentFS. // This ensures we fail fast on API errors rather than attempting fallback behavior when // cluster access is unavailable (since the ClusterObjectSet controller also requires @@ -479,7 +519,7 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust } // Generate desired revision - desiredRevision, err := bc.RevisionGenerator.GenerateRevision(ctx, contentFS, ext, objectLabels, revisionAnnotations) + desiredRevision, err := bc.RevisionGenerator.GenerateRevision(ctx, contentFS, ext, objectLabels, revisionAnnotations, nsConfig) if err != nil { return false, "", err } diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 25963c9a01..31e3a64ed7 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -89,69 +89,40 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) "my-label": "my-value", } - rev, err := g.GenerateRevisionFromHelmRelease(t.Context(), helmRelease, ext, objectLabels) + rev, err := g.GenerateRevisionFromHelmRelease(t.Context(), helmRelease, ext, objectLabels, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) 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) + + // Verify phases - should have namespaces phase and configuration phase + require.Len(t, rev.Spec.Phases, 2) + + // Verify namespace phase + namespacesPhase := rev.Spec.Phases[0] + assert.Equal(t, "namespaces", *namespacesPhase.Name) + require.Len(t, namespacesPhase.Objects, 1) + assert.Equal(t, "Namespace", namespacesPhase.Objects[0].Object.GetKind()) + assert.Equal(t, "test-namespace", namespacesPhase.Objects[0].Object.GetName()) + + // Verify configuration phase + configPhase := rev.Spec.Phases[1] + 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) { @@ -202,7 +173,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{ labels.BundleVersionKey: "1.0.0", labels.PackageNameKey: "test-package", - }) + }, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) require.NoError(t, err) t.Log("by checking the olm.operatorframework.io/owner-name and owner-kind labels are set") @@ -216,6 +187,21 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { require.Equal(t, ptr.To(ocv1.CollisionProtectionPrevent), rev.Spec.CollisionProtection) t.Log("by checking the rendered objects are present in the correct phases") require.Equal(t, []ocv1ac.ClusterObjectSetPhaseApplyConfiguration{ + *ocv1ac.ClusterObjectSetPhase(). + WithName(string(applier.PhaseNamespaces)). + WithObjects( + ocv1ac.ClusterObjectSetObject(). + WithObject(unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "test-namespace", + "labels": map[string]interface{}{}, + }, + }, + }), + ), *ocv1ac.ClusterObjectSetPhase(). WithName(string(applier.PhaseInfrastructure)). WithObjects( @@ -298,7 +284,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing. WithCSV(bundlecsv.Builder().WithName("test-csv").Build()). Build() - rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}) + rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}, nil) require.NoError(t, err) t.Log("by checking bundle properties are added to the revision annotations") @@ -312,7 +298,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing. WithCSV(bundlecsv.Builder().WithName("test-csv").Build()). Build() - rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}) + rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}, nil) require.NoError(t, err) t.Log("by checking olm.properties is not present in the revision annotations") @@ -332,7 +318,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing. Build()). Build() - rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}) + rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, map[string]string{}, map[string]string{}, nil) require.NoError(t, err) t.Log("by checking csv annotations are not added to the revision annotations") @@ -341,7 +327,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision_BundleAnnotations(t *testing. }) t.Run("errors getting bundle properties are surfaced", func(t *testing.T) { - _, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, map[string]string{}, map[string]string{}) + _, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, map[string]string{}, map[string]string{}, nil) require.Error(t, err) require.Contains(t, err.Error(), "metadata/annotations.yaml: file does not exist") }) @@ -367,7 +353,7 @@ func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) { ManifestProvider: r, } - _, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{}) + _, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{}, nil) require.NoError(t, err) } @@ -409,15 +395,22 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t }, }, map[string]string{ "some": "value", - }, revAnnotations) + }, revAnnotations, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) require.NoError(t, err) 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") @@ -473,7 +466,7 @@ func Test_SimpleRevisionGenerator_PropagatesProgressDeadlineMinutes(t *testing.T ext.Spec.ProgressDeadlineMinutes = *pd } - rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, empty, empty) + rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, empty, empty, nil) require.NoError(t, err) require.Equal(t, tc.want.progressDeadlineMinutes, rev.Spec.ProgressDeadlineMinutes) }) @@ -494,7 +487,7 @@ func Test_SimpleRevisionGenerator_Failure(t *testing.T) { Spec: ocv1.ClusterExtensionSpec{ Namespace: "test-namespace", }, - }, map[string]string{}, map[string]string{}) + }, map[string]string{}, map[string]string{}, nil) require.Nil(t, rev) t.Log("by checking rendering errors are propagated") require.Error(t, err) @@ -577,8 +570,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return ocv1ac.ClusterObjectSet(""). WithAnnotations(revisionAnnotations). WithLabels(map[string]string{ @@ -603,7 +596,7 @@ func TestBoxcutter_Apply(t *testing.T) { ), ), nil }).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, validate: func(t *testing.T, c client.Client) { @@ -625,8 +618,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return ocv1ac.ClusterObjectSet(""). WithAnnotations(revisionAnnotations). WithLabels(map[string]string{ @@ -651,7 +644,7 @@ func TestBoxcutter_Apply(t *testing.T) { ), ), nil }).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, existingObjs: []client.Object{ @@ -671,8 +664,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return ocv1ac.ClusterObjectSet(""). WithAnnotations(revisionAnnotations). WithLabels(map[string]string{ @@ -697,7 +690,7 @@ func TestBoxcutter_Apply(t *testing.T) { ), ), nil }).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, clientIterceptor: allowedRevisionValue(2), @@ -729,8 +722,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("render boom")).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("render boom")).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, expectedErr: "render boom", @@ -747,8 +740,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return ocv1ac.ClusterObjectSet(""). WithAnnotations(revisionAnnotations). WithLabels(map[string]string{ @@ -756,7 +749,7 @@ func TestBoxcutter_Apply(t *testing.T) { }). WithSpec(ocv1ac.ClusterObjectSetSpec()), nil }).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, existingObjs: []client.Object{ @@ -853,8 +846,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return ocv1ac.ClusterObjectSet(""). WithAnnotations(revisionAnnotations). WithLabels(map[string]string{ @@ -862,7 +855,7 @@ func TestBoxcutter_Apply(t *testing.T) { }). WithSpec(ocv1ac.ClusterObjectSetSpec()), nil }).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, existingObjs: []client.Object{ @@ -975,8 +968,8 @@ func TestBoxcutter_Apply(t *testing.T) { mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevision(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, bundleFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return ocv1ac.ClusterObjectSet(""). WithAnnotations(revisionAnnotations). WithLabels(map[string]string{ @@ -1001,7 +994,7 @@ func TestBoxcutter_Apply(t *testing.T) { ), ), nil }).AnyTimes() - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() return m }, existingObjs: []client.Object{ @@ -1088,7 +1081,7 @@ func TestBoxcutter_Apply(t *testing.T) { labels.PackageNameKey: "test-package", } } - completed, status, err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations) + completed, status, err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations, nil) // Assert if tc.expectedErr != "" { @@ -1129,8 +1122,8 @@ func TestBoxcutterStorageMigrator(t *testing.T) { newStorageMigratorGenerator := func(t *testing.T) *mockapplier.MockClusterObjectSetGenerator { ctrl := gomock.NewController(t) m := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, helmRelease *release.Release, e *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + m.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, helmRelease *release.Release, e *ocv1.ClusterExtension, objectLabels map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return defaultHelmRevisionResult(e), nil }).AnyTimes() return m @@ -1141,7 +1134,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 +1207,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 +1262,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 +1335,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 +1418,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 +1475,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", @@ -1492,8 +1485,8 @@ func TestBoxcutterStorageMigrator(t *testing.T) { ctrl := gomock.NewController(t) brb := mockapplier.NewMockClusterObjectSetGenerator(ctrl) - brb.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), expectedRelease, gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, helmRelease *release.Release, e *ocv1.ClusterExtension, objectLabels map[string]string) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { + brb.EXPECT().GenerateRevisionFromHelmRelease(gomock.Any(), expectedRelease, gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx context.Context, helmRelease *release.Release, e *ocv1.ClusterExtension, objectLabels map[string]string, nsConfig *applier.NamespaceConfig) (*ocv1ac.ClusterObjectSetApplyConfiguration, error) { return defaultHelmRevisionResult(e), nil }).Times(1) @@ -1579,7 +1572,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 +1619,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 +1644,393 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, err) }) } + +func Test_SimpleRevisionGenerator_GenerateRevision_NamespaceInjection(t *testing.T) { + ctrl := gomock.NewController(t) + r := mockapplier.NewMockManifestProvider(ctrl) + r.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]client.Object{}, 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", + }, + }, + } + + objectLabels := map[string]string{ + "test-label": "test-value", + } + + t.Run("with namespace template", func(t *testing.T) { + nsTemplate := `{ + "metadata": { + "name": "template-name", + "labels": { + "template-label": "template-value", + "security": "restricted" + }, + "annotations": { + "template-annotation": "template-annotation-value" + } + } + }` + + bundleFS := bundlefs.Builder(). + WithPackageName("test-package"). + WithCSV(bundlecsv.Builder(). + WithName("test-csv"). + WithAnnotations(map[string]string{ + applier.AnnotationSuggestedNamespaceTemplate: nsTemplate, + }). + Build()). + Build() + + // Parse the namespace template to pass via NamespaceConfig + bundleAnnotations, err := applier.GetBundleAnnotations(bundleFS) + require.NoError(t, err) + parsedTemplate, err := applier.ParseNamespaceTemplate(bundleAnnotations) + require.NoError(t, err) + + rev, err := b.GenerateRevision(t.Context(), bundleFS, ext, objectLabels, map[string]string{}, &applier.NamespaceConfig{Managed: true, Name: "test-namespace", Template: parsedTemplate}) + require.NoError(t, err) + require.NotNil(t, rev) + + // 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") + require.Len(t, namespacesPhase.Objects, 1, "namespaces phase should contain exactly one object") + + nsObj := namespacesPhase.Objects[0].Object + require.NotNil(t, nsObj) + require.Equal(t, "v1", nsObj.GetAPIVersion()) + require.Equal(t, "Namespace", nsObj.GetKind()) + require.Equal(t, "test-namespace", nsObj.GetName(), "namespace name should match ext.Spec.Namespace, not template name") + + // Verify template labels are present + labels := nsObj.GetLabels() + require.Equal(t, "template-value", labels["template-label"]) + require.Equal(t, "restricted", labels["security"]) + // Verify objectLabels are also present + require.Equal(t, "test-value", labels["test-label"]) + + // Verify template annotations are present + annotations := nsObj.GetAnnotations() + require.Equal(t, "template-annotation-value", annotations["template-annotation"]) + }) + + t.Run("without namespace template", func(t *testing.T) { + rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, objectLabels, map[string]string{}, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) + require.NoError(t, err) + require.NotNil(t, rev) + + // 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 even without template") + require.Len(t, namespacesPhase.Objects, 1, "namespaces phase should contain exactly one object") + + nsObj := namespacesPhase.Objects[0].Object + require.NotNil(t, nsObj) + require.Equal(t, "v1", nsObj.GetAPIVersion()) + require.Equal(t, "Namespace", nsObj.GetKind()) + require.Equal(t, "test-namespace", nsObj.GetName()) + + // Verify objectLabels are present + labels := nsObj.GetLabels() + require.Equal(t, "test-value", labels["test-label"]) + + // Should not have template-specific labels + require.NotContains(t, labels, "template-label") + require.NotContains(t, labels, "security") + }) +} + +func Test_SimpleRevisionGenerator_GenerateRevision_NamespacePhaseCollisionProtection(t *testing.T) { + ctrl := gomock.NewController(t) + r := mockapplier.NewMockManifestProvider(ctrl) + r.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]client.Object{ + &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{}, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) + 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_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease_IncludesNamespace(t *testing.T) { + g := &applier.SimpleRevisionGenerator{} + + helmRelease := &release.Release{ + Name: "test-123", + Manifest: `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"test-cm"}}`, + Labels: map[string]string{ + labels.BundleNameKey: "my-bundle", + labels.PackageNameKey: "my-package", + labels.BundleVersionKey: "1.0.0", + }, + } + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test-123"}, + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "test-namespace", + ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"}, //nolint:staticcheck // deprecated field used in test + }, + } + + rev, err := g.GenerateRevisionFromHelmRelease(t.Context(), helmRelease, ext, nil, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) + require.NoError(t, err) + + var foundNS bool + for _, phase := range rev.Spec.Phases { + if *phase.Name == "namespaces" { + for _, obj := range phase.Objects { + if obj.Object.GetKind() == "Namespace" { + foundNS = true + assert.Equal(t, "test-namespace", obj.Object.GetName()) + } + } + } + } + assert.True(t, foundNS, "expected Namespace in Helm migration revision") +} + +func Test_GenerateRevision_NamespacePhaseIsFirst(t *testing.T) { + ctrl := gomock.NewController(t) + r := mockapplier.NewMockManifestProvider(ctrl) + r.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]client.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, + }, + &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{}, &applier.NamespaceConfig{Managed: true, Name: "test-namespace"}) + 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{}, nil) + 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") +} + +func Test_ResolveNamespaceName_WithBundleFS(t *testing.T) { + t.Run("resolves name from suggested-namespace-template annotation", func(t *testing.T) { + nsTemplate := `{"metadata":{"name":"operator-ns","labels":{"pod-security.kubernetes.io/enforce":"privileged"}}}` + bundleFS := bundlefs.Builder(). + WithPackageName("test-package"). + WithCSV(bundlecsv.Builder(). + WithName("test-csv"). + WithAnnotations(map[string]string{ + applier.AnnotationSuggestedNamespaceTemplate: nsTemplate, + }). + Build()). + Build() + + annotations, err := applier.GetBundleAnnotations(bundleFS) + require.NoError(t, err) + + name, template, err := applier.ResolveNamespaceName(annotations, "test-package") + require.NoError(t, err) + assert.Equal(t, "operator-ns", name) + require.NotNil(t, template) + assert.Equal(t, "privileged", template.Labels["pod-security.kubernetes.io/enforce"]) + }) + + t.Run("resolves name from suggested-namespace annotation", func(t *testing.T) { + bundleFS := bundlefs.Builder(). + WithPackageName("test-package"). + WithCSV(bundlecsv.Builder(). + WithName("test-csv"). + WithAnnotations(map[string]string{ + applier.AnnotationSuggestedNamespace: "my-operator-ns", + }). + Build()). + Build() + + annotations, err := applier.GetBundleAnnotations(bundleFS) + require.NoError(t, err) + + name, template, err := applier.ResolveNamespaceName(annotations, "test-package") + require.NoError(t, err) + assert.Equal(t, "my-operator-ns", name) + assert.Nil(t, template) + }) + + t.Run("falls back to packageName-system", func(t *testing.T) { + bundleFS := bundlefs.Builder(). + WithPackageName("test-package"). + WithCSV(bundlecsv.Builder().WithName("test-csv").Build()). + Build() + + annotations, err := applier.GetBundleAnnotations(bundleFS) + require.NoError(t, err) + + name, template, err := applier.ResolveNamespaceName(annotations, "test-package") + require.NoError(t, err) + assert.Equal(t, "test-package-system", name) + assert.Nil(t, template) + }) + + t.Run("template takes priority over suggested-namespace", func(t *testing.T) { + bundleFS := bundlefs.Builder(). + WithPackageName("test-package"). + WithCSV(bundlecsv.Builder(). + WithName("test-csv"). + WithAnnotations(map[string]string{ + applier.AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"from-template"}}`, + applier.AnnotationSuggestedNamespace: "from-annotation", + }). + Build()). + Build() + + annotations, err := applier.GetBundleAnnotations(bundleFS) + require.NoError(t, err) + + name, _, err := applier.ResolveNamespaceName(annotations, "test-package") + require.NoError(t, err) + assert.Equal(t, "from-template", name) + }) +} diff --git a/internal/operator-controller/applier/namespace.go b/internal/operator-controller/applier/namespace.go new file mode 100644 index 0000000000..26d6d2f2c0 --- /dev/null +++ b/internal/operator-controller/applier/namespace.go @@ -0,0 +1,113 @@ +package applier + +import ( + "encoding/json" + "fmt" + "regexp" + + 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" +) + +var dns1123LabelRegexp = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + +const ( + AnnotationSuggestedNamespaceTemplate = "operatorframework.io/suggested-namespace-template" + AnnotationSuggestedNamespace = "operators.operatorframework.io/suggested-namespace" +) + +func ParseNamespaceTemplate(csvAnnotations map[string]string) (*corev1.Namespace, error) { + if csvAnnotations == nil { + return nil, nil + } + + 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 +} + +// ResolveNamespaceName resolves the namespace name from CSV annotations using the +// fallback chain: suggested-namespace-template name → suggested-namespace → -system. +// Returns the resolved name, the parsed template (if any), and an error. +func ResolveNamespaceName(csvAnnotations map[string]string, packageName string) (string, *corev1.Namespace, error) { + // Try suggested-namespace-template first + template, err := ParseNamespaceTemplate(csvAnnotations) + if err != nil { + return "", nil, err + } + + var name string + if template != nil && template.Name != "" { + name = template.Name + } else if csvAnnotations != nil { + if n, ok := csvAnnotations[AnnotationSuggestedNamespace]; ok && n != "" { + name = n + } + } + + if name == "" { + name = fmt.Sprintf("%s-system", packageName) + } + + if err := validateNamespaceName(name); err != nil { + return "", nil, err + } + + return name, template, nil +} + +func validateNamespaceName(name string) error { + if name == "" { + return fmt.Errorf("resolved namespace name is empty") + } + if len(name) > 63 { + return fmt.Errorf("resolved namespace name %q exceeds 63 characters", name) + } + if !dns1123LabelRegexp.MatchString(name) { + return fmt.Errorf("resolved namespace name %q is not a valid DNS1123 label", name) + } + return nil +} + +func BuildNamespaceObject(name string, template *corev1.Namespace) (unstructured.Unstructured, 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 unstructured.Unstructured{}, fmt.Errorf("failed to convert namespace to unstructured: %w", err) + } + + // Remove empty spec/status to avoid apply drift, consistent with how + // other bundle objects are sanitized before storage in a ClusterObjectSet. + delete(unstructuredObj, "status") + delete(unstructuredObj, "spec") + + return unstructured.Unstructured{Object: unstructuredObj}, nil +} diff --git a/internal/operator-controller/applier/namespace_test.go b/internal/operator-controller/applier/namespace_test.go new file mode 100644 index 0000000000..f0fbd859f5 --- /dev/null +++ b/internal/operator-controller/applier/namespace_test.go @@ -0,0 +1,434 @@ +package applier + +import ( + "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" +) + +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, + expectError: false, + }, + { + name: "empty map", + annotations: map[string]string{}, + expected: nil, + expectError: false, + }, + { + name: "annotation absent", + annotations: map[string]string{ + "some.other/annotation": "value", + }, + expected: nil, + expectError: false, + }, + { + name: "empty string value", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: "", + }, + expected: nil, + expectError: false, + }, + { + name: "valid template with PSA labels", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{ + "metadata": { + "labels": { + "pod-security.kubernetes.io/enforce": "restricted", + "pod-security.kubernetes.io/audit": "restricted", + "pod-security.kubernetes.io/warn": "restricted" + } + } + }`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "pod-security.kubernetes.io/enforce": "restricted", + "pod-security.kubernetes.io/audit": "restricted", + "pod-security.kubernetes.io/warn": "restricted", + }, + }, + }, + expectError: false, + }, + { + name: "valid template with annotations", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{ + "metadata": { + "annotations": { + "openshift.io/description": "Operator namespace", + "openshift.io/display-name": "My Operator" + } + } + }`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "openshift.io/description": "Operator namespace", + "openshift.io/display-name": "My Operator", + }, + }, + }, + expectError: false, + }, + { + name: "valid template with both labels and annotations", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{ + "metadata": { + "labels": { + "pod-security.kubernetes.io/enforce": "baseline" + }, + "annotations": { + "openshift.io/description": "Operator namespace" + } + } + }`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "pod-security.kubernetes.io/enforce": "baseline", + }, + Annotations: map[string]string{ + "openshift.io/description": "Operator namespace", + }, + }, + }, + expectError: false, + }, + { + name: "invalid JSON", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata": invalid json}`, + }, + expected: nil, + 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) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +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", + "pod-security.kubernetes.io/audit": "restricted", + "pod-security.kubernetes.io/warn": "restricted", + }, + }, + }, + validate: func(t *testing.T, obj map[string]interface{}) { + assert.Equal(t, "v1", obj["apiVersion"]) + assert.Equal(t, "Namespace", obj["kind"]) + + metadata, ok := obj["metadata"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "my-ns", metadata["name"]) + + labels, ok := metadata["labels"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "restricted", labels["pod-security.kubernetes.io/enforce"]) + assert.Equal(t, "restricted", labels["pod-security.kubernetes.io/audit"]) + assert.Equal(t, "restricted", labels["pod-security.kubernetes.io/warn"]) + }, + }, + { + name: "nil template", + nsName: "my-ns", + template: nil, + validate: func(t *testing.T, obj map[string]interface{}) { + assert.Equal(t, "v1", obj["apiVersion"]) + assert.Equal(t, "Namespace", obj["kind"]) + + metadata, ok := obj["metadata"].(map[string]interface{}) + require.True(t, ok) + 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, ok := obj["metadata"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "override", metadata["name"]) + }, + }, + { + name: "with template annotations", + nsName: "my-ns", + template: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "openshift.io/description": "Operator namespace", + "openshift.io/display-name": "My Operator", + }, + }, + }, + validate: func(t *testing.T, obj map[string]interface{}) { + assert.Equal(t, "v1", obj["apiVersion"]) + assert.Equal(t, "Namespace", obj["kind"]) + + metadata, ok := obj["metadata"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "my-ns", metadata["name"]) + + annotations, ok := metadata["annotations"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "Operator namespace", annotations["openshift.io/description"]) + assert.Equal(t, "My Operator", annotations["openshift.io/display-name"]) + }, + }, + } + + 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.Object) + }) + } +} + +func TestResolveNamespaceName(t *testing.T) { + tests := []struct { + name string + csvAnnotations map[string]string + packageName string + wantName string + wantTemplate bool + }{ + { + name: "suggested-namespace-template with name", + csvAnnotations: 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", + csvAnnotations: map[string]string{ + AnnotationSuggestedNamespace: "my-custom-ns", + }, + packageName: "my-operator", + wantName: "my-custom-ns", + wantTemplate: false, + }, + { + name: "template takes priority over suggested-namespace", + csvAnnotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"from-template"}}`, + AnnotationSuggestedNamespace: "from-annotation", + }, + packageName: "my-operator", + wantName: "from-template", + wantTemplate: true, + }, + { + name: "fallback to packageName-system", + csvAnnotations: map[string]string{}, + packageName: "my-operator", + wantName: "my-operator-system", + wantTemplate: false, + }, + { + name: "nil annotations fallback", + csvAnnotations: nil, + packageName: "my-operator", + wantName: "my-operator-system", + wantTemplate: false, + }, + { + name: "template without name falls back to suggested-namespace", + csvAnnotations: 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", + csvAnnotations: 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 := ResolveNamespaceName(tt.csvAnnotations, tt.packageName) + require.NoError(t, err) + require.Equal(t, tt.wantName, name) + if tt.wantTemplate { + require.NotNil(t, template) + } else { + require.Nil(t, template) + } + }) + } +} + +func TestResolveNamespaceName_InvalidTemplate(t *testing.T) { + _, _, err := ResolveNamespaceName(map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{invalid json`, + }, "pkg") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse namespace template") +} + +func TestResolveNamespaceName_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: "rejects name starting with hyphen", + annotations: map[string]string{ + AnnotationSuggestedNamespace: "-invalid", + }, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + { + name: "accepts valid fallback name", + annotations: nil, + packageName: "my-package", + expectErr: false, + }, + { + name: "accepts valid suggested-namespace", + annotations: map[string]string{ + AnnotationSuggestedNamespace: "valid-ns-123", + }, + packageName: "pkg", + expectErr: false, + }, + { + 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 := ResolveNamespaceName(tt.annotations, tt.packageName) + if tt.expectErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestBuildNamespaceObject_ReturnsError(t *testing.T) { + // BuildNamespaceObject with valid input should not error + _, err := BuildNamespaceObject("valid-ns", nil) + require.NoError(t, err) + + // With template + template := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"key": "value"}, + }, + } + obj, err := BuildNamespaceObject("valid-ns", template) + require.NoError(t, err) + require.Equal(t, "valid-ns", obj.GetName()) + require.Equal(t, "value", obj.GetLabels()["key"]) +} diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index e82d17ba46..6fa9a0f16b 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -187,7 +187,7 @@ func extensionConfigBytes(ext *ocv1.ClusterExtension) []byte { return nil } -func getBundleAnnotations(bundleFS fs.FS) (map[string]string, error) { +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/controllers/boxcutter_reconcile_steps.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go index f340520fc7..00439ebc9d 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" ) @@ -100,7 +101,7 @@ func MigrateStorage(m StorageMigrator) ReconcileStepFunc { } } -func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (bool, string, error)) ReconcileStepFunc { +func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (bool, string, error)) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) revisionAnnotations := map[string]string{ @@ -118,7 +119,11 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e } l.Info("applying bundle contents") - _, _, err := apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations) + _, _, err := apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations, &applier.NamespaceConfig{ + Name: state.resolvedNamespace, + Managed: state.namespaceManaged, + Template: state.namespaceTemplate, + }) if err != nil { // If there was an error applying the resolved bundle, // report the error via the Progressing condition. @@ -132,6 +137,7 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e return nil, err } + ext.Status.Namespace = state.resolvedNamespace ext.Status.ActiveRevisions = []ocv1.RevisionStatus{} // Mirror Available/Progressing conditions from the installed revision if i := state.revisionStates.Installed; i != nil { diff --git a/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go b/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go index 78adb70090..45eafc0495 100644 --- a/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go +++ b/internal/operator-controller/controllers/boxcutter_reconcile_steps_apply_test.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/applier" ) func TestApplyBundleWithBoxcutter(t *testing.T) { @@ -133,7 +134,7 @@ func TestApplyBundleWithBoxcutter(t *testing.T) { imageFS: fstest.MapFS{}, } - stepFunc := ApplyBundleWithBoxcutter(func(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension, _, _ map[string]string) (bool, string, error) { + stepFunc := ApplyBundleWithBoxcutter(func(_ context.Context, _ fs.FS, _ *ocv1.ClusterExtension, _, _ map[string]string, _ *applier.NamespaceConfig) (bool, string, error) { return true, "", nil }) result, err := stepFunc(ctx, state, ext) 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.go b/internal/operator-controller/controllers/clusterextension_controller.go index 0fa5fdee2c..c6e667cc45 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -27,6 +27,7 @@ import ( "github.com/go-logr/logr" "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" @@ -66,6 +67,10 @@ type reconcileState struct { imageFS fs.FS resolvedDeprecation *declcfg.Deprecation hasCatalogData bool + + resolvedNamespace string + namespaceManaged bool + namespaceTemplate *corev1.Namespace } // ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process. diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 2637457752..d8c18c4b73 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,92 @@ func TestValidateClusterExtension(t *testing.T) { } } +func TestResolveNamespace(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.ResolveNamespace(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) + require.Equal(t, metav1.ConditionFalse, progressingCond.Status) + require.Equal(t, ocv1.ReasonBlocked, 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..73eeb38526 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -21,14 +21,18 @@ 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" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" ocv1 "github.com/operator-framework/operator-controller/api/v1" + "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" @@ -402,6 +406,79 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { } } +func ResolveNamespace(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 != "" { + 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) { + termErr := reconcile.TerminalError(fmt.Errorf("namespace %q not found; spec.namespace must reference an existing namespace", ext.Spec.Namespace)) + setStatusProgressing(ext, termErr) + return nil, termErr + } + if err != nil { + return nil, fmt.Errorf("error checking namespace %q: %w", ext.Spec.Namespace, err) + } + state.resolvedNamespace = ext.Spec.Namespace + state.namespaceManaged = false + return nil, nil + } + + // Managed mode: resolve name from bundle annotations. + // If imageFS is nil (fallback path), preserve the previously resolved namespace + // from status to avoid wiping it in later steps. + if state.imageFS == nil { + if ext.Status.Namespace != "" { + state.resolvedNamespace = ext.Status.Namespace + state.namespaceManaged = true + } + return nil, nil + } + + bundleAnnotations, err := applier.GetBundleAnnotations(state.imageFS) + if err != nil { + return nil, fmt.Errorf("error reading bundle annotations: %w", err) + } + + packageName := getPackageName(ext) + resolvedName, template, err := applier.ResolveNamespaceName(bundleAnnotations, packageName) + if err != nil { + termErr := reconcile.TerminalError(fmt.Errorf("error resolving namespace from bundle annotations: %w", err)) + setStatusProgressing(ext, termErr) + return nil, termErr + } + + if ext.Status.Namespace != "" && ext.Status.Namespace != resolvedName { + termErr := reconcile.TerminalError(fmt.Errorf("bundle upgrade changes managed namespace from %q to %q; this is not supported", ext.Status.Namespace, resolvedName)) + setStatusProgressing(ext, termErr) + return nil, termErr + } + + // On first install, verify managed namespace does not already exist. + // On upgrade (status.Namespace already set), skip this check — the namespace was created by us. + if ext.Status.Namespace == "" { + l.V(1).Info("checking managed namespace does not already exist", "namespace", resolvedName) + _, getErr := nsClient.Namespaces().Get(ctx, resolvedName, metav1.GetOptions{}) + if getErr == nil { + termErr := reconcile.TerminalError(fmt.Errorf("managed namespace %q already exists; use spec.namespace to install into an existing namespace", resolvedName)) + setStatusProgressing(ext, termErr) + return nil, termErr + } + if !apierrors.IsNotFound(getErr) { + return nil, fmt.Errorf("error checking namespace %q: %w", resolvedName, getErr) + } + } + + state.resolvedNamespace = resolvedName + state.namespaceManaged = true + state.namespaceTemplate = template + l.Info("resolved managed namespace", "namespace", resolvedName) + 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) @@ -419,6 +496,12 @@ func ApplyBundle(a Applier) ReconcileStepFunc { labels.OwnerNameKey: ext.GetName(), } + if state.namespaceManaged { + termErr := reconcile.TerminalError(fmt.Errorf("managed namespace mode (omitting spec.namespace) requires the BoxcutterRuntime feature gate")) + setStatusProgressing(ext, termErr) + return nil, termErr + } + l.Info("applying bundle contents") // NOTE: We need to be cautious of eating errors here. // We should always return any error that occurs during an @@ -431,6 +514,8 @@ func ApplyBundle(a Applier) ReconcileStepFunc { // The only way to eventually recover from permission errors is to keep retrying). rolloutSucceeded, rolloutStatus, err := a.Apply(ctx, state.imageFS, ext, objLbls, revisionAnnotations) + ext.Status.Namespace = state.resolvedNamespace + // Set installed status if rolloutSucceeded { state.revisionStates = &RevisionStates{Installed: state.resolvedRevisionMetadata} diff --git a/internal/operator-controller/controllers/clusterobjectset_controller.go b/internal/operator-controller/controllers/clusterobjectset_controller.go index e42e3c6144..4c86dc2fe2 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 is already managed by another controller", 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/testutil/mock/applier/mock_applier.go b/internal/testutil/mock/applier/mock_applier.go index 5ff9b4e08c..82d203438e 100644 --- a/internal/testutil/mock/applier/mock_applier.go +++ b/internal/testutil/mock/applier/mock_applier.go @@ -16,6 +16,7 @@ import ( v1 "github.com/operator-framework/operator-controller/api/v1" v10 "github.com/operator-framework/operator-controller/applyconfigurations/api/v1" + applier "github.com/operator-framework/operator-controller/internal/operator-controller/applier" gomock "go.uber.org/mock/gomock" chart "helm.sh/helm/v3/pkg/chart" release "helm.sh/helm/v3/pkg/release" @@ -177,33 +178,33 @@ func (m *MockClusterObjectSetGenerator) EXPECT() *MockClusterObjectSetGeneratorM } // GenerateRevision mocks base method. -func (m *MockClusterObjectSetGenerator) GenerateRevision(ctx context.Context, bundleFS fs.FS, ext *v1.ClusterExtension, objectLabels, revisionAnnotations map[string]string) (*v10.ClusterObjectSetApplyConfiguration, error) { +func (m *MockClusterObjectSetGenerator) GenerateRevision(ctx context.Context, bundleFS fs.FS, ext *v1.ClusterExtension, objectLabels, revisionAnnotations map[string]string, nsConfig *applier.NamespaceConfig) (*v10.ClusterObjectSetApplyConfiguration, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GenerateRevision", ctx, bundleFS, ext, objectLabels, revisionAnnotations) + ret := m.ctrl.Call(m, "GenerateRevision", ctx, bundleFS, ext, objectLabels, revisionAnnotations, nsConfig) ret0, _ := ret[0].(*v10.ClusterObjectSetApplyConfiguration) ret1, _ := ret[1].(error) return ret0, ret1 } // GenerateRevision indicates an expected call of GenerateRevision. -func (mr *MockClusterObjectSetGeneratorMockRecorder) GenerateRevision(ctx, bundleFS, ext, objectLabels, revisionAnnotations any) *gomock.Call { +func (mr *MockClusterObjectSetGeneratorMockRecorder) GenerateRevision(ctx, bundleFS, ext, objectLabels, revisionAnnotations, nsConfig any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateRevision", reflect.TypeOf((*MockClusterObjectSetGenerator)(nil).GenerateRevision), ctx, bundleFS, ext, objectLabels, revisionAnnotations) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateRevision", reflect.TypeOf((*MockClusterObjectSetGenerator)(nil).GenerateRevision), ctx, bundleFS, ext, objectLabels, revisionAnnotations, nsConfig) } // GenerateRevisionFromHelmRelease mocks base method. -func (m *MockClusterObjectSetGenerator) GenerateRevisionFromHelmRelease(ctx context.Context, helmRelease *release.Release, ext *v1.ClusterExtension, objectLabels map[string]string) (*v10.ClusterObjectSetApplyConfiguration, error) { +func (m *MockClusterObjectSetGenerator) GenerateRevisionFromHelmRelease(ctx context.Context, helmRelease *release.Release, ext *v1.ClusterExtension, objectLabels map[string]string, nsConfig *applier.NamespaceConfig) (*v10.ClusterObjectSetApplyConfiguration, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GenerateRevisionFromHelmRelease", ctx, helmRelease, ext, objectLabels) + ret := m.ctrl.Call(m, "GenerateRevisionFromHelmRelease", ctx, helmRelease, ext, objectLabels, nsConfig) ret0, _ := ret[0].(*v10.ClusterObjectSetApplyConfiguration) ret1, _ := ret[1].(error) return ret0, ret1 } // GenerateRevisionFromHelmRelease indicates an expected call of GenerateRevisionFromHelmRelease. -func (mr *MockClusterObjectSetGeneratorMockRecorder) GenerateRevisionFromHelmRelease(ctx, helmRelease, ext, objectLabels any) *gomock.Call { +func (mr *MockClusterObjectSetGeneratorMockRecorder) GenerateRevisionFromHelmRelease(ctx, helmRelease, ext, objectLabels, nsConfig any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateRevisionFromHelmRelease", reflect.TypeOf((*MockClusterObjectSetGenerator)(nil).GenerateRevisionFromHelmRelease), ctx, helmRelease, ext, objectLabels) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateRevisionFromHelmRelease", reflect.TypeOf((*MockClusterObjectSetGenerator)(nil).GenerateRevisionFromHelmRelease), ctx, helmRelease, ext, objectLabels, nsConfig) } // MockManifestProvider is a mock of ManifestProvider interface. diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 6d9346b4ae..e78935e44c 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 is optional. When set, it specifies an existing namespace where + namespace-scoped resources for the extension are applied. The namespace must + already exist on the cluster. When omitted, operator-controller resolves and + creates a managed namespace from bundle metadata. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + The mode (set vs omitted) is locked at creation time and cannot be changed. + When set, the value is immutable. + + 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: @@ -1339,6 +1344,12 @@ spec: required: - bundle type: object + namespace: + description: |- + namespace is the resolved namespace where the extension is installed. + For user-provided namespaces, this mirrors spec.namespace. + For managed namespaces, this shows the name resolved from bundle metadata. + type: string type: object type: object served: true diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index f8c3add53b..a7979475b5 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 is optional. When set, it specifies an existing namespace where + namespace-scoped resources for the extension are applied. The namespace must + already exist on the cluster. When omitted, operator-controller resolves and + creates a managed namespace from bundle metadata. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + The mode (set vs omitted) is locked at creation time and cannot be changed. + When set, the value is immutable. + + 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: @@ -1300,6 +1305,12 @@ spec: required: - bundle type: object + namespace: + description: |- + namespace is the resolved namespace where the extension is installed. + For user-provided namespaces, this mirrors spec.namespace. + For managed namespaces, this shows the name resolved from bundle metadata. + type: string type: object type: object served: true diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 28dca6563d..0dfb098d87 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -723,12 +723,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 is optional. When set, it specifies an existing namespace where + namespace-scoped resources for the extension are applied. The namespace must + already exist on the cluster. When omitted, operator-controller resolves and + creates a managed namespace from bundle metadata. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + The mode (set vs omitted) is locked at creation time and cannot be changed. + When set, the value is immutable. + + 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 +739,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 == '' serviceAccount: description: |- serviceAccount is a deprecated field and is completely ignored. @@ -1059,7 +1065,6 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object status: @@ -1183,6 +1188,12 @@ spec: required: - bundle type: object + namespace: + description: |- + namespace is the resolved namespace where the extension is installed. + For user-provided namespaces, this mirrors spec.namespace. + For managed namespaces, this shows the name resolved from bundle metadata. + type: string type: object type: object served: true diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 71c7677772..fec7e8438f 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -684,12 +684,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 is optional. When set, it specifies an existing namespace where + namespace-scoped resources for the extension are applied. The namespace must + already exist on the cluster. When omitted, operator-controller resolves and + creates a managed namespace from bundle metadata. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + The mode (set vs omitted) is locked at creation time and cannot be changed. + When set, the value is immutable. + + 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 +700,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 == '' serviceAccount: description: |- serviceAccount is a deprecated field and is completely ignored. @@ -1020,7 +1026,6 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object status: @@ -1144,6 +1149,12 @@ spec: required: - bundle type: object + namespace: + description: |- + namespace is the resolved namespace where the extension is installed. + For user-provided namespaces, this mirrors spec.namespace. + For managed namespaces, this shows the name resolved from bundle metadata. + type: string type: object type: object served: true diff --git a/test/e2e/features/namespace.feature b/test/e2e/features/namespace.feature new file mode 100644 index 0000000000..76174c3a27 --- /dev/null +++ b/test/e2e/features/namespace.feature @@ -0,0 +1,61 @@ +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 + + 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,