diff --git a/internal/helm/chart/builder_local_test.go b/internal/helm/chart/builder_local_test.go index 7f42ee90..5691371f 100644 --- a/internal/helm/chart/builder_local_test.go +++ b/internal/helm/chart/builder_local_test.go @@ -30,7 +30,6 @@ import ( "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/repo" - "github.com/fluxcd/source-controller/internal/helm/getter" "github.com/fluxcd/source-controller/internal/helm/repository" ) @@ -43,7 +42,7 @@ func TestLocalBuilder_Build(t *testing.T) { g.Expect(chartB).ToNot(BeEmpty()) mockRepo := func() *repository.ChartRepository { return &repository.ChartRepository{ - Client: &getter.MockGetter{ + Client: &mockGetter{ Response: chartB, }, Index: &repo.IndexFile{ diff --git a/internal/helm/chart/builder_remote_test.go b/internal/helm/chart/builder_remote_test.go index e8ad6be5..80534c60 100644 --- a/internal/helm/chart/builder_remote_test.go +++ b/internal/helm/chart/builder_remote_test.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "os" + "path/filepath" "strings" "sync" "testing" @@ -337,6 +338,30 @@ func Test_mergeChartValues(t *testing.T) { } } +func Test_validatePackageAndWriteToPath(t *testing.T) { + g := NewWithT(t) + + tmpDir, err := os.MkdirTemp("", "validate-pkg-chart-") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + validF, err := os.Open("./../testdata/charts/helmchart-0.1.0.tgz") + g.Expect(err).ToNot(HaveOccurred()) + defer validF.Close() + + chartPath := filepath.Join(tmpDir, "chart.tgz") + defer os.Remove(chartPath) + err = validatePackageAndWriteToPath(validF, chartPath) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(chartPath).To(BeARegularFile()) + + emptyF, err := os.Open("./../testdata/charts/empty.tgz") + defer emptyF.Close() + g.Expect(err).ToNot(HaveOccurred()) + err = validatePackageAndWriteToPath(emptyF, filepath.Join(tmpDir, "out.tgz")) + g.Expect(err).To(HaveOccurred()) +} + func Test_pathIsDir(t *testing.T) { tests := []struct { name string diff --git a/internal/helm/chart/builder_test.go b/internal/helm/chart/builder_test.go index 05b3ec1b..87f0b93d 100644 --- a/internal/helm/chart/builder_test.go +++ b/internal/helm/chart/builder_test.go @@ -28,6 +28,82 @@ import ( "helm.sh/helm/v3/pkg/chartutil" ) +func TestLocalReference_Validate(t *testing.T) { + tests := []struct { + name string + ref LocalReference + wantErr string + }{ + { + name: "ref with path", + ref: LocalReference{Path: "/a/path"}, + }, + { + name: "ref with path and work dir", + ref: LocalReference{Path: "/a/path", WorkDir: "/with/a/workdir"}, + }, + { + name: "ref without path", + ref: LocalReference{WorkDir: "/just/a/workdir"}, + wantErr: "no path set for local chart reference", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + err := tt.ref.Validate() + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + +func TestRemoteReference_Validate(t *testing.T) { + tests := []struct { + name string + ref RemoteReference + wantErr string + }{ + { + name: "ref with name", + ref: RemoteReference{Name: "valid-chart-name"}, + }, + { + name: "ref with invalid name", + ref: RemoteReference{Name: "iNvAlID-ChArT-NAmE!"}, + wantErr: "invalid chart name 'iNvAlID-ChArT-NAmE!'", + }, + { + name: "ref with Artifactory specific invalid format", + ref: RemoteReference{Name: "i-shall/not"}, + wantErr: "invalid chart name 'i-shall/not'", + }, + { + name: "ref without name", + ref: RemoteReference{}, + wantErr: "no name set for remote chart reference", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + err := tt.ref.Validate() + if tt.wantErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} + func TestBuildOptions_GetValueFiles(t *testing.T) { tests := []struct { name string diff --git a/internal/helm/chart/dependency_manager.go b/internal/helm/chart/dependency_manager.go index 2fa1df32..798f6df9 100644 --- a/internal/helm/chart/dependency_manager.go +++ b/internal/helm/chart/dependency_manager.go @@ -95,7 +95,9 @@ func (dm *DependencyManager) Clear() []error { var errs []error for _, v := range dm.repositories { v.Unload() - errs = append(errs, v.RemoveCache()) + if err := v.RemoveCache(); err != nil { + errs = append(errs, err) + } } return errs } diff --git a/internal/helm/chart/dependency_manager_test.go b/internal/helm/chart/dependency_manager_test.go index da4b70a6..04c0fc46 100644 --- a/internal/helm/chart/dependency_manager_test.go +++ b/internal/helm/chart/dependency_manager_test.go @@ -17,6 +17,7 @@ limitations under the License. package chart import ( + "bytes" "context" "errors" "fmt" @@ -28,12 +29,48 @@ import ( . "github.com/onsi/gomega" helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" + helmgetter "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" - "github.com/fluxcd/source-controller/internal/helm/getter" "github.com/fluxcd/source-controller/internal/helm/repository" ) +// mockGetter is a simple mocking getter.Getter implementation, returning +// a byte response to any provided URL. +type mockGetter struct { + Response []byte +} + +func (g *mockGetter) Get(_ string, _ ...helmgetter.Option) (*bytes.Buffer, error) { + r := g.Response + return bytes.NewBuffer(r), nil +} + +func TestDependencyManager_Clear(t *testing.T) { + g := NewWithT(t) + + repos := map[string]*repository.ChartRepository{ + "with index": { + Index: repo.NewIndexFile(), + RWMutex: &sync.RWMutex{}, + }, + "cached cache path": { + CachePath: "/invalid/path/resets", + Cached: true, + RWMutex: &sync.RWMutex{}, + }, + } + + dm := NewDependencyManager(WithRepositories(repos)) + g.Expect(dm.Clear()).To(BeNil()) + g.Expect(dm.repositories).To(HaveLen(len(repos))) + for _, v := range repos { + g.Expect(v.Index).To(BeNil()) + g.Expect(v.CachePath).To(BeEmpty()) + g.Expect(v.Cached).To(BeFalse()) + } +} + func TestDependencyManager_Build(t *testing.T) { g := NewWithT(t) @@ -45,7 +82,7 @@ func TestDependencyManager_Build(t *testing.T) { mockRepo := func() *repository.ChartRepository { return &repository.ChartRepository{ - Client: &getter.MockGetter{ + Client: &mockGetter{ Response: chartGrafana, }, Index: &repo.IndexFile{ @@ -286,7 +323,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { name: "adds remote dependency", repositories: map[string]*repository.ChartRepository{ "https://example.com/": { - Client: &getter.MockGetter{ + Client: &mockGetter{ Response: chartB, }, Index: &repo.IndexFile{ @@ -403,7 +440,7 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) { name: "chart load error", repositories: map[string]*repository.ChartRepository{ "https://example.com/": { - Client: &getter.MockGetter{}, + Client: &mockGetter{}, Index: &repo.IndexFile{ Entries: map[string]repo.ChartVersions{ chartName: { diff --git a/internal/helm/getter/mock.go b/internal/helm/getter/mock.go deleted file mode 100644 index 91cd2b7b..00000000 --- a/internal/helm/getter/mock.go +++ /dev/null @@ -1,41 +0,0 @@ -/* -Copyright 2021 The Flux 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 getter - -import ( - "bytes" - - "helm.sh/helm/v3/pkg/getter" -) - -// MockGetter can be used as a simple mocking getter.Getter implementation. -type MockGetter struct { - Response []byte - - requestedURL string -} - -func (g *MockGetter) Get(u string, _ ...getter.Option) (*bytes.Buffer, error) { - g.requestedURL = u - r := g.Response - return bytes.NewBuffer(r), nil -} - -// LastGet returns the last requested URL for Get. -func (g *MockGetter) LastGet() string { - return g.requestedURL -} diff --git a/internal/helm/repository/chart_repository_test.go b/internal/helm/repository/chart_repository_test.go index b6f191f3..22d3e664 100644 --- a/internal/helm/repository/chart_repository_test.go +++ b/internal/helm/repository/chart_repository_test.go @@ -29,8 +29,6 @@ import ( "helm.sh/helm/v3/pkg/chart" helmgetter "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" - - "github.com/fluxcd/source-controller/internal/helm/getter" ) var now = time.Now() @@ -41,6 +39,19 @@ const ( unorderedTestFile = "../testdata/local-index-unordered.yaml" ) +// mockGetter is a simple mocking getter.Getter implementation, returning +// a byte response to any provided URL. +type mockGetter struct { + Response []byte + LastCalledURL string +} + +func (g *mockGetter) Get(u string, _ ...helmgetter.Option) (*bytes.Buffer, error) { + r := g.Response + g.LastCalledURL = u + return bytes.NewBuffer(r), nil +} + func TestNewChartRepository(t *testing.T) { repositoryURL := "https://example.com" providers := helmgetter.Providers{ @@ -220,7 +231,7 @@ func TestChartRepository_DownloadChart(t *testing.T) { g := NewWithT(t) t.Parallel() - mg := getter.MockGetter{} + mg := mockGetter{} r := &ChartRepository{ URL: tt.url, Client: &mg, @@ -231,7 +242,7 @@ func TestChartRepository_DownloadChart(t *testing.T) { g.Expect(res).To(BeNil()) return } - g.Expect(mg.LastGet()).To(Equal(tt.wantURL)) + g.Expect(mg.LastCalledURL).To(Equal(tt.wantURL)) g.Expect(res).ToNot(BeNil()) g.Expect(err).ToNot(HaveOccurred()) }) @@ -244,7 +255,7 @@ func TestChartRepository_DownloadIndex(t *testing.T) { b, err := os.ReadFile(chartmuseumTestFile) g.Expect(err).ToNot(HaveOccurred()) - mg := getter.MockGetter{Response: b} + mg := mockGetter{Response: b} r := &ChartRepository{ URL: "https://example.com", Client: &mg, @@ -253,7 +264,7 @@ func TestChartRepository_DownloadIndex(t *testing.T) { buf := bytes.NewBuffer([]byte{}) g.Expect(r.DownloadIndex(buf)).To(Succeed()) g.Expect(buf.Bytes()).To(Equal(b)) - g.Expect(mg.LastGet()).To(Equal(r.URL + "/index.yaml")) + g.Expect(mg.LastCalledURL).To(Equal(r.URL + "/index.yaml")) g.Expect(err).To(BeNil()) } @@ -374,7 +385,7 @@ func TestChartRepository_LoadIndexFromFile(t *testing.T) { func TestChartRepository_CacheIndex(t *testing.T) { g := NewWithT(t) - mg := getter.MockGetter{Response: []byte("foo")} + mg := mockGetter{Response: []byte("foo")} expectSum := fmt.Sprintf("%x", sha256.Sum256(mg.Response)) r := newChartRepository() @@ -393,6 +404,31 @@ func TestChartRepository_CacheIndex(t *testing.T) { g.Expect(sum).To(BeEquivalentTo(expectSum)) } +func TestChartRepository_StrategicallyLoadIndex(t *testing.T) { + g := NewWithT(t) + + r := newChartRepository() + r.Index = repo.NewIndexFile() + g.Expect(r.StrategicallyLoadIndex()).To(Succeed()) + g.Expect(r.CachePath).To(BeEmpty()) + g.Expect(r.Cached).To(BeFalse()) + + r.Index = nil + r.CachePath = "/invalid/cache/index/path.yaml" + err := r.StrategicallyLoadIndex() + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("/invalid/cache/index/path.yaml: no such file or directory")) + g.Expect(r.Cached).To(BeFalse()) + + r.CachePath = "" + r.Client = &mockGetter{} + err = r.StrategicallyLoadIndex() + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("no API version specified")) + g.Expect(r.Cached).To(BeTrue()) + g.Expect(r.RemoveCache()).To(Succeed()) +} + func TestChartRepository_LoadFromCache(t *testing.T) { tests := []struct { name string @@ -443,6 +479,15 @@ func TestChartRepository_HasIndex(t *testing.T) { g.Expect(r.HasIndex()).To(BeTrue()) } +func TestChartRepository_HasCacheFile(t *testing.T) { + g := NewWithT(t) + + r := newChartRepository() + g.Expect(r.HasCacheFile()).To(BeFalse()) + r.CachePath = "foo" + g.Expect(r.HasCacheFile()).To(BeTrue()) +} + func TestChartRepository_UnloadIndex(t *testing.T) { g := NewWithT(t) @@ -522,3 +567,27 @@ func verifyLocalIndex(t *testing.T, i *repo.IndexFile) { g.Expect(tt.Keywords).To(ContainElements(expect.Keywords)) } } + +func TestChartRepository_RemoveCache(t *testing.T) { + g := NewWithT(t) + + tmpFile, err := os.CreateTemp("", "remove-cache-") + g.Expect(err).ToNot(HaveOccurred()) + defer os.Remove(tmpFile.Name()) + + r := newChartRepository() + r.CachePath = tmpFile.Name() + r.Cached = true + + g.Expect(r.RemoveCache()).To(Succeed()) + g.Expect(r.CachePath).To(BeEmpty()) + g.Expect(r.Cached).To(BeFalse()) + g.Expect(tmpFile.Name()).ToNot(BeAnExistingFile()) + + r.CachePath = tmpFile.Name() + r.Cached = true + + g.Expect(r.RemoveCache()).To(Succeed()) + g.Expect(r.CachePath).To(BeEmpty()) + g.Expect(r.Cached).To(BeFalse()) +}