From d14da9c8f572ad17b7f01b80c0f44deee74f22ea Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Fri, 11 Jul 2025 10:49:16 +0200 Subject: [PATCH] Refactor applier to use HelmChartLoader Signed-off-by: Per Goncalves da Silva --- cmd/operator-controller/main.go | 20 +++- internal/operator-controller/applier/helm.go | 40 +++---- .../operator-controller/applier/helm_test.go | 109 ++++++++++-------- .../operator-controller/applier/loader.go | 66 +++++++++++ internal/shared/util/image/helm.go | 48 +++++--- internal/shared/util/image/helm_test.go | 62 ++-------- 6 files changed, 198 insertions(+), 147 deletions(-) create mode 100644 internal/operator-controller/applier/loader.go diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index d426793d4..04a007a6c 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -439,14 +439,26 @@ func run() error { isWebhookSupportEnabled = true } + // create and configure chart loaders + bundleLoaders := map[applier.BundleType]applier.HelmChartLoader{ + applier.BundleTypeRegistryV1: &applier.RegistryV1BundleLoader{ + BundleToHelmChartConverter: convert.BundleToHelmChartConverter{ + BundleRenderer: registryv1.Renderer, + CertificateProvider: certProvider, + IsWebhookSupportEnabled: isWebhookSupportEnabled, + }, + }, + } + if features.OperatorControllerFeatureGate.Enabled(features.HelmChartSupport) { + bundleLoaders[applier.BundleTypeHelm] = &applier.HelmBundleLoader{} + } + // now initialize the helmApplier, assigning the potentially nil preAuth helmApplier := &applier.Helm{ ActionClientGetter: acg, Preflights: preflights, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{ - BundleRenderer: registryv1.Renderer, - CertificateProvider: certProvider, - IsWebhookSupportEnabled: isWebhookSupportEnabled, + HelmChartLoader: applier.BundleFSChartLoader{ + HelmChartLoaders: bundleLoaders, }, PreAuthorizer: preAuth, } diff --git a/internal/operator-controller/applier/helm.go b/internal/operator-controller/applier/helm.go index ecfb3fdc2..7a72175a5 100644 --- a/internal/operator-controller/applier/helm.go +++ b/internal/operator-controller/applier/helm.go @@ -26,11 +26,8 @@ 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/features" - "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" - imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" ) const ( @@ -56,15 +53,19 @@ type Preflight interface { Upgrade(context.Context, *release.Release) error } -type BundleToHelmChartConverter interface { - ToHelmChart(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) +// HelmChartLoader loads helm charts from bundle filesystems +type HelmChartLoader interface { + // Load loads a Helm Chart from the filesytem and applies the Install- and WatchNamespace values (if appropriate). + // If the returned chart is nil and there is no error, this indicates that the underlying filesystem is not a + // Helm bundle. If there is an error, this is indeterminate. + Load(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) } type Helm struct { - ActionClientGetter helmclient.ActionClientGetter - Preflights []Preflight - PreAuthorizer authorization.PreAuthorizer - BundleToHelmChartConverter BundleToHelmChartConverter + ActionClientGetter helmclient.ActionClientGetter + Preflights []Preflight + PreAuthorizer authorization.PreAuthorizer + HelmChartLoader HelmChartLoader } // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND @@ -122,7 +123,7 @@ func (h *Helm) runPreAuthorizationChecks(ctx context.Context, ext *ocv1.ClusterE } func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) { - chrt, err := h.buildHelmChart(contentFS, ext) + chrt, err := h.loadHelmChart(contentFS, ext) if err != nil { return nil, "", err } @@ -203,26 +204,15 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte return relObjects, state, nil } -func (h *Helm) buildHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { - if h.BundleToHelmChartConverter == nil { - return nil, errors.New("BundleToHelmChartConverter is nil") +func (h *Helm) loadHelmChart(bundleFS fs.FS, ext *ocv1.ClusterExtension) (*chart.Chart, error) { + if h.HelmChartLoader == nil { + return nil, fmt.Errorf("HelmChartLoader is not set") } watchNamespace, err := GetWatchNamespace(ext) if err != nil { return nil, err } - if features.OperatorControllerFeatureGate.Enabled(features.HelmChartSupport) { - meta := new(chart.Metadata) - if ok, _ := imageutil.IsBundleSourceChart(bundleFS, meta); ok { - return imageutil.LoadChartFSWithOptions( - bundleFS, - fmt.Sprintf("%s-%s.tgz", meta.Name, meta.Version), - imageutil.WithInstallNamespace(ext.Spec.Namespace), - ) - } - } - - return h.BundleToHelmChartConverter.ToHelmChart(source.FromFS(bundleFS), ext.Spec.Namespace, watchNamespace) + return h.HelmChartLoader.Load(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 89c94df88..d345f1927 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" @@ -26,8 +27,6 @@ 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" ) type mockPreflight struct { @@ -197,8 +196,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, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -211,8 +210,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, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -230,8 +229,8 @@ func TestApply_Installation(t *testing.T) { dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -248,9 +247,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}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -266,8 +265,8 @@ func TestApply_Installation(t *testing.T) { installErr: errors.New("failed installing chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -286,8 +285,8 @@ func TestApply_Installation(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -306,8 +305,8 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { dryRunInstallErr: errors.New("failed attempting to dry-run install chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -328,10 +327,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}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + PreAuthorizer: &mockPreAuthorizer{nil, nil}, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -350,9 +349,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{nil, errPreAuth}, + HelmChartLoader: noOpHelmChartLoader(), } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -379,9 +378,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{missingRBAC, nil}, + HelmChartLoader: noOpHelmChartLoader(), } // Use a ClusterExtension with valid Spec fields. validCE := &ocv1.ClusterExtension{ @@ -408,9 +407,9 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) { }, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - PreAuthorizer: &mockPreAuthorizer{nil, nil}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + PreAuthorizer: &mockPreAuthorizer{nil, nil}, + HelmChartLoader: noOpHelmChartLoader(), } // Use a ClusterExtension with valid Spec fields. @@ -442,8 +441,8 @@ func TestApply_Upgrade(t *testing.T) { dryRunUpgradeErr: errors.New("failed attempting to dry-run upgrade chart"), } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -464,9 +463,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}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -488,7 +487,7 @@ func TestApply_Upgrade(t *testing.T) { mockPf := &mockPreflight{} helmApplier := applier.Helm{ ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -509,9 +508,9 @@ func TestApply_Upgrade(t *testing.T) { } mockPf := &mockPreflight{} helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - Preflights: []applier.Preflight{mockPf}, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + Preflights: []applier.Preflight{mockPf}, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -530,8 +529,8 @@ func TestApply_Upgrade(t *testing.T) { desiredRel: &testDesiredRelease, } helmApplier := applier.Helm{ - ActionClientGetter: mockAcg, - BundleToHelmChartConverter: &convert.BundleToHelmChartConverter{}, + ActionClientGetter: mockAcg, + HelmChartLoader: noOpHelmChartLoader(), } objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels) @@ -556,8 +555,8 @@ func TestApply_InstallationWithSingleOwnNamespaceInstallSupportEnabled(t *testin Manifest: validManifest, }, }, - BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{ - fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + HelmChartLoader: fakeHelmChartLoader{ + fn: func(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { require.Equal(t, expectedWatchNamespace, watchNamespace) return nil, nil }, @@ -589,8 +588,8 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { Manifest: validManifest, }, }, - BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{ - fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + HelmChartLoader: fakeHelmChartLoader{ + fn: func(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { require.Equal(t, expectedWatchNamespace, watchNamespace) return nil, nil }, @@ -609,8 +608,8 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { Manifest: validManifest, }, }, - BundleToHelmChartConverter: &fakeBundleToHelmChartConverter{ - fn: func(bundle source.BundleSource, installNamespace string, watchNamespace string) (*chart.Chart, error) { + HelmChartLoader: fakeHelmChartLoader{ + fn: func(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { return nil, errors.New("some error") }, }, @@ -621,10 +620,18 @@ func TestApply_RegistryV1ToChartConverterIntegration(t *testing.T) { }) } -type fakeBundleToHelmChartConverter struct { - fn func(source.BundleSource, string, string) (*chart.Chart, error) +func noOpHelmChartLoader() applier.HelmChartLoader { + return fakeHelmChartLoader{ + fn: func(bundleFS fs.FS, _ string, _ string) (*chart.Chart, error) { + return nil, nil + }, + } +} + +type fakeHelmChartLoader struct { + fn func(fs.FS, 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) +func (f fakeHelmChartLoader) Load(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return f.fn(bundleFS, installNamespace, watchNamespace) } diff --git a/internal/operator-controller/applier/loader.go b/internal/operator-controller/applier/loader.go new file mode 100644 index 000000000..ff48f691f --- /dev/null +++ b/internal/operator-controller/applier/loader.go @@ -0,0 +1,66 @@ +package applier + +import ( + "fmt" + "io/fs" + + "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/convert" + imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" +) + +const ( + BundleTypeRegistryV1 BundleType = "registry-v1" + BundleTypeHelm BundleType = "helm" +) + +type BundleType string + +type BundleFSChartLoader struct { + HelmChartLoaders map[BundleType]HelmChartLoader +} + +func (b BundleFSChartLoader) Load(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + bundleType, err := b.determineBundleType(bundleFS) + if err != nil { + return nil, fmt.Errorf("error determining bundle type: %w", err) + } + if loader, ok := b.HelmChartLoaders[bundleType]; ok { + return loader.Load(bundleFS, installNamespace, watchNamespace) + } + return nil, fmt.Errorf("unsupported bundle type: %s", bundleType) +} + +func (b BundleFSChartLoader) determineBundleType(bundleFS fs.FS) (BundleType, error) { + isHelmChartBundle, err := imageutil.IsBundleSourceChart(bundleFS) + if err != nil { + return "", err + } + if isHelmChartBundle { + return BundleTypeHelm, nil + } + return BundleTypeRegistryV1, nil +} + +type HelmBundleLoader struct{} + +func (h HelmBundleLoader) Load(bundleFS fs.FS, installNamespace string, _ string) (*chart.Chart, error) { + isHelmChartBundle, err := imageutil.IsBundleSourceChart(bundleFS) + if err != nil { + return nil, err + } + if isHelmChartBundle { + return imageutil.LoadChartFSWithOptions(bundleFS, imageutil.WithInstallNamespace(installNamespace)) + } + return nil, nil +} + +type RegistryV1BundleLoader struct { + convert.BundleToHelmChartConverter +} + +func (r RegistryV1BundleLoader) Load(bundleFS fs.FS, installNamespace string, watchNamespace string) (*chart.Chart, error) { + return r.ToHelmChart(source.FromFS(bundleFS), installNamespace, watchNamespace) +} diff --git a/internal/shared/util/image/helm.go b/internal/shared/util/image/helm.go index c00d4000b..ac7d0439c 100644 --- a/internal/shared/util/image/helm.go +++ b/internal/shared/util/image/helm.go @@ -103,28 +103,25 @@ func inspectChart(data []byte, metadata *chart.Metadata) (chartInspectionResult, return report, nil } -func IsBundleSourceChart(bundleFS fs.FS, metadata *chart.Metadata) (bool, error) { - var chartPath string - files, _ := fs.ReadDir(bundleFS, ".") - for _, file := range files { - if strings.HasSuffix(file.Name(), ".tgz") || - strings.HasSuffix(file.Name(), ".tar.gz") { - chartPath = file.Name() - break - } +// IsBundleSourceChart is a stand-in method until metadata can accompany the bundle filesystem +// and provide a bundle type. If an error is returned, the result is indeterminate. +func IsBundleSourceChart(bundleFS fs.FS) (bool, error) { + chartPath := findChartArchive(bundleFS) + if chartPath == "" { + return false, nil } - chartData, err := fs.ReadFile(bundleFS, chartPath) + f, err := bundleFS.Open(chartPath) if err != nil { return false, err } - result, err := inspectChart(chartData, metadata) + c, err := loader.LoadArchive(f) if err != nil { - return false, fmt.Errorf("reading %s from fs: %w", chartPath, err) + return false, nil } - return (result.templatesExist && result.chartfileExists), nil + return len(c.Templates) > 0 && c.Metadata != nil, nil } type ChartOption func(*chart.Chart) @@ -139,8 +136,12 @@ func WithInstallNamespace(namespace string) ChartOption { } } -func LoadChartFSWithOptions(bundleFS fs.FS, filename string, options ...ChartOption) (*chart.Chart, error) { - ch, err := loadChartFS(bundleFS, filename) +func LoadChartFSWithOptions(bundleFS fs.FS, options ...ChartOption) (*chart.Chart, error) { + chartPath := findChartArchive(bundleFS) + if chartPath == "" { + return nil, errors.New("unsupported filesystem: no chart archive found") + } + ch, err := loadChartFS(bundleFS, chartPath) if err != nil { return nil, err } @@ -167,9 +168,22 @@ func loadChartFS(bundleFS fs.FS, filename string) (*chart.Chart, error) { return nil, fmt.Errorf("chart file name was not provided") } - tarball, err := fs.ReadFile(bundleFS, filename) + f, err := bundleFS.Open(filename) if err != nil { return nil, fmt.Errorf("reading chart %s; %+v", filename, err) } - return loader.LoadArchive(bytes.NewBuffer(tarball)) + return loader.LoadArchive(f) +} + +func findChartArchive(bundleFS fs.FS) string { + var chartPath string + files, _ := fs.ReadDir(bundleFS, ".") + for _, file := range files { + if strings.HasSuffix(file.Name(), ".tgz") || + strings.HasSuffix(file.Name(), ".tar.gz") { + chartPath = file.Name() + break + } + } + return chartPath } diff --git a/internal/shared/util/image/helm_test.go b/internal/shared/util/image/helm_test.go index d7fa6d3de..9fcd99cff 100644 --- a/internal/shared/util/image/helm_test.go +++ b/internal/shared/util/image/helm_test.go @@ -327,8 +327,7 @@ func TestIsBundleSourceChart(t *testing.T) { }, }, want: want{ - value: false, - errStr: "reading testchart-0.1.0.tgz from fs: loading chart archive: Chart.yaml file is missing", + value: false, }, }, { @@ -343,8 +342,7 @@ func TestIsBundleSourceChart(t *testing.T) { }, }, want: want{ - value: false, - errStr: "reading testchart-0.1.0.tgz from fs: loading chart archive: Chart.yaml file is missing", + value: false, }, }, } @@ -352,7 +350,7 @@ func TestIsBundleSourceChart(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { chartFS, _ := createTempFS(t, mockHelmChartTgz(t, tc.args.files)) - got, err := IsBundleSourceChart(chartFS, tc.args.meta) + got, err := IsBundleSourceChart(chartFS) if tc.want.errStr != "" { require.Error(t, err, "chart validation error required") require.EqualError(t, err, tc.want.errStr, "chart error") @@ -466,8 +464,7 @@ func Test_loadChartFS(t *testing.T) { func TestLoadChartFSWithOptions(t *testing.T) { type args struct { - filename string - files []fileContent + files []fileContent } type want struct { name string @@ -481,31 +478,18 @@ func TestLoadChartFSWithOptions(t *testing.T) { expect func(*chart.Chart, want, error) }{ { - name: "empty filename is provided", - args: args{ - filename: "", - files: []fileContent{ - { - name: "testchart/Chart.yaml", - content: []byte("apiVersion: v2\nname: testchart\nversion: 0.1.0"), - }, - { - name: "testchart/templates/deployment.yaml", - content: []byte("kind: Deployment\napiVersion: apps/v1"), - }, - }, - }, + name: "no chart archive in filesystem", + args: args{}, want: want{ - errMsg: "chart file name was not provided", + errMsg: "no chart archive found", }, expect: func(chart *chart.Chart, want want, err error) { - require.Error(t, err, want.errMsg) + require.Contains(t, err.Error(), want.errMsg) }, }, { name: "load sample chart", args: args{ - filename: "testchart-0.1.0.tgz", files: []fileContent{ { name: "testchart/Chart.yaml", @@ -527,35 +511,12 @@ func TestLoadChartFSWithOptions(t *testing.T) { assert.Equal(t, want.version, chart.Metadata.Version, "chart version") }, }, - { - name: "load nonexistent chart", - args: args{ - filename: "nonexistent-chart-0.1.0.tgz", - files: []fileContent{ - { - name: "testchart/Chart.yaml", - content: []byte("apiVersion: v2\nname: testchart\nversion: 0.1.0"), - }, - { - name: "testchart/templates/deployment.yaml", - content: []byte("kind: Deployment\napiVersion: apps/v1"), - }, - }, - }, - want: want{ - errMsg: "reading chart nonexistent-chart-0.1.0.tgz; open nonexistent-chart-0.1.0.tgz: no such file or directory", - }, - expect: func(chart *chart.Chart, want want, err error) { - require.Error(t, err, want.errMsg) - assert.Nil(t, chart) - }, - }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { chartFS, _ := createTempFS(t, mockHelmChartTgz(t, tc.args.files)) - got, err := LoadChartFSWithOptions(chartFS, tc.args.filename, WithInstallNamespace("metrics-server-system")) + got, err := LoadChartFSWithOptions(chartFS, WithInstallNamespace("metrics-server-system")) require.NotNil(t, tc.expect) tc.expect(got, tc.want, err) }) @@ -636,7 +597,9 @@ type fileContent struct { } func mockHelmChartTgz(t *testing.T, contents []fileContent) []byte { - require.NotEmpty(t, contents, "chart content required") + if len(contents) == 0 { + return nil + } var buf bytes.Buffer tw := tar.NewWriter(&buf) @@ -662,7 +625,6 @@ func mockHelmChartTgz(t *testing.T, contents []fileContent) []byte { } func createTempFS(t *testing.T, data []byte) (fs.FS, error) { - require.NotEmpty(t, data, "chart data") tmpDir, _ := os.MkdirTemp(t.TempDir(), "bundlefs-") if len(data) == 0 { return os.DirFS(tmpDir), nil