internal/helm: add more tests

Signed-off-by: Hidde Beydals <hello@hidde.co>

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{

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

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

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
 }

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: {

internal/helm/getter/mock.go

@@ -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
-}

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())
+}