Skip to content

Commit 01b28a8

Browse files
author
Per Goncalves da Silva
committed
Update WithTargetNamespace behavior and MultiNamespace validation
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent 4fc183a commit 01b28a8

File tree

2 files changed

+36
-134
lines changed

2 files changed

+36
-134
lines changed

‎internal/operator-controller/rukpak/render/render.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,14 @@ func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) {
8585

8686
type Option func(*Options)
8787

88+
// WithTargetNamespaces sets the target namespaces to be used when rendering the bundle
89+
// The value will only be used if len(namespaces) > 0. Otherwise, the default value for the bundle
90+
// derived from its install mode support will be used (if such a value can be defined).
8891
func WithTargetNamespaces(namespaces ...string) Option {
8992
return func(o *Options) {
90-
o.TargetNamespaces = namespaces
93+
if len(namespaces) > 0 {
94+
o.TargetNamespaces = namespaces
95+
}
9196
}
9297
}
9398

@@ -149,6 +154,14 @@ func validateTargetNamespaces(rv1 *bundle.RegistryV1, installNamespace string, t
149154
set := sets.New[string](targetNamespaces...)
150155
switch {
151156
case set.Len() == 0:
157+
// Note: this function generally expects targetNamespace to contain at least one value set by default
158+
// in case the user does not specify the value. The option to set the targetNamespace is a no-op if it is empty.
159+
// The only case for which a default targetNamespace is undefined is in the case of a bundle that only
160+
// supports SingleNamespace install mode. The if statement here is added to provide a more friendly error
161+
// message than just the generic (at least one target namespace must be specified) which would occur
162+
// in case only the MultiNamespace install mode is supported by the bundle.
163+
// If AllNamespaces mode is supported, the default will be [""] -> watch all namespaces
164+
// If only OwnNamespace is supported, the default will be [install-namespace] -> only watch the install/own namespace
152165
if supportedInstallModes.Len() == 1 && supportedInstallModes.Has(v1alpha1.InstallModeTypeSingleNamespace) {
153166
return errors.New("exactly one target namespace must be specified")
154167
}
@@ -169,6 +182,9 @@ func validateTargetNamespaces(rv1 *bundle.RegistryV1, installNamespace string, t
169182
return nil
170183
}
171184
default:
185+
if !supportedInstallModes.Has(v1alpha1.InstallModeTypeOwnNamespace) && set.Has(installNamespace) {
186+
return fmt.Errorf("supported install modes %v do not support targeting own namespace", sets.List(supportedInstallModes))
187+
}
172188
if supportedInstallModes.Has(v1alpha1.InstallModeTypeMultiNamespace) && !set.Has("") {
173189
return nil
174190
}

‎internal/operator-controller/rukpak/render/render_test.go‎

Lines changed: 19 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -42,136 +42,23 @@ func Test_BundleRenderer_ValidatesBundle(t *testing.T) {
4242
require.Contains(t, err.Error(), "this bundle is invalid")
4343
}
4444

45-
func Test_BundleRenderer_CreatesCorrectRenderOptions_WithDefaults(t *testing.T) {
46-
const (
47-
expectedInstallNamespace = "install-namespace"
48-
)
45+
func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) {
46+
expectedInstallNamespace := "install-namespace"
47+
expectedTargetNamespaces := []string{""}
48+
expectedUniqueNameGenerator := render.DefaultUniqueNameGenerator
4949

50-
for _, tc := range []struct {
51-
name string
52-
csv v1alpha1.ClusterServiceVersion
53-
validate func(t *testing.T, opts render.Options)
54-
expectedErrMsgFragment string
55-
}{
56-
{
57-
name: "sets install-namespace correctly",
58-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)),
59-
validate: func(t *testing.T, opts render.Options) {
50+
renderer := render.BundleRenderer{
51+
ResourceGenerators: []render.ResourceGenerator{
52+
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
6053
require.Equal(t, expectedInstallNamespace, opts.InstallNamespace)
54+
require.Equal(t, expectedTargetNamespaces, opts.TargetNamespaces)
55+
require.Equal(t, reflect.ValueOf(expectedUniqueNameGenerator).Pointer(), reflect.ValueOf(render.DefaultUniqueNameGenerator).Pointer(), "options has unexpected default unique name generator")
56+
return nil, nil
6157
},
62-
}, {
63-
name: "uses DefaultUniqueNameGenerator by default",
64-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)),
65-
validate: func(t *testing.T, opts render.Options) {
66-
require.Equal(t, reflect.ValueOf(render.DefaultUniqueNameGenerator).Pointer(), reflect.ValueOf(opts.UniqueNameGenerator).Pointer(), "options has unexpected default unique name generator")
67-
},
68-
}, {
69-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces]",
70-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)),
71-
validate: func(t *testing.T, opts render.Options) {
72-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
73-
},
74-
}, {
75-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, SingleNamespace]",
76-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace)),
77-
validate: func(t *testing.T, opts render.Options) {
78-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
79-
},
80-
}, {
81-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, OwnNamespace]",
82-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace)),
83-
validate: func(t *testing.T, opts render.Options) {
84-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
85-
},
86-
}, {
87-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, MultiNamespace]",
88-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeMultiNamespace)),
89-
validate: func(t *testing.T, opts render.Options) {
90-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
91-
},
92-
}, {
93-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, OwnNamespace, SingleNamespace]",
94-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace)),
95-
validate: func(t *testing.T, opts render.Options) {
96-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
97-
},
98-
}, {
99-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, OwnNamespace, MultiNamespace]",
100-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace)),
101-
validate: func(t *testing.T, opts render.Options) {
102-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
103-
},
104-
}, {
105-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, SingleNamespace, MultiNamespace]",
106-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace)),
107-
validate: func(t *testing.T, opts render.Options) {
108-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
109-
},
110-
}, {
111-
name: "sets target namespaces to [corev1.NamespaceAll] by default if bundle supports install modes [AllNamespaces, SingleNamespace, OwnNamespace, MultiNamespace]",
112-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace)),
113-
validate: func(t *testing.T, opts render.Options) {
114-
require.Equal(t, []string{corev1.NamespaceAll}, opts.TargetNamespaces)
115-
},
116-
}, {
117-
name: "sets target namespaces to [install-namespace] by default if bundle supports install modes [OwnNamespace]",
118-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace)),
119-
validate: func(t *testing.T, opts render.Options) {
120-
require.Equal(t, []string{expectedInstallNamespace}, opts.TargetNamespaces)
121-
},
122-
}, {
123-
name: "sets target namespaces to [install-namespace] by default if bundle supports install modes [OwnNamespace, SingleNamespace]",
124-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace)),
125-
validate: func(t *testing.T, opts render.Options) {
126-
require.Equal(t, []string{expectedInstallNamespace}, opts.TargetNamespaces)
127-
},
128-
}, {
129-
name: "sets target namespaces to [install-namespace] by default if bundle supports install modes [OwnNamespace, MultiNamespace]",
130-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeMultiNamespace)),
131-
validate: func(t *testing.T, opts render.Options) {
132-
require.Equal(t, []string{expectedInstallNamespace}, opts.TargetNamespaces)
133-
},
134-
}, {
135-
name: "sets target namespaces to [install-namespace] by default if bundle supports install modes [OwnNamespace, SingleNamespace, MultiNamespace]",
136-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace, v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace)),
137-
validate: func(t *testing.T, opts render.Options) {
138-
require.Equal(t, []string{expectedInstallNamespace}, opts.TargetNamespaces)
139-
},
140-
}, {
141-
name: "returns error if target namespaces is not set bundle supports install modes [SingleNamespace]",
142-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace)),
143-
expectedErrMsgFragment: "exactly one target namespace must be specified",
144-
}, {
145-
name: "returns error if target namespaces is not set bundle supports install modes [MultiNamespace]",
146-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace)),
147-
expectedErrMsgFragment: "at least one target namespace must be specified",
148-
}, {
149-
name: "returns error if target namespaces is not set bundle supports install modes [SingleNamespace, MultiNamespace]",
150-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace)),
151-
expectedErrMsgFragment: "at least one target namespace must be specified",
15258
},
153-
} {
154-
t.Run(tc.name, func(t *testing.T) {
155-
renderer := render.BundleRenderer{
156-
ResourceGenerators: []render.ResourceGenerator{
157-
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
158-
tc.validate(t, opts)
159-
return nil, nil
160-
},
161-
},
162-
}
163-
164-
_, err := renderer.Render(bundle.RegistryV1{
165-
CSV: tc.csv,
166-
}, expectedInstallNamespace)
167-
if tc.expectedErrMsgFragment == "" {
168-
require.NoError(t, err)
169-
} else {
170-
require.Error(t, err)
171-
require.Contains(t, err.Error(), tc.expectedErrMsgFragment)
172-
}
173-
})
17459
}
60+
61+
_, _ = renderer.Render(bundle.RegistryV1{}, expectedInstallNamespace)
17562
}
17663

