Skip to content

Commit b4aeb92

Browse files
perdasilvaPer Goncalves da Silva
perdasilva
and
Per Goncalves da Silva
authored
✨ OPRUN-4113: Compute target namespace defaults (#2178)
* Compute target namespace defaults Signed-off-by: Per Goncalves da Silva <[email protected]> * Update target namespace validation to reject own namespace installation on single namespace mode Signed-off-by: Per Goncalves da Silva <[email protected]> * Update WithTargetNamespace behavior and MultiNamespace validation Signed-off-by: Per Goncalves da Silva <[email protected]> --------- Signed-off-by: Per Goncalves da Silva <[email protected]> Co-authored-by: Per Goncalves da Silva <[email protected]>
1 parent 48b81ed commit b4aeb92

File tree

2 files changed

+68
-21
lines changed

2 files changed

+68
-21
lines changed

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

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"errors"
55
"fmt"
66

7-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
7+
corev1 "k8s.io/api/core/v1"
88
"k8s.io/apimachinery/pkg/util/sets"
99
"sigs.k8s.io/controller-runtime/pkg/client"
1010

@@ -74,9 +74,6 @@ func (o *Options) apply(opts ...Option) *Options {
7474

7575
func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) {
7676
var errs []error
77-
if len(o.TargetNamespaces) == 0 {
78-
errs = append(errs, errors.New("at least one target namespace must be specified"))
79-
}
8077
if o.UniqueNameGenerator == nil {
8178
errs = append(errs, errors.New("unique name generator must be specified"))
8279
}
@@ -88,9 +85,14 @@ func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) {
8885

8986
type Option func(*Options)
9087

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).
9191
func WithTargetNamespaces(namespaces ...string) Option {
9292
return func(o *Options) {
93-
o.TargetNamespaces = namespaces
93+
if len(namespaces) > 0 {
94+
o.TargetNamespaces = namespaces
95+
}
9496
}
9597
}
9698

@@ -121,7 +123,7 @@ func (r BundleRenderer) Render(rv1 bundle.RegistryV1, installNamespace string, o
121123
genOpts, errs := (&Options{
122124
// default options
123125
InstallNamespace: installNamespace,
124-
TargetNamespaces: []string{metav1.NamespaceAll},
126+
TargetNamespaces: defaultTargetNamespacesForBundle(&rv1, installNamespace),
125127
UniqueNameGenerator: DefaultUniqueNameGenerator,
126128
CertificateProvider: nil,
127129
}).apply(opts...).validate(&rv1)
@@ -147,31 +149,69 @@ func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) {
147149
}
148150

149151
func validateTargetNamespaces(rv1 *bundle.RegistryV1, installNamespace string, targetNamespaces []string) error {
150-
supportedInstallModes := sets.New[string]()
151-
for _, im := range rv1.CSV.Spec.InstallModes {
152-
if im.Supported {
153-
supportedInstallModes.Insert(string(im.Type))
154-
}
155-
}
152+
supportedInstallModes := supportedBundleInstallModes(rv1)
156153

157154
set := sets.New[string](targetNamespaces...)
158155
switch {
159-
case set.Len() == 0 || (set.Len() == 1 && set.Has("")):
160-
if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) {
156+
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
165+
if supportedInstallModes.Len() == 1 && supportedInstallModes.Has(v1alpha1.InstallModeTypeSingleNamespace) {
166+
return errors.New("exactly one target namespace must be specified")
167+
}
168+
return errors.New("at least one target namespace must be specified")
169+
case set.Len() == 1 && set.Has(""):
170+
if supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
161171
return nil
162172
}
163173
return fmt.Errorf("supported install modes %v do not support targeting all namespaces", sets.List(supportedInstallModes))
164174
case set.Len() == 1 && !set.Has(""):
165-
if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) {
175+
if targetNamespaces[0] == installNamespace {
176+
if !supportedInstallModes.Has(v1alpha1.InstallModeTypeOwnNamespace) {
177+
return fmt.Errorf("supported install modes %v do not support targeting own namespace", sets.List(supportedInstallModes))
178+
}
166179
return nil
167180
}
168-
if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNamespaces[0] == installNamespace {
181+
if supportedInstallModes.Has(v1alpha1.InstallModeTypeSingleNamespace) {
169182
return nil
170183
}
171184
default:
172-
if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") {
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+
}
188+
if supportedInstallModes.Has(v1alpha1.InstallModeTypeMultiNamespace) && !set.Has("") {
173189
return nil
174190
}
175191
}
176-
return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces)
192+
return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[v1alpha1.InstallModeType](supportedInstallModes), targetNamespaces)
193+
}
194+
195+
func defaultTargetNamespacesForBundle(rv1 *bundle.RegistryV1, installNamespace string) []string {
196+
supportedInstallModes := supportedBundleInstallModes(rv1)
197+
198+
if supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
199+
return []string{corev1.NamespaceAll}
200+
}
201+
202+
if supportedInstallModes.Has(v1alpha1.InstallModeTypeOwnNamespace) {
203+
return []string{installNamespace}
204+
}
205+
206+
return nil
207+
}
208+
209+
func supportedBundleInstallModes(rv1 *bundle.RegistryV1) sets.Set[v1alpha1.InstallModeType] {
210+
supportedInstallModes := sets.New[v1alpha1.InstallModeType]()
211+
for _, im := range rv1.CSV.Spec.InstallModes {
212+
if im.Supported {
213+
supportedInstallModes.Insert(im.Type)
214+
}
215+
}
216+
return supportedInstallModes
177217
}

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,12 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) {
7070
err error
7171
}{
7272
{
73-
name: "rejects empty targetNamespaces",
73+
name: "accepts empty targetNamespaces (because it is ignored)",
7474
installNamespace: "install-namespace",
7575
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)),
7676
opts: []render.Option{
7777
render.WithTargetNamespaces(),
7878
},
79-
err: errors.New("invalid option(s): at least one target namespace must be specified"),
8079
}, {
8180
name: "rejects nil unique name generator",
8281
installNamespace: "install-namespace",
@@ -100,7 +99,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) {
10099
opts: []render.Option{
101100
render.WithTargetNamespaces("install-namespace"),
102101
},
103-
err: errors.New("invalid option(s): invalid target namespaces [install-namespace]: supported install modes [AllNamespaces] do not support target namespaces [install-namespace]"),
102+
err: errors.New("invalid option(s): invalid target namespaces [install-namespace]: supported install modes [AllNamespaces] do not support targeting own namespace"),
104103
}, {
105104
name: "rejects install out of own namespace if only OwnNamespace install mode is supported",
106105
installNamespace: "install-namespace",
@@ -160,6 +159,14 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) {
160159
opts: []render.Option{
161160
render.WithTargetNamespaces("n1", "n2", "n3"),
162161
},
162+
}, {
163+
name: "reject multi namespace render if OwnNamespace install mode is not supported and target namespaces include install namespace",
164+
installNamespace: "install-namespace",
165+
csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace)),
166+
opts: []render.Option{
167+
render.WithTargetNamespaces("n1", "n2", "n3", "install-namespace"),
168+
},
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"),
163170
},
164171
} {
165172
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)