From 9c32775980127cdef66fad9521d82011405fa793 Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Wed, 16 Sep 2026 14:21:24 -0400 Subject: [PATCH 1/2] feat(applier): support a system-managed install namespace at runtime When spec.namespace is empty the applier stops passing WithSelfManagedInstallNamespace, so the renderer resolves the install namespace from bundle metadata and emits the Namespace object itself. This is gated on BoxcutterRuntime; with the gate off an empty spec.namespace is a terminal configuration error rather than a silent fallback. Signed-off-by: Nader Ziada --- cmd/operator-controller/main.go | 12 +- .../operator-controller/applier/boxcutter.go | 5 + .../applier/boxcutter_test.go | 43 ++++-- .../operator-controller/applier/provider.go | 23 +++- .../applier/provider_test.go | 122 ++++++++++++++++-- .../clusterextension_controller_test.go | 95 ++++++++++++++ .../clusterextension_reconcile_steps.go | 31 +++++ 7 files changed, 301 insertions(+), 30 deletions(-) diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 2fcea83ef0..b6ea5c6823 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -502,11 +502,12 @@ func run() error { certProvider := getCertificateProvider() regv1ManifestProvider := &applier.RegistryV1ManifestProvider{ - BundleRenderer: registryv1.Renderer, - CertificateProvider: certProvider, - IsWebhookSupportEnabled: certProvider != nil, - IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), - IsDeploymentConfigEnabled: features.OperatorControllerFeatureGate.Enabled(features.DeploymentConfig), + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, + IsWebhookSupportEnabled: certProvider != nil, + IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), + IsDeploymentConfigEnabled: features.OperatorControllerFeatureGate.Enabled(features.DeploymentConfig), + IsNamespaceManagementEnabled: features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime), } var cerCfg reconcilerConfigurator if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { @@ -659,6 +660,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl controllers.RetrieveRevisionStates(revisionStatesGetter), controllers.ResolveBundle(c.resolver, c.mgr.GetClient()), controllers.UnpackBundle(c.imagePuller, c.imageCache), + controllers.ValidateInstallNamespace(coreClient), controllers.ApplyBundleWithBoxcutter(appl.Apply), } diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index a52fa21c7e..5914e864c4 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -273,6 +273,11 @@ type boxcutterStorageMigratorClient interface { // Migrate creates a ClusterObjectSet from an existing Helm release if no revisions exist yet. // The migration is idempotent and skipped if revisions already exist or no Helm release is found. func (m *BoxcutterStorageMigrator) Migrate(ctx context.Context, ext *ocv1.ClusterExtension, objectLabels map[string]string) error { + // Managed namespace mode (spec.namespace empty) means this is a new-style extension + // that never had a Helm release, so there's nothing to migrate. + if ext.Spec.Namespace == "" { + return nil + } existingRevisionList := ocv1.ClusterObjectSetList{} if err := m.Client.List(ctx, &existingRevisionList, client.MatchingLabels{ labels.OwnerNameKey: ext.Name, diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index 25963c9a01..d33c6aa55e 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -1141,7 +1141,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 +1214,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 +1269,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 +1342,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 +1425,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 +1482,7 @@ func TestBoxcutterStorageMigrator(t *testing.T) { require.NoError(t, ocv1.AddToScheme(testScheme)) ext := &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, Spec: ocv1.ClusterExtensionSpec{Namespace: "test-namespace"}, } expectedRelease := &release.Release{ Name: "test123", @@ -1579,7 +1579,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 +1626,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) @@ -1650,4 +1650,31 @@ func TestBoxcutterStorageMigrator(t *testing.T) { err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) require.NoError(t, err) }) + + t.Run("skips migration for managed namespace mode (empty spec.namespace)", func(t *testing.T) { + testScheme := runtime.NewScheme() + require.NoError(t, ocv1.AddToScheme(testScheme)) + + ext := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test123"}, + } + ctrl := gomock.NewController(t) + // A managed-namespace extension never had a Helm release, so migration must be a no-op: + // no List, no action client, and no revision generation. No expectations are set, so + // gomock fails the test if any of these are called. + brb := mockapplier.NewMockClusterObjectSetGenerator(ctrl) + mag := newMockActionGetter(ctrl, mockActionGetterConfig{}) + mockClient := mockctrlclient.NewMockClient(ctrl) + + sm := &applier.BoxcutterStorageMigrator{ + RevisionGenerator: brb, + ActionClientGetter: mag, + Client: mockClient, + Scheme: testScheme, + FieldOwner: "test-owner", + } + + err := sm.Migrate(t.Context(), ext, map[string]string{"my-label": "my-value"}) + require.NoError(t, err) + }) } diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index 77343cf9da..d4db0a8570 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -29,11 +29,12 @@ type ManifestProvider interface { // RegistryV1ManifestProvider generates the manifests that should be installed for a registry+v1 bundle // given the user specified configuration given by the ClusterExtension API surface type RegistryV1ManifestProvider struct { - BundleRenderer render.BundleRenderer - CertificateProvider render.CertificateProvider - IsWebhookSupportEnabled bool - IsSingleOwnNamespaceEnabled bool - IsDeploymentConfigEnabled bool + BundleRenderer render.BundleRenderer + CertificateProvider render.CertificateProvider + IsWebhookSupportEnabled bool + IsSingleOwnNamespaceEnabled bool + IsDeploymentConfigEnabled bool + IsNamespaceManagementEnabled bool } func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { @@ -67,9 +68,19 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens return nil, fmt.Errorf("unsupported bundle: bundle must support at least one of [AllNamespaces SingleNamespace OwnNamespace] install modes") } + if ext.Spec.Namespace == "" && !r.IsNamespaceManagementEnabled { + return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("spec.namespace is required unless the BoxcutterRuntime feature gate is enabled")) + } + opts := []render.Option{ render.WithCertificateProvider(r.CertificateProvider), - render.WithSelfManagedInstallNamespace(ext.Spec.Namespace), + } + + // When the user set spec.namespace, render into that caller-managed (already-existing) + // namespace and do not emit a Namespace object. Otherwise the renderer resolves the + // bundle's system-managed namespace and emits the Namespace object for it. + if ext.Spec.Namespace != "" { + opts = append(opts, render.WithSelfManagedInstallNamespace(ext.Spec.Namespace)) } // Always validate inline config when present so that disabled features produce diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index 6fb9760417..d1b26faf54 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -2,6 +2,7 @@ package applier_test import ( "errors" + "io/fs" "testing" "testing/fstest" @@ -139,17 +140,7 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) { provider := applier.RegistryV1ManifestProvider{ BundleRenderer: registryv1.Renderer, } - bundleFS := bundlefs.Builder().WithPackageName("test"). - WithCSV(bundlecsv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). - WithBundleResource("service.yaml", &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Service", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-service", - }, - }).Build() + bundleFS := newAllNamespacesBundleFS(t) ext := &ocv1.ClusterExtension{ Spec: ocv1.ClusterExtensionSpec{ Namespace: "install-namespace", @@ -174,6 +165,115 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) { require.Equal(t, []client.Object{exp}, objs) }) + + t.Run("emits a system-managed Namespace object when spec.namespace is empty", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsNamespaceManagementEnabled: true, + } + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(bundlecsv.Builder(). + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces). + WithAnnotations(map[string]string{ + render.AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"managed-ns","labels":{"pod-security.kubernetes.io/enforce":"privileged"},"annotations":{"example.com/note":"hello"}}}`, + }).Build()). + WithBundleResource("service.yaml", &corev1.Service{ + TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Service"}, + ObjectMeta: metav1.ObjectMeta{Name: "test-service"}, + }).Build() + // No spec.namespace -> system-managed: the renderer resolves the name from + // bundle annotations and emits the Namespace object. + ext := &ocv1.ClusterExtension{} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + require.NotEmpty(t, objs) + + t.Log("by checking the Namespace object is emitted first") + ns := objs[0] + require.Equal(t, "Namespace", ns.GetObjectKind().GroupVersionKind().Kind) + require.Equal(t, "managed-ns", ns.GetName()) + + t.Log("by checking template labels and annotations are applied") + require.Equal(t, "privileged", ns.GetLabels()["pod-security.kubernetes.io/enforce"]) + require.Equal(t, "hello", ns.GetAnnotations()["example.com/note"]) + }) + + t.Run("does not emit a Namespace object when spec.namespace is set", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Namespace: "install-namespace"}} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + for _, o := range objs { + require.NotEqual(t, "Namespace", o.GetObjectKind().GroupVersionKind().Kind, "no Namespace should be emitted when spec.namespace is set") + } + }) +} + +func Test_RegistryV1ManifestProvider_BoxcutterRuntimeGate(t *testing.T) { + t.Run("rejects empty spec.namespace when the BoxcutterRuntime feature gate is disabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsNamespaceManagementEnabled: false, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{} + + _, err := provider.Get(bundleFS, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "spec.namespace is required unless the BoxcutterRuntime feature gate is enabled") + require.ErrorIs(t, err, reconcile.TerminalError(nil), "namespace gate error should be terminal") + }) + + t.Run("allows empty spec.namespace and renders a managed Namespace when the BoxcutterRuntime feature gate is enabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsNamespaceManagementEnabled: true, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + require.Contains(t, collectKinds(objs), "Namespace") + }) + + t.Run("ignores the BoxcutterRuntime feature gate when spec.namespace is set", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + IsNamespaceManagementEnabled: false, + } + bundleFS := newAllNamespacesBundleFS(t) + ext := &ocv1.ClusterExtension{Spec: ocv1.ClusterExtensionSpec{Namespace: "install-namespace"}} + + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + require.NotContains(t, collectKinds(objs), "Namespace") + }) +} + +// newAllNamespacesBundleFS returns a minimal registry+v1 bundle FS that supports the +// AllNamespaces install mode and includes a single Service resource named "test-service". +func newAllNamespacesBundleFS(t *testing.T) fs.FS { + t.Helper() + return bundlefs.Builder().WithPackageName("test"). + WithCSV(bundlecsv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). + WithBundleResource("service.yaml", &corev1.Service{ + TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Service"}, + ObjectMeta: metav1.ObjectMeta{Name: "test-service"}, + }).Build() +} + +func collectKinds(objs []client.Object) []string { + kinds := make([]string, 0, len(objs)) + for _, o := range objs { + kinds = append(kinds, o.GetObjectKind().GroupVersionKind().Kind) + } + return kinds } func Test_RegistryV1ManifestProvider_APIServiceSupport(t *testing.T) { diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 2637457752..5f721adb8e 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,98 @@ func TestValidateClusterExtension(t *testing.T) { } } +func TestValidateInstallNamespace(t *testing.T) { + tests := []struct { + name string + specNamespace string + namespaceObjects []runtime.Object + expectError bool + errorMessageIncludes string + }{ + { + name: "user-provided namespace exists", + specNamespace: "existing-ns", + namespaceObjects: []runtime.Object{ + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "existing-ns"}}, + }, + }, + { + name: "user-provided namespace not found", + specNamespace: "missing-ns", + expectError: true, + errorMessageIncludes: `namespace "missing-ns" not found`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + fakeClient := fake.NewClientset(tt.namespaceObjects...) + + cl := newClient(t) + reconciler := &controllers.ClusterExtensionReconciler{ + Client: cl, + ReconcileSteps: controllers.ReconcileSteps{ + controllers.HandleFinalizers(crfinalizer.NewFinalizers()), + controllers.ValidateInstallNamespace(fakeClient.CoreV1()), + }, + } + + extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} + clusterExtension := &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, + Spec: ocv1.ClusterExtensionSpec{ + Source: ocv1.SourceConfig{ + SourceType: "Catalog", + Catalog: &ocv1.CatalogFilter{PackageName: "test-package"}, + }, + Namespace: tt.specNamespace, + ServiceAccount: ocv1.ServiceAccountReference{ //nolint:staticcheck // deprecated field used in test + Name: "test-sa", + }, + }, + } + require.NoError(t, cl.Create(ctx, clusterExtension)) + + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) + require.Equal(t, ctrl.Result{}, res) + if !tt.expectError { + require.NoError(t, err) + require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) + return + } + + require.Error(t, err) + require.Contains(t, err.Error(), tt.errorMessageIncludes) + + require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) + progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing) + require.NotNil(t, progressingCond) + // A missing namespace is retryable (not terminal): the user can create it and + // the next reconcile succeeds, so Progressing stays True with Reason=Retrying. + require.Equal(t, metav1.ConditionTrue, progressingCond.Status) + require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason) + require.Contains(t, progressingCond.Message, tt.errorMessageIncludes) + require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{})) + }) + } +} + +// The CRD still requires a non-empty spec.namespace, so this case cannot be driven through the +// API server. Call the step directly to cover the system-managed short-circuit. +func TestValidateInstallNamespaceSkipsSystemManaged(t *testing.T) { + fakeClient := fake.NewClientset() + step := controllers.ValidateInstallNamespace(fakeClient.CoreV1()) + + res, err := step(context.Background(), nil, &ocv1.ClusterExtension{ + ObjectMeta: metav1.ObjectMeta{Name: "test-extension"}, + }) + + require.NoError(t, err) + require.Nil(t, res) + require.Empty(t, fakeClient.Actions(), "no namespace lookup should happen for a system-managed namespace") +} + 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..c541202d2d 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -21,8 +21,10 @@ import ( "errors" "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/finalizer" @@ -402,6 +404,35 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc { } } +// ValidateInstallNamespace verifies that a user-provided spec.namespace exists. +// +// A missing namespace is recoverable — the user can create it — so it is surfaced as a retryable +// error rather than a terminal one: the next reconcile succeeds once the namespace exists. +// +// When spec.namespace is omitted the namespace is system-managed and the renderer creates it, so +// there is nothing to check. +func ValidateInstallNamespace(nsClient corev1client.NamespacesGetter) ReconcileStepFunc { + return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { + if ext.Spec.Namespace == "" { + return nil, nil + } + + l := log.FromContext(ctx) + l.V(1).Info("validating user-provided namespace exists", "namespace", ext.Spec.Namespace) + + _, err := nsClient.Namespaces().Get(ctx, ext.Spec.Namespace, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + nsErr := fmt.Errorf("namespace %q not found; spec.namespace must reference an existing namespace", ext.Spec.Namespace) + setStatusProgressing(ext, nsErr) + return nil, nsErr + } + if err != nil { + return nil, fmt.Errorf("error checking namespace %q: %w", ext.Spec.Namespace, err) + } + return nil, nil + } +} + func ApplyBundle(a Applier) ReconcileStepFunc { return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) { l := log.FromContext(ctx) From 2ac3035fd9a58c2dd34b28786c4dfdda5eb949d6 Mon Sep 17 00:00:00 2001 From: Nader Ziada Date: Wed, 16 Sep 2026 16:14:13 -0400 Subject: [PATCH 2/2] feat(api): make spec.namespace optional in the experimental channel On the experimental channel spec.namespace may now be omitted, in which case operator-controller resolves and creates a managed namespace from the bundle's metadata. Whether the field is set or omitted is locked at creation time: it cannot be added, removed, or changed afterwards. The standard channel keeps the existing required and immutable contract. Signed-off-by: Nader Ziada --- Makefile | 9 ++ api/v1/clusterextension_types.go | 30 +++- .../api/v1/clusterextension.go | 2 + .../api/v1/clusterextensionspec.go | 24 +++- docs/api-reference/olmv1-api-reference.md | 40 +++--- docs/draft/concepts/managed-namespaces.md | 53 +++++++ .../namespace-configuration-for-authors.md | 63 ++++++++ hack/tools/crd-generator/main.go | 9 +- hack/tools/crd-generator/main_test.go | 59 ++++++++ ...peratorframework.io_clusterextensions.yaml | 4 +- ...peratorframework.io_clusterextensions.yaml | 4 +- ...peratorframework.io_clusterextensions.yaml | 18 ++- .../clusterextension_admission_test.go | 135 +++++++++++++++++- manifests/experimental-e2e.yaml | 18 ++- manifests/experimental.yaml | 18 ++- test/e2e/features/namespace.feature | 62 ++++++++ test/e2e/steps/steps.go | 54 +++++++ test/internal/catalog/bundle.go | 21 +++ 18 files changed, 572 insertions(+), 51 deletions(-) create mode 100644 docs/draft/concepts/managed-namespaces.md create mode 100644 docs/draft/howto/namespace-configuration-for-authors.md create mode 100644 test/e2e/features/namespace.feature diff --git a/Makefile b/Makefile index e9237b0ac5..888c5b4f14 100644 --- a/Makefile +++ b/Makefile @@ -685,6 +685,15 @@ crd-ref-docs: $(CRD_REF_DOCS) #EXHELP Generate the API Reference Documents. $(CRD_REF_DOCS) --source-path=$(ROOT_DIR)/api/ \ --config=$(API_REFERENCE_DIR)/crd-ref-docs-gen-config.yaml \ --renderer=markdown --output-path=$(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME); + # crd-ref-docs renders doc-comment text verbatim, including internal generator + # directives. The reference covers both channels at once, so label the channel-specific + # description blocks rather than dropping the tags silently -- otherwise a field documented + # per channel reads as self-contradictory. Remaining directives are stripped. + sed -E -e 's##**Standard channel:** #g' \ + -e 's##**Experimental channel:** #g' \ + -e 's#]*>##g' \ + $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME) > $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME).tmp + mv $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME).tmp $(API_REFERENCE_DIR)/$(API_REFERENCE_FILENAME) VENVDIR := $(abspath docs/.venv) diff --git a/api/v1/clusterextension_types.go b/api/v1/clusterextension_types.go index 6f7912ae9b..f0fbfd6e08 100644 --- a/api/v1/clusterextension_types.go +++ b/api/v1/clusterextension_types.go @@ -50,7 +50,7 @@ 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. + // 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. // @@ -59,12 +59,32 @@ type ClusterExtensionSpec struct { // and be no longer than 63 characters. // // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // + // + // It designates the default namespace where namespace-scoped resources for the extension + // are applied to. + // + // namespace is optional. When set, it must reference an existing namespace on the cluster. + // When omitted, operator-controller resolves and creates a managed namespace from the + // bundle's metadata. Whether namespace is set or omitted is fixed at creation time and + // cannot be changed afterwards. + // + // The namespace field follows the DNS label standard as defined in [RFC 1123]. + // It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character, + // and be no longer than 63 characters. + // + // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // + // + // + // + // + // + // // // +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 - Namespace string `json:"namespace"` + Namespace string `json:"namespace,omitzero"` // serviceAccount is a deprecated field and is completely ignored. // OLMv1 is a single-tenant system where users with ClusterExtension write access are @@ -586,6 +606,8 @@ type ClusterExtension struct { metav1.ObjectMeta `json:"metadata,omitempty"` // spec is an optional field that defines the desired state of the ClusterExtension. + // + // // +optional Spec ClusterExtensionSpec `json:"spec,omitempty"` diff --git a/applyconfigurations/api/v1/clusterextension.go b/applyconfigurations/api/v1/clusterextension.go index d195f0fadf..e212bd6a51 100644 --- a/applyconfigurations/api/v1/clusterextension.go +++ b/applyconfigurations/api/v1/clusterextension.go @@ -36,6 +36,8 @@ type ClusterExtensionApplyConfiguration struct { // More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata *metav1.ObjectMetaApplyConfiguration `json:"metadata,omitempty"` // spec is an optional field that defines the desired state of the ClusterExtension. + // + // Spec *ClusterExtensionSpecApplyConfiguration `json:"spec,omitempty"` // status is an optional field that defines the observed state of the ClusterExtension. Status *ClusterExtensionStatusApplyConfiguration `json:"status,omitempty"` diff --git a/applyconfigurations/api/v1/clusterextensionspec.go b/applyconfigurations/api/v1/clusterextensionspec.go index 47d810a74a..8a99964b12 100644 --- a/applyconfigurations/api/v1/clusterextensionspec.go +++ b/applyconfigurations/api/v1/clusterextensionspec.go @@ -23,7 +23,7 @@ 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. + // 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. // @@ -32,6 +32,28 @@ type ClusterExtensionSpecApplyConfiguration struct { // and be no longer than 63 characters. // // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // + // + // It designates the default namespace where namespace-scoped resources for the extension + // are applied to. + // + // namespace is optional. When set, it must reference an existing namespace on the cluster. + // When omitted, operator-controller resolves and creates a managed namespace from the + // bundle's metadata. Whether namespace is set or omitted is fixed at creation time and + // cannot be changed afterwards. + // + // The namespace field follows the DNS label standard as defined in [RFC 1123]. + // It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character, + // and be no longer than 63 characters. + // + // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // + // + // + // + // + // + // Namespace *string `json:"namespace,omitempty"` // serviceAccount is a deprecated field and is completely ignored. // OLMv1 is a single-tenant system where users with ClusterExtension write access are diff --git a/docs/api-reference/olmv1-api-reference.md b/docs/api-reference/olmv1-api-reference.md index 1d686238ca..eec19cb7e7 100644 --- a/docs/api-reference/olmv1-api-reference.md +++ b/docs/api-reference/olmv1-api-reference.md @@ -29,10 +29,10 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `type` _[ProbeType](#probetype)_ | type is a required field which specifies the type of probe to use.
The allowed probe types are "ConditionEqual", "FieldsEqual", and "FieldValue".
When set to "ConditionEqual", the probe checks objects that have reached a condition of specified type and status.
When set to "FieldsEqual", the probe checks that the values found at two provided field paths are matching.
When set to "FieldValue", the probe checks that the value found at the provided field path matches what was specified.
| | Enum: [ConditionEqual FieldsEqual FieldValue]
Required: \{\}
| -| `conditionEqual` _[ConditionEqualProbe](#conditionequalprobe)_ | conditionEqual contains the expected condition type and status.
| | Optional: \{\}
| -| `fieldsEqual` _[FieldsEqualProbe](#fieldsequalprobe)_ | fieldsEqual contains the two field paths whose values are expected to match.
| | Optional: \{\}
| -| `fieldValue` _[FieldValueProbe](#fieldvalueprobe)_ | fieldValue contains the expected field path and value found within.
| | Optional: \{\}
| +| `type` _[ProbeType](#probetype)_ | type is a required field which specifies the type of probe to use.
The allowed probe types are "ConditionEqual", "FieldsEqual", and "FieldValue".
When set to "ConditionEqual", the probe checks objects that have reached a condition of specified type and status.
When set to "FieldsEqual", the probe checks that the values found at two provided field paths are matching.
When set to "FieldValue", the probe checks that the value found at the provided field path matches what was specified.
| | Enum: [ConditionEqual FieldsEqual FieldValue]
Required: \{\}
| +| `conditionEqual` _[ConditionEqualProbe](#conditionequalprobe)_ | conditionEqual contains the expected condition type and status.
| | Optional: \{\}
| +| `fieldsEqual` _[FieldsEqualProbe](#fieldsequalprobe)_ | fieldsEqual contains the two field paths whose values are expected to match.
| | Optional: \{\}
| +| `fieldValue` _[FieldValueProbe](#fieldvalueprobe)_ | fieldValue contains the expected field path and value found within.
| | Optional: \{\}
| #### AvailabilityMode @@ -67,7 +67,7 @@ _Appears in:_ | --- | --- | --- | --- | | `name` _string_ | name is required and follows the DNS subdomain standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters, hyphens (-) or periods (.),
start and end with an alphanumeric character, and be no longer than 253 characters. | | Required: \{\}
| | `version` _string_ | version is required and references the version that this bundle represents.
It follows the semantic versioning standard as defined in https://semver.org/. | | Required: \{\}
| -| `release` _string_ | release is an optional field that identifies a specific release of this bundle's version.
A release represents a re-publication of the same version, typically used to deliver
packaging or metadata changes without changing the version number. When multiple
releases exist for the same version, higher releases are preferred. An unset release
is less preferred than all other release values.
The value consists of dot-separated identifiers, where each identifier is either a
numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
"3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
compared as integers, alphanumeric identifiers are compared lexically, and numeric
identifiers always sort before alphanumeric identifiers.
For bundles with explicit pkg.Release metadata, this field contains that release value.
For registry+v1 bundles lacking an explicit release value, this field contains the release
extracted from version's build metadata (e.g., '2' from '1.0.0+2').
This field is omitted when the bundle's release value is unset.
| | MaxLength: 20
Optional: \{\}
| +| `release` _string_ | release is an optional field that identifies a specific release of this bundle's version.
A release represents a re-publication of the same version, typically used to deliver
packaging or metadata changes without changing the version number. When multiple
releases exist for the same version, higher releases are preferred. An unset release
is less preferred than all other release values.
The value consists of dot-separated identifiers, where each identifier is either a
numeric value (without leading zeros) or an alphanumeric string (e.g., "2", "1.el9",
"3.alpha.1"). Releases are compared identifier by identifier: numeric identifiers are
compared as integers, alphanumeric identifiers are compared lexically, and numeric
identifiers always sort before alphanumeric identifiers.
For bundles with explicit pkg.Release metadata, this field contains that release value.
For registry+v1 bundles lacking an explicit release value, this field contains the release
extracted from version's build metadata (e.g., '2' from '1.0.0+2').
This field is omitted when the bundle's release value is unset.
| | MaxLength: 20
Optional: \{\}
| #### CRDUpgradeSafetyEnforcement @@ -255,7 +255,7 @@ _Appears in:_ | `kind` _string_ | Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | | Optional: \{\}
| | `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | | Optional: \{\}
| | `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. | | Optional: \{\}
| -| `spec` _[ClusterExtensionSpec](#clusterextensionspec)_ | spec is an optional field that defines the desired state of the ClusterExtension. | | Optional: \{\}
| +| `spec` _[ClusterExtensionSpec](#clusterextensionspec)_ | spec is an optional field that defines the desired state of the ClusterExtension.
| | Optional: \{\}
| | `status` _[ClusterExtensionStatus](#clusterextensionstatus)_ | status is an optional field that defines the observed state of the ClusterExtension. | | Optional: \{\}
| @@ -358,12 +358,12 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `namespace` _string_ | namespace specifies a Kubernetes namespace.
It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster.
Some extensions may contain namespace-scoped resources to be applied in other namespaces.
This namespace must exist.
The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
and be no longer than 63 characters.
[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63
Required: \{\}
| +| `namespace` _string_ | namespace specifies a Kubernetes namespace.
**Standard channel:** It designates the default namespace where namespace-scoped resources for the extension are applied to the cluster.
Some extensions may contain namespace-scoped resources to be applied in other namespaces.
This namespace must exist.
The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
and be no longer than 63 characters.
[RFC 1123]: https://tools.ietf.org/html/rfc1123

**Experimental channel:**
It designates the default namespace where namespace-scoped resources for the extension
are applied to.
namespace is optional. When set, it must reference an existing namespace on the cluster.
When omitted, operator-controller resolves and creates a managed namespace from the
bundle's metadata. Whether namespace is set or omitted is fixed at creation time and
cannot be changed afterwards.
The namespace field follows the DNS label standard as defined in [RFC 1123].
It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character,
and be no longer than 63 characters.
[RFC 1123]: https://tools.ietf.org/html/rfc1123





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

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

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

When the ClusterExtension is sourced from a catalog, it surfaces deprecation conditions based on catalog metadata.
These are indications from a package owner to guide users away from a particular package, channel, or bundle:
- BundleDeprecated is True if the installed bundle is marked deprecated, False if not deprecated, or Unknown if no bundle is installed yet or if catalog data is unavailable.
- ChannelDeprecated is True if any requested channel is marked deprecated, False if not deprecated, or Unknown if catalog data is unavailable.
- PackageDeprecated is True if the requested package is marked deprecated, False if not deprecated, or Unknown if catalog data is unavailable.
- Deprecated is a rollup condition that is True when any deprecation exists, False when none exist, or Unknown when catalog data is unavailable. | | Optional: \{\}
| | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | Optional: \{\}
| -| `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | activeRevisions holds a list of currently active (non-archived) ClusterObjectSets,
including both installed and rolling out revisions.
| | Optional: \{\}
| +| `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | activeRevisions holds a list of currently active (non-archived) ClusterObjectSets,
including both installed and rolling out revisions.
| | Optional: \{\}
| @@ -399,8 +399,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `type` _string_ | type sets the expected condition type, i.e. "Ready".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| -| `status` _string_ | status sets the expected condition status.
Allowed values are "True" and "False".
| | Enum: [True False]
Required: \{\}
| +| `type` _string_ | type sets the expected condition type, i.e. "Ready".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `status` _string_ | status sets the expected condition status.
Allowed values are "True" and "False".
| | Enum: [True False]
Required: \{\}
| #### FieldValueProbe @@ -416,8 +416,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `fieldPath` _string_ | fieldPath sets the field path for the field to check, i.e. "status.phase". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| -| `value` _string_ | value sets the expected value found at fieldPath, i.e. "Bound".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `fieldPath` _string_ | fieldPath sets the field path for the field to check, i.e. "status.phase". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `value` _string_ | value sets the expected value found at fieldPath, i.e. "Bound".
| | MaxLength: 200
MinLength: 1
Required: \{\}
| #### FieldsEqualProbe @@ -433,8 +433,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `fieldA` _string_ | fieldA sets the field path for the first field, i.e. "spec.replicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| -| `fieldB` _string_ | fieldB sets the field path for the second field, i.e. "status.readyReplicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `fieldA` _string_ | fieldA sets the field path for the first field, i.e. "spec.replicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| +| `fieldB` _string_ | fieldB sets the field path for the second field, i.e. "status.readyReplicas". The probe will fail
if the path does not exist.
| | MaxLength: 200
MinLength: 1
Required: \{\}
| #### ImageSource @@ -470,9 +470,9 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `type` _[SelectorType](#selectortype)_ | type is a required field which specifies the type of selector to use.
The allowed selector types are "GroupKind" and "Label".
When set to "GroupKind", all objects which match the specified group and kind will be selected.
When set to "Label", all objects which match the specified labels and/or expressions will be selected.
| | Enum: [GroupKind Label]
Required: \{\}
| -| `groupKind` _[GroupKind](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#groupkind-v1-meta)_ | groupKind specifies the group and kind of objects to select.
Required when type is "GroupKind".
Uses the Kubernetes format specified here:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#GroupKind
| | Optional: \{\}
| -| `label` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#labelselector-v1-meta)_ | label is the label selector definition.
Required when type is "Label".
A probe using a Label selector will be executed against every object matching the labels or expressions; you must use care
when using this type of selector. For example, if multiple Kind objects are selected via labels then the probe is
likely to fail because the values of different Kind objects rarely share the same schema.
The LabelSelector field uses the following Kubernetes format:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#LabelSelector
Requires exactly one of matchLabels or matchExpressions.
| | Optional: \{\}
| +| `type` _[SelectorType](#selectortype)_ | type is a required field which specifies the type of selector to use.
The allowed selector types are "GroupKind" and "Label".
When set to "GroupKind", all objects which match the specified group and kind will be selected.
When set to "Label", all objects which match the specified labels and/or expressions will be selected.
| | Enum: [GroupKind Label]
Required: \{\}
| +| `groupKind` _[GroupKind](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#groupkind-v1-meta)_ | groupKind specifies the group and kind of objects to select.
Required when type is "GroupKind".
Uses the Kubernetes format specified here:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#GroupKind
| | Optional: \{\}
| +| `label` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#labelselector-v1-meta)_ | label is the label selector definition.
Required when type is "Label".
A probe using a Label selector will be executed against every object matching the labels or expressions; you must use care
when using this type of selector. For example, if multiple Kind objects are selected via labels then the probe is
likely to fail because the values of different Kind objects rarely share the same schema.
The LabelSelector field uses the following Kubernetes format:
https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#LabelSelector
Requires exactly one of matchLabels or matchExpressions.
| | Optional: \{\}
| diff --git a/docs/draft/concepts/managed-namespaces.md b/docs/draft/concepts/managed-namespaces.md new file mode 100644 index 0000000000..d2bdc1f0bd --- /dev/null +++ b/docs/draft/concepts/managed-namespaces.md @@ -0,0 +1,53 @@ +# Managed Namespaces + +## What is a managed namespace? + +> **Note:** Managed namespaces (omitting `spec.namespace`) are available only in the +> experimental feature set, which enables the `BoxcutterRuntime` feature gate. In the +> standard feature set, `spec.namespace` is required. + +For registry+v1 bundles, when you create a ClusterExtension without specifying `spec.namespace`, operator-controller automatically creates and manages a namespace for the operator. The namespace name comes from the bundle's metadata or defaults to `-system`. + +When you specify `spec.namespace`, the namespace must already exist on the cluster and operator-controller installs into it without managing its lifecycle. + +The mode is locked at creation time: you cannot switch between managed and user-provided after the ClusterExtension is created. + +Managed mode requires the `BoxcutterRuntime` feature gate. Without it, omitting `spec.namespace` results in a terminal error, so you must set `spec.namespace` to an existing namespace instead. + +> **Note:** The behavior described in this document applies to the registry+v1 bundle format. Other bundle formats are likely to handle namespace management differently — for example, by including namespace objects directly in their manifests. This points toward namespace configuration being bundle-format-specific rather than a top-level ClusterExtension concern. + +## Namespace resolution + +For registry+v1 bundles in managed mode, the namespace name is resolved from CSV annotations in this order: + +1. `operatorframework.io/suggested-namespace-template`: the `metadata.name` field from the JSON template +2. `operatorframework.io/suggested-namespace`: a plain string with the preferred name +3. `-system`: convention fallback + +## What belongs in a managed namespace + +- The operator's own workloads (deployments, services, configmaps) +- The operator's RBAC resources (service accounts, roles, role bindings) +- CRDs and webhooks installed by the operator + +## What does NOT belong in a managed namespace + +- User application workloads +- Shared services used by multiple operators +- Persistent data that should survive operator uninstallation + +## Deletion behavior + +Deleting a ClusterExtension with a managed namespace **deletes the entire namespace and everything in it.** If you have created resources in the managed namespace that are not part of the operator, they will be lost. + +If you need the namespace to persist beyond the operator's lifecycle, use `spec.namespace` to point at an existing namespace you manage yourself. + +## PSA labels + +If the bundle declares PSA requirements via `operatorframework.io/suggested-namespace-template`, those labels are applied to the managed namespace automatically. This ensures the namespace has the correct Pod Security Admission level for the operator's workloads without manual configuration. + +## Drift protection + +Managed namespaces are reconciled by the ClusterObjectSet controller. If someone manually modifies or removes labels that the controller owns (e.g., PSA labels from the template), they are automatically restored. + +Labels or annotations added by other actors that don't conflict with controller-owned fields are preserved. diff --git a/docs/draft/howto/namespace-configuration-for-authors.md b/docs/draft/howto/namespace-configuration-for-authors.md new file mode 100644 index 0000000000..ff71370bb9 --- /dev/null +++ b/docs/draft/howto/namespace-configuration-for-authors.md @@ -0,0 +1,63 @@ +# Namespace Configuration for Bundle Authors + +> **Note:** Managed namespaces (omitting `spec.namespace`) are available only in the +> experimental feature set, which enables the `BoxcutterRuntime` feature gate. In the +> standard feature set, `spec.namespace` is required, and the annotations described +> below are not consulted. + +Bundle authors can specify their preferred namespace configuration through CSV annotations. These annotations are used by operator-controller when the cluster admin does not provide an explicit `spec.namespace`, which requires the experimental feature set. + +## Annotations + +### `operatorframework.io/suggested-namespace-template` + +Full namespace template with metadata. Use this when your operator needs specific labels or annotations on its namespace (e.g., PSA labels). + +```yaml +apiVersion: operators.coreos.com/v1alpha1 +kind: ClusterServiceVersion +metadata: + name: my-operator.v1.0.0 + annotations: + operatorframework.io/suggested-namespace-template: | + { + "apiVersion": "v1", + "kind": "Namespace", + "metadata": { + "name": "my-operator-system", + "labels": { + "pod-security.kubernetes.io/enforce": "privileged", + "pod-security.kubernetes.io/audit": "privileged", + "pod-security.kubernetes.io/warn": "privileged" + } + } + } +``` + +### `operatorframework.io/suggested-namespace` + +Simple namespace name without metadata. Use this when you want a specific name but don't need labels or annotations. + +```yaml +annotations: + operatorframework.io/suggested-namespace: my-operator-system +``` + +### No annotation + +If neither annotation is present, operator-controller uses `-system` as the namespace name. + +## Priority + +If both annotations are present, `suggested-namespace-template` takes priority. + +## Guidelines + +- Always include PSA labels if your operator runs privileged containers. +- Use a descriptive, unique namespace name that includes your package name to avoid collisions. +- Do not assume the namespace name will be exactly what you suggest as cluster admins can override it by setting `spec.namespace`. +- The namespace name from the template is used only when `spec.namespace` is omitted. When set, the admin's choice takes precedence and no namespace object is created. + +## Consistency across bundle formats + +The `operatorframework.io/suggested-namespace-template` and `operatorframework.io/suggested-namespace` annotations are the canonical way to declare namespace preferences. Future bundle formats should use the same annotation keys to avoid divergence across the ecosystem. diff --git a/hack/tools/crd-generator/main.go b/hack/tools/crd-generator/main.go index edc254494e..af0181208d 100644 --- a/hack/tools/crd-generator/main.go +++ b/hack/tools/crd-generator/main.go @@ -251,7 +251,10 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche } } - celRe := regexp.MustCompile(validationPrefix + "XValidation:rule=\"([^\"]*)\",message=\"([^\"]*)\">") + // The rule is captured non-greedily so it may itself contain double quotes (e.g. a CEL + // self.matches("...") call); it extends to the real ",message=" delimiter. The message + // remains quote-free. + celRe := regexp.MustCompile(validationPrefix + "XValidation:rule=\"(.*?)\",message=\"([^\"]*)\">") celMatches := celRe.FindAllStringSubmatch(jsonProps.Description, 64) for _, celMatch := range celMatches { if len(celMatch) != 3 { @@ -260,8 +263,8 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche numValid++ jsonProps.XValidations = append(jsonProps.XValidations, apiextensionsv1.ValidationRule{ - Message: celMatch[1], - Rule: celMatch[2], + Rule: celMatch[1], + Message: celMatch[2], }) } optReqRe := regexp.MustCompile(validationPrefix + "(Optional|Required)>") diff --git a/hack/tools/crd-generator/main_test.go b/hack/tools/crd-generator/main_test.go index aebef0b336..48080b777b 100644 --- a/hack/tools/crd-generator/main_test.go +++ b/hack/tools/crd-generator/main_test.go @@ -13,6 +13,65 @@ import ( const controllerToolsVersion = "v0.21.0" +// TestOpconTweaksXValidation verifies that a channel-specific XValidation opcon +// tag maps the captured rule and message into the correct ValidationRule fields. +func TestOpconTweaksXValidation(t *testing.T) { + tests := []struct { + name string + channel string + description string + expectRule string + expectMessage string + expectRuleCount int + }{ + { + name: "experimental xvalidation applied in experimental channel", + channel: ExperimentalChannel, + description: `Field description.` + "\n" + ``, + expectRule: "oldSelf != '' || self == ''", + expectMessage: "mode is locked at creation time", + expectRuleCount: 1, + }, + { + name: "experimental xvalidation ignored in standard channel", + channel: StandardChannel, + description: `Field description.` + "\n" + ``, + expectRuleCount: 0, + }, + { + name: "rule containing double quotes is captured in full", + channel: StandardChannel, + description: `Field description.` + "\n" + ``, + expectRule: `self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")`, + expectMessage: "namespace must be a valid DNS1123 label", + expectRuleCount: 1, + }, + { + name: "quoted rule combined with disjunction is captured in full", + channel: ExperimentalChannel, + description: `Field description.` + "\n" + ``, + expectRule: `self == '' || self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")`, + expectMessage: "namespace must be a valid DNS1123 label", + expectRuleCount: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + jsonProps := apiextensionsv1.JSONSchemaProps{ + Description: tt.description, + Type: "string", + } + out, _ := opconTweaks(tt.channel, "namespace", jsonProps) + require.Len(t, out.XValidations, tt.expectRuleCount) + if tt.expectRuleCount > 0 { + require.Equal(t, tt.expectRule, out.XValidations[0].Rule) + require.Equal(t, tt.expectMessage, out.XValidations[0].Message) + } + }) + } +} + func TestRunGenerator(t *testing.T) { here, err := os.Getwd() require.NoError(t, err) diff --git a/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml b/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml index 73505ecd50..3cdf61c83d 100644 --- a/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml +++ b/hack/tools/crd-generator/testdata/output/experimental/olm.operatorframework.io_clusterextensions.yaml @@ -128,8 +128,8 @@ spec: x-kubernetes-validations: - message: namespace must be a valid DNS1123 label rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - message: self == oldSelf - rule: namespace really is immutable + - message: namespace really is immutable + rule: self == oldSelf serviceAccount: description: |- serviceAccount is a reference to a ServiceAccount used to perform all interactions diff --git a/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml b/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml index 90c33c902a..b724eece53 100644 --- a/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml +++ b/hack/tools/crd-generator/testdata/output/standard/olm.operatorframework.io_clusterextensions.yaml @@ -128,8 +128,8 @@ spec: x-kubernetes-validations: - message: namespace must be a valid DNS1123 label rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - message: self == oldSelf - rule: namespace is immutable + - message: namespace is immutable + rule: self == oldSelf serviceAccount: description: |- serviceAccount is a reference to a ServiceAccount used to perform all interactions diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml index 3082a69946..28d2ec68b0 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 @@ -148,11 +148,16 @@ spec: 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. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + It designates the default namespace where namespace-scoped resources for the extension + are applied to. + + namespace is optional. When set, it must reference an existing namespace on the cluster. + When omitted, operator-controller resolves and creates a managed namespace from the + bundle's metadata. Whether namespace is set or omitted is fixed at creation time and + cannot be changed afterwards. + + The namespace field follows the DNS label standard as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character, and be no longer than 63 characters. @@ -493,9 +498,12 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object + x-kubernetes-validations: + - message: namespace presence is immutable; it cannot be added or removed + after creation + rule: has(oldSelf.namespace) == has(self.namespace) status: description: status is an optional field that defines the observed state of the ClusterExtension. diff --git a/internal/operator-controller/controllers/clusterextension_admission_test.go b/internal/operator-controller/controllers/clusterextension_admission_test.go index 14cfea8fc9..62aef51b60 100644 --- a/internal/operator-controller/controllers/clusterextension_admission_test.go +++ b/internal/operator-controller/controllers/clusterextension_admission_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/require" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" ocv1 "github.com/operator-framework/operator-controller/api/v1" ) @@ -286,7 +288,7 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) { }{ {"just alphanumeric", "justalphanumberic1", ""}, {"hyphen-separated", "hyphenated-name", ""}, - {"no install namespace", "", regexMismatchError}, + {"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 +327,134 @@ 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", + }, + // The typed client omits an empty namespace (omitzero), so the cases below exercise + // a genuinely absent field rather than an empty string. + { + name: "omitted to set - rejected", + initialNS: "", + updatedNS: "my-ns", + expectErr: true, + errContains: "namespace presence is immutable", + }, + { + name: "omitted to omitted - allowed", + initialNS: "", + updatedNS: "", + expectErr: false, + }, + { + name: "set to omitted - rejected", + initialNS: "my-ns", + updatedNS: "", + expectErr: true, + errContains: "namespace presence is immutable", + }, + } + + 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) + } + }) + } +} + +// TestClusterExtensionAdmissionNamespaceEncoding pins down the wire encoding of +// managed-namespace mode. Admission locks whether spec.namespace is present, so an absent field +// and an empty string must not both be creatable — otherwise clients that disagree about which +// one to send would produce extensions the other can never write to. +func TestClusterExtensionAdmissionNamespaceEncoding(t *testing.T) { + newExt := func(spec map[string]any) *unstructured.Unstructured { + spec["source"] = map[string]any{ + "sourceType": "Catalog", + "catalog": map[string]any{"packageName": "package"}, + } + return &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": ocv1.GroupVersion.String(), + "kind": "ClusterExtension", + "metadata": map[string]any{"generateName": "test-extension-"}, + "spec": spec, + }} + } + + t.Parallel() + t.Run("explicit empty namespace is rejected at creation", func(t *testing.T) { + t.Parallel() + cl := newClient(t) + + err := cl.Create(context.Background(), newExt(map[string]any{"namespace": ""})) + require.Error(t, err) + require.Contains(t, err.Error(), "namespace must be a valid DNS1123 label") + }) + + t.Run("typed client can update an extension created with namespace omitted", func(t *testing.T) { + t.Parallel() + cl := newClient(t) + ctx := context.Background() + + ext := newExt(map[string]any{}) + require.NoError(t, cl.Create(ctx, ext)) + + typed := &ocv1.ClusterExtension{} + require.NoError(t, cl.Get(ctx, client.ObjectKey{Name: ext.GetName()}, typed)) + require.Empty(t, typed.Spec.Namespace) + + typed.Labels = map[string]string{"touched": "yes"} + require.NoError(t, cl.Update(ctx, typed)) + }) +} + 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/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index 6d9346b4ae..885d648b4a 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -762,11 +762,16 @@ spec: 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. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + It designates the default namespace where namespace-scoped resources for the extension + are applied to. + + namespace is optional. When set, it must reference an existing namespace on the cluster. + When omitted, operator-controller resolves and creates a managed namespace from the + bundle's metadata. Whether namespace is set or omitted is fixed at creation time and + cannot be changed afterwards. + + The namespace field follows the DNS label standard as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character, and be no longer than 63 characters. @@ -1107,9 +1112,12 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object + x-kubernetes-validations: + - message: namespace presence is immutable; it cannot be added or removed + after creation + rule: has(oldSelf.namespace) == has(self.namespace) status: description: status is an optional field that defines the observed state of the ClusterExtension. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index f8c3add53b..2ceb70b4b5 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -723,11 +723,16 @@ spec: 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. - The namespace field is required, immutable, and follows the DNS label standard as defined in [RFC 1123]. + It designates the default namespace where namespace-scoped resources for the extension + are applied to. + + namespace is optional. When set, it must reference an existing namespace on the cluster. + When omitted, operator-controller resolves and creates a managed namespace from the + bundle's metadata. Whether namespace is set or omitted is fixed at creation time and + cannot be changed afterwards. + + The namespace field follows the DNS label standard as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-), start and end with an alphanumeric character, and be no longer than 63 characters. @@ -1068,9 +1073,12 @@ spec: rule: 'has(self.sourceType) && self.sourceType == ''Catalog'' ? has(self.catalog) : !has(self.catalog)' required: - - namespace - source type: object + x-kubernetes-validations: + - message: namespace presence is immutable; it cannot be added or removed + after creation + rule: has(oldSelf.namespace) == has(self.namespace) status: description: status is an optional field that defines the observed state of the ClusterExtension. diff --git a/test/e2e/features/namespace.feature b/test/e2e/features/namespace.feature new file mode 100644 index 0000000000..6da7e7838e --- /dev/null +++ b/test/e2e/features/namespace.feature @@ -0,0 +1,62 @@ +Feature: Namespace PSA Management + + As an OLM user, when I install an operator that declares PSA requirements + via the suggested-namespace-template CSV annotation, operator-controller + should create a managed namespace with PSA labels applied. + + Background: + Given OLM is available + And an image registry is available + + @BoxcutterRuntime + Scenario: Managed namespace with PSA template applies labels + Given a catalog "test" with packages: + | package | version | channel | replaces | contents | + | test | 1.0.0 | stable | | CRD, Deployment, NSTemplate(privileged) | + When ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + source: + sourceType: Catalog + catalog: + packageName: ${PACKAGE:test} + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": ${CATALOG:test} + """ + Then ClusterExtension is rolled out + And ClusterExtension is available + And namespace "${PACKAGE:test}-system" has labels + | key | value | + | pod-security.kubernetes.io/enforce | privileged | + | pod-security.kubernetes.io/audit | privileged | + | pod-security.kubernetes.io/warn | privileged | + + Scenario: User-provided namespace does not get PSA labels + Given namespace "${TEST_NAMESPACE}" is available + And a catalog "test" with packages: + | package | version | channel | replaces | contents | + | test | 1.0.0 | stable | | CRD, Deployment, ConfigMap | + When ClusterExtension is applied + """ + apiVersion: olm.operatorframework.io/v1 + kind: ClusterExtension + metadata: + name: ${NAME} + spec: + namespace: ${TEST_NAMESPACE} + source: + sourceType: Catalog + catalog: + packageName: ${PACKAGE:test} + selector: + matchLabels: + "olm.operatorframework.io/metadata.name": ${CATALOG:test} + """ + Then ClusterExtension is rolled out + And ClusterExtension is available + And namespace "${TEST_NAMESPACE}" does not have label "pod-security.kubernetes.io/enforce" diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go index 31abf4bc0b..d3edd5169c 100644 --- a/test/e2e/steps/steps.go +++ b/test/e2e/steps/steps.go @@ -185,6 +185,9 @@ func RegisterSteps(sc *godog.ScenarioContext) { sc.Step(`^(?i)catalog "([^"]+)" is labeled with "([^"]+)"$`, CatalogIsLabeledWith) sc.Step(`^(?i)ValidatingAdmissionPolicy "([^"]+)" is active$`, ValidatingAdmissionPolicyIsActive) + sc.Step(`^(?i)namespace "([^"]+)" has labels$`, NamespaceHasLabels) + sc.Step(`^(?i)namespace "([^"]+)" does not have label "([^"]+)"$`, NamespaceDoesNotHaveLabel) + sc.Step(`^(?i)operator "([^"]+)" target namespace is "([^"]+)"$`, OperatorTargetNamespace) sc.Step(`^(?i)Prometheus metrics are returned in the response$`, PrometheusMetricsAreReturned) @@ -1968,6 +1971,10 @@ func parseContents(contents string) ([]catalog.BundleOption, error) { dir := part[len("StaticBundleDir(") : len(part)-1] absDir := filepath.Join(projectRootDir(), dir) opts = append(opts, catalog.StaticBundleDir(absDir)) + case strings.HasPrefix(part, "NSTemplate(") && strings.HasSuffix(part, ")"): + // NSTemplate(privileged) or NSTemplate(baseline) or NSTemplate(restricted) + level := part[len("NSTemplate(") : len(part)-1] + opts = append(opts, catalog.WithNSTemplate(level)) } } return opts, nil @@ -2459,6 +2466,53 @@ func ResourceHasLabels(ctx context.Context, resourceName string, table *godog.Ta return nil } +// NamespaceHasLabels waits for a namespace (cluster-scoped) to have all labels specified in the data table. +func NamespaceHasLabels(ctx context.Context, nsName string, table *godog.Table) error { + sc := scenarioCtx(ctx) + nsName = substituteScenarioVars(nsName, sc) + + expected, err := parseKeyValueTable(table, sc) + if err != nil { + return fmt.Errorf("invalid labels table: %w", err) + } + + waitFor(ctx, func() bool { + out, err := k8sClient(ctx, "get", "namespace", nsName, "-o", "json") + if err != nil { + return false + } + var obj unstructured.Unstructured + if err := json.Unmarshal([]byte(out), &obj); err != nil { + return false + } + if key, got, ok := matchLabels(obj.GetLabels(), expected); !ok { + logger.V(1).Info("Namespace label not yet present or value mismatch", "namespace", nsName, "key", key, "expected", expected[key], "actual", got) + return false + } + return true + }) + return nil +} + +// NamespaceDoesNotHaveLabel verifies a namespace does not have the specified label. +func NamespaceDoesNotHaveLabel(ctx context.Context, nsName string, labelKey string) error { + sc := scenarioCtx(ctx) + nsName = substituteScenarioVars(nsName, sc) + + out, err := k8sClient(ctx, "get", "namespace", nsName, "-o", "json") + if err != nil { + return fmt.Errorf("failed to get namespace %q: %w", nsName, err) + } + var obj unstructured.Unstructured + if err := json.Unmarshal([]byte(out), &obj); err != nil { + return fmt.Errorf("failed to unmarshal namespace: %w", err) + } + if v, ok := obj.GetLabels()[labelKey]; ok { + return fmt.Errorf("namespace %q has unexpected label %s=%s", nsName, labelKey, v) + } + return nil +} + // nestedString traverses a nested map[string]interface{} by the given keys // and returns the leaf value as a string. func nestedString(obj map[string]interface{}, keys ...string) (string, bool) { diff --git a/test/internal/catalog/bundle.go b/test/internal/catalog/bundle.go index 7bb80b5bce..491846c654 100644 --- a/test/internal/catalog/bundle.go +++ b/test/internal/catalog/bundle.go @@ -41,6 +41,7 @@ type bundleConfig struct { largeCRDFieldCount int // if > 0, generate a CRD with this many fields staticBundleDir string // if set, read bundle from this directory (no parameterization) clusterRegistryOverride string // if set, use this host in the FBC image ref instead of the default + csvAnnotations map[string]string } // bundleSpec is the resolved bundle: version + file map ready for crane.Image(). @@ -109,6 +110,22 @@ func WithBundleProperty(propertyType, value string) BundleOption { } } +// WithCSVAnnotation adds an annotation to the bundle's CSV. +func WithCSVAnnotation(key, value string) BundleOption { + return func(c *bundleConfig) { + if c.csvAnnotations == nil { + c.csvAnnotations = make(map[string]string) + } + c.csvAnnotations[key] = value + } +} + +// WithNSTemplate adds a suggested namespace template annotation to the CSV with the specified PSA level. +func WithNSTemplate(psaLevel string) BundleOption { + template := fmt.Sprintf(`{"apiVersion":"v1","kind":"Namespace","metadata":{"labels":{"pod-security.kubernetes.io/enforce":"%s","pod-security.kubernetes.io/audit":"%s","pod-security.kubernetes.io/warn":"%s"}}}`, psaLevel, psaLevel, psaLevel) + return WithCSVAnnotation("operatorframework.io/suggested-namespace-template", template) +} + // BadImage produces a bundle with CRD and deployment but uses "wrong/image" as // the container image, causing ImagePullBackOff at runtime. func BadImage() BundleOption { @@ -164,6 +181,10 @@ func buildBundle(scenarioID, packageName, version string, opts []BundleOption) ( WithName(fmt.Sprintf("%s.v%s", packageName, version)). WithInstallModeSupportFor(installModes...) + if len(cfg.csvAnnotations) > 0 { + csvBuilder = csvBuilder.WithAnnotations(cfg.csvAnnotations) + } + if cfg.hasCRD { csvBuilder = csvBuilder.WithOwnedCRDs(v1alpha1.CRDDescription{ Name: crdName,