diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 0ebce0f71..c3241ce63 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -448,10 +448,18 @@ func run() error { return err } + certProvider := getCertificateProvider() + regv1ManifestProvider := &applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, + IsWebhookSupportEnabled: certProvider != nil, + IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), + } + if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { - err = setupBoxcutter(mgr, ceReconciler, preflights) + err = setupBoxcutter(mgr, ceReconciler, preflights, regv1ManifestProvider) } else { - err = setupHelm(mgr, ceReconciler, preflights, ceController, clusterExtensionFinalizers) + err = setupHelm(mgr, ceReconciler, preflights, ceController, clusterExtensionFinalizers, regv1ManifestProvider) } if err != nil { setupLog.Error(err, "unable to setup lifecycler") @@ -512,9 +520,12 @@ func getCertificateProvider() render.CertificateProvider { return nil } -func setupBoxcutter(mgr manager.Manager, ceReconciler *controllers.ClusterExtensionReconciler, preflights []applier.Preflight) error { - certProvider := getCertificateProvider() - +func setupBoxcutter( + mgr manager.Manager, + ceReconciler *controllers.ClusterExtensionReconciler, + preflights []applier.Preflight, + regv1ManifestProvider applier.ManifestProvider, +) error { coreClient, err := corev1client.NewForConfig(mgr.GetConfig()) if err != nil { return fmt.Errorf("unable to create core client: %w", err) @@ -541,11 +552,8 @@ func setupBoxcutter(mgr manager.Manager, ceReconciler *controllers.ClusterExtens // TODO: better scheme handling - which types do we want to support? _ = apiextensionsv1.AddToScheme(mgr.GetScheme()) rg := &applier.SimpleRevisionGenerator{ - Scheme: mgr.GetScheme(), - BundleRenderer: &applier.RegistryV1BundleRenderer{ - BundleRenderer: registryv1.Renderer, - CertificateProvider: certProvider, - }, + Scheme: mgr.GetScheme(), + ManifestProvider: regv1ManifestProvider, } ceReconciler.Applier = &applier.Boxcutter{ Client: mgr.GetClient(), @@ -611,6 +619,7 @@ func setupHelm( preflights []applier.Preflight, ceController crcontroller.Controller, clusterExtensionFinalizers crfinalizer.Registerer, + regv1ManifestProvider applier.ManifestProvider, ) error { coreClient, err := corev1client.NewForConfig(mgr.GetConfig()) if err != nil { @@ -658,17 +667,12 @@ func setupHelm( return err } - certProvider := getCertificateProvider() - // now initialize the helmApplier, assigning the potentially nil preAuth ceReconciler.Applier = &applier.Helm{ ActionClientGetter: acg, Preflights: preflights, HelmChartProvider: &applier.RegistryV1HelmChartProvider{ - BundleRenderer: registryv1.Renderer, - CertificateProvider: certProvider, - IsWebhookSupportEnabled: certProvider != nil, - IsSingleOwnNamespaceEnabled: features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport), + ManifestProvider: regv1ManifestProvider, }, HelmReleaseToObjectsConverter: &applier.HelmReleaseToObjectsConverter{}, PreAuthorizer: preAuth, diff --git a/commitchecker.yaml b/commitchecker.yaml index 33a29d3f9..69e6b3a5a 100644 --- a/commitchecker.yaml +++ b/commitchecker.yaml @@ -1,4 +1,4 @@ -expectedMergeBase: 6604f2a4e24ca0c4abce99389b1e5dbbe8d8dbfa +expectedMergeBase: 56213a4044e0acadc639a8c2eff38d2a4271cc12 upstreamBranch: main upstreamOrg: operator-framework upstreamRepo: operator-controller diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index e56304d98..14159c180 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -27,8 +27,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" hashutil "github.com/operator-framework/operator-controller/internal/shared/util/hash" ) @@ -46,8 +44,8 @@ type ClusterExtensionRevisionGenerator interface { } type SimpleRevisionGenerator struct { - Scheme *runtime.Scheme - BundleRenderer BundleRenderer + Scheme *runtime.Scheme + ManifestProvider ManifestProvider } func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( @@ -92,7 +90,7 @@ func (r *SimpleRevisionGenerator) GenerateRevision( objectLabels, revisionAnnotations map[string]string, ) (*ocv1.ClusterExtensionRevision, error) { // extract plain manifests - plain, err := r.BundleRenderer.Render(bundleFS, ext) + plain, err := r.ManifestProvider.Get(bundleFS, ext) if err != nil { return nil, err } @@ -359,34 +357,6 @@ func latestRevisionNumber(prevRevisions []ocv1.ClusterExtensionRevision) int64 { return prevRevisions[len(prevRevisions)-1].Spec.Revision } -// TODO: in the next refactor iteration BundleRenderer and RegistryV1BundleRenderer into the RegistryV1ChartProvider - -type BundleRenderer interface { - Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) -} - -type RegistryV1BundleRenderer struct { - BundleRenderer render.BundleRenderer - CertificateProvider render.CertificateProvider -} - -func (r *RegistryV1BundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { - reg, err := source.FromFS(bundleFS).GetBundle() - if err != nil { - return nil, err - } - - if len(reg.CSV.Spec.WebhookDefinitions) > 0 && r.CertificateProvider == nil { - return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported") - } - - watchNamespace, err := GetWatchNamespace(ext) - if err != nil { - return nil, err - } - return r.BundleRenderer.Render(reg, ext.Spec.Namespace, render.WithTargetNamespaces(watchNamespace), render.WithCertificateProvider(r.CertificateProvider)) -} - func splitManifestDocuments(file string) []string { //nolint:prealloc var docs []string diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go index cd38ac5ed..5679f4b8a 100644 --- a/internal/operator-controller/applier/boxcutter_test.go +++ b/internal/operator-controller/applier/boxcutter_test.go @@ -24,75 +24,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "github.com/operator-framework/api/pkg/operators/v1alpha1" - ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" "github.com/operator-framework/operator-controller/internal/operator-controller/controllers" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" - "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/bundlefs" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion" ) -func Test_RegistryV1BundleRenderer_Render_Success(t *testing.T) { - expectedObjs := []client.Object{ - &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-service", - }, - }, - } - r := applier.RegistryV1BundleRenderer{ - BundleRenderer: render.BundleRenderer{ - ResourceGenerators: []render.ResourceGenerator{ - func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { - require.Equal(t, []string{""}, opts.TargetNamespaces) - require.Equal(t, "some-namespace", opts.InstallNamespace) - return expectedObjs, nil - }, - }, - }, - } - bundleFS := bundlefs.Builder(). - WithPackageName("some-package"). - WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). - Build() - objs, err := r.Render(bundleFS, &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "some-namespace", - }, - }) - require.NoError(t, err) - require.Equal(t, expectedObjs, objs) -} - -func Test_RegistryV1BundleRenderer_Render_Failure(t *testing.T) { - var expectedObjs []client.Object - r := applier.RegistryV1BundleRenderer{ - BundleRenderer: render.BundleRenderer{ - ResourceGenerators: []render.ResourceGenerator{ - func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { - return expectedObjs, fmt.Errorf("some-error") - }, - }, - }, - } - bundleFS := bundlefs.Builder(). - WithPackageName("some-package"). - WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). - Build() - objs, err := r.Render(bundleFS, &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "some-namespace", - }, - }) - require.Nil(t, objs) - require.Error(t, err) - require.Contains(t, err.Error(), "some-error") -} - func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) { g := &applier.SimpleRevisionGenerator{} @@ -175,24 +112,26 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T) } func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) { - var r mockBundleRenderer = func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) { - return []client.Object{ - &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-service", + r := &FakeManifestProvider{ + GetFn: func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) { + return []client.Object{ + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, }, - }, - &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-deployment", + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + }, }, - }, - }, nil + }, nil + }, } b := applier.SimpleRevisionGenerator{ - Scheme: k8scheme.Scheme, - BundleRenderer: r, + Scheme: k8scheme.Scheme, + ManifestProvider: r, } ext := &ocv1.ClusterExtension{ @@ -266,15 +205,17 @@ func Test_SimpleRevisionGenerator_Renderer_Integration(t *testing.T) { Name: "test-extension", }, } - var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { - t.Log("by checking renderer was called with the correct parameters") - require.Equal(t, bundleFS, b) - require.Equal(t, ext, e) - return nil, nil + r := &FakeManifestProvider{ + GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { + t.Log("by checking renderer was called with the correct parameters") + require.Equal(t, bundleFS, b) + require.Equal(t, ext, e) + return nil, nil + }, } b := applier.SimpleRevisionGenerator{ - Scheme: k8scheme.Scheme, - BundleRenderer: r, + Scheme: k8scheme.Scheme, + ManifestProvider: r, } _, err := b.GenerateRevision(bundleFS, ext, map[string]string{}, map[string]string{}) @@ -300,12 +241,15 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t }, }, } - var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { - return renderedObjs, nil + r := &FakeManifestProvider{ + GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { + return renderedObjs, nil + }, } + b := applier.SimpleRevisionGenerator{ - Scheme: k8scheme.Scheme, - BundleRenderer: r, + Scheme: k8scheme.Scheme, + ManifestProvider: r, } revAnnotations := map[string]string{ @@ -330,12 +274,14 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t } func Test_SimpleRevisionGenerator_Failure(t *testing.T) { - var r mockBundleRenderer = func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { - return nil, fmt.Errorf("some-error") + r := &FakeManifestProvider{ + GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) { + return nil, fmt.Errorf("some-error") + }, } b := applier.SimpleRevisionGenerator{ - Scheme: k8scheme.Scheme, - BundleRenderer: r, + Scheme: k8scheme.Scheme, + ManifestProvider: r, } rev, err := b.GenerateRevision(fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{}, map[string]string{}) @@ -928,12 +874,6 @@ func (m *mockBundleRevisionBuilder) GenerateRevisionFromHelmRelease( return nil, nil } -type mockBundleRenderer func(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) - -func (f mockBundleRenderer) Render(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { - return f(bundleFS, ext) -} - type clientMock struct { mock.Mock } diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index 3723fbc0e..4e7026894 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -29,14 +29,13 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" "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/util" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) // HelmChartProvider provides helm charts from bundle sources and cluster extensions type HelmChartProvider interface { - Get(bundle source.BundleSource, clusterExtension *ocv1.ClusterExtension) (*chart.Chart, error) + Get(bundle fs.FS, clusterExtension *ocv1.ClusterExtension) (*chart.Chart, error) } type HelmReleaseToObjectsConverter struct { @@ -212,7 +211,7 @@ func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*char ) } } - return h.HelmChartProvider.Get(source.FromFS(bundleFS), ext) + return h.HelmChartProvider.Get(bundleFS, ext) } 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 7bed5e263..8d07de912 100644 --- a/internal/operator-controller/applier/helm_test.go +++ b/internal/operator-controller/applier/helm_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "io" + "io/fs" "os" "testing" "testing/fstest" @@ -23,7 +24,6 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/authorization" "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager" cmcache "github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager/cache" - "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source" ) var _ contentmanager.Manager = (*mockManagedContentCacheManager)(nil) @@ -239,7 +239,7 @@ func TestApply_Base(t *testing.T) { mockAcg := &mockActionGetter{actionClientForErr: errors.New("failed getting action client")} helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -253,7 +253,7 @@ func TestApply_Base(t *testing.T) { mockAcg := &mockActionGetter{getClientErr: errors.New("failed getting current release")} helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -272,7 +272,7 @@ func TestApply_Installation(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -291,7 +291,7 @@ func TestApply_Installation(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -309,7 +309,7 @@ func TestApply_Installation(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -330,7 +330,7 @@ func TestApply_Installation(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, Manager: &mockManagedContentCacheManager{ cache: &mockManagedContentCache{}, @@ -352,7 +352,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -376,7 +376,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, PreAuthorizer: &mockPreAuthorizer{nil, nil}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -398,7 +398,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -427,7 +427,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -456,7 +456,7 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, PreAuthorizer: &mockPreAuthorizer{nil, nil}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, Manager: &mockManagedContentCacheManager{ cache: &mockManagedContentCache{}, @@ -491,7 +491,7 @@ func TestApply_Upgrade(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, } installSucceeded, installStatus, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -514,7 +514,7 @@ func TestApply_Upgrade(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -537,7 +537,7 @@ func TestApply_Upgrade(t *testing.T) { mockPf := &mockPreflight{} helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -561,7 +561,7 @@ func TestApply_Upgrade(t *testing.T) { helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, } @@ -582,7 +582,7 @@ func TestApply_Upgrade(t *testing.T) { } helmApplier := applier.Helm{ ActionClientGetter: mockAcg, - HelmChartProvider: &applier.RegistryV1HelmChartProvider{}, + HelmChartProvider: DummyHelmChartProvider, HelmReleaseToObjectsConverter: mockHelmReleaseToObjectsConverter{}, Manager: &mockManagedContentCacheManager{ cache: &mockManagedContentCache{}, @@ -606,8 +606,8 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { Manifest: validManifest, }, }, - HelmChartProvider: &fakeRegistryV1HelmChartProvider{ - fn: func(bundle source.BundleSource, ext *ocv1.ClusterExtension) (*chart.Chart, error) { + HelmChartProvider: &FakeHelmChartProvider{ + fn: func(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { require.Equal(t, testCE, ext) return nil, nil }, @@ -630,8 +630,8 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { Manifest: validManifest, }, }, - HelmChartProvider: &fakeRegistryV1HelmChartProvider{ - fn: func(bundle source.BundleSource, ext *ocv1.ClusterExtension) (*chart.Chart, error) { + HelmChartProvider: &FakeHelmChartProvider{ + fn: func(bundleFs fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { return nil, errors.New("some error") }, }, @@ -645,10 +645,16 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { }) } -type fakeRegistryV1HelmChartProvider struct { - fn func(source.BundleSource, *ocv1.ClusterExtension) (*chart.Chart, error) +type FakeHelmChartProvider struct { + fn func(fs.FS, *ocv1.ClusterExtension) (*chart.Chart, error) } -func (f fakeRegistryV1HelmChartProvider) Get(bundle source.BundleSource, ext *ocv1.ClusterExtension) (*chart.Chart, error) { +func (f FakeHelmChartProvider) Get(bundle fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { return f.fn(bundle, ext) } + +var DummyHelmChartProvider = &FakeHelmChartProvider{ + fn: func(fs fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { + return &chart.Chart{}, nil + }, +} diff --git a/internal/operator-controller/applier/provider.go b/internal/operator-controller/applier/provider.go index 8a7123243..ed1a1c5ec 100644 --- a/internal/operator-controller/applier/provider.go +++ b/internal/operator-controller/applier/provider.go @@ -4,9 +4,12 @@ import ( "crypto/sha256" "encoding/json" "fmt" + "io/fs" "helm.sh/helm/v3/pkg/chart" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -15,15 +18,23 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" ) -type RegistryV1HelmChartProvider struct { +// ManifestProvider returns the manifests that should be applied by OLM given a bundle and its associated ClusterExtension +type ManifestProvider interface { + // Get returns a set of resource manifests in bundle that take into account the configuration in ext + Get(bundle fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) +} + +// RegistryV1ManifestProvider generates the manifests that should be installed for a registry+v1 bundle +// given the user specified configuration given by the ClusterExtension API surface +type RegistryV1ManifestProvider struct { BundleRenderer render.BundleRenderer CertificateProvider render.CertificateProvider IsWebhookSupportEnabled bool IsSingleOwnNamespaceEnabled bool } -func (r *RegistryV1HelmChartProvider) Get(bundle source.BundleSource, ext *ocv1.ClusterExtension) (*chart.Chart, error) { - rv1, err := bundle.GetBundle() +func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { + rv1, err := source.FromFS(bundleFS).GetBundle() if err != nil { return nil, err } @@ -57,12 +68,7 @@ func (r *RegistryV1HelmChartProvider) Get(bundle source.BundleSource, ext *ocv1. render.WithCertificateProvider(r.CertificateProvider), } - // TODO: in a follow up PR we'll split this into two components: - // 1. takes a bundle + cluster extension => manifests - // 2. takes a bundle + cluster extension => chart (which will use the component in 1. under the hood) - // GetWatchNamespace will move under the component in 1. and also be reused by the component that - // takes bundle + cluster extension => revision - watchNamespace, err := GetWatchNamespace(ext) + watchNamespace, err := r.getWatchNamespace(ext) if err != nil { return nil, err } @@ -71,13 +77,55 @@ func (r *RegistryV1HelmChartProvider) Get(bundle source.BundleSource, ext *ocv1. opts = append(opts, render.WithTargetNamespaces(watchNamespace)) } - objs, err := r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...) + return r.BundleRenderer.Render(rv1, ext.Spec.Namespace, opts...) +} + +// getWatchNamespace determines the watch namespace the ClusterExtension should use based on the +// configuration in .spec.config.Inline. Only active if SingleOwnNamespace support is enabled. +func (r *RegistryV1ManifestProvider) getWatchNamespace(ext *ocv1.ClusterExtension) (string, error) { + if !r.IsSingleOwnNamespaceEnabled { + return "", nil + } + + var watchNamespace string + if ext.Spec.Config != nil && ext.Spec.Config.Inline != nil { + cfg := struct { + WatchNamespace string `json:"watchNamespace"` + }{} + if err := json.Unmarshal(ext.Spec.Config.Inline.Raw, &cfg); err != nil { + return "", fmt.Errorf("invalid bundle configuration: %w", err) + } + watchNamespace = cfg.WatchNamespace + } else { + return "", nil + } + + if errs := validation.IsDNS1123Subdomain(watchNamespace); len(errs) > 0 { + return "", fmt.Errorf("invalid watch namespace '%s': namespace must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", watchNamespace) + } + + return watchNamespace, nil +} + +// RegistryV1HelmChartProvider creates a Helm-Chart from a registry+v1 bundle and its associated ClusterExtension +type RegistryV1HelmChartProvider struct { + ManifestProvider ManifestProvider +} +func (r *RegistryV1HelmChartProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { + objs, err := r.ManifestProvider.Get(bundleFS, ext) if err != nil { - return nil, fmt.Errorf("error rendering bundle: %w", err) + return nil, err } chrt := &chart.Chart{Metadata: &chart.Metadata{}} + // The need to get the underlying bundle in order to extract its annotations + // will go away once with have a bundle interface that can surface the annotations independently of the + // underlying bundle format... + rv1, err := source.FromFS(bundleFS).GetBundle() + if err != nil { + return nil, err + } chrt.Metadata.Annotations = rv1.CSV.GetAnnotations() for _, obj := range objs { jsonData, err := json.Marshal(obj) diff --git a/internal/operator-controller/applier/provider_test.go b/internal/operator-controller/applier/provider_test.go index 74795f544..1816bdd33 100644 --- a/internal/operator-controller/applier/provider_test.go +++ b/internal/operator-controller/applier/provider_test.go @@ -2,99 +2,115 @@ package applier_test import ( "errors" + "io/fs" "testing" + "testing/fstest" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/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" - featuregatetesting "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/applier" - "github.com/operator-framework/operator-controller/internal/operator-controller/features" "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/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/bundlefs" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing/clusterserviceversion" ) -func Test_RegistryV1HelmChartProvider_Get_ReturnsBundleSourceFailures(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{} - var failingBundleSource FakeBundleSource = func() (bundle.RegistryV1, error) { - return bundle.RegistryV1{}, errors.New("some error") - } - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - }, - } - _, err := provider.Get(failingBundleSource, ext) - require.Error(t, err) - require.Contains(t, err.Error(), "some error") -} +func Test_RegistryV1ManifestProvider_Integration(t *testing.T) { + t.Run("surfaces bundle source errors", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{} + ext := &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + } + _, err := provider.Get(fstest.MapFS{}, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "metadata/annotations.yaml: file does not exist") + }) -func Test_RegistryV1HelmChartProvider_Get_ReturnsBundleRendererFailures(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{ - BundleRenderer: render.BundleRenderer{ - ResourceGenerators: []render.ResourceGenerator{ - func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { - return nil, errors.New("some error") + t.Run("surfaces bundle renderer errors", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + 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: clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - }, - ) + // The contents of the bundle are not important for this tesy, only that it be a valid bundle + // to avoid errors in the deserialization process + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()).Build() - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - }, - } - _, err := provider.Get(b, ext) - require.Error(t, err) - require.Contains(t, err.Error(), "some error") -} + ext := &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + } -func Test_RegistryV1HelmChartProvider_Get_NoAPIServiceDefinitions(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{} + _, err := provider.Get(bundleFS, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "some error") + }) - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{}).Build(), - }, - ) + t.Run("returns rendered manifests", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, + } + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build()). + WithBundleResource("service.yaml", &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + }, + }).Build() + ext := &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + } + objs, err := provider.Get(bundleFS, ext) + require.NoError(t, err) - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - }, - } + exp := ToUnstructuredT(t, &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Service", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: "install-namespace", + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{}, + }, + }) - _, err := provider.Get(b, ext) - require.Error(t, err) - require.Contains(t, err.Error(), "unsupported bundle: apiServiceDefintions are not supported") + require.Equal(t, []client.Object{exp}, objs) + }) } -func Test_RegistryV1HelmChartProvider_Get_SingleOwnNamespace(t *testing.T) { - t.Run("rejects bundles without AllNamespaces install mode support if SingleOwnNamespace is not enabled", func(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{} +func Test_RegistryV1ManifestProvider_APIServiceSupport(t *testing.T) { + t.Run("rejects registry+v1 bundles with API service definitions", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{} - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build(), - }, - ) + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{Name: "test-apiservice"}).Build()).Build() ext := &ocv1.ClusterExtension{ Spec: ocv1.ClusterExtensionSpec{ @@ -102,50 +118,40 @@ func Test_RegistryV1HelmChartProvider_Get_SingleOwnNamespace(t *testing.T) { }, } - _, err := provider.Get(b, ext) + _, err := provider.Get(bundleFS, ext) require.Error(t, err) - require.Contains(t, err.Error(), "unsupported bundle: bundle does not support AllNamespaces install mode") + require.Contains(t, err.Error(), "unsupported bundle: apiServiceDefintions are not supported") }) - t.Run("accepts bundles with SingleNamespace install mode support if SingleOwnNamespace is enabled", func(t *testing.T) { - // TODO: this will be removed in a follow-up PR that will refactor GetWatchNamespace's location - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) - provider := applier.RegistryV1HelmChartProvider{ - IsSingleOwnNamespaceEnabled: true, +} + +func Test_RegistryV1ManifestProvider_WebhookSupport(t *testing.T) { + t.Run("rejects bundles with webhook definitions if support is disabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + IsWebhookSupportEnabled: false, } - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build(), - }, - ) + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithWebhookDefinitions(v1alpha1.WebhookDescription{}).Build()).Build() ext := &ocv1.ClusterExtension{ Spec: ocv1.ClusterExtensionSpec{ Namespace: "install-namespace", - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace": "some-namespace"}`), - }, - }, }, } - _, err := provider.Get(b, ext) - require.NoError(t, err) + _, err := provider.Get(bundleFS, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "webhookDefinitions are not supported") }) - t.Run("accepts bundles with OwnNamespace install mode support if SingleOwnNamespace is enabled", func(t *testing.T) { - // TODO: this will be removed in a follow-up PR that will refactor GetWatchNamespace's location - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) - provider := applier.RegistryV1HelmChartProvider{ - IsSingleOwnNamespaceEnabled: true, + + t.Run("fails if bundle contains webhook definitions, webhook support is enabled, but the certificate provider is undefined", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + IsWebhookSupportEnabled: true, + CertificateProvider: nil, } - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build(), - }, - ) + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithWebhookDefinitions(v1alpha1.WebhookDescription{}).Build()).Build() ext := &ocv1.ClusterExtension{ Spec: ocv1.ClusterExtensionSpec{ @@ -153,184 +159,170 @@ func Test_RegistryV1HelmChartProvider_Get_SingleOwnNamespace(t *testing.T) { }, } - _, err := provider.Get(b, ext) - require.NoError(t, err) + _, err := provider.Get(bundleFS, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "webhookDefinitions are not supported") }) -} - -func Test_RegistryV1HelmChartProvider_Get_NoWebhooksWithoutCertProvider(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{ - IsWebhookSupportEnabled: true, - } - - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithWebhookDefinitions(v1alpha1.WebhookDescription{}).Build(), - }, - ) - - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - }, - } - _, err := provider.Get(b, ext) - require.Error(t, err) - require.Contains(t, err.Error(), "webhookDefinitions are not supported") -} - -func Test_RegistryV1HelmChartProvider_Get_WebhooksSupportDisabled(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{ - IsWebhookSupportEnabled: false, - } + t.Run("accepts bundles with webhook definitions if support is enabled and a certificate provider is defined", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + CertificateProvider: FakeCertProvider{}, + IsWebhookSupportEnabled: true, + } - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithWebhookDefinitions(v1alpha1.WebhookDescription{}).Build(), - }, - ) + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV( + clusterserviceversion.Builder(). + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces). + WithWebhookDefinitions(v1alpha1.WebhookDescription{}).Build()). + Build() - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - }, - } + ext := &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + } - _, err := provider.Get(b, ext) - require.Error(t, err) - require.Contains(t, err.Error(), "webhookDefinitions are not supported") + _, err := provider.Get(bundleFS, ext) + require.NoError(t, err) + }) } -func Test_RegistryV1HelmChartProvider_Get_WebhooksWithCertProvider(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{ - CertificateProvider: FakeCertProvider{}, - IsWebhookSupportEnabled: true, - } - - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder(). - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces). - WithWebhookDefinitions(v1alpha1.WebhookDescription{}).Build(), - }, - ) - - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - }, - } - - _, err := provider.Get(b, ext) - require.NoError(t, err) -} +func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) { + t.Run("rejects bundles without AllNamespaces install mode when Single/OwnNamespace install mode support is disabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + IsSingleOwnNamespaceEnabled: false, + } -func Test_RegistryV1HelmChartProvider_Get_BundleRendererIntegration(t *testing.T) { - expectedInstallNamespace := "install-namespace" - expectedCertProvider := FakeCertProvider{} - watchNamespace := "some-namespace" + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build() - ext := &ocv1.ClusterExtension{ - Spec: ocv1.ClusterExtensionSpec{ - Namespace: "install-namespace", - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace": "` + watchNamespace + `"}`), - }, + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", }, - }, - } - - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace).Build(), - }, - ) + }) + require.Equal(t, "unsupported bundle: bundle does not support AllNamespaces install mode", err.Error()) + }) - t.Run("SingleOwnNamespace install mode support off", func(t *testing.T) { - provider := applier.RegistryV1HelmChartProvider{ + t.Run("accepts bundles without AllNamespaces install mode and with SingleNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) { + expectedWatchNamespace := "some-namespace" + provider := applier.RegistryV1ManifestProvider{ 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, expectedCertProvider, opts.CertificateProvider) - - // target namespaces should not set to {""} (AllNamespaces) if the SingleOwnNamespace feature flag is off - t.Log("check that targetNamespaces option is set to AllNamespaces") - require.Equal(t, []string{""}, opts.TargetNamespaces) + t.Log("ensure watch namespace is appropriately configured") + require.Equal(t, []string{expectedWatchNamespace}, opts.TargetNamespaces) return nil, nil }, }, }, - CertificateProvider: expectedCertProvider, + IsSingleOwnNamespaceEnabled: true, } - _, err := provider.Get(b, ext) + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build() + + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + Config: &ocv1.ClusterExtensionConfig{ + ConfigType: ocv1.ClusterExtensionConfigTypeInline, + Inline: &apiextensionsv1.JSON{ + Raw: []byte(`{"watchNamespace": "` + expectedWatchNamespace + `"}`), + }, + }, + }, + }) require.NoError(t, err) }) - t.Run("feature on", func(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) + t.Run("accepts bundles without AllNamespaces install mode and with OwnNamespace support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + IsSingleOwnNamespaceEnabled: true, + } + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace).Build()).Build() + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + }) + require.NoError(t, err) + }) + + t.Run("rejects bundles without AllNamespaces, SingleNamespace, or OwnNamespace install mode support when Single/OwnNamespace install mode support is enabled", func(t *testing.T) { + provider := applier.RegistryV1ManifestProvider{ + IsSingleOwnNamespaceEnabled: true, + } + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace).Build()).Build() + _, err := provider.Get(bundleFS, &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + }) + require.Equal(t, "unsupported bundle: bundle must support at least one of [AllNamespaces SingleNamespace OwnNamespace] install modes", err.Error()) + }) +} +func Test_RegistryV1HelmChartProvider_Integration(t *testing.T) { + t.Run("surfaces bundle source errors", func(t *testing.T) { provider := applier.RegistryV1HelmChartProvider{ - 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, expectedCertProvider, opts.CertificateProvider) + ManifestProvider: DummyManifestProvider, + } + ext := &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + } + _, err := provider.Get(fstest.MapFS{}, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "metadata/annotations.yaml: file does not exist") + }) - // targetNamespace must be set if the feature flag is on - t.Log("check that targetNamespaces option is set") - require.Equal(t, []string{watchNamespace}, opts.TargetNamespaces) - return nil, nil - }, + t.Run("surfaces manifest provider failures", func(t *testing.T) { + provider := applier.RegistryV1HelmChartProvider{ + ManifestProvider: &FakeManifestProvider{ + GetFn: func(bundle fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { + return nil, errors.New("some error") }, }, - CertificateProvider: expectedCertProvider, } - _, err := provider.Get(b, ext) - require.NoError(t, err) + ext := &ocv1.ClusterExtension{ + Spec: ocv1.ClusterExtensionSpec{ + Namespace: "install-namespace", + }, + } + _, err := provider.Get(fstest.MapFS{}, ext) + require.Error(t, err) + require.Contains(t, err.Error(), "some error") }) } -func Test_RegistryV1HelmChartProvider_Get_Success(t *testing.T) { +func Test_RegistryV1HelmChartProvider_Chart(t *testing.T) { provider := applier.RegistryV1HelmChartProvider{ - 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 - }, - }, + ManifestProvider: &applier.RegistryV1ManifestProvider{ + BundleRenderer: registryv1.Renderer, }, } - b := source.FromBundle( - bundle.RegistryV1{ - CSV: clusterserviceversion.Builder(). + bundleFS := bundlefs.Builder().WithPackageName("test"). + WithCSV( + clusterserviceversion.Builder(). WithAnnotations(map[string]string{"foo": "bar"}). - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces).Build(), - Others: []unstructured.Unstructured{ - *ToUnstructuredT(t, &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - APIVersion: corev1.SchemeGroupVersion.String(), - Kind: "Service", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "testService", - }, - }), + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces). + Build()). + WithBundleResource("service.yaml", &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Service", }, - }, - ) + ObjectMeta: metav1.ObjectMeta{ + Name: "testService", + }, + }).Build() ext := &ocv1.ClusterExtension{ Spec: ocv1.ClusterExtensionSpec{ @@ -338,7 +330,7 @@ func Test_RegistryV1HelmChartProvider_Get_Success(t *testing.T) { }, } - chart, err := provider.Get(b, ext) + chart, err := provider.Get(bundleFS, ext) require.NoError(t, err) require.NotNil(t, chart) require.NotNil(t, chart.Metadata) @@ -349,3 +341,17 @@ func Test_RegistryV1HelmChartProvider_Get_Success(t *testing.T) { t.Log("Check Chart templates have the same number of resources generated by the renderer") require.Len(t, chart.Templates, 1) } + +var DummyManifestProvider = &FakeManifestProvider{ + GetFn: func(bundle fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { + return []client.Object{}, nil + }, +} + +type FakeManifestProvider struct { + GetFn func(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) +} + +func (f *FakeManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtension) ([]client.Object, error) { + return f.GetFn(bundleFS, ext) +} diff --git a/internal/operator-controller/applier/watchnamespace.go b/internal/operator-controller/applier/watchnamespace.go deleted file mode 100644 index 4ef2b2267..000000000 --- a/internal/operator-controller/applier/watchnamespace.go +++ /dev/null @@ -1,40 +0,0 @@ -package applier - -import ( - "encoding/json" - "fmt" - - "k8s.io/apimachinery/pkg/util/validation" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/features" -) - -// GetWatchNamespace determines the watch namespace the ClusterExtension should use -// Note: this is a temporary artifice to enable gated use of single/own namespace install modes -// for registry+v1 bundles. This will go away once the ClusterExtension API is updated to include -// (opaque) runtime configuration. -func GetWatchNamespace(ext *ocv1.ClusterExtension) (string, error) { - if !features.OperatorControllerFeatureGate.Enabled(features.SingleOwnNamespaceInstallSupport) { - return "", nil - } - - var watchNamespace string - if ext.Spec.Config != nil && ext.Spec.Config.Inline != nil { - cfg := struct { - WatchNamespace string `json:"watchNamespace"` - }{} - if err := json.Unmarshal(ext.Spec.Config.Inline.Raw, &cfg); err != nil { - return "", fmt.Errorf("invalid bundle configuration: %w", err) - } - watchNamespace = cfg.WatchNamespace - } else { - return "", nil - } - - if errs := validation.IsDNS1123Subdomain(watchNamespace); len(errs) > 0 { - return "", fmt.Errorf("invalid watch namespace '%s': namespace must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", watchNamespace) - } - - return watchNamespace, nil -} diff --git a/internal/operator-controller/applier/watchnamespace_test.go b/internal/operator-controller/applier/watchnamespace_test.go deleted file mode 100644 index 274ee7212..000000000 --- a/internal/operator-controller/applier/watchnamespace_test.go +++ /dev/null @@ -1,164 +0,0 @@ -package applier_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - featuregatetesting "k8s.io/component-base/featuregate/testing" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" - "github.com/operator-framework/operator-controller/internal/operator-controller/applier" - "github.com/operator-framework/operator-controller/internal/operator-controller/features" -) - -func TestGetWatchNamespacesWhenFeatureGateIsDisabled(t *testing.T) { - watchNamespace, err := applier.GetWatchNamespace(&ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace":"watch-namespace"}`), - }, - }, - }, - }) - require.NoError(t, err) - t.Log("Check watchNamespace is '' even if the configuration is set") - require.Equal(t, corev1.NamespaceAll, watchNamespace) -} - -func TestGetWatchNamespace(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.SingleOwnNamespaceInstallSupport, true) - - for _, tt := range []struct { - name string - want string - ce *ocv1.ClusterExtension - expectError bool - }{ - { - name: "no watch namespace is configured in a ClusterExtension CR", - want: corev1.NamespaceAll, - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - Annotations: nil, - }, - Spec: ocv1.ClusterExtensionSpec{}, - }, - expectError: false, - }, { - name: "a watch namespace is configured in a ClusterExtension CR", - want: "watch-namespace", - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace":"watch-namespace"}`), - }, - }, - }, - }, - expectError: false, - }, { - name: "a watch namespace is configured in a ClusterExtension CR but with invalid namespace", - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace":"watch-namespace-"}`), - }, - }, - }, - }, - expectError: true, - }, { - name: "a watch namespace is configured in a ClusterExtension CR with an empty string as the namespace", - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace":""}`), - }, - }, - }, - }, - expectError: true, - }, { - name: "an invalid watchNamespace value is configured in a ClusterExtension CR: multiple watch namespaces", - want: "", - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace":"watch-namespace,watch-namespace2,watch-namespace3"}`), - }, - }, - }, - }, - expectError: true, - }, { - name: "an invalid watchNamespace value is configured in a ClusterExtension CR: invalid name", - want: "", - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`{"watchNamespace":"watch-namespace-"}`), - }, - }, - }, - }, - expectError: true, - }, { - name: "an invalid watchNamespace value is configured in a ClusterExtension CR: invalid json", - want: "", - ce: &ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Name: "extension", - }, - Spec: ocv1.ClusterExtensionSpec{ - Config: &ocv1.ClusterExtensionConfig{ - ConfigType: ocv1.ClusterExtensionConfigTypeInline, - Inline: &apiextensionsv1.JSON{ - Raw: []byte(`invalid json`), - }, - }, - }, - }, - expectError: true, - }, - } { - t.Run(tt.name, func(t *testing.T) { - got, err := applier.GetWatchNamespace(tt.ce) - require.Equal(t, tt.want, got) - require.Equal(t, tt.expectError, err != nil) - }) - } -} diff --git a/internal/operator-controller/rukpak/util/util.go b/internal/operator-controller/rukpak/util/util.go index 503d7afa7..067722c47 100644 --- a/internal/operator-controller/rukpak/util/util.go +++ b/internal/operator-controller/rukpak/util/util.go @@ -41,7 +41,6 @@ func ToUnstructured(obj client.Object) (*unstructured.Unstructured, error) { return nil, fmt.Errorf("convert %s %q to unstructured: %w", gvk.Kind, obj.GetName(), err) } unstructured.RemoveNestedField(uObj, "metadata", "creationTimestamp") - unstructured.RemoveNestedField(uObj, "status") u.Object = uObj u.SetGroupVersionKind(gvk) return &u, nil diff --git a/vendor/k8s.io/component-base/featuregate/testing/feature_gate.go b/vendor/k8s.io/component-base/featuregate/testing/feature_gate.go deleted file mode 100644 index 1d7fc4676..000000000 --- a/vendor/k8s.io/component-base/featuregate/testing/feature_gate.go +++ /dev/null @@ -1,200 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package testing - -import ( - "fmt" - "strings" - "sync" - - "k8s.io/apimachinery/pkg/util/version" - "k8s.io/component-base/featuregate" -) - -var ( - overrideLock sync.Mutex - featureFlagOverride map[featuregate.Feature]string - emulationVersionOverride string - emulationVersionOverrideValue *version.Version -) - -func init() { - featureFlagOverride = map[featuregate.Feature]string{} -} - -// SetFeatureGateDuringTest sets the specified gate to the specified value for duration of the test. -// Fails when it detects second call to the same flag or is unable to set or restore feature flag. -// -// WARNING: Can leak set variable when called in test calling t.Parallel(), however second attempt to set the same feature flag will cause fatal. -// -// Example use: -// -// featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features., true) -func SetFeatureGateDuringTest(tb TB, gate featuregate.FeatureGate, f featuregate.Feature, value bool) { - tb.Helper() - detectParallelOverrideCleanup := detectParallelOverride(tb, f) - originalValue := gate.Enabled(f) - originalEmuVer := gate.(featuregate.MutableVersionedFeatureGate).EmulationVersion() - originalExplicitlySet := gate.(featuregate.MutableVersionedFeatureGate).ExplicitlySet(f) - - // Specially handle AllAlpha and AllBeta - if f == "AllAlpha" || f == "AllBeta" { - // Iterate over individual gates so their individual values get restored - for k, v := range gate.(featuregate.MutableFeatureGate).GetAll() { - if k == "AllAlpha" || k == "AllBeta" { - continue - } - if (f == "AllAlpha" && v.PreRelease == featuregate.Alpha) || (f == "AllBeta" && v.PreRelease == featuregate.Beta) { - SetFeatureGateDuringTest(tb, gate, k, value) - } - } - } - - if err := gate.(featuregate.MutableFeatureGate).Set(fmt.Sprintf("%s=%v", f, value)); err != nil { - if s := suggestChangeEmulationVersion(tb, gate, f, value); s != "" { - tb.Errorf("error setting %s=%v: %v. %s", f, value, err, s) - } else { - tb.Errorf("error setting %s=%v: %v", f, value, err) - } - } - - tb.Cleanup(func() { - tb.Helper() - detectParallelOverrideCleanup() - emuVer := gate.(featuregate.MutableVersionedFeatureGate).EmulationVersion() - if !emuVer.EqualTo(originalEmuVer) { - tb.Fatalf("change of feature gate emulation version from %s to %s in the chain of SetFeatureGateDuringTest is not allowed\nuse SetFeatureGateEmulationVersionDuringTest to change emulation version in tests", - originalEmuVer.String(), emuVer.String()) - } - if originalExplicitlySet { - if err := gate.(featuregate.MutableFeatureGate).Set(fmt.Sprintf("%s=%v", f, originalValue)); err != nil { - tb.Errorf("error restoring %s=%v: %v", f, originalValue, err) - } - } else { - if err := gate.(featuregate.MutableVersionedFeatureGate).ResetFeatureValueToDefault(f); err != nil { - tb.Errorf("error restoring %s=%v: %v", f, originalValue, err) - } - } - }) -} - -func suggestChangeEmulationVersion(tb TB, gate featuregate.FeatureGate, f featuregate.Feature, value bool) string { - mutableVersionedFeatureGate, ok := gate.(featuregate.MutableVersionedFeatureGate) - if !ok { - return "" - } - - emuVer := mutableVersionedFeatureGate.EmulationVersion() - versionedSpecs, ok := mutableVersionedFeatureGate.GetAllVersioned()[f] - if !ok { - return "" - } - if len(versionedSpecs) > 1 { - // check if the feature is locked - lastLifecycle := versionedSpecs[len(versionedSpecs)-1] - if lastLifecycle.LockToDefault && !lastLifecycle.Version.GreaterThan(emuVer) && lastLifecycle.Default != value { - // if the feature is locked, set the emulation version to the previous version when the feature is not locked. - return fmt.Sprintf("Feature %s is locked at version %s. Try adding SetFeatureGateEmulationVersionDuringTest(t, gate, version.MustParse(\"1.%d\")) at the beginning of your test.", f, emuVer.String(), lastLifecycle.Version.SubtractMinor(1).Minor()) - } - } - return "" -} - -// SetFeatureGateEmulationVersionDuringTest sets the specified gate to the specified emulation version for duration of the test. -// Fails when it detects second call to set a different emulation version or is unable to set or restore emulation version. -// WARNING: Can leak set variable when called in test calling t.Parallel(), however second attempt to set a different emulation version will cause fatal. -// Example use: - -// featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.31")) -func SetFeatureGateEmulationVersionDuringTest(tb TB, gate featuregate.FeatureGate, ver *version.Version) { - tb.Helper() - detectParallelOverrideCleanup := detectParallelOverrideEmulationVersion(tb, ver) - originalEmuVer := gate.(featuregate.MutableVersionedFeatureGate).EmulationVersion() - if err := gate.(featuregate.MutableVersionedFeatureGate).SetEmulationVersion(ver); err != nil { - tb.Fatalf("failed to set emulation version to %s during test: %v", ver.String(), err) - } - tb.Cleanup(func() { - tb.Helper() - detectParallelOverrideCleanup() - if err := gate.(featuregate.MutableVersionedFeatureGate).SetEmulationVersion(originalEmuVer); err != nil { - tb.Fatalf("failed to restore emulation version to %s during test", originalEmuVer.String()) - } - }) -} - -func detectParallelOverride(tb TB, f featuregate.Feature) func() { - tb.Helper() - overrideLock.Lock() - defer overrideLock.Unlock() - beforeOverrideTestName := featureFlagOverride[f] - if beforeOverrideTestName != "" && !sameTestOrSubtest(tb, beforeOverrideTestName) { - tb.Fatalf("Detected parallel setting of a feature gate by both %q and %q", beforeOverrideTestName, tb.Name()) - } - featureFlagOverride[f] = tb.Name() - - return func() { - tb.Helper() - overrideLock.Lock() - defer overrideLock.Unlock() - if afterOverrideTestName := featureFlagOverride[f]; afterOverrideTestName != tb.Name() { - tb.Fatalf("Detected parallel setting of a feature gate between both %q and %q", afterOverrideTestName, tb.Name()) - } - featureFlagOverride[f] = beforeOverrideTestName - } -} - -func detectParallelOverrideEmulationVersion(tb TB, ver *version.Version) func() { - tb.Helper() - overrideLock.Lock() - defer overrideLock.Unlock() - beforeOverrideTestName := emulationVersionOverride - beforeOverrideValue := emulationVersionOverrideValue - if ver.EqualTo(beforeOverrideValue) { - return func() {} - } - if beforeOverrideTestName != "" && !sameTestOrSubtest(tb, beforeOverrideTestName) { - tb.Fatalf("Detected parallel setting of a feature gate emulation version by both %q and %q", beforeOverrideTestName, tb.Name()) - } - emulationVersionOverride = tb.Name() - emulationVersionOverrideValue = ver - - return func() { - tb.Helper() - overrideLock.Lock() - defer overrideLock.Unlock() - if afterOverrideTestName := emulationVersionOverride; afterOverrideTestName != tb.Name() { - tb.Fatalf("Detected parallel setting of a feature gate emulation version between both %q and %q", afterOverrideTestName, tb.Name()) - } - emulationVersionOverride = beforeOverrideTestName - emulationVersionOverrideValue = beforeOverrideValue - } -} - -func sameTestOrSubtest(tb TB, testName string) bool { - // Assumes that "/" is not used in test names. - return tb.Name() == testName || strings.HasPrefix(tb.Name(), testName+"/") -} - -type TB interface { - Cleanup(func()) - Error(args ...any) - Errorf(format string, args ...any) - Fatal(args ...any) - Fatalf(format string, args ...any) - Helper() - Name() string -} diff --git a/vendor/modules.txt b/vendor/modules.txt index 89433dba9..3f19a6069 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1858,7 +1858,6 @@ k8s.io/client-go/util/workqueue k8s.io/component-base/cli/flag k8s.io/component-base/compatibility k8s.io/component-base/featuregate -k8s.io/component-base/featuregate/testing k8s.io/component-base/metrics k8s.io/component-base/metrics/legacyregistry k8s.io/component-base/metrics/prometheus/compatversion