17764
func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) {
@@ -183,13 +70,12 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) {
18370
err error
18471
}{
18572
{
186-
name: "rejects empty targetNamespaces",
73+
name: "accepts empty targetNamespaces (because it is ignored)",
18774
installNamespace: "install-namespace",
18875
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)),
18976
opts: []render.Option{
19077
render.WithTargetNamespaces(),
19178
},
192-
err: errors.New("invalid option(s): invalid target namespaces []: at least one target namespace must be specified"),
19379
}, {
19480
name: "rejects nil unique name generator",
19581
installNamespace: "install-namespace",
@@ -267,20 +153,20 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) {
267153
render.WithTargetNamespaces("some-namespace"),
268154
},
269155
}, {
270-
name: "rejects single namespace render if OwnNamespace install mode is unsupported and targets own namespace",
156+
name: "accepts multi namespace render if MultiNamespace install mode is supported",
271157
installNamespace: "install-namespace",
272-
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace)),
158+
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace)),
273159
opts: []render.Option{
274-
render.WithTargetNamespaces("install-namespace"),
160+
render.WithTargetNamespaces("n1", "n2", "n3"),
275161
},
276-
err: errors.New("invalid option(s): invalid target namespaces [install-namespace]: supported install modes [SingleNamespace] do not support targeting own namespace"),
277162
}, {
278-
name: "accepts multi namespace render if MultiNamespace install mode is supported",
163+
name: "reject multi namespace render if OwnNamespace install mode is not supported and target namespaces include install namespace",
279164
installNamespace: "install-namespace",
280165
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace)),
281166
opts: []render.Option{
282-
render.WithTargetNamespaces("n1", "n2", "n3"),
167+
render.WithTargetNamespaces("n1", "n2", "n3", "install-namespace"),
283168
},
169+
err: errors.New("invalid option(s): invalid target namespaces [n1 n2 n3 install-namespace]: supported install modes [MultiNamespace] do not support targeting own namespace"),
284170
},
285171
} {
286172
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)