diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 70063f1d48..384b16796c 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -4,7 +4,7 @@ import ( "errors" "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" @@ -74,9 +74,6 @@ func (o *Options) apply(opts ...Option) *Options { func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) { var errs []error - if len(o.TargetNamespaces) == 0 { - errs = append(errs, errors.New("at least one target namespace must be specified")) - } if o.UniqueNameGenerator == nil { errs = append(errs, errors.New("unique name generator must be specified")) } @@ -88,9 +85,14 @@ func (o *Options) validate(rv1 *bundle.RegistryV1) (*Options, []error) { type Option func(*Options) +// 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). func WithTargetNamespaces(namespaces ...string) Option { return func(o *Options) { - o.TargetNamespaces = namespaces + if len(namespaces) > 0 { + o.TargetNamespaces = namespaces + } } } @@ -121,7 +123,7 @@ func (r BundleRenderer) Render(rv1 bundle.RegistryV1, installNamespace string, o genOpts, errs := (&Options{ // default options InstallNamespace: installNamespace, - TargetNamespaces: []string{metav1.NamespaceAll}, + TargetNamespaces: defaultTargetNamespacesForBundle(&rv1, installNamespace), UniqueNameGenerator: DefaultUniqueNameGenerator, CertificateProvider: nil, }).apply(opts...).validate(&rv1) @@ -147,31 +149,69 @@ func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) { } func validateTargetNamespaces(rv1 *bundle.RegistryV1, installNamespace string, targetNamespaces []string) error { - supportedInstallModes := sets.New[string]() - for _, im := range rv1.CSV.Spec.InstallModes { - if im.Supported { - supportedInstallModes.Insert(string(im.Type)) - } - } + supportedInstallModes := supportedBundleInstallModes(rv1) set := sets.New[string](targetNamespaces...) switch { - case set.Len() == 0 || (set.Len() == 1 && set.Has("")): - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) { + case set.Len() == 0: + // Note: this function generally expects targetNamespace to contain at least one value set by default + // in case the user does not specify the value. The option to set the targetNamespace is a no-op if it is empty. + // The only case for which a default targetNamespace is undefined is in the case of a bundle that only + // supports SingleNamespace install mode. The if statement here is added to provide a more friendly error + // message than just the generic (at least one target namespace must be specified) which would occur + // in case only the MultiNamespace install mode is supported by the bundle. + // If AllNamespaces mode is supported, the default will be [""] -> watch all namespaces + // If only OwnNamespace is supported, the default will be [install-namespace] -> only watch the install/own namespace + if supportedInstallModes.Len() == 1 && supportedInstallModes.Has(v1alpha1.InstallModeTypeSingleNamespace) { + return errors.New("exactly one target namespace must be specified") + } + return errors.New("at least one target namespace must be specified") + case set.Len() == 1 && set.Has(""): + if supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) { return nil } return fmt.Errorf("supported install modes %v do not support targeting all namespaces", sets.List(supportedInstallModes)) case set.Len() == 1 && !set.Has(""): - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) { + if targetNamespaces[0] == installNamespace { + if !supportedInstallModes.Has(v1alpha1.InstallModeTypeOwnNamespace) { + return fmt.Errorf("supported install modes %v do not support targeting own namespace", sets.List(supportedInstallModes)) + } return nil } - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNamespaces[0] == installNamespace { + if supportedInstallModes.Has(v1alpha1.InstallModeTypeSingleNamespace) { return nil } default: - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") { + if !supportedInstallModes.Has(v1alpha1.InstallModeTypeOwnNamespace) && set.Has(installNamespace) { + return fmt.Errorf("supported install modes %v do not support targeting own namespace", sets.List(supportedInstallModes)) + } + if supportedInstallModes.Has(v1alpha1.InstallModeTypeMultiNamespace) && !set.Has("") { return nil } } - return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces) + return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[v1alpha1.InstallModeType](supportedInstallModes), targetNamespaces) +} + +func defaultTargetNamespacesForBundle(rv1 *bundle.RegistryV1, installNamespace string) []string { + supportedInstallModes := supportedBundleInstallModes(rv1) + + if supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) { + return []string{corev1.NamespaceAll} + } + + if supportedInstallModes.Has(v1alpha1.InstallModeTypeOwnNamespace) { + return []string{installNamespace} + } + + return nil +} + +func supportedBundleInstallModes(rv1 *bundle.RegistryV1) sets.Set[v1alpha1.InstallModeType] { + supportedInstallModes := sets.New[v1alpha1.InstallModeType]() + for _, im := range rv1.CSV.Spec.InstallModes { + if im.Supported { + supportedInstallModes.Insert(im.Type) + } + } + return supportedInstallModes } diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go index 0760c4fa10..0461ea3bea 100644 --- a/internal/operator-controller/rukpak/render/render_test.go +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -70,13 +70,12 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { err error }{ { - name: "rejects empty targetNamespaces", + name: "accepts empty targetNamespaces (because it is ignored)", installNamespace: "install-namespace", csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), opts: []render.Option{ render.WithTargetNamespaces(), }, - err: errors.New("invalid option(s): at least one target namespace must be specified"), }, { name: "rejects nil unique name generator", installNamespace: "install-namespace", @@ -100,7 +99,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { opts: []render.Option{ render.WithTargetNamespaces("install-namespace"), }, - err: errors.New("invalid option(s): invalid target namespaces [install-namespace]: supported install modes [AllNamespaces] do not support target namespaces [install-namespace]"), + err: errors.New("invalid option(s): invalid target namespaces [install-namespace]: supported install modes [AllNamespaces] do not support targeting own namespace"), }, { name: "rejects install out of own namespace if only OwnNamespace install mode is supported", installNamespace: "install-namespace", @@ -160,6 +159,14 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { opts: []render.Option{ render.WithTargetNamespaces("n1", "n2", "n3"), }, + }, { + name: "reject multi namespace render if OwnNamespace install mode is not supported and target namespaces include install namespace", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces("n1", "n2", "n3", "install-namespace"), + }, + err: errors.New("invalid option(s): invalid target namespaces [n1 n2 n3 install-namespace]: supported install modes [MultiNamespace] do not support targeting own namespace"), }, } { t.Run(tc.name, func(t *testing.T) {