Repository: operator-framework/operator-controller
Pull request #1945 (2025-04-30 07:22:23)

:seedling: Make target namespaces optional
# Description

This PR updates the BundleRenderer's Render method signature by removing the targetNamespaces parameter as a required parameter in favor of making it an option. It also adds additional unit tests around options and fixes an NPE caused by passing nil as the options parameter.

## Reviewer Checklist

- [ ] API Go Documentation
- [ ] Tests: Unit Tests (and E2E Tests, if appropriate)
- [ ] Comprehensive Commit Messages
- [ ] Links to related GitHub Issue(s)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1945?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 66.77%. Comparing base [(`1171691`)](https://app.codecov.io/gh/operator-framework/operator-controller/commit/1171691da36dd01cc3ee38f7e3642a85709f1478?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) to head [(`c603c5f`)](https://app.codecov.io/gh/operator-framework/operator-controller/commit/c603c5fbfa46e46c715a041cdbaea0224203ff27?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).
> Report is 1 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1945      +/-   ##
==========================================
+ Coverage   66.67%   66.77%   +0.09%     
==========================================
  Files          75       75              
  Lines        6326     6335       +9     
==========================================
+ Hits         4218     4230      +12     
+ Misses       1843     1841       -2     
+ Partials      265      264       -1     
```

| [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1945/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | Coverage Δ | |
|---|---|---|
| [e2e](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1945/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `45.21% <71.42%> (+0.08%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1945/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `56.36% <100.00%> (+0.10%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/operator-framework/operator-controller/pull/1945?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go
index 68244d3..1417239 100644
--- a/internal/operator-controller/rukpak/convert/registryv1.go
+++ b/internal/operator-controller/rukpak/convert/registryv1.go
@@ -261,7 +261,7 @@ func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targe
 		return nil, fmt.Errorf("webhookDefinitions are not supported")
 	}
 
-	objs, err := c.BundleRenderer.Render(rv1, installNamespace, targetNamespaces)
+	objs, err := c.BundleRenderer.Render(rv1, installNamespace, render.WithTargetNamespaces(targetNamespaces...))
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go
index af904ae..279320a 100644
--- a/internal/operator-controller/rukpak/render/render.go
+++ b/internal/operator-controller/rukpak/render/render.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 
 	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"
 
@@ -69,19 +70,33 @@ type Options struct {
 
 func (o *Options) apply(opts ...Option) *Options {
 	for _, opt := range opts {
-		opt(o)
+		if opt != nil {
+			opt(o)
+		}
 	}
 	return o
 }
 
 type Option func(*Options)
 
+func WithTargetNamespaces(namespaces ...string) Option {
+	return func(o *Options) {
+		o.TargetNamespaces = namespaces
+	}
+}
+
+func WithUniqueNameGenerator(generator UniqueNameGenerator) Option {
+	return func(o *Options) {
+		o.UniqueNameGenerator = generator
+	}
+}
+
 type BundleRenderer struct {
 	BundleValidator    BundleValidator
 	ResourceGenerators []ResourceGenerator
 }
 
-func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, watchNamespaces []string, opts ...Option) ([]client.Object, error) {
+func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, opts ...Option) ([]client.Object, error) {
 	// validate bundle
 	if err := r.BundleValidator.Validate(&rv1); err != nil {
 		return nil, err
@@ -89,7 +104,7 @@ func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, watchNam
 
 	genOpts := (&Options{
 		InstallNamespace:    installNamespace,
-		TargetNamespaces:    watchNamespaces,
+		TargetNamespaces:    []string{metav1.NamespaceAll},
 		UniqueNameGenerator: DefaultUniqueNameGenerator,
 	}).apply(opts...)
 


Test patch:
diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go
index ede57ff..510a629 100644
--- a/internal/operator-controller/rukpak/render/render_test.go
+++ b/internal/operator-controller/rukpak/render/render_test.go
@@ -29,7 +29,7 @@ func Test_BundleRenderer_ValidatesBundle(t *testing.T) {
 			},
 		},
 	}
-	objs, err := renderer.Render(render.RegistryV1{}, "", nil)
+	objs, err := renderer.Render(render.RegistryV1{}, "")
 	require.Nil(t, objs)
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "this bundle is invalid")
@@ -37,7 +37,7 @@ func Test_BundleRenderer_ValidatesBundle(t *testing.T) {
 
 func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) {
 	expectedInstallNamespace := "install-namespace"
-	expectedTargetNamespaces := []string{"ns-one", "ns-two"}
+	expectedTargetNamespaces := []string{""}
 	expectedUniqueNameGenerator := render.DefaultUniqueNameGenerator
 
 	renderer := render.BundleRenderer{
@@ -51,7 +51,34 @@ func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) {
 		},
 	}
 
-	_, _ = renderer.Render(render.RegistryV1{}, expectedInstallNamespace, expectedTargetNamespaces)
+	_, _ = renderer.Render(render.RegistryV1{}, expectedInstallNamespace)
+}
+
+func Test_BundleRenderer_AppliesUserOptions(t *testing.T) {
+	isOptionApplied := false
+	_, _ = render.BundleRenderer{}.Render(render.RegistryV1{}, "install-namespace", func(options *render.Options) {
+		isOptionApplied = true
+	})
+	require.True(t, isOptionApplied)
+}
+
+func Test_WithTargetNamespaces(t *testing.T) {
+	opts := &render.Options{
+		TargetNamespaces: []string{"target-namespace"},
+	}
+	render.WithTargetNamespaces("a", "b", "c")(opts)
+	require.Equal(t, []string{"a", "b", "c"}, opts.TargetNamespaces)
+}
+
+func Test_WithUniqueNameGenerator(t *testing.T) {
+	opts := &render.Options{
+		UniqueNameGenerator: render.DefaultUniqueNameGenerator,
+	}
+	render.WithUniqueNameGenerator(func(s string, i interface{}) (string, error) {
+		return "a man needs a name", nil
+	})(opts)
+	generatedName, _ := opts.UniqueNameGenerator("", nil)
+	require.Equal(t, "a man needs a name", generatedName)
 }
 
 func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) {
@@ -65,7 +92,7 @@ func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) {
 			},
 		},
 	}
-	objs, err := renderer.Render(render.RegistryV1{}, "", nil)
+	objs, err := renderer.Render(render.RegistryV1{}, "")
 	require.NoError(t, err)
 	require.Equal(t, []client.Object{&corev1.Namespace{}, &corev1.Service{}, &appsv1.Deployment{}}, objs)
 }
@@ -81,7 +108,7 @@ func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) {
 			},
 		},
 	}
-	objs, err := renderer.Render(render.RegistryV1{}, "", nil)
+	objs, err := renderer.Render(render.RegistryV1{}, "")
 	require.Nil(t, objs)
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "generator error")