diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index e82d17ba46..77343cf9da 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -69,6 +69,7 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens opts := []render.Option{ render.WithCertificateProvider(r.CertificateProvider), + render.WithSelfManagedInstallNamespace(ext.Spec.Namespace), } // Always validate inline config when present so that disabled features produce @@ -82,7 +83,7 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens } opts = append(opts, configOpts...) } - return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...) + return r.BundleRenderer.Render(rv1, opts...) } // extractBundleConfigOptions extracts and validates configuration options from a ClusterExtension. diff --git a/internal/operator-controller/rukpak/render/namespace.go b/internal/operator-controller/rukpak/render/namespace.go new file mode 100644 index 0000000000..7747854e20 --- /dev/null +++ b/internal/operator-controller/rukpak/render/namespace.go @@ -0,0 +1,159 @@ +package render + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash" +) + +const ( + // AnnotationSuggestedNamespaceTemplate is a CSV annotation carrying a JSON + // Namespace template whose metadata seeds the system-managed namespace. + AnnotationSuggestedNamespaceTemplate = "operatorframework.io/suggested-namespace-template" + // AnnotationSuggestedNamespace is a CSV annotation carrying the preferred + // namespace name for the operator. + AnnotationSuggestedNamespace = "operatorframework.io/suggested-namespace" +) + +var dns1123LabelRegexp = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + +// resolveSystemManagedNamespace derives the name of the namespace OLM should +// create and manage for a bundle, using the precedence: +// +// suggested-namespace-template name → suggested-namespace → -system +// +// It returns the resolved name and the parsed template (if any) so the caller can +// seed labels/annotations (e.g. PSA) on the emitted Namespace object. +func resolveSystemManagedNamespace(rv1 *bundle.RegistryV1) (string, *corev1.Namespace, error) { + csvAnnotations := rv1.CSV.GetAnnotations() + + template, err := parseNamespaceTemplate(csvAnnotations) + if err != nil { + return "", nil, err + } + + var name string + switch { + case template != nil && template.Name != "": + name = template.Name + case csvAnnotations[AnnotationSuggestedNamespace] != "": + name = csvAnnotations[AnnotationSuggestedNamespace] + default: + // The auto-derived default must always be a valid namespace, even for package names + // with disallowed characters (e.g. dots) or names that are too long. + name = defaultInstallNamespace(rv1.PackageName) + } + + if err := validateNamespaceName(name); err != nil { + return "", nil, err + } + + return name, template, nil +} + +func parseNamespaceTemplate(csvAnnotations map[string]string) (*corev1.Namespace, error) { + templateJSON, exists := csvAnnotations[AnnotationSuggestedNamespaceTemplate] + if !exists || templateJSON == "" { + return nil, nil + } + + var ns corev1.Namespace + if err := json.Unmarshal([]byte(templateJSON), &ns); err != nil { + return nil, fmt.Errorf("failed to parse namespace template: %w", err) + } + + return &ns, nil +} + +const maxNamespaceNameLength = 63 + +func validateNamespaceName(name string) error { + if name == "" { + return fmt.Errorf("resolved namespace name is empty") + } + if len(name) > maxNamespaceNameLength { + return fmt.Errorf("resolved namespace name %q exceeds %d characters", name, maxNamespaceNameLength) + } + if !dns1123LabelRegexp.MatchString(name) { + return fmt.Errorf("resolved namespace name %q is not a valid DNS1123 label", name) + } + return nil +} + +// defaultInstallNamespace derives a deterministic, DNS1123-label-valid namespace name for a +// package when the bundle does not suggest one. It normalizes disallowed characters (e.g. dots) +// and enforces the namespace length limit. Whenever the package name has to be altered to fit — +// by normalization or truncation — a short hash of the original name is appended, so packages +// that would otherwise reduce to the same label keep distinct namespaces. +func defaultInstallNamespace(packageName string) string { + const ( + suffix = "system" + hashLength = 8 + // maxBase is the room left for the sanitized package name in "--system"; + // it is a compile-time constant (47), so the truncation below can never be negative. + maxBase = maxNamespaceNameLength - len(suffix) - hashLength - 2 + ) + + base := sanitizeDNS1123Label(packageName) + + // Fast path: a package name that is already a valid, short label keeps the historical + // "-system" name. Names that sanitization had to alter take the hashed path + // instead, so packages that normalize to the same label (e.g. "foo.bar" and "foo-bar") + // do not both claim one namespace. + if base != "" && base == packageName && len(base)+1+len(suffix) <= maxNamespaceNameLength { + return base + "-" + suffix + } + + // Otherwise keep the name deterministic and collision-resistant: append a short hash of the + // original package name and truncate the base to fit within the length limit. + hash := hashutil.DeepHashObject(packageName)[:hashLength] + if len(base) > maxBase { + base = base[:maxBase] + } + base = strings.Trim(base, "-") + if base == "" { + return hash + "-" + suffix + } + return base + "-" + hash + "-" + suffix +} + +// sanitizeDNS1123Label lowercases s, replaces each run of disallowed characters with a single +// hyphen, and trims leading/trailing hyphens so the result is a valid DNS1123 label (or empty). +func sanitizeDNS1123Label(s string) string { + var b strings.Builder + lastHyphen := false + for _, r := range strings.ToLower(s) { + switch { + case (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9'): + b.WriteRune(r) + lastHyphen = false + case !lastHyphen: + b.WriteByte('-') + lastHyphen = true + } + } + return strings.Trim(b.String(), "-") +} + +// BuildNamespaceObject returns the Namespace object to include in the rendered set, +// seeding the given labels and annotations. +func BuildNamespaceObject(name string, labels, annotations map[string]string) *corev1.Namespace { + return &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + Kind: "Namespace", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + Annotations: annotations, + }, + } +} diff --git a/internal/operator-controller/rukpak/render/namespace_test.go b/internal/operator-controller/rukpak/render/namespace_test.go new file mode 100644 index 0000000000..ac2ad5ec40 --- /dev/null +++ b/internal/operator-controller/rukpak/render/namespace_test.go @@ -0,0 +1,352 @@ +package render + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/testing/bundle/csv" +) + +func rv1WithAnnotations(pkg string, annotations map[string]string) *bundle.RegistryV1 { + return &bundle.RegistryV1{ + PackageName: pkg, + CSV: csv.Builder().WithName("test-csv").WithAnnotations(annotations).Build(), + } +} + +func TestParseNamespaceTemplate(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected *corev1.Namespace + expectError bool + }{ + { + name: "nil annotations", + annotations: nil, + expected: nil, + }, + { + name: "empty map", + annotations: map[string]string{}, + expected: nil, + }, + { + name: "annotation absent", + annotations: map[string]string{"some.other/annotation": "value"}, + expected: nil, + }, + { + name: "empty string value", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: ""}, + expected: nil, + }, + { + name: "valid template with PSA labels", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"labels":{"pod-security.kubernetes.io/enforce":"restricted"}}}`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"pod-security.kubernetes.io/enforce": "restricted"}, + }, + }, + }, + { + name: "valid template with annotations", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"annotations":{"openshift.io/description":"Operator namespace"}}}`, + }, + expected: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"openshift.io/description": "Operator namespace"}, + }, + }, + }, + { + name: "invalid JSON", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata": invalid json}`}, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := parseNamespaceTemplate(tt.annotations) + if tt.expectError { + require.Error(t, err) + assert.Nil(t, result) + return + } + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestResolveSystemManagedNamespace(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + packageName string + wantName string + wantTemplate bool + }{ + { + name: "suggested-namespace-template with name", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"from-template","labels":{"pod-security.kubernetes.io/enforce":"privileged"}}}`}, + packageName: "my-operator", + wantName: "from-template", + wantTemplate: true, + }, + { + name: "suggested-namespace without template", + annotations: map[string]string{AnnotationSuggestedNamespace: "my-custom-ns"}, + packageName: "my-operator", + wantName: "my-custom-ns", + }, + { + name: "template takes priority over suggested-namespace", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"from-template"}}`, + AnnotationSuggestedNamespace: "from-annotation", + }, + packageName: "my-operator", + wantName: "from-template", + wantTemplate: true, + }, + { + name: "fallback to packageName-system", + annotations: map[string]string{}, + packageName: "my-operator", + wantName: "my-operator-system", + }, + { + name: "nil annotations fallback", + annotations: nil, + packageName: "my-operator", + wantName: "my-operator-system", + }, + { + name: "template without name falls back to suggested-namespace", + annotations: map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{"metadata":{"labels":{"foo":"bar"}}}`, + AnnotationSuggestedNamespace: "from-annotation", + }, + packageName: "my-operator", + wantName: "from-annotation", + wantTemplate: true, + }, + { + name: "template without name and no suggested-namespace falls back to convention", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata":{"labels":{"foo":"bar"}}}`}, + packageName: "my-operator", + wantName: "my-operator-system", + wantTemplate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + name, template, err := resolveSystemManagedNamespace(rv1WithAnnotations(tt.packageName, tt.annotations)) + require.NoError(t, err) + require.Equal(t, tt.wantName, name) + if tt.wantTemplate { + require.NotNil(t, template) + } else { + require.Nil(t, template) + } + }) + } +} + +func TestResolveSystemManagedNamespace_InvalidTemplate(t *testing.T) { + _, _, err := resolveSystemManagedNamespace(rv1WithAnnotations("pkg", map[string]string{ + AnnotationSuggestedNamespaceTemplate: `{invalid json`, + })) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse namespace template") +} + +func TestResolveSystemManagedNamespace_Validation(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + packageName string + expectErr bool + errContains string + }{ + { + name: "rejects uppercase characters in suggested-namespace", + annotations: map[string]string{AnnotationSuggestedNamespace: "Invalid-NS"}, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + { + name: "rejects name exceeding 63 characters", + annotations: map[string]string{AnnotationSuggestedNamespace: "a234567890123456789012345678901234567890123456789012345678901234"}, + packageName: "pkg", + expectErr: true, + errContains: "exceeds 63 characters", + }, + { + name: "rejects name with dots", + annotations: map[string]string{AnnotationSuggestedNamespace: "my.namespace"}, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + { + name: "accepts valid fallback name", + annotations: nil, + packageName: "my-package", + }, + { + name: "accepts valid suggested-namespace", + annotations: map[string]string{AnnotationSuggestedNamespace: "valid-ns-123"}, + packageName: "pkg", + }, + { + name: "rejects invalid name from template", + annotations: map[string]string{AnnotationSuggestedNamespaceTemplate: `{"metadata":{"name":"INVALID"}}`}, + packageName: "pkg", + expectErr: true, + errContains: "not a valid DNS1123 label", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := resolveSystemManagedNamespace(rv1WithAnnotations(tt.packageName, tt.annotations)) + if tt.expectErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestBuildNamespaceObject(t *testing.T) { + tests := []struct { + name string + nsName string + labels map[string]string + annotations map[string]string + want *corev1.Namespace + }{ + { + name: "with labels and annotations", + nsName: "my-ns", + labels: map[string]string{"pod-security.kubernetes.io/enforce": "restricted"}, + annotations: map[string]string{"some.io/annotation": "value"}, + want: &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-ns", + Labels: map[string]string{"pod-security.kubernetes.io/enforce": "restricted"}, + Annotations: map[string]string{"some.io/annotation": "value"}, + }, + }, + }, + { + name: "no labels or annotations", + nsName: "my-ns", + want: &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{Kind: "Namespace", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-ns"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, BuildNamespaceObject(tt.nsName, tt.labels, tt.annotations)) + }) + } +} + +func TestDefaultInstallNamespace(t *testing.T) { + tests := []struct { + name string + packageName string + want string // exact expected name; empty means only assert validity + // wantPrefix asserts the stable prefix of a hashed name, for package names that + // sanitization had to alter and whose hash suffix cannot be written out here. + wantPrefix string + }{ + { + name: "valid short name keeps -system", + packageName: "argocd-operator", + want: "argocd-operator-system", + }, + { + name: "dotted package name is normalized and hashed", + packageName: "my.operator", + wantPrefix: "my-operator-", + }, + { + name: "uppercase and underscores are normalized and hashed", + packageName: "My_Operator", + wantPrefix: "my-operator-", + }, + { + name: "overlong package name is truncated to a valid label", + packageName: strings.Repeat("a", 80), + // no exact expectation; validated below + }, + { + name: "package name with no valid characters still yields a valid namespace", + packageName: "...", + // no exact expectation; validated below + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := defaultInstallNamespace(tt.packageName) + + // The default must always be a valid, length-bounded namespace name. + require.NoError(t, validateNamespaceName(got)) + + // It must be deterministic. + require.Equal(t, got, defaultInstallNamespace(tt.packageName)) + + if tt.want != "" { + require.Equal(t, tt.want, got) + } + if tt.wantPrefix != "" { + require.True(t, strings.HasPrefix(got, tt.wantPrefix), "got %q, want prefix %q", got, tt.wantPrefix) + require.NotEqual(t, tt.wantPrefix+"system", got, + "a normalized name must not take the plain -system form, which belongs to the package spelled that way") + } + }) + } + + t.Run("distinct overlong names that share a prefix do not collide", func(t *testing.T) { + a := defaultInstallNamespace(strings.Repeat("a", 70) + "-one") + b := defaultInstallNamespace(strings.Repeat("a", 70) + "-two") + require.NoError(t, validateNamespaceName(a)) + require.NoError(t, validateNamespaceName(b)) + require.NotEqual(t, a, b) + }) + + t.Run("names that normalize to the same label do not collide", func(t *testing.T) { + // "foo.bar" sanitizes to "foo-bar", which is itself a valid package name. Both are + // installable at once, so they must not claim the same namespace. + dotted := defaultInstallNamespace("foo.bar") + hyphenated := defaultInstallNamespace("foo-bar") + require.NoError(t, validateNamespaceName(dotted)) + require.NoError(t, validateNamespaceName(hyphenated)) + require.NotEqual(t, dotted, hyphenated) + require.Equal(t, "foo-bar-system", hyphenated, "an already-valid package name keeps the plain form") + }) +} diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go index 454d4944fd..da21861aa2 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go @@ -56,6 +56,19 @@ var certVolumeConfigs = []certVolumeConfig{ }, } +// BundleInstallNamespaceGenerator emits the install Namespace object for the system-managed +// install namespace (the default), whose name, labels, and annotations are resolved during Render +// setup. When the caller opted into a self-managed install namespace +// (opts.SelfManagedInstallNamespace), that namespace is assumed to already exist and this is a no-op. +func BundleInstallNamespaceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + if opts.SelfManagedInstallNamespace { + return nil, nil + } + return []client.Object{ + render.BuildNamespaceObject(opts.InstallNamespace, opts.InstallNamespaceLabels, opts.InstallNamespaceAnnotations), + }, nil +} + // BundleCSVDeploymentGenerator generates all deployments defined in rv1's cluster service version (CSV). The generated // resource aim to have parity with OLMv0 generated Deployment resources: // - olm.targetNamespaces annotation is set with the opts.TargetNamespace value diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go index 931e4429d3..4dcea4c642 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go @@ -62,6 +62,40 @@ func Test_ResourceGenerators_Errors(t *testing.T) { require.Contains(t, err.Error(), "generator error") } +func Test_BundleInstallNamespaceGenerator(t *testing.T) { + t.Run("is a no-op for a self-managed install namespace", func(t *testing.T) { + objs, err := generators.BundleInstallNamespaceGenerator(&bundle.RegistryV1{}, render.Options{ + InstallNamespace: "install-namespace", + SelfManagedInstallNamespace: true, + }) + require.NoError(t, err) + require.Empty(t, objs) + }) + + t.Run("emits a Namespace object for the system-managed install namespace by default", func(t *testing.T) { + objs, err := generators.BundleInstallNamespaceGenerator(&bundle.RegistryV1{}, render.Options{ + InstallNamespace: "install-namespace", + }) + require.NoError(t, err) + require.Len(t, objs, 1) + require.Equal(t, "install-namespace", objs[0].GetName()) + require.Equal(t, "Namespace", objs[0].GetObjectKind().GroupVersionKind().Kind) + }) + + t.Run("seeds labels and annotations from the template", func(t *testing.T) { + objs, err := generators.BundleInstallNamespaceGenerator(&bundle.RegistryV1{}, render.Options{ + InstallNamespace: "install-namespace", + InstallNamespaceLabels: map[string]string{"pod-security.kubernetes.io/enforce": "privileged"}, + InstallNamespaceAnnotations: map[string]string{"example.com/foo": "bar"}, + }) + require.NoError(t, err) + require.Len(t, objs, 1) + require.Equal(t, "install-namespace", objs[0].GetName()) + require.Equal(t, map[string]string{"pod-security.kubernetes.io/enforce": "privileged"}, objs[0].GetLabels()) + require.Equal(t, map[string]string{"example.com/foo": "bar"}, objs[0].GetAnnotations()) + }) +} + func Test_BundleCSVDeploymentGenerator_Succeeds(t *testing.T) { for _, tc := range []struct { name string diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1.go b/internal/operator-controller/rukpak/render/registryv1/registryv1.go index 87ab11ba43..63cad34f9e 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1.go +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1.go @@ -38,6 +38,7 @@ var ResourceGenerators = []render.ResourceGenerator{ // NOTE: if you update this list, Test_ResourceGeneratorsHasAllGenerators will fail until // you bring the same changes over to that test. This helps ensure all validation rules are executed // while giving us the flexibility to test each generator individually + generators.BundleInstallNamespaceGenerator, generators.BundleCSVServiceAccountGenerator, generators.BundleCSVPermissionsGenerator, generators.BundleCSVClusterPermissionsGenerator, diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go index f84a2305ed..b996bd9007 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go @@ -46,6 +46,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) { func Test_ResourceGeneratorsHasAllGenerators(t *testing.T) { expectedGenerators := []render.ResourceGenerator{ + generators.BundleInstallNamespaceGenerator, generators.BundleCSVServiceAccountGenerator, generators.BundleCSVPermissionsGenerator, generators.BundleCSVClusterPermissionsGenerator, @@ -84,7 +85,7 @@ func Test_Renderer_Success(t *testing.T) { }, } - objs, err := registryv1.Renderer.Render(someBundle, "install-namespace") + objs, err := registryv1.Renderer.Render(someBundle, render.WithSelfManagedInstallNamespace("install-namespace")) t.Log("Check renderer returns objects and no errors") require.NoError(t, err) require.NotEmpty(t, objs) @@ -98,6 +99,38 @@ func Test_Renderer_Success(t *testing.T) { require.Equal(t, "install-namespace", objs[0].GetNamespace()) } +func Test_Renderer_SystemManagedInstallNamespace(t *testing.T) { + someBundle := bundle.RegistryV1{ + PackageName: "my-package", + CSV: csv.Builder(). + WithName("test-bundle"). + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), + Others: []unstructured.Unstructured{ + *ToUnstructuredT(t, &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-service", + }, + }), + }, + } + + objs, err := registryv1.Renderer.Render(someBundle) + require.NoError(t, err) + + t.Log("Check the install namespace defaults to -system and a Namespace object is emitted") + require.Len(t, objs, 2) + require.Equal(t, "Namespace", objs[0].GetObjectKind().GroupVersionKind().Kind) + require.Equal(t, "my-package-system", objs[0].GetName()) + + t.Log("Check namespace-scoped resources are rendered into the defaulted install namespace") + require.Equal(t, "my-service", objs[1].GetName()) + require.Equal(t, "my-package-system", objs[1].GetNamespace()) +} + func Test_Renderer_Failure_UnsupportedKind(t *testing.T) { someBundle := bundle.RegistryV1{ PackageName: "my-package", @@ -117,7 +150,7 @@ func Test_Renderer_Failure_UnsupportedKind(t *testing.T) { }, } - objs, err := registryv1.Renderer.Render(someBundle, "install-namespace") + objs, err := registryv1.Renderer.Render(someBundle, render.WithSelfManagedInstallNamespace("install-namespace")) t.Log("Check renderer returns objects and no errors") require.Error(t, err) require.Contains(t, err.Error(), "unsupported resource") diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 86eb2ff492..1d532a2c6c 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -66,6 +66,17 @@ type Options struct { // DeploymentConfig contains optional customizations to apply to CSV deployments. // If nil, no customizations are applied. DeploymentConfig *config.DeploymentConfig + + // SelfManagedInstallNamespace, when true, means the caller supplies InstallNamespace and + // manages that namespace itself: it is assumed to already exist and no Namespace object is + // rendered. When false (the default), the renderer resolves the bundle's system-managed + // namespace and emits a Namespace object for it (see WithSelfManagedInstallNamespace). + SelfManagedInstallNamespace bool + // InstallNamespaceLabels and InstallNamespaceAnnotations seed metadata (e.g. PSA) on the + // emitted Namespace object. They are defaulted from the bundle's suggested-namespace-template + // annotation during Render setup and only consulted when SelfManagedInstallNamespace is false. + InstallNamespaceLabels map[string]string + InstallNamespaceAnnotations map[string]string } func (o *Options) apply(opts ...Option) *Options { @@ -82,6 +93,9 @@ func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) { if o.UniqueNameGenerator == nil { errs = append(errs, errors.New("unique name generator must be specified")) } + if o.SelfManagedInstallNamespace && o.InstallNamespace == "" { + errs = append(errs, errors.New("self-managed install namespace requires a namespace name")) + } if err := validateTargetNamespaces(rv1, o.InstallNamespace, o.TargetNamespaces); err != nil { errs = append(errs, fmt.Errorf("invalid target namespaces %v: %w", o.TargetNamespaces, err)) } @@ -90,6 +104,17 @@ func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) { type Option func(*Options) +// WithSelfManagedInstallNamespace renders namespace-scoped resources into ns and treats it as +// a caller-managed namespace: it is assumed to already exist and no Namespace object is emitted. +// Without this option the renderer defaults to the bundle's system-managed namespace (resolved +// from CSV annotations, else "-system") and emits a Namespace object for it. +func WithSelfManagedInstallNamespace(ns string) Option { + return func(o *Options) { + o.InstallNamespace = ns + o.SelfManagedInstallNamespace = true + } +} + // WithTargetNamespaces sets the target namespaces to be used when rendering the bundle // The value will only be used if len(namespaces) > 0. Otherwise, the default value for the bundle // derived from its install mode support will be used (if such a value can be defined). @@ -126,31 +151,39 @@ type BundleRenderer struct { ResourceGenerators []ResourceGenerator } -func (r BundleRenderer) Render(rv1 bundle.RegistryV1, installNamespace string, opts ...Option) ([]client.Object, error) { +func (r BundleRenderer) Render(rv1 bundle.RegistryV1, opts ...Option) ([]client.Object, error) { // validate bundle if err := r.BundleValidator.Validate(&rv1); err != nil { return nil, err } - // generate bundle objects - genOpts, errs := (&Options{ + genOpts := (&Options{ // default options - InstallNamespace: installNamespace, TargetNamespaces: defaultTargetNamespacesForBundle(&rv1), UniqueNameGenerator: DefaultUniqueNameGenerator, CertificateProvider: nil, - }).apply(opts...).validate(&rv1) + }).apply(opts...) - if len(errs) > 0 { - return nil, fmt.Errorf("invalid option(s): %w", errors.Join(errs...)) + // Unless the caller opted into a self-managed install namespace, resolve the bundle's + // system-managed namespace (and its Namespace template) from CSV metadata. The template + // seeds labels/annotations on the Namespace object that BundleInstallNamespaceGenerator emits. + if !genOpts.SelfManagedInstallNamespace { + name, template, err := resolveSystemManagedNamespace(&rv1) + if err != nil { + return nil, err + } + genOpts.InstallNamespace = name + if template != nil { + genOpts.InstallNamespaceLabels = template.Labels + genOpts.InstallNamespaceAnnotations = template.Annotations + } } - objs, err := ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts) - if err != nil { - return nil, err + if _, errs := genOpts.validate(&rv1); len(errs) > 0 { + return nil, fmt.Errorf("invalid option(s): %w", errors.Join(errs...)) } - return objs, nil + return ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts) } func DefaultUniqueNameGenerator(base string, o interface{}) string { diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go index fb24b7d3b1..11c1ccff4f 100644 --- a/internal/operator-controller/rukpak/render/render_test.go +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -26,7 +26,7 @@ func Test_BundleRenderer_NoConfig(t *testing.T) { objs, err := renderer.Render( bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, "", nil) + }, render.WithSelfManagedInstallNamespace("install-namespace"), nil) require.NoError(t, err) require.Empty(t, objs) } @@ -39,7 +39,7 @@ func Test_BundleRenderer_ValidatesBundle(t *testing.T) { }, }, } - objs, err := renderer.Render(bundle.RegistryV1{}, "") + objs, err := renderer.Render(bundle.RegistryV1{}, render.WithSelfManagedInstallNamespace("install-namespace")) require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "this bundle is invalid") @@ -61,7 +61,7 @@ func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) { }, } - _, _ = renderer.Render(bundle.RegistryV1{}, expectedInstallNamespace) + _, _ = renderer.Render(bundle.RegistryV1{}, render.WithSelfManagedInstallNamespace(expectedInstallNamespace)) } func Test_BundleRenderer_DefaultTargetNamespaces(t *testing.T) { @@ -160,7 +160,7 @@ func Test_BundleRenderer_DefaultTargetNamespaces(t *testing.T) { CSV: csv.Builder(). WithName("test"). WithInstallModeSupportFor(tc.supportedInstallModes...).Build(), - }, "some-namespace") + }, render.WithSelfManagedInstallNamespace("some-namespace")) if tc.expectedErrMsg != "" { require.Error(t, err) require.Contains(t, err.Error(), tc.expectedErrMsg) @@ -283,8 +283,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { renderer := render.BundleRenderer{} _, err := renderer.Render( bundle.RegistryV1{CSV: tc.csv}, - tc.installNamespace, - tc.opts..., + append([]render.Option{render.WithSelfManagedInstallNamespace(tc.installNamespace)}, tc.opts...)..., ) if tc.err == nil { require.NoError(t, err) @@ -298,7 +297,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { func Test_BundleRenderer_AppliesUserOptions(t *testing.T) { isOptionApplied := false - _, _ = render.BundleRenderer{}.Render(bundle.RegistryV1{}, "install-namespace", func(options *render.Options) { + _, _ = render.BundleRenderer{}.Render(bundle.RegistryV1{}, render.WithSelfManagedInstallNamespace("install-namespace"), func(options *render.Options) { isOptionApplied = true }) require.True(t, isOptionApplied) @@ -345,7 +344,7 @@ func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { objs, err := renderer.Render( bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, "") + }, render.WithSelfManagedInstallNamespace("install-namespace")) require.NoError(t, err) require.Equal(t, []client.Object{&corev1.Namespace{}, &corev1.Service{}, &appsv1.Deployment{}}, objs) } @@ -364,7 +363,7 @@ func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) { objs, err := renderer.Render( bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, "") + }, render.WithSelfManagedInstallNamespace("install-namespace")) require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "generator error") @@ -408,7 +407,7 @@ func Test_WithDeploymentConfig(t *testing.T) { bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), }, - "test-namespace", + render.WithSelfManagedInstallNamespace("test-namespace"), render.WithDeploymentConfig(expectedConfig), ) @@ -431,7 +430,7 @@ func Test_WithDeploymentConfig(t *testing.T) { bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), }, - "test-namespace", + render.WithSelfManagedInstallNamespace("test-namespace"), ) require.NoError(t, err) @@ -453,7 +452,7 @@ func Test_WithDeploymentConfig(t *testing.T) { bundle.RegistryV1{ CSV: csv.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), }, - "test-namespace", + render.WithSelfManagedInstallNamespace("test-namespace"), render.WithDeploymentConfig(nil), ) diff --git a/test/regression/convert/generate-manifests.go b/test/regression/convert/generate-manifests.go index a3e3197e6e..5c52616be3 100644 --- a/test/regression/convert/generate-manifests.go +++ b/test/regression/convert/generate-manifests.go @@ -275,11 +275,14 @@ func generateManifests(outputPath, bundleDir, installNamespace, watchNamespace s } // Convert RegistryV1 to plain manifests - opts := []render.Option{render.WithTargetNamespaces(watchNamespace)} + opts := []render.Option{ + render.WithSelfManagedInstallNamespace(installNamespace), + render.WithTargetNamespaces(watchNamespace), + } if deploymentConfig != nil { opts = append(opts, render.WithDeploymentConfig(deploymentConfig)) } - objs, err := registryv1.Renderer.Render(regv1, installNamespace, opts...) + objs, err := registryv1.Renderer.Render(regv1, opts...) if err != nil { return fmt.Errorf("error converting registry+v1 bundle: %w", err) }