diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 592d52507..59613489d 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -67,6 +67,9 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/resolve" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/certproviders" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" "github.com/operator-framework/operator-controller/internal/operator-controller/scheme" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" httputil "github.com/operator-framework/operator-controller/internal/shared/util/http" @@ -189,7 +192,7 @@ func run() error { secretParts := strings.Split(cfg.globalPullSecret, "/") if len(secretParts) != 2 { err := fmt.Errorf("incorrect number of components") - setupLog.Error(err, "value of global-pull-secret should be of the format /") + setupLog.Error(err, "Value of global-pull-secret should be of the format /") return err } globalPullSecretKey = &k8stypes.NamespacedName{Name: secretParts[1], Namespace: secretParts[0]} @@ -421,12 +424,25 @@ func run() error { preAuth = authorization.NewRBACPreAuthorizer(mgr.GetClient()) } + // determine if a certificate provider should be set in the bundle renderer and feature support for the provider + // based on the feature flag + var certProvider render.CertificateProvider + var isWebhookSupportEnabled bool + if features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) { + certProvider = certproviders.CertManagerCertificateProvider{} + isWebhookSupportEnabled = true + } + // now initialize the helmApplier, assigning the potentially nil preAuth helmApplier := &applier.Helm{ - ActionClientGetter: acg, - Preflights: preflights, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, - PreAuthorizer: preAuth, + ActionClientGetter: acg, + Preflights: preflights, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{ + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, + IsWebhookSupportEnabled: isWebhookSupportEnabled, + }, + PreAuthorizer: preAuth, } cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()) diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 7691989e6..cc47cc5a3 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -26,6 +26,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) @@ -53,13 +54,15 @@ type Preflight interface { Upgrade(context.Context, *release.Release) error } -type BundleToHelmChartFn func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) +type BundleToHelmChartConverter interface { + ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) +} type Helm struct { - ActionClientGetter helmclient.ActionClientGetter - Preflights []Preflight - PreAuthorizer authorization.PreAuthorizer - BundleToHelmChartFn BundleToHelmChartFn + ActionClientGetter helmclient.ActionClientGetter + Preflights []Preflight + PreAuthorizer authorization.PreAuthorizer + BundleToHelmChartConverter BundleToHelmChartConverter } // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND @@ -199,14 +202,14 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte } func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { - if h.BundleToHelmChartFn == nil { - return nil, errors.New("BundleToHelmChartFn is nil") + if h.BundleToHelmChartConverter == nil { + return nil, errors.New("BundleToHelmChartConverter is nil") } watchNamespace, err := GetWatchNamespace(ext) if err != nil { return nil, err } - return h.BundleToHelmChartFn(bundleFS, ext.Spec.Namespace, watchNamespace) + return h.BundleToHelmChartConverter.ToHelmChart(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace) } func (h *Helm) renderClientOnlyRelease(ctx context.Context, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, error) { diff --git a/internal/operator-controller/applier/helm_test.go b/internal/operator-controller/applier/helm_test.go index bfb7a67a1..66017eafa 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "io" - "io/fs" "os" "testing" "testing/fstest" @@ -27,6 +26,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/features" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" ) @@ -197,8 +197,8 @@ func TestApply_Base(t *testing.T) { t.Run("fails trying to obtain an action client", func(t *testing.T) { mockAcg := &mockActionGetter{actionClientForErr: errors.New("failed getting action client")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -211,8 +211,8 @@ func TestApply_Base(t *testing.T) { t.Run("fails getting current release and !driver.ErrReleaseNotFound", func(t *testing.T) { mockAcg := &mockActionGetter{getClientErr: errors.New("failed getting current release")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -230,8 +230,8 @@ func TestApply_Installation(t *testing.T) { dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -248,9 +248,9 @@ func TestApply_Installation(t *testing.T) { } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -266,8 +266,8 @@ func TestApply_Installation(t *testing.T) { installErr: errors.New("failed installing chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -286,8 +286,8 @@ func TestApply_Installation(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -306,8 +306,8 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -328,10 +328,10 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { } mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + PreAuthorizer: &mockPreAuthorizer{nil, nil}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -350,9 +350,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -379,9 +379,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -408,9 +408,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{nil, nil}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } // Use a ClusterExtension with valid Spec fields. @@ -442,8 +442,8 @@ func TestApply_Upgrade(t *testing.T) { dryRunUpgradeErr: errors.New("failed attempting to dry-run upgrade chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -464,9 +464,9 @@ func TestApply_Upgrade(t *testing.T) { } mockPf := &mockPreflight{upgradeErr: errors.New("failed during upgrade pre-flight check")} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -488,7 +488,7 @@ func TestApply_Upgrade(t *testing.T) { mockPf := &mockPreflight{} helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -509,9 +509,9 @@ func TestApply_Upgrade(t *testing.T) { } mockPf := &mockPreflight{} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -530,8 +530,8 @@ func TestApply_Upgrade(t *testing.T) { desiredRel: &testDesiredRelease, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartFn: convert.RegistryV1ToHelmChart, + ActionClientGetter: mockAcg, + BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -556,9 +556,11 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin Manifest: validManifest, }, }, - BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { - require.Equal(t, expectedWatchNamespace, watchNamespace) - return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace) + BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{ + fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + require.Equal(t, expectedWatchNamespace, watchNamespace) + return nil, nil + }, }, } @@ -587,9 +589,11 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { Manifest: validManifest, }, }, - BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { - require.Equal(t, expectedWatchNamespace, watchNamespace) - return convert.RegistryV1ToHelmChart(rv1, installNamespace, watchNamespace) + BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{ + fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + require.Equal(t, expectedWatchNamespace, watchNamespace) + return nil, nil + }, }, } @@ -605,8 +609,10 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { Manifest: validManifest, }, }, - BundleToHelmChartFn: func(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { - return nil, errors.New("some error") + BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{ + fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return nil, errors.New("some error") + }, }, } @@ -614,3 +620,11 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { require.Error(t, err) }) } + +type fakeBundleToHelmChartConverter struct { + fn func(source.BundleSource, string, string) (*chart.Chart, error) +} + +func (f fakeBundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return f.fn(bundle, installNamespace, watchNamespace) +} diff --git a/internal/operator-controller/rukpak/bundle/registryv1.go b/internal/operator-controller/rukpak/bundle/registryv1.go new file mode 100644 index 000000000..bc757e63d --- /dev/null +++ b/internal/operator-controller/rukpak/bundle/registryv1.go @@ -0,0 +1,15 @@ +package bundle + +import ( + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" +) + +type RegistryV1 struct { + PackageName string + CSV v1alpha1.ClusterServiceVersion + CRDs []apiextensionsv1.CustomResourceDefinition + Others []unstructured.Unstructured +} diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/bundle/source/source.go similarity index 54% rename from internal/operator-controller/rukpak/convert/registryv1.go rename to internal/operator-controller/rukpak/bundle/source/source.go index 7c87b7783..ad8570179 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/bundle/source/source.go @@ -1,76 +1,62 @@ -package convert +package source import ( - "crypto/sha256" "encoding/json" "errors" "fmt" "io/fs" "path/filepath" - "helm.sh/helm/v3/pkg/chart" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/resource" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/alpha/property" - "github.com/operator-framework/operator-controller/internal/operator-controller/features" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" registry "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/operator-registry" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/certproviders" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" ) -type Plain struct { - Objects []client.Object +type BundleSource interface { + GetBundle() (bundle.RegistryV1, error) } -func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { - reg, err := ParseFS(rv1) - if err != nil { - return nil, err - } - - plain, err := PlainConverter.Convert(reg, installNamespace, []string{watchNamespace}) - if err != nil { - return nil, err - } +// identitySource is a bundle source that returns itself +type identitySource bundle.RegistryV1 - chrt := &chart.Chart{Metadata: &chart.Metadata{}} - chrt.Metadata.Annotations = reg.CSV.GetAnnotations() - for _, obj := range plain.Objects { - jsonData, err := json.Marshal(obj) - if err != nil { - return nil, err - } - hash := sha256.Sum256(jsonData) - chrt.Templates = append(chrt.Templates, &chart.File{ - Name: fmt.Sprintf("object-%x.json", hash[0:8]), - Data: jsonData, - }) - } +func (r identitySource) GetBundle() (bundle.RegistryV1, error) { + return bundle.RegistryV1(r), nil +} - return chrt, nil +func FromBundle(rv1 bundle.RegistryV1) BundleSource { + return identitySource(rv1) } -// ParseFS converts the rv1 filesystem into a render.RegistryV1. -// ParseFS expects the filesystem to conform to the registry+v1 format: +// FromFS returns a BundleSource that loads a registry+v1 bundle from a filesystem. +// The filesystem is expected to conform to the registry+v1 format: // metadata/annotations.yaml +// metadata/properties.yaml // manifests/ // - csv.yaml // - ... // -// manifests directory does not contain subdirectories -func ParseFS(rv1 fs.FS) (render.RegistryV1, error) { - reg := render.RegistryV1{} - annotationsFileData, err := fs.ReadFile(rv1, filepath.Join("metadata", "annotations.yaml")) +// manifests directory should not contain subdirectories +func FromFS(fs fs.FS) BundleSource { + return fsBundleSource{ + FS: fs, + } +} + +type fsBundleSource struct { + FS fs.FS +} + +func (f fsBundleSource) GetBundle() (bundle.RegistryV1, error) { + reg := bundle.RegistryV1{} + annotationsFileData, err := fs.ReadFile(f.FS, filepath.Join("metadata", "annotations.yaml")) if err != nil { return reg, err } @@ -82,7 +68,7 @@ func ParseFS(rv1 fs.FS) (render.RegistryV1, error) { const manifestsDir = "manifests" foundCSV := false - if err := fs.WalkDir(rv1, manifestsDir, func(path string, e fs.DirEntry, err error) error { + if err := fs.WalkDir(f.FS, manifestsDir, func(path string, e fs.DirEntry, err error) error { if err != nil { return err } @@ -92,7 +78,7 @@ func ParseFS(rv1 fs.FS) (render.RegistryV1, error) { } return fmt.Errorf("subdirectories are not allowed within the %q directory of the bundle image filesystem: found %q", manifestsDir, path) } - manifestFile, err := rv1.Open(path) + manifestFile, err := f.FS.Open(path) if err != nil { return err } @@ -136,7 +122,7 @@ func ParseFS(rv1 fs.FS) (render.RegistryV1, error) { return reg, fmt.Errorf("no ClusterServiceVersion found in %q", manifestsDir) } - if err := copyMetadataPropertiesToCSV(®.CSV, rv1); err != nil { + if err := copyMetadataPropertiesToCSV(®.CSV, f.FS); err != nil { return reg, err } @@ -190,64 +176,3 @@ func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS csv.Annotations["olm.properties"] = string(allPropertiesJSON) return nil } - -var PlainConverter = Converter{ - BundleRenderer: render.BundleRenderer{ - BundleValidator: validators.RegistryV1BundleValidator, - ResourceGenerators: []render.ResourceGenerator{ - generators.BundleCSVRBACResourceGenerator.ResourceGenerator(), - generators.BundleCRDGenerator, - generators.BundleAdditionalResourcesGenerator, - generators.BundleCSVDeploymentGenerator, - generators.BundleValidatingWebhookResourceGenerator, - generators.BundleMutatingWebhookResourceGenerator, - generators.BundleWebhookServiceResourceGenerator, - generators.CertProviderResourceGenerator, - }, - }, -} - -type Converter struct { - render.BundleRenderer -} - -func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) { - if installNamespace == "" { - installNamespace = rv1.CSV.Annotations["operatorframework.io/suggested-namespace"] - } - if installNamespace == "" { - installNamespace = fmt.Sprintf("%s-system", rv1.PackageName) - } - supportedInstallModes := sets.New[string]() - for _, im := range rv1.CSV.Spec.InstallModes { - if im.Supported { - supportedInstallModes.Insert(string(im.Type)) - } - } - if len(targetNamespaces) == 0 { - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) { - targetNamespaces = []string{""} - } else if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) { - targetNamespaces = []string{installNamespace} - } - } - - if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 { - return nil, fmt.Errorf("apiServiceDefintions are not supported") - } - - if !features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) && len(rv1.CSV.Spec.WebhookDefinitions) > 0 { - return nil, fmt.Errorf("webhookDefinitions are not supported") - } - - objs, err := c.BundleRenderer.Render( - rv1, - installNamespace, - render.WithTargetNamespaces(targetNamespaces...), - render.WithCertificateProvider(certproviders.CertManagerCertificateProvider{}), - ) - if err != nil { - return nil, err - } - return &Plain{Objects: objs}, nil -} diff --git a/internal/operator-controller/rukpak/bundle/source/source_test.go b/internal/operator-controller/rukpak/bundle/source/source_test.go new file mode 100644 index 000000000..2ee948638 --- /dev/null +++ b/internal/operator-controller/rukpak/bundle/source/source_test.go @@ -0,0 +1,111 @@ +package source_test + +import ( + "io/fs" + "strings" + "testing" + "testing/fstest" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" +) + +const ( + olmProperties = "olm.properties" + + bundlePathAnnotations = "metadata/annotations.yaml" + bundlePathProperties = "metadata/properties.yaml" + bundlePathCSV = "manifests/csv.yaml" +) + +func Test_FromBundle_Success(t *testing.T) { + expectedBundle := bundle.RegistryV1{ + PackageName: "my-package", + } + b, err := source.FromBundle(expectedBundle).GetBundle() + require.NoError(t, err) + require.Equal(t, expectedBundle, b) +} + +func Test_FromFS_Success(t *testing.T) { + rv1, err := source.FromFS(newBundleFS()).GetBundle() + require.NoError(t, err) + + t.Log("Check package name is correctly taken from metadata/annotations.yaml") + require.Equal(t, "test", rv1.PackageName) + + t.Log("Check metadata/properties.yaml is merged into csv.annotations[olm.properties]") + require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`, rv1.CSV.Annotations[olmProperties]) +} + +func Test_FromFS_Fails(t *testing.T) { + for _, tt := range []struct { + name string + FS fs.FS + }{ + { + name: "bundle missing ClusterServiceVersion manifest", + FS: removePaths(newBundleFS(), bundlePathCSV), + }, { + name: "bundle missing metadata/annotations.yaml", + FS: removePaths(newBundleFS(), bundlePathAnnotations), + }, { + name: "bundle missing metadata/ directory", + FS: removePaths(newBundleFS(), "metadata/"), + }, { + name: "bundle missing manifests/ directory", + FS: removePaths(newBundleFS(), "manifests/"), + }, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := source.FromFS(tt.FS).GetBundle() + require.Error(t, err) + }) + } +} + +func newBundleFS() fstest.MapFS { + annotationsYml := ` +annotations: + operators.operatorframework.io.bundle.mediatype.v1: registry+v1 + operators.operatorframework.io.bundle.package.v1: test +` + + propertiesYml := ` +properties: + - type: "from-file-key" + value: "from-file-value" +` + + csvYml := ` +apiVersion: operators.operatorframework.io/v1alpha1 +kind: ClusterServiceVersion +metadata: + name: test.v1.0.0 + annotations: + olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]' +spec: + installModes: + - type: AllNamespaces + supported: true +` + + return fstest.MapFS{ + bundlePathAnnotations: &fstest.MapFile{Data: []byte(strings.Trim(annotationsYml, "\n"))}, + bundlePathProperties: &fstest.MapFile{Data: []byte(strings.Trim(propertiesYml, "\n"))}, + bundlePathCSV: &fstest.MapFile{Data: []byte(strings.Trim(csvYml, "\n"))}, + } +} + +func removePaths(mapFs fstest.MapFS, paths ...string) fstest.MapFS { + for k := range mapFs { + for _, path := range paths { + if strings.HasPrefix(k, path) { + delete(mapFs, k) + } + } + } + return mapFs +} diff --git a/internal/operator-controller/rukpak/convert/helm.go b/internal/operator-controller/rukpak/convert/helm.go new file mode 100644 index 000000000..531c10502 --- /dev/null +++ b/internal/operator-controller/rukpak/convert/helm.go @@ -0,0 +1,67 @@ +package convert + +import ( + "crypto/sha256" + "encoding/json" + "fmt" + + "helm.sh/helm/v3/pkg/chart" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" +) + +type BundleToHelmChartConverter struct { + BundleRenderer render.BundleRenderer + CertificateProvider render.CertificateProvider + IsWebhookSupportEnabled bool +} + +func (r *BundleToHelmChartConverter) ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + rv1, err := bundle.GetBundle() + if err != nil { + return nil, err + } + + if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 { + return nil, fmt.Errorf("unsupported bundle: apiServiceDefintions are not supported") + } + + if len(rv1.CSV.Spec.WebhookDefinitions) > 0 { + if !r.IsWebhookSupportEnabled { + return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported") + } else if r.CertificateProvider == nil { + return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported: certificate provider is nil") + } + } + + if r.CertificateProvider == nil && len(rv1.CSV.Spec.WebhookDefinitions) > 0 { + return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported") + } + + objs, err := r.BundleRenderer.Render( + rv1, installNamespace, + render.WithTargetNamespaces(watchNamespace), + render.WithCertificateProvider(r.CertificateProvider), + ) + + if err != nil { + return nil, fmt.Errorf("error rendering bundle: %w", err) + } + + chrt := &chart.Chart{Metadata: &chart.Metadata{}} + chrt.Metadata.Annotations = rv1.CSV.GetAnnotations() + for _, obj := range objs { + jsonData, err := json.Marshal(obj) + if err != nil { + return nil, err + } + hash := sha256.Sum256(jsonData) + chrt.Templates = append(chrt.Templates, &chart.File{ + Name: fmt.Sprintf("object-%x.json", hash[0:8]), + Data: jsonData, + }) + } + + return chrt, nil +} diff --git a/internal/operator-controller/rukpak/convert/helm_test.go b/internal/operator-controller/rukpak/convert/helm_test.go new file mode 100644 index 000000000..fdfa9812b --- /dev/null +++ b/internal/operator-controller/rukpak/convert/helm_test.go @@ -0,0 +1,194 @@ +package convert_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" +) + +func Test_BundleToHelmChartConverter_ToHelmChart_ReturnsBundleSourceFailures(t *testing.T) { + converter := convert.BundleToHelmChartConverter{} + var failingBundleSource FakeBundleSource = func() (bundle.RegistryV1, error) { + return bundle.RegistryV1{}, errors.New("some error") + } + _, err := converter.ToHelmChart(failingBundleSource, "install-namespace", "watch-namespace") + require.Error(t, err) + require.Contains(t, err.Error(), "some error") +} + +func Test_BundleToHelmChartConverter_ToHelmChart_ReturnsBundleRendererFailures(t *testing.T) { + converter := convert.BundleToHelmChartConverter{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + return nil, errors.New("some error") + }, + }, + }, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + }, + ) + + _, err := converter.ToHelmChart(b, "install-namespace", "") + require.Error(t, err) + require.Contains(t, err.Error(), "some error") +} + +func Test_BundleToHelmChartConverter_ToHelmChart_NoAPIServiceDefinitions(t *testing.T) { + converter := convert.BundleToHelmChartConverter{} + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV(WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{})), + }, + ) + + _, err := converter.ToHelmChart(b, "install-namespace", "") + require.Error(t, err) + require.Contains(t, err.Error(), "unsupported bundle: apiServiceDefintions are not supported") +} + +func Test_BundleToHelmChartConverter_ToHelmChart_NoWebhooksWithoutCertProvider(t *testing.T) { + converter := convert.BundleToHelmChartConverter{ + IsWebhookSupportEnabled: true, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV(WithWebhookDefinitions(v1alpha1.WebhookDescription{})), + }, + ) + + _, err := converter.ToHelmChart(b, "install-namespace", "") + require.Error(t, err) + require.Contains(t, err.Error(), "webhookDefinitions are not supported") +} + +func Test_BundleToHelmChartConverter_ToHelmChart_WebhooksSupportDisabled(t *testing.T) { + converter := convert.BundleToHelmChartConverter{ + IsWebhookSupportEnabled: false, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV(WithWebhookDefinitions(v1alpha1.WebhookDescription{})), + }, + ) + + _, err := converter.ToHelmChart(b, "install-namespace", "") + require.Error(t, err) + require.Contains(t, err.Error(), "webhookDefinitions are not supported") +} + +func Test_BundleToHelmChartConverter_ToHelmChart_WebhooksWithCertProvider(t *testing.T) { + converter := convert.BundleToHelmChartConverter{ + CertificateProvider: FakeCertProvider{}, + IsWebhookSupportEnabled: true, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + WithWebhookDefinitions(v1alpha1.WebhookDescription{}), + ), + }, + ) + + _, err := converter.ToHelmChart(b, "install-namespace", "") + require.NoError(t, err) +} + +func Test_BundleToHelmChartConverter_ToHelmChart_BundleRendererIntegration(t *testing.T) { + expectedInstallNamespace := "install-namespace" + expectedWatchNamespace := "" + expectedCertProvider := FakeCertProvider{} + + converter := convert.BundleToHelmChartConverter{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + // ensure correct options are being passed down to the bundle renderer + require.Equal(t, expectedInstallNamespace, opts.InstallNamespace) + require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces) + require.Equal(t, expectedCertProvider, opts.CertificateProvider) + return nil, nil + }, + }, + }, + CertificateProvider: expectedCertProvider, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + }, + ) + + _, err := converter.ToHelmChart(b, expectedInstallNamespace, expectedWatchNamespace) + require.NoError(t, err) +} + +func Test_BundleToHelmChartConverter_ToHelmChart_Success(t *testing.T) { + converter := convert.BundleToHelmChartConverter{ + BundleRenderer: render.BundleRenderer{ + ResourceGenerators: []render.ResourceGenerator{ + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + out := make([]client.Object, 0, len(rv1.Others)) + for i := range rv1.Others { + out = append(out, &rv1.Others[i]) + } + return out, nil + }, + }, + }, + } + + b := source.FromBundle( + bundle.RegistryV1{ + CSV: MakeCSV( + WithAnnotations(map[string]string{"foo": "bar"}), + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + ), + Others: []unstructured.Unstructured{ + *ToUnstructuredT(t, &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "testService", + }, + }), + }, + }, + ) + + chart, err := converter.ToHelmChart(b, "install-namespace", "") + require.NoError(t, err) + require.NotNil(t, chart) + require.NotNil(t, chart.Metadata) + + t.Log("Check Chart metadata contains CSV annotations") + require.Equal(t, map[string]string{"foo": "bar"}, chart.Metadata.Annotations) + + t.Log("Check Chart templates have the same number of resources generated by the renderer") + require.Len(t, chart.Templates, 1) +} diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go deleted file mode 100644 index 516bc1da2..000000000 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ /dev/null @@ -1,413 +0,0 @@ -package convert_test - -import ( - "io/fs" - "os" - "strings" - "testing" - "testing/fstest" - - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - schedulingv1 "k8s.io/api/scheduling/v1" - 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" - "k8s.io/apimachinery/pkg/runtime/schema" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" - - "github.com/operator-framework/operator-controller/internal/operator-controller/features" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" - . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" -) - -const ( - olmNamespaces = "olm.targetNamespaces" - olmProperties = "olm.properties" - installNamespace = "testInstallNamespace" - - bundlePathAnnotations = "metadata/annotations.yaml" - bundlePathCSV = "manifests/csv.yaml" -) - -func getCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) { - csv := MakeCSV(WithName("testCSV"), WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)) - svc := corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testService", - }, - } - svc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}) - return csv, svc -} - -func TestPlainConverterUsedRegV1Validator(t *testing.T) { - require.Equal(t, validators.RegistryV1BundleValidator, convert.PlainConverter.BundleValidator) -} - -func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) { - var targetNamespaces []string - - t.Log("RegistryV1 Suite Convert") - t.Log("It should set the namespaces of the object correctly") - t.Log("It should set the namespace to installnamespace if not available") - - t.Log("By creating a registry v1 bundle") - csv, svc := getCsvAndService() - - unstructuredSvc := *ToUnstructuredT(t, &svc) - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 1) - - t.Log("By verifying if ns has been set correctly") - resObj := findObjectByName(svc.Name, plainBundle.Objects) - require.NotNil(t, resObj) - require.Equal(t, installNamespace, resObj.GetNamespace()) -} - -func TestRegistryV1SuiteNamespaceAvailable(t *testing.T) { - var targetNamespaces []string - - t.Log("RegistryV1 Suite Convert") - t.Log("It should set the namespaces of the object correctly") - t.Log("It should override namespace if already available") - - t.Log("By creating a registry v1 bundle") - csv, svc := getCsvAndService() - - svc.SetNamespace("otherNs") - unstructuredSvc := *ToUnstructuredT(t, &svc) - unstructuredSvc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}) - - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 1) - - t.Log("By verifying if ns has been set correctly") - resObj := findObjectByName(svc.Name, plainBundle.Objects) - require.NotNil(t, plainBundle) - require.Equal(t, installNamespace, resObj.GetNamespace()) -} - -func TestRegistryV1SuiteNamespaceUnsupportedKind(t *testing.T) { - var targetNamespaces []string - - t.Log("RegistryV1 Suite Convert") - t.Log("It should set the namespaces of the object correctly") - t.Log("It should error when object is not supported") - t.Log("It should error when unsupported GVK is passed") - - t.Log("By creating a registry v1 bundle") - csv, _ := getCsvAndService() - - t.Log("By creating an unsupported kind") - event := &corev1.Event{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Event", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "testEvent", - }, - } - - unstructuredEvt := *ToUnstructuredT(t, event) - unstructuredEvt.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Event"}) - - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: csv, - Others: []unstructured.Unstructured{unstructuredEvt}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces) - require.Error(t, err) - require.ErrorContains(t, err, "bundle contains unsupported resource") - require.Nil(t, plainBundle) -} - -func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) { - var targetNamespaces []string - - t.Log("RegistryV1 Suite Convert") - t.Log("It should set the namespaces of the object correctly") - t.Log("It should not set ns cluster scoped object is passed") - t.Log("It should not error when cluster scoped obj is passed and not set its namespace") - - t.Log("By creating a registry v1 bundle") - csv, _ := getCsvAndService() - - t.Log("By creating an unsupported kind") - pc := &schedulingv1.PriorityClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: schedulingv1.SchemeGroupVersion.String(), - Kind: "PriorityClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "testPriorityClass", - }, - } - - unstructuredpriorityclass := *ToUnstructuredT(t, pc) - unstructuredpriorityclass.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "PriorityClass"}) - - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: csv, - Others: []unstructured.Unstructured{unstructuredpriorityclass}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 1) - - t.Log("By verifying if ns has been set correctly") - resObj := findObjectByName(pc.Name, plainBundle.Objects) - require.NotNil(t, resObj) - require.Empty(t, resObj.GetNamespace()) -} - -func TestRegistryV1SuiteReadBundleFileSystem(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should read the registry+v1 bundle filesystem correctly") - t.Log("It should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata") - fsys := os.DirFS("testdata/combine-properties-bundle") - - chrt, err := convert.RegistryV1ToHelmChart(fsys, "", "") - require.NoError(t, err) - require.NotNil(t, chrt) - require.NotNil(t, chrt.Metadata) - require.Contains(t, chrt.Metadata.Annotations, olmProperties) - require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`, chrt.Metadata.Annotations[olmProperties]) -} - -func TestParseFSFails(t *testing.T) { - for _, tt := range []struct { - name string - FS fs.FS - }{ - { - name: "bundle missing ClusterServiceVersion manifest", - FS: removePaths(newBundleFS(), bundlePathCSV), - }, { - name: "bundle missing metadata/annotations.yaml", - FS: removePaths(newBundleFS(), bundlePathAnnotations), - }, { - name: "bundle missing metadata/ directory", - FS: removePaths(newBundleFS(), "metadata/"), - }, { - name: "bundle missing manifests/ directory", - FS: removePaths(newBundleFS(), "manifests/"), - }, - } { - t.Run(tt.name, func(t *testing.T) { - _, err := convert.ParseFS(tt.FS) - require.Error(t, err) - }) - } -} - -func TestRegistryV1SuiteReadBundleFileSystemFailsOnNoCSV(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should read the registry+v1 bundle filesystem correctly") - t.Log("It should include metadata/properties.yaml and csv.metadata.annotations['olm.properties'] in chart metadata") - fsys := os.DirFS("testdata/combine-properties-bundle") - - chrt, err := convert.RegistryV1ToHelmChart(fsys, "", "") - - require.NoError(t, err) - require.NotNil(t, chrt) - require.NotNil(t, chrt.Metadata) - require.Contains(t, chrt.Metadata.Annotations, olmProperties) - require.JSONEq(t, `[{"type":"from-csv-annotations-key","value":"from-csv-annotations-value"},{"type":"from-file-key","value":"from-file-value"}]`, chrt.Metadata.Annotations[olmProperties]) -} - -func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should enforce limitations") - t.Log("It should not allow bundles with webhooks") - t.Log("By creating a registry v1 bundle") - csv := v1alpha1.ClusterServiceVersion{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testCSV", - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}}, - WebhookDefinitions: []v1alpha1.WebhookDescription{{ConversionCRDs: []string{"fake-webhook.package-with-webhooks.io"}}}, - }, - } - watchNamespaces := []string{metav1.NamespaceAll} - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: csv, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.Error(t, err) - require.ErrorContains(t, err, "webhookDefinitions are not supported") - require.Nil(t, plainBundle) -} - -func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.WebhookProviderCertManager, true) - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should enforce limitations") - t.Log("It should allow bundles with webhooks") - t.Log("By creating a registry v1 bundle") - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CRDs: []apiextensionsv1.CustomResourceDefinition{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "fake-webhook.package-with-webhooks", - }, - }, - }, - CSV: MakeCSV( - WithName("testCSV"), - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), - WithOwnedCRDs( - v1alpha1.CRDDescription{ - Name: "fake-webhook.package-with-webhooks", - }, - ), - WithStrategyDeploymentSpecs( - v1alpha1.StrategyDeploymentSpec{ - Name: "some-deployment", - }, - ), - WithWebhookDefinitions( - v1alpha1.WebhookDescription{ - Type: v1alpha1.ConversionWebhook, - ConversionCRDs: []string{"fake-webhook.package-with-webhooks"}, - DeploymentName: "some-deployment", - GenerateName: "my-conversion-webhook", - }, - ), - ), - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, []string{metav1.NamespaceAll}) - require.NoError(t, err) - require.NotNil(t, plainBundle) -} - -func TestRegistryV1SuiteGenerateNoAPIServiceDefinitions(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should enforce limitations") - t.Log("It should not allow bundles with API service definitions") - t.Log("By creating a registry v1 bundle") - csv := v1alpha1.ClusterServiceVersion{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testCSV", - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}}, - APIServiceDefinitions: v1alpha1.APIServiceDefinitions{ - Owned: []v1alpha1.APIServiceDescription{{Name: "fake-owned-api-definition"}}, - }, - }, - } - watchNamespaces := []string{metav1.NamespaceAll} - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: csv, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.Error(t, err) - require.ErrorContains(t, err, "apiServiceDefintions are not supported") - require.Nil(t, plainBundle) -} - -func findObjectByName(name string, result []client.Object) client.Object { - for _, o := range result { - // Since this is a controlled env, comparing only the names is sufficient for now. - // In the future, compare GVKs too by ensuring its set on the unstructuredObj. - if o.GetName() == name { - return o - } - } - return nil -} - -func newBundleFS() fstest.MapFS { - annotationsYml := ` -annotations: - operators.operatorframework.io.bundle.mediatype.v1: registry+v1 - operators.operatorframework.io.bundle.package.v1: test -` - - csvYml := ` -apiVersion: operators.operatorframework.io/v1alpha1 -kind: ClusterServiceVersion -metadata: - name: test.v1.0.0 - annotations: - olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]' -spec: - installModes: - - type: AllNamespaces - supported: true -` - - return fstest.MapFS{ - bundlePathAnnotations: &fstest.MapFile{Data: []byte(strings.Trim(annotationsYml, "\n"))}, - bundlePathCSV: &fstest.MapFile{Data: []byte(strings.Trim(csvYml, "\n"))}, - } -} - -func removePaths(mapFs fstest.MapFS, paths ...string) fstest.MapFS { - for k := range mapFs { - for _, path := range paths { - if strings.HasPrefix(k, path) { - delete(mapFs, k) - } - } - } - return mapFs -} diff --git a/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/manifests/csv.yaml b/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/manifests/csv.yaml deleted file mode 100644 index a2a620439..000000000 --- a/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/manifests/csv.yaml +++ /dev/null @@ -1,10 +0,0 @@ -apiVersion: operators.operatorframework.io/v1alpha1 -kind: ClusterServiceVersion -metadata: - name: test.v1.0.0 - annotations: - olm.properties: '[{"type":"from-csv-annotations-key", "value":"from-csv-annotations-value"}]' -spec: - installModes: - - type: AllNamespaces - supported: true diff --git a/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/metadata/annotations.yaml b/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/metadata/annotations.yaml deleted file mode 100644 index e75867f85..000000000 --- a/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/metadata/annotations.yaml +++ /dev/null @@ -1,3 +0,0 @@ -annotations: - operators.operatorframework.io.bundle.mediatype.v1: registry+v1 - operators.operatorframework.io.bundle.package.v1: test diff --git a/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/metadata/properties.yaml b/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/metadata/properties.yaml deleted file mode 100644 index 1a6e7abfb..000000000 --- a/internal/operator-controller/rukpak/convert/testdata/combine-properties-bundle/metadata/properties.yaml +++ /dev/null @@ -1,3 +0,0 @@ -properties: - - type: "from-file-key" - value: "from-file-value" diff --git a/internal/operator-controller/rukpak/render/generators/generators.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go similarity index 95% rename from internal/operator-controller/rukpak/render/generators/generators.go rename to internal/operator-controller/rukpak/render/registryv1/generators/generators.go index 5e702c492..cf17142fa 100644 --- a/internal/operator-controller/rukpak/render/generators/generators.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go @@ -21,6 +21,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) @@ -36,20 +37,12 @@ var certVolumeMounts = map[string]corev1.VolumeMount{ }, } -// BundleCSVRBACResourceGenerator generates all ServiceAccounts, ClusterRoles, ClusterRoleBindings, Roles, RoleBindings -// defined in the RegistryV1 bundle's cluster service version (CSV) -var BundleCSVRBACResourceGenerator = render.ResourceGenerators{ - BundleCSVServiceAccountGenerator, - BundleCSVPermissionsGenerator, - BundleCSVClusterPermissionsGenerator, -} - // 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 // - the deployment spec's revision history limit is set to 1 // - merges csv annotations to the deployment template's annotations -func BundleCSVDeploymentGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleCSVDeploymentGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -96,7 +89,7 @@ func BundleCSVDeploymentGenerator(rv1 *render.RegistryV1, opts render.Options) ( // BundleCSVPermissionsGenerator generates the Roles and RoleBindings based on bundle's cluster service version // permission spec. If the bundle is being installed in AllNamespaces mode (opts.TargetNamespaces = [”]) // no resources will be generated as these permissions will be promoted to ClusterRole/Bunding(s) -func BundleCSVPermissionsGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleCSVPermissionsGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -136,7 +129,7 @@ func BundleCSVPermissionsGenerator(rv1 *render.RegistryV1, opts render.Options) // (opts.TargetNamespaces = [”]), the CSV's permission spec will be promoted to ClusterRole and ClusterRoleBinding // resources. To keep parity with OLMv0, these will also include an extra rule to get, list, watch namespaces // (see https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/operators/olm/operatorgroup.go#L539) -func BundleCSVClusterPermissionsGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleCSVClusterPermissionsGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -178,7 +171,7 @@ func BundleCSVClusterPermissionsGenerator(rv1 *render.RegistryV1, opts render.Op // if multiple permissions reference the same service account, only one resource will be generated). // If a clusterPermission, or permission, references an empty (”) service account, this is considered to be the // namespace 'default' service account. A resource for the namespace 'default' service account is not generated. -func BundleCSVServiceAccountGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleCSVServiceAccountGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -205,7 +198,7 @@ func BundleCSVServiceAccountGenerator(rv1 *render.RegistryV1, opts render.Option // BundleCRDGenerator generates CustomResourceDefinition resources from the registry+v1 bundle. If the CRD is referenced // by any conversion webhook defined in the bundle's cluster service version spec, the CRD is modified // by the CertificateProvider in opts to add any annotations or modifications necessary for certificate injection. -func BundleCRDGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleCRDGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -268,7 +261,7 @@ func BundleCRDGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.O // BundleAdditionalResourcesGenerator generates resources for the additional resources included in the // bundle. If the bundle resource is namespace scoped, its namespace will be set to the value of opts.InstallNamespace. -func BundleAdditionalResourcesGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleAdditionalResourcesGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -292,7 +285,7 @@ func BundleAdditionalResourcesGenerator(rv1 *render.RegistryV1, opts render.Opti // BundleValidatingWebhookResourceGenerator generates ValidatingAdmissionWebhookConfiguration resources based on // the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts // to add any annotations or modifications necessary for certificate injection. -func BundleValidatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleValidatingWebhookResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -340,7 +333,7 @@ func BundleValidatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts rende // BundleMutatingWebhookResourceGenerator generates MutatingAdmissionWebhookConfiguration resources based on // the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts // to add any annotations or modifications necessary for certificate injection. -func BundleMutatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleMutatingWebhookResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -389,7 +382,7 @@ func BundleMutatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts render. // BundleWebhookServiceResourceGenerator generates Service resources based that support the webhooks defined in // the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts // to add any annotations or modifications necessary for certificate injection. -func BundleWebhookServiceResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func BundleWebhookServiceResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } @@ -443,7 +436,7 @@ func BundleWebhookServiceResourceGenerator(rv1 *render.RegistryV1, opts render.O // CertProviderResourceGenerator generates any resources necessary for the CertificateProvider // in opts to function correctly, e.g. Issuer or Certificate resources. -func CertProviderResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { +func CertProviderResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { deploymentsWithWebhooks := sets.Set[string]{} for _, wh := range rv1.CSV.Spec.WebhookDefinitions { diff --git a/internal/operator-controller/rukpak/render/generators/generators_test.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go similarity index 96% rename from internal/operator-controller/rukpak/render/generators/generators_test.go rename to internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go index 0dcb9b11e..91d02de02 100644 --- a/internal/operator-controller/rukpak/render/generators/generators_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go @@ -3,7 +3,6 @@ package generators_test import ( "cmp" "fmt" - "reflect" "slices" "testing" @@ -21,51 +20,38 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/generators" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) -func Test_BundleCSVRBACResourceGenerator_HasCorrectGenerators(t *testing.T) { - expectedResourceGenerators := []render.ResourceGenerator{ - generators.BundleCSVServiceAccountGenerator, - generators.BundleCSVPermissionsGenerator, - generators.BundleCSVClusterPermissionsGenerator, - } - actualResourceGenerators := generators.BundleCSVRBACResourceGenerator - - require.Equal(t, len(expectedResourceGenerators), len(actualResourceGenerators)) - for i := range expectedResourceGenerators { - require.Equal(t, reflect.ValueOf(expectedResourceGenerators[i]).Pointer(), reflect.ValueOf(actualResourceGenerators[i]).Pointer(), "bundle validator has unexpected validation function") - } -} - func Test_ResourceGenerators(t *testing.T) { g := render.ResourceGenerators{ - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return []client.Object{&corev1.Service{}}, nil }, - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return []client.Object{&corev1.ConfigMap{}}, nil }, } - objs, err := g.GenerateResources(&render.RegistryV1{}, render.Options{}) + objs, err := g.GenerateResources(&bundle.RegistryV1{}, render.Options{}) require.NoError(t, err) require.Equal(t, []client.Object{&corev1.Service{}, &corev1.ConfigMap{}}, objs) } func Test_ResourceGenerators_Errors(t *testing.T) { g := render.ResourceGenerators{ - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return []client.Object{&corev1.Service{}}, nil }, - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return nil, fmt.Errorf("generator error") }, } - objs, err := g.GenerateResources(&render.RegistryV1{}, render.Options{}) + objs, err := g.GenerateResources(&bundle.RegistryV1{}, render.Options{}) require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "generator error") @@ -74,13 +60,13 @@ func Test_ResourceGenerators_Errors(t *testing.T) { func Test_BundleCSVDeploymentGenerator_Succeeds(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 opts render.Options expectedResources []client.Object }{ { name: "generates deployment resources", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithAnnotations(map[string]string{ "csv": "annotation", @@ -187,7 +173,7 @@ func Test_BundleCSVDeploymentGenerator_WithCertWithCertProvider_Succeeds(t *test }, } - bundle := &render.RegistryV1{ + bundle := &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -345,7 +331,7 @@ func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { for _, tc := range []struct { name string opts render.Options - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedResources []client.Object }{ { @@ -355,7 +341,7 @@ func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{""}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -381,7 +367,7 @@ func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{"watch-namespace"}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -456,7 +442,7 @@ func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{"watch-namespace", "watch-namespace-two"}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -575,7 +561,7 @@ func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{"watch-namespace"}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -692,7 +678,7 @@ func Test_BundleCSVPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{"watch-namespace"}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -779,7 +765,7 @@ func Test_BundleCSVClusterPermissionsGenerator_Succeeds(t *testing.T) { for _, tc := range []struct { name string opts render.Options - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedResources []client.Object }{ { @@ -789,7 +775,7 @@ func Test_BundleCSVClusterPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{""}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -910,7 +896,7 @@ func Test_BundleCSVClusterPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{"watch-namespace"}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithClusterPermissions( @@ -1023,7 +1009,7 @@ func Test_BundleCSVClusterPermissionsGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{"watch-namespace"}, UniqueNameGenerator: fakeUniqueNameGenerator, }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithClusterPermissions( @@ -1104,7 +1090,7 @@ func Test_BundleCSVServiceAccountGenerator_Succeeds(t *testing.T) { for _, tc := range []struct { name string opts render.Options - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedResources []client.Object }{ { @@ -1112,7 +1098,7 @@ func Test_BundleCSVServiceAccountGenerator_Succeeds(t *testing.T) { opts: render.Options{ InstallNamespace: "install-namespace", }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -1199,7 +1185,7 @@ func Test_BundleCSVServiceAccountGenerator_Succeeds(t *testing.T) { opts: render.Options{ InstallNamespace: "install-namespace", }, - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithName("csv"), WithPermissions( @@ -1258,7 +1244,7 @@ func Test_BundleCRDGenerator_Succeeds(t *testing.T) { TargetNamespaces: []string{""}, } - bundle := &render.RegistryV1{ + bundle := &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, @@ -1279,7 +1265,7 @@ func Test_BundleCRDGenerator_WithConversionWebhook_Succeeds(t *testing.T) { TargetNamespaces: []string{""}, } - bundle := &render.RegistryV1{ + bundle := &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, @@ -1360,7 +1346,7 @@ func Test_BundleCRDGenerator_WithConversionWebhook_Fails(t *testing.T) { TargetNamespaces: []string{""}, } - bundle := &render.RegistryV1{ + bundle := &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ { ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}, @@ -1405,7 +1391,7 @@ func Test_BundleCRDGenerator_WithCertProvider_Succeeds(t *testing.T) { CertificateProvider: fakeProvider, } - bundle := &render.RegistryV1{ + bundle := &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, @@ -1443,7 +1429,7 @@ func Test_BundleAdditionalResourcesGenerator_Succeeds(t *testing.T) { InstallNamespace: "install-namespace", } - bundle := &render.RegistryV1{ + bundle := &bundle.RegistryV1{ Others: []unstructured.Unstructured{ *ToUnstructuredT(t, &corev1.Service{ @@ -1493,13 +1479,13 @@ func Test_BundleValidatingWebhookResourceGenerator_Succeeds(t *testing.T) { } for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 opts render.Options expectedResources []client.Object }{ { name: "generates validating webhook configuration resources described in the bundle's cluster service version", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -1592,7 +1578,7 @@ func Test_BundleValidatingWebhookResourceGenerator_Succeeds(t *testing.T) { }, { name: "removes any - suffixes from the webhook name (v0 used GenerateName to allow multiple operator installations - we don't want that in v1)", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -1685,7 +1671,7 @@ func Test_BundleValidatingWebhookResourceGenerator_Succeeds(t *testing.T) { }, { name: "generates validating webhook configuration resources with certificate provider modifications", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -1757,13 +1743,13 @@ func Test_BundleMutatingWebhookResourceGenerator_Succeeds(t *testing.T) { } for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 opts render.Options expectedResources []client.Object }{ { name: "generates validating webhook configuration resources described in the bundle's cluster service version", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -1858,7 +1844,7 @@ func Test_BundleMutatingWebhookResourceGenerator_Succeeds(t *testing.T) { }, { name: "removes any - suffixes from the webhook name (v0 used GenerateName to allow multiple operator installations - we don't want that in v1)", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -1953,7 +1939,7 @@ func Test_BundleMutatingWebhookResourceGenerator_Succeeds(t *testing.T) { }, { name: "generates validating webhook configuration resources with certificate provider modifications", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -2025,13 +2011,13 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { } for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 opts render.Options expectedResources []client.Object }{ { name: "generates webhook services using container port 443 and target port 443 by default", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2076,7 +2062,7 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { }, { name: "generates webhook services using the given container port and setting target port the same as the container port if not given", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2122,7 +2108,7 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { }, { name: "generates webhook services using given container port of 443 and given target port", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2171,7 +2157,7 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { }, { name: "generates webhook services using given container port and target port", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2221,7 +2207,7 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { }, { name: "generates webhook services using referenced deployment defined label selector", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2281,7 +2267,7 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { }, { name: "aggregates all webhook definitions referencing the same deployment into a single service", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2379,7 +2365,7 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { }, { name: "applies cert provider modifiers to webhook service", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ @@ -2454,7 +2440,7 @@ func Test_CertProviderResourceGenerator_Succeeds(t *testing.T) { }, } - objs, err := generators.CertProviderResourceGenerator(&render.RegistryV1{ + objs, err := generators.CertProviderResourceGenerator(&bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( // only generate resources for deployments referenced by webhook definitions diff --git a/internal/operator-controller/rukpak/render/generators/resources.go b/internal/operator-controller/rukpak/render/registryv1/generators/resources.go similarity index 100% rename from internal/operator-controller/rukpak/render/generators/resources.go rename to internal/operator-controller/rukpak/render/registryv1/generators/resources.go diff --git a/internal/operator-controller/rukpak/render/generators/resources_test.go b/internal/operator-controller/rukpak/render/registryv1/generators/resources_test.go similarity index 99% rename from internal/operator-controller/rukpak/render/generators/resources_test.go rename to internal/operator-controller/rukpak/render/registryv1/generators/resources_test.go index 7d6a95e33..aa3227987 100644 --- a/internal/operator-controller/rukpak/render/generators/resources_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/resources_test.go @@ -12,7 +12,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/generators" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1.go b/internal/operator-controller/rukpak/render/registryv1/registryv1.go new file mode 100644 index 000000000..61f0e6ef0 --- /dev/null +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1.go @@ -0,0 +1,48 @@ +package registryv1 + +import ( + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/generators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/validators" +) + +// Renderer renders registry+v1 bundles into plain kubernetes manifests +var Renderer = render.BundleRenderer{ + BundleValidator: BundleValidator, + ResourceGenerators: ResourceGenerators, +} + +// BundleValidator validates RegistryV1 bundles +var BundleValidator = render.BundleValidator{ + // NOTE: if you update this list, Test_BundleValidatorHasAllValidationFns 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 validation function individually + validators.CheckDeploymentSpecUniqueness, + validators.CheckDeploymentNameIsDNS1123SubDomain, + validators.CheckCRDResourceUniqueness, + validators.CheckOwnedCRDExistence, + validators.CheckPackageNameNotEmpty, + validators.CheckWebhookDeploymentReferentialIntegrity, + validators.CheckWebhookNameUniqueness, + validators.CheckWebhookNameIsDNS1123SubDomain, + validators.CheckConversionWebhookCRDReferenceUniqueness, + validators.CheckConversionWebhooksReferenceOwnedCRDs, +} + +// ResourceGenerators a slice of ResourceGenerators required to generate plain resource manifests for +// registry+v1 bundles +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.BundleCSVServiceAccountGenerator, + generators.BundleCSVPermissionsGenerator, + generators.BundleCSVClusterPermissionsGenerator, + generators.BundleCRDGenerator, + generators.BundleAdditionalResourcesGenerator, + generators.BundleCSVDeploymentGenerator, + generators.BundleValidatingWebhookResourceGenerator, + generators.BundleMutatingWebhookResourceGenerator, + generators.BundleWebhookServiceResourceGenerator, + generators.CertProviderResourceGenerator, +} diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go new file mode 100644 index 000000000..ed1b3294f --- /dev/null +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go @@ -0,0 +1,123 @@ +package registryv1_test + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/generators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/validators" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" +) + +func Test_BundleValidatorHasAllValidationFns(t *testing.T) { + expectedValidationFns := []func(v1 *bundle.RegistryV1) []error{ + validators.CheckDeploymentSpecUniqueness, + validators.CheckDeploymentNameIsDNS1123SubDomain, + validators.CheckCRDResourceUniqueness, + validators.CheckOwnedCRDExistence, + validators.CheckPackageNameNotEmpty, + validators.CheckWebhookDeploymentReferentialIntegrity, + validators.CheckWebhookNameUniqueness, + validators.CheckWebhookNameIsDNS1123SubDomain, + validators.CheckConversionWebhookCRDReferenceUniqueness, + validators.CheckConversionWebhooksReferenceOwnedCRDs, + } + actualValidationFns := registryv1.BundleValidator + + require.Equal(t, len(expectedValidationFns), len(actualValidationFns)) + for i := range expectedValidationFns { + require.Equal(t, reflect.ValueOf(expectedValidationFns[i]).Pointer(), reflect.ValueOf(actualValidationFns[i]).Pointer(), "bundle validator has unexpected validation function") + } +} + +func Test_ResourceGeneratorsHasAllGenerators(t *testing.T) { + expectedGenerators := []render.ResourceGenerator{ + generators.BundleCSVServiceAccountGenerator, + generators.BundleCSVPermissionsGenerator, + generators.BundleCSVClusterPermissionsGenerator, + generators.BundleCRDGenerator, + generators.BundleAdditionalResourcesGenerator, + generators.BundleCSVDeploymentGenerator, + generators.BundleValidatingWebhookResourceGenerator, + generators.BundleMutatingWebhookResourceGenerator, + generators.BundleWebhookServiceResourceGenerator, + generators.CertProviderResourceGenerator, + } + actualGenerators := registryv1.ResourceGenerators + + require.Equal(t, len(expectedGenerators), len(actualGenerators)) + for i := range expectedGenerators { + require.Equal(t, reflect.ValueOf(expectedGenerators[i]).Pointer(), reflect.ValueOf(actualGenerators[i]).Pointer(), "bundle validator has unexpected validation function") + } +} + +func Test_Renderer_Success(t *testing.T) { + bundle := bundle.RegistryV1{ + PackageName: "my-package", + CSV: MakeCSV( + WithName("test-bundle"), + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + ), + 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(bundle, "install-namespace") + t.Log("Check renderer returns objects and no errors") + require.NoError(t, err) + require.NotEmpty(t, objs) + + t.Log("Check renderer returns a single object") + // bundle only contains a service - bundle csv is empty + require.Len(t, objs, 1) + + t.Log("Check correct name and that the correct namespace was applied") + require.Equal(t, "my-service", objs[0].GetName()) + require.Equal(t, "install-namespace", objs[0].GetNamespace()) +} + +func Test_Renderer_Failure_UnsupportedKind(t *testing.T) { + bundle := bundle.RegistryV1{ + PackageName: "my-package", + CSV: MakeCSV( + WithName("test-bundle"), + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + ), + Others: []unstructured.Unstructured{ + *ToUnstructuredT(t, &corev1.Event{ + TypeMeta: metav1.TypeMeta{ + Kind: "Event", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "testEvent", + }, + }), + }, + } + + objs, err := registryv1.Renderer.Render(bundle, "install-namespace") + t.Log("Check renderer returns objects and no errors") + require.Error(t, err) + require.Contains(t, err.Error(), "unsupported resource") + require.Empty(t, objs) +} diff --git a/internal/operator-controller/rukpak/render/validators/validator.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go similarity index 86% rename from internal/operator-controller/rukpak/render/validators/validator.go rename to internal/operator-controller/rukpak/render/registryv1/validators/validator.go index 4d9568375..61d0aad7c 100644 --- a/internal/operator-controller/rukpak/render/validators/validator.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go @@ -13,29 +13,12 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" ) -// RegistryV1BundleValidator validates RegistryV1 bundles -var RegistryV1BundleValidator = render.BundleValidator{ - // NOTE: if you update this list, Test_BundleValidatorHasAllValidationFns 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 validation function individually - CheckDeploymentSpecUniqueness, - CheckDeploymentNameIsDNS1123SubDomain, - CheckCRDResourceUniqueness, - CheckOwnedCRDExistence, - CheckPackageNameNotEmpty, - CheckWebhookDeploymentReferentialIntegrity, - CheckWebhookNameUniqueness, - CheckWebhookNameIsDNS1123SubDomain, - CheckConversionWebhookCRDReferenceUniqueness, - CheckConversionWebhooksReferenceOwnedCRDs, -} - // CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name. // Errors are sorted by deployment name. -func CheckDeploymentSpecUniqueness(rv1 *render.RegistryV1) []error { +func CheckDeploymentSpecUniqueness(rv1 *bundle.RegistryV1) []error { deploymentNameSet := sets.Set[string]{} duplicateDeploymentNames := sets.Set[string]{} for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { @@ -54,7 +37,7 @@ func CheckDeploymentSpecUniqueness(rv1 *render.RegistryV1) []error { // CheckDeploymentNameIsDNS1123SubDomain checks each deployment strategy spec name complies with the Kubernetes // resource naming conversions -func CheckDeploymentNameIsDNS1123SubDomain(rv1 *render.RegistryV1) []error { +func CheckDeploymentNameIsDNS1123SubDomain(rv1 *bundle.RegistryV1) []error { deploymentNameErrMap := map[string][]string{} for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { errs := validation.IsDNS1123Subdomain(dep.Name) @@ -72,7 +55,7 @@ func CheckDeploymentNameIsDNS1123SubDomain(rv1 *render.RegistryV1) []error { } // CheckOwnedCRDExistence checks bundle owned custom resource definitions declared in the csv exist in the bundle -func CheckOwnedCRDExistence(rv1 *render.RegistryV1) []error { +func CheckOwnedCRDExistence(rv1 *bundle.RegistryV1) []error { crdsNames := sets.Set[string]{} for _, crd := range rv1.CRDs { crdsNames.Insert(crd.Name) @@ -93,7 +76,7 @@ func CheckOwnedCRDExistence(rv1 *render.RegistryV1) []error { } // CheckCRDResourceUniqueness checks that the bundle CRD names are unique -func CheckCRDResourceUniqueness(rv1 *render.RegistryV1) []error { +func CheckCRDResourceUniqueness(rv1 *bundle.RegistryV1) []error { crdsNames := sets.Set[string]{} duplicateCRDNames := sets.Set[string]{} for _, crd := range rv1.CRDs { @@ -111,7 +94,7 @@ func CheckCRDResourceUniqueness(rv1 *render.RegistryV1) []error { } // CheckPackageNameNotEmpty checks that PackageName is not empty -func CheckPackageNameNotEmpty(rv1 *render.RegistryV1) []error { +func CheckPackageNameNotEmpty(rv1 *bundle.RegistryV1) []error { if rv1.PackageName == "" { return []error{errors.New("package name is empty")} } @@ -125,7 +108,7 @@ func CheckPackageNameNotEmpty(rv1 *render.RegistryV1) []error { // Since OLMv1 considers APIs to be cluster-scoped, we initially extend this constraint to validating and mutating webhooks. // While this might restrict the number of supported bundles, we can tackle the issue of relaxing this constraint in turn // after getting the webhook support working. -func CheckWebhookSupport(rv1 *render.RegistryV1) []error { +func CheckWebhookSupport(rv1 *bundle.RegistryV1) []error { if len(rv1.CSV.Spec.WebhookDefinitions) > 0 { supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{} for _, mode := range rv1.CSV.Spec.InstallModes { @@ -142,7 +125,7 @@ func CheckWebhookSupport(rv1 *render.RegistryV1) []error { // CheckWebhookDeploymentReferentialIntegrity checks that each webhook definition in the csv // references an existing strategy deployment spec. Errors are sorted by strategy deployment spec name, // webhook type, and webhook name. -func CheckWebhookDeploymentReferentialIntegrity(rv1 *render.RegistryV1) []error { +func CheckWebhookDeploymentReferentialIntegrity(rv1 *bundle.RegistryV1) []error { webhooksByDeployment := map[string][]v1alpha1.WebhookDescription{} for _, wh := range rv1.CSV.Spec.WebhookDefinitions { webhooksByDeployment[wh.DeploymentName] = append(webhooksByDeployment[wh.DeploymentName], wh) @@ -169,7 +152,7 @@ func CheckWebhookDeploymentReferentialIntegrity(rv1 *render.RegistryV1) []error // CheckWebhookNameUniqueness checks that each webhook definition of each type (validating, mutating, or conversion) // has a unique name. Webhooks of different types can have the same name. Errors are sorted by webhook type // and name. -func CheckWebhookNameUniqueness(rv1 *render.RegistryV1) []error { +func CheckWebhookNameUniqueness(rv1 *bundle.RegistryV1) []error { webhookNameSetByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{} duplicateWebhooksByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{} for _, wh := range rv1.CSV.Spec.WebhookDefinitions { @@ -196,7 +179,7 @@ func CheckWebhookNameUniqueness(rv1 *render.RegistryV1) []error { // CheckConversionWebhooksReferenceOwnedCRDs checks defined conversion webhooks reference bundle owned CRDs. // Errors are sorted by webhook name and CRD name. -func CheckConversionWebhooksReferenceOwnedCRDs(rv1 *render.RegistryV1) []error { +func CheckConversionWebhooksReferenceOwnedCRDs(rv1 *bundle.RegistryV1) []error { //nolint:prealloc var conversionWebhooks []v1alpha1.WebhookDescription for _, wh := range rv1.CSV.Spec.WebhookDefinitions { @@ -233,7 +216,7 @@ func CheckConversionWebhooksReferenceOwnedCRDs(rv1 *render.RegistryV1) []error { } // CheckConversionWebhookCRDReferenceUniqueness checks no two (or more) conversion webhooks reference the same CRD. -func CheckConversionWebhookCRDReferenceUniqueness(rv1 *render.RegistryV1) []error { +func CheckConversionWebhookCRDReferenceUniqueness(rv1 *bundle.RegistryV1) []error { // collect webhooks by crd crdToWh := map[string][]string{} for _, wh := range rv1.CSV.Spec.WebhookDefinitions { @@ -260,7 +243,7 @@ func CheckConversionWebhookCRDReferenceUniqueness(rv1 *render.RegistryV1) []erro } // CheckWebhookNameIsDNS1123SubDomain checks each webhook configuration name complies with the Kubernetes resource naming conversions -func CheckWebhookNameIsDNS1123SubDomain(rv1 *render.RegistryV1) []error { +func CheckWebhookNameIsDNS1123SubDomain(rv1 *bundle.RegistryV1) []error { invalidWebhooksByType := map[v1alpha1.WebhookAdmissionType]map[string][]string{} for _, wh := range rv1.CSV.Spec.WebhookDefinitions { if _, ok := invalidWebhooksByType[wh.Type]; !ok { diff --git a/internal/operator-controller/rukpak/render/validators/validator_test.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go similarity index 92% rename from internal/operator-controller/rukpak/render/validators/validator_test.go rename to internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go index b6feae4bd..6c1d7491b 100644 --- a/internal/operator-controller/rukpak/render/validators/validator_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go @@ -2,7 +2,6 @@ package validators_test import ( "errors" - "reflect" "testing" "github.com/stretchr/testify/require" @@ -11,41 +10,20 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1/validators" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) -func Test_BundleValidatorHasAllValidationFns(t *testing.T) { - expectedValidationFns := []func(v1 *render.RegistryV1) []error{ - validators.CheckDeploymentSpecUniqueness, - validators.CheckDeploymentNameIsDNS1123SubDomain, - validators.CheckCRDResourceUniqueness, - validators.CheckOwnedCRDExistence, - validators.CheckPackageNameNotEmpty, - validators.CheckWebhookDeploymentReferentialIntegrity, - validators.CheckWebhookNameUniqueness, - validators.CheckWebhookNameIsDNS1123SubDomain, - validators.CheckConversionWebhookCRDReferenceUniqueness, - validators.CheckConversionWebhooksReferenceOwnedCRDs, - } - actualValidationFns := validators.RegistryV1BundleValidator - - require.Equal(t, len(expectedValidationFns), len(actualValidationFns)) - for i := range expectedValidationFns { - require.Equal(t, reflect.ValueOf(expectedValidationFns[i]).Pointer(), reflect.ValueOf(actualValidationFns[i]).Pointer(), "bundle validator has unexpected validation function") - } -} - func Test_CheckDeploymentSpecUniqueness(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with unique deployment strategy spec names", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -56,7 +34,7 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with duplicate deployment strategy spec names", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -70,7 +48,7 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { }, }, { name: "errors are ordered by deployment strategy spec name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-a"}, @@ -97,12 +75,12 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { func Test_CheckDeploymentNameIsDNS1123SubDomain(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts valid deployment strategy spec names", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -113,7 +91,7 @@ func Test_CheckDeploymentNameIsDNS1123SubDomain(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with invalid deployment strategy spec names - errors are sorted by name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "-bad-name"}, @@ -139,12 +117,12 @@ func Test_CheckDeploymentNameIsDNS1123SubDomain(t *testing.T) { func Test_CRDResourceUniqueness(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with unique custom resource definition resources", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b.crd.something"}}, @@ -153,7 +131,7 @@ func Test_CRDResourceUniqueness(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with duplicate custom resource definition resources", - bundle: &render.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ + bundle: &bundle.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, }}, @@ -162,7 +140,7 @@ func Test_CRDResourceUniqueness(t *testing.T) { }, }, { name: "errors are ordered by custom resource definition name", - bundle: &render.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ + bundle: &bundle.RegistryV1{CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "c.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "c.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, @@ -184,12 +162,12 @@ func Test_CRDResourceUniqueness(t *testing.T) { func Test_CheckOwnedCRDExistence(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with existing owned custom resource definition resources", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{ {ObjectMeta: metav1.ObjectMeta{Name: "a.crd.something"}}, {ObjectMeta: metav1.ObjectMeta{Name: "b.crd.something"}}, @@ -204,7 +182,7 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { expectedErrs: []error{}, }, { name: "rejects bundles with missing owned custom resource definition resources", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{}, CSV: MakeCSV( WithOwnedCRDs(v1alpha1.CRDDescription{Name: "a.crd.something"}), @@ -215,7 +193,7 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { }, }, { name: "errors are ordered by owned custom resource definition name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CRDs: []apiextensionsv1.CustomResourceDefinition{}, CSV: MakeCSV( WithOwnedCRDs( @@ -242,17 +220,17 @@ func Test_CheckOwnedCRDExistence(t *testing.T) { func Test_CheckPackageNameNotEmpty(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with non-empty package name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ PackageName: "not-empty", }, }, { name: "rejects bundles with empty package name", - bundle: &render.RegistryV1{}, + bundle: &bundle.RegistryV1{}, expectedErrs: []error{ errors.New("package name is empty"), }, @@ -268,12 +246,12 @@ func Test_CheckPackageNameNotEmpty(t *testing.T) { func Test_CheckWebhookSupport(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles with validating webhook definitions when they only support AllNamespaces install mode", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithWebhookDefinitions( @@ -286,7 +264,7 @@ func Test_CheckWebhookSupport(t *testing.T) { }, { name: "accepts bundles with mutating webhook definitions when they only support AllNamespaces install mode", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithWebhookDefinitions( @@ -299,7 +277,7 @@ func Test_CheckWebhookSupport(t *testing.T) { }, { name: "accepts bundles with conversion webhook definitions when they only support AllNamespaces install mode", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithWebhookDefinitions( @@ -312,7 +290,7 @@ func Test_CheckWebhookSupport(t *testing.T) { }, { name: "rejects bundles with validating webhook definitions when they support more modes than AllNamespaces install mode", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), WithWebhookDefinitions( @@ -326,7 +304,7 @@ func Test_CheckWebhookSupport(t *testing.T) { }, { name: "accepts bundles with mutating webhook definitions when they support more modes than AllNamespaces install mode", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), WithWebhookDefinitions( @@ -340,7 +318,7 @@ func Test_CheckWebhookSupport(t *testing.T) { }, { name: "accepts bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), WithWebhookDefinitions( @@ -363,12 +341,12 @@ func Test_CheckWebhookSupport(t *testing.T) { func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles where webhook definitions reference existing strategy deployment specs", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -385,7 +363,7 @@ func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { }, }, { name: "rejects bundles with webhook definitions that reference non-existing strategy deployment specs", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -404,7 +382,7 @@ func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { }, }, { name: "errors are ordered by deployment strategy spec name, webhook type, and webhook name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, @@ -461,17 +439,17 @@ func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { func Test_CheckWebhookNameUniqueness(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles without webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV(), }, }, { name: "accepts bundles with unique webhook names", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -498,7 +476,7 @@ func Test_CheckWebhookNameUniqueness(t *testing.T) { }, }, { name: "accepts bundles with webhooks with the same name but different types", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -516,7 +494,7 @@ func Test_CheckWebhookNameUniqueness(t *testing.T) { }, }, { name: "rejects bundles with duplicate validating webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -534,7 +512,7 @@ func Test_CheckWebhookNameUniqueness(t *testing.T) { }, }, { name: "rejects bundles with duplicate mutating webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -552,7 +530,7 @@ func Test_CheckWebhookNameUniqueness(t *testing.T) { }, }, { name: "rejects bundles with duplicate conversion webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -570,7 +548,7 @@ func Test_CheckWebhookNameUniqueness(t *testing.T) { }, }, { name: "orders errors by webhook type and name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -637,15 +615,15 @@ func Test_CheckWebhookNameUniqueness(t *testing.T) { func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles without webhook definitions", - bundle: &render.RegistryV1{}, + bundle: &bundle.RegistryV1{}, }, { name: "accepts bundles without conversion webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -661,7 +639,7 @@ func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) { }, }, { name: "accepts bundles with conversion webhooks that reference owned CRDs", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithOwnedCRDs( v1alpha1.CRDDescription{Name: "some.crd.something"}, @@ -681,7 +659,7 @@ func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) { }, }, { name: "rejects bundles with conversion webhooks that reference existing CRDs that are not owned", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithOwnedCRDs( v1alpha1.CRDDescription{Name: "some.crd.something"}, @@ -703,7 +681,7 @@ func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) { }, }, { name: "errors are ordered by webhook name and CRD name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithOwnedCRDs( v1alpha1.CRDDescription{Name: "b.crd.something"}, @@ -751,17 +729,17 @@ func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) { func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles without webhook definitions", - bundle: &render.RegistryV1{}, + bundle: &bundle.RegistryV1{}, expectedErrs: []error{}, }, { name: "accepts bundles without conversion webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ @@ -779,7 +757,7 @@ func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { }, { name: "accepts bundles with conversion webhooks that reference different CRDs", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithOwnedCRDs( v1alpha1.CRDDescription{Name: "some.crd.something"}, @@ -807,7 +785,7 @@ func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { }, { name: "rejects bundles with conversion webhooks that reference the same CRD", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithOwnedCRDs( v1alpha1.CRDDescription{Name: "some.crd.something"}, @@ -836,7 +814,7 @@ func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { }, { name: "errors are ordered by CRD name and webhook names", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithOwnedCRDs( v1alpha1.CRDDescription{Name: "b.crd.something"}, @@ -885,17 +863,17 @@ func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { func Test_CheckWebhookNameIsDNS1123SubDomain(t *testing.T) { for _, tc := range []struct { name string - bundle *render.RegistryV1 + bundle *bundle.RegistryV1 expectedErrs []error }{ { name: "accepts bundles without webhook definitions", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV(), }, }, { name: "rejects bundles with invalid webhook definitions names and orders errors by webhook type and name", - bundle: &render.RegistryV1{ + bundle: &bundle.RegistryV1{ CSV: MakeCSV( WithWebhookDefinitions( v1alpha1.WebhookDescription{ diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index a9dbb6a84..70063f1d4 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -4,29 +4,21 @@ import ( "errors" "fmt" - 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" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) -type RegistryV1 struct { - PackageName string - CSV v1alpha1.ClusterServiceVersion - CRDs []apiextensionsv1.CustomResourceDefinition - Others []unstructured.Unstructured -} - // BundleValidator validates a RegistryV1 bundle by executing a series of // checks on it and collecting any errors that were found -type BundleValidator []func(v1 *RegistryV1) []error +type BundleValidator []func(v1 *bundle.RegistryV1) []error -func (v BundleValidator) Validate(rv1 *RegistryV1) error { +func (v BundleValidator) Validate(rv1 *bundle.RegistryV1) error { var errs []error for _, validator := range v { errs = append(errs, validator(rv1)...) @@ -35,9 +27,9 @@ func (v BundleValidator) Validate(rv1 *RegistryV1) error { } // ResourceGenerator generates resources given a registry+v1 bundle and options -type ResourceGenerator func(rv1 *RegistryV1, opts Options) ([]client.Object, error) +type ResourceGenerator func(rv1 *bundle.RegistryV1, opts Options) ([]client.Object, error) -func (g ResourceGenerator) GenerateResources(rv1 *RegistryV1, opts Options) ([]client.Object, error) { +func (g ResourceGenerator) GenerateResources(rv1 *bundle.RegistryV1, opts Options) ([]client.Object, error) { return g(rv1, opts) } @@ -45,7 +37,7 @@ func (g ResourceGenerator) GenerateResources(rv1 *RegistryV1, opts Options) ([]c // generated resources. type ResourceGenerators []ResourceGenerator -func (r ResourceGenerators) GenerateResources(rv1 *RegistryV1, opts Options) ([]client.Object, error) { +func (r ResourceGenerators) GenerateResources(rv1 *bundle.RegistryV1, opts Options) ([]client.Object, error) { //nolint:prealloc var renderedObjects []client.Object for _, generator := range r { @@ -80,7 +72,7 @@ func (o *Options) apply(opts ...Option) *Options { return o } -func (o *Options) validate(rv1 *RegistryV1) (*Options, []error) { +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")) @@ -119,7 +111,7 @@ type BundleRenderer struct { ResourceGenerators []ResourceGenerator } -func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, opts ...Option) ([]client.Object, error) { +func (r BundleRenderer) Render(rv1 bundle.RegistryV1, installNamespace string, opts ...Option) ([]client.Object, error) { // validate bundle if err := r.BundleValidator.Validate(&rv1); err != nil { return nil, err @@ -154,7 +146,7 @@ func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) { return util.ObjectNameForBaseAndSuffix(base, hashStr), nil } -func validateTargetNamespaces(rv1 *RegistryV1, installNamespace string, targetNamespaces []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 { diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go index 23455a8be..0760c4fa1 100644 --- a/internal/operator-controller/rukpak/render/render_test.go +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -13,6 +13,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) @@ -20,7 +21,7 @@ import ( func Test_BundleRenderer_NoConfig(t *testing.T) { renderer := render.BundleRenderer{} objs, err := renderer.Render( - render.RegistryV1{ + bundle.RegistryV1{ CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), }, "", nil) require.NoError(t, err) @@ -30,12 +31,12 @@ func Test_BundleRenderer_NoConfig(t *testing.T) { func Test_BundleRenderer_ValidatesBundle(t *testing.T) { renderer := render.BundleRenderer{ BundleValidator: render.BundleValidator{ - func(v1 *render.RegistryV1) []error { + func(v1 *bundle.RegistryV1) []error { return []error{errors.New("this bundle is invalid")} }, }, } - objs, err := renderer.Render(render.RegistryV1{}, "") + objs, err := renderer.Render(bundle.RegistryV1{}, "") require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "this bundle is invalid") @@ -48,7 +49,7 @@ func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) { renderer := render.BundleRenderer{ ResourceGenerators: []render.ResourceGenerator{ - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { require.Equal(t, expectedInstallNamespace, opts.InstallNamespace) require.Equal(t, expectedTargetNamespaces, opts.TargetNamespaces) require.Equal(t, reflect.ValueOf(expectedUniqueNameGenerator).Pointer(), reflect.ValueOf(render.DefaultUniqueNameGenerator).Pointer(), "options has unexpected default unique name generator") @@ -57,7 +58,7 @@ func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) { }, } - _, _ = renderer.Render(render.RegistryV1{}, expectedInstallNamespace) + _, _ = renderer.Render(bundle.RegistryV1{}, expectedInstallNamespace) } func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { @@ -164,7 +165,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { t.Run(tc.name, func(t *testing.T) { renderer := render.BundleRenderer{} _, err := renderer.Render( - render.RegistryV1{CSV: tc.csv}, + bundle.RegistryV1{CSV: tc.csv}, tc.installNamespace, tc.opts..., ) @@ -180,7 +181,7 @@ func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { func Test_BundleRenderer_AppliesUserOptions(t *testing.T) { isOptionApplied := false - _, _ = render.BundleRenderer{}.Render(render.RegistryV1{}, "install-namespace", func(options *render.Options) { + _, _ = render.BundleRenderer{}.Render(bundle.RegistryV1{}, "install-namespace", func(options *render.Options) { isOptionApplied = true }) require.True(t, isOptionApplied) @@ -215,16 +216,16 @@ func Test_WithCertificateProvide(t *testing.T) { func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { renderer := render.BundleRenderer{ ResourceGenerators: []render.ResourceGenerator{ - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return []client.Object{&corev1.Namespace{}, &corev1.Service{}}, nil }, - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return []client.Object{&appsv1.Deployment{}}, nil }, }, } objs, err := renderer.Render( - render.RegistryV1{ + bundle.RegistryV1{ CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), }, "") require.NoError(t, err) @@ -234,16 +235,16 @@ func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) { renderer := render.BundleRenderer{ ResourceGenerators: []render.ResourceGenerator{ - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return []client.Object{&corev1.Namespace{}, &corev1.Service{}}, nil }, - func(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { return nil, fmt.Errorf("generator error") }, }, } objs, err := renderer.Render( - render.RegistryV1{ + bundle.RegistryV1{ CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), }, "") require.Nil(t, objs) @@ -254,11 +255,11 @@ func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) { func Test_BundleValidatorCallsAllValidationFnsInOrder(t *testing.T) { actual := "" val := render.BundleValidator{ - func(v1 *render.RegistryV1) []error { + func(v1 *bundle.RegistryV1) []error { actual += "h" return nil }, - func(v1 *render.RegistryV1) []error { + func(v1 *bundle.RegistryV1) []error { actual += "i" return nil }, diff --git a/internal/operator-controller/rukpak/util/testing/testing.go b/internal/operator-controller/rukpak/util/testing/testing.go index 0a4ec84fe..f5c9b36a3 100644 --- a/internal/operator-controller/rukpak/util/testing/testing.go +++ b/internal/operator-controller/rukpak/util/testing/testing.go @@ -10,6 +10,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) @@ -72,6 +73,12 @@ func WithWebhookDefinitions(webhookDefinitions ...v1alpha1.WebhookDescription) C } } +func WithOwnedAPIServiceDescriptions(ownedAPIServiceDescriptions ...v1alpha1.APIServiceDescription) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Spec.APIServiceDefinitions.Owned = ownedAPIServiceDescriptions + } +} + func MakeCSV(opts ...CSVOption) v1alpha1.ClusterServiceVersion { csv := v1alpha1.ClusterServiceVersion{ TypeMeta: metav1.TypeMeta{ @@ -103,6 +110,12 @@ func (f FakeCertProvider) GetCertSecretInfo(cfg render.CertificateProvisionerCon return f.GetCertSecretInfoFn(cfg) } +type FakeBundleSource func() (bundle.RegistryV1, error) + +func (f FakeBundleSource) GetBundle() (bundle.RegistryV1, error) { + return f() +} + func ToUnstructuredT(t *testing.T, obj client.Object) *unstructured.Unstructured { u, err := util.ToUnstructured(obj) require.NoError(t, err) diff --git a/test/convert/generate-manifests.go b/test/convert/generate-manifests.go index c80d0c06f..147b05e36 100644 --- a/test/convert/generate-manifests.go +++ b/test/convert/generate-manifests.go @@ -11,7 +11,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" ) func main() { @@ -60,14 +62,14 @@ func main() { func generateManifests(outputPath, bundleDir, installNamespace, watchNamespace string) error { // Parse bundleFS into RegistryV1 - regv1, err := convert.ParseFS(os.DirFS(bundleDir)) + regv1, err := source.FromFS(os.DirFS(bundleDir)).GetBundle() if err != nil { fmt.Printf("error parsing bundle directory: %v\n", err) os.Exit(1) } // Convert RegistryV1 to plain manifests - plain, err := convert.PlainConverter.Convert(regv1, installNamespace, []string{watchNamespace}) + objs, err := registryv1.Renderer.Render(regv1, installNamespace, render.WithTargetNamespaces(watchNamespace)) if err != nil { return fmt.Errorf("error converting registry+v1 bundle: %w", err) } @@ -78,7 +80,7 @@ func generateManifests(outputPath, bundleDir, installNamespace, watchNamespace s } if err := func() error { - for idx, obj := range slices.SortedFunc(slices.Values(plain.Objects), orderByKindNamespaceName) { + for idx, obj := range slices.SortedFunc(slices.Values(objs), orderByKindNamespaceName) { kind := obj.GetObjectKind().GroupVersionKind().Kind fileName := fmt.Sprintf("%02d_%s_%s.yaml", idx, strings.ToLower(kind), obj.GetName()) data, err := yaml.Marshal(obj)