From 470a238219d259b31fb2c915f5751401d68b0669 Mon Sep 17 00:00:00 2001 From: Igor Zibarev Date: Wed, 20 Sep 2023 20:10:41 +0300 Subject: [PATCH] ISSUE-300 Fix init command when repos file not exist --- .golangci.yml | 1 - .tool-versions | 2 +- cmd/helm-s3/init.go | 45 ++++++++++++++++++++-------- hack/test-e2e-local.sh | 4 +-- internal/helmutil/helm_v2.go | 11 ++++++- internal/helmutil/repo_entry.go | 4 +++ internal/helmutil/repo_entry_test.go | 42 +++++++++++++++++++++++--- internal/helmutil/repo_entry_v2.go | 4 +-- internal/helmutil/repo_entry_v3.go | 4 +-- internal/helmutil/testing_test.go | 1 + tests/e2e/init_test.go | 26 ++++++++++++++++ 11 files changed, 118 insertions(+), 26 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 6997ee18..7eae62b8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,7 +22,6 @@ linters: - godot - gofmt - goheader - - goimports # - gomnd - gomodguard - goprintffuncname diff --git a/.tool-versions b/.tool-versions index bb8b8360..9b6bbe05 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ -golang 1.19.3 +golang 1.19.13 helm 3.12.3 golangci-lint 1.48.0 diff --git a/cmd/helm-s3/init.go b/cmd/helm-s3/init.go index d4199461..56ef8d7e 100644 --- a/cmd/helm-s3/init.go +++ b/cmd/helm-s3/init.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "io/fs" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -79,19 +80,8 @@ func (act *initAction) run(ctx context.Context) error { return newSilentError() } - repoEntry, ok, err := helmutil.LookupRepoEntryByURL(act.uri) - if err != nil { - return fmt.Errorf("lookup repo entry by url: %v", err) - } - if ok { - if act.ignoreIfExists { - return act.ignoreIfExistsError(repoEntry.Name()) - } - if !act.force { - return act.alreadyExistsError(repoEntry.Name()) - } - - // fallthrough on --force + if err := act.checkRepoEntry(); err != nil { + return err } r, err := helmutil.NewIndex().Reader() @@ -133,6 +123,35 @@ func (act *initAction) run(ctx context.Context) error { return nil } +func (act *initAction) checkRepoEntry() error { + repoEntry, ok, err := helmutil.LookupRepoEntryByURL(act.uri) + if errors.Is(err, fs.ErrNotExist) { + // Repo file may not exist, this is OK for instance when the helm is + // just installed (e.g. in docker). + return nil + } + if err != nil { + return fmt.Errorf("lookup repo entry by url: %v", err) + } + + if !ok { + // Repo entry not found - all is good. + return nil + } + + // Repo entry exists. + + if act.ignoreIfExists { + return act.ignoreIfExistsError(repoEntry.Name()) + } + if !act.force { + return act.alreadyExistsError(repoEntry.Name()) + } + + // fallthrough on --force + return nil +} + func (act *initAction) ignoreIfExistsError(name string) error { act.printer.Printf( "The repository with the provided URI already exists under name %q, ignore init operation.\n", diff --git a/hack/test-e2e-local.sh b/hack/test-e2e-local.sh index 1a891d85..2e04c6d5 100755 --- a/hack/test-e2e-local.sh +++ b/hack/test-e2e-local.sh @@ -35,10 +35,10 @@ docker container run -d --rm --name "${DOCKER_NAME}" \ -e MINIO_SECRET_KEY=$AWS_SECRET_ACCESS_KEY \ minio/minio:latest server /data >/dev/null -PATH=${GOPATH}/bin:${PATH} +PATH=$(go env GOPATH)/bin:${PATH} if [ ! -x "$(which mc 2>/dev/null)" ]; then pushd /tmp > /dev/null - go get github.com/minio/mc + go install github.com/minio/mc@latest popd > /dev/null fi diff --git a/internal/helmutil/helm_v2.go b/internal/helmutil/helm_v2.go index d53c286a..058b7921 100644 --- a/internal/helmutil/helm_v2.go +++ b/internal/helmutil/helm_v2.go @@ -13,7 +13,16 @@ import ( // setupHelm2 sets up environment and function bindings for helm v2. func setupHelm2() { helm2Home = resolveHome() - helm2LoadRepoFile = repo.LoadRepositoriesFile + helm2LoadRepoFile = func(path string) (*repo.RepoFile, error) { + // This is needed because LoadRepositoriesFile returns custom (not + // wrapped) error if the file does not exist. + _, err := os.Stat(path) + if err != nil { + return nil, err + } + + return repo.LoadRepositoriesFile(path) + } } var ( diff --git a/internal/helmutil/repo_entry.go b/internal/helmutil/repo_entry.go index 4afa7b04..2dbef8e0 100644 --- a/internal/helmutil/repo_entry.go +++ b/internal/helmutil/repo_entry.go @@ -25,6 +25,8 @@ type RepoEntry interface { } // LookupRepoEntry returns an entry from helm's repositories.yaml file by name. +// If repositories.yaml file is not found, errors.Is(err, fs.ErrNotExist) will +// return true. func LookupRepoEntry(name string) (RepoEntry, error) { if IsHelm3() { return lookupV3(name) @@ -34,6 +36,8 @@ func LookupRepoEntry(name string) (RepoEntry, error) { // LookupRepoEntryByURL returns an entry from helm's repositories.yaml file by // repo URL. If not found, returns false and error. +// If repositories.yaml file is not found, errors.Is(err, fs.ErrNotExist) will +// return true. func LookupRepoEntryByURL(url string) (RepoEntry, bool, error) { if IsHelm3() { return lookupByURLV3(url) diff --git a/internal/helmutil/repo_entry_test.go b/internal/helmutil/repo_entry_test.go index 42e40339..219f2faf 100644 --- a/internal/helmutil/repo_entry_test.go +++ b/internal/helmutil/repo_entry_test.go @@ -1,6 +1,8 @@ package helmutil import ( + "io/fs" + "os" "testing" "github.com/stretchr/testify/assert" @@ -14,7 +16,7 @@ func TestLookupRepoEntry(t *testing.T) { setup func() func() name string expectedEntry RepoEntry - expectError bool + assertError assert.ErrorAssertionFunc }{ "helm v2": { setup: func() func() { @@ -41,7 +43,21 @@ func TestLookupRepoEntry(t *testing.T) { URL: "s3://my-charts", }, }, - expectError: false, + assertError: assert.NoError, + }, + "helm v2 repo file not found": { + setup: func() func() { + helm2LoadRepoFile = func(path string) (file *repo2.RepoFile, e error) { + _, err := os.Stat("foobarbaz") + return nil, err + } + return mockEnv(t, "TILLER_HOST", "1") + }, + name: "my-charts", + expectedEntry: RepoEntryV2{}, + assertError: func(t assert.TestingT, err error, i ...interface{}) bool { + return assert.ErrorIs(t, err, fs.ErrNotExist) + }, }, "helm v3": { setup: func() func() { @@ -72,7 +88,25 @@ func TestLookupRepoEntry(t *testing.T) { URL: "s3://my-charts", }, }, - expectError: false, + assertError: assert.NoError, + }, + "helm v3 repo file not found": { + setup: func() func() { + helm3LoadRepoFile = func(path string) (file *repo3.File, e error) { + _, err := os.Stat("foobarbaz") + return nil, err + } + helm3Env = cli.New() + helm3Detected = func() bool { + return true + } + return func() {} + }, + name: "my-charts", + expectedEntry: RepoEntryV3{}, + assertError: func(t assert.TestingT, err error, i ...interface{}) bool { + return assert.ErrorIs(t, err, fs.ErrNotExist) + }, }, } @@ -83,7 +117,7 @@ func TestLookupRepoEntry(t *testing.T) { defer teardown() entry, err := LookupRepoEntry(tc.name) - assertError(t, err, tc.expectError) + tc.assertError(t, err) assert.Equal(t, tc.expectedEntry, entry) }) } diff --git a/internal/helmutil/repo_entry_v2.go b/internal/helmutil/repo_entry_v2.go index e0209ba5..c7dfa75b 100644 --- a/internal/helmutil/repo_entry_v2.go +++ b/internal/helmutil/repo_entry_v2.go @@ -37,7 +37,7 @@ func (r RepoEntryV2) CacheFile() string { func lookupV2(name string) (RepoEntryV2, error) { repoFile, err := helm2LoadRepoFile(repoFilePathV2()) if err != nil { - return RepoEntryV2{}, errors.Wrap(err, "load repo file") + return RepoEntryV2{}, fmt.Errorf("load repo file: %w", err) } if entry, ok := repoFile.Get(name); ok { @@ -50,7 +50,7 @@ func lookupV2(name string) (RepoEntryV2, error) { func lookupByURLV2(url string) (RepoEntryV2, bool, error) { repoFile, err := helm2LoadRepoFile(repoFilePathV2()) if err != nil { - return RepoEntryV2{}, false, fmt.Errorf("load repo file: %v", err) + return RepoEntryV2{}, false, fmt.Errorf("load repo file: %w", err) } url = strings.TrimSuffix(url, "/") diff --git a/internal/helmutil/repo_entry_v3.go b/internal/helmutil/repo_entry_v3.go index bb885ba6..6ee8aee1 100644 --- a/internal/helmutil/repo_entry_v3.go +++ b/internal/helmutil/repo_entry_v3.go @@ -33,7 +33,7 @@ func (r RepoEntryV3) CacheFile() string { func lookupV3(name string) (RepoEntryV3, error) { repoFile, err := helm3LoadRepoFile(repoFilePathV3()) if err != nil { - return RepoEntryV3{}, errors.Wrap(err, "load repo file") + return RepoEntryV3{}, fmt.Errorf("load repo file: %w", err) } entry := repoFile.Get(name) @@ -47,7 +47,7 @@ func lookupV3(name string) (RepoEntryV3, error) { func lookupByURLV3(url string) (RepoEntryV3, bool, error) { repoFile, err := helm3LoadRepoFile(repoFilePathV3()) if err != nil { - return RepoEntryV3{}, false, fmt.Errorf("load repo file: %v", err) + return RepoEntryV3{}, false, fmt.Errorf("load repo file: %w", err) } url = strings.TrimSuffix(url, "/") diff --git a/internal/helmutil/testing_test.go b/internal/helmutil/testing_test.go index ddf19f0b..f78857a5 100644 --- a/internal/helmutil/testing_test.go +++ b/internal/helmutil/testing_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" ) +// TODO: replace with t.SetEnv. func mockEnv(t *testing.T, name, value string) func() { old := os.Getenv(name) diff --git a/tests/e2e/init_test.go b/tests/e2e/init_test.go index 1392d968..66be590f 100644 --- a/tests/e2e/init_test.go +++ b/tests/e2e/init_test.go @@ -181,3 +181,29 @@ func TestInitForceAndIgnoreIfExists(t *testing.T) { expectedStderr := "The --force and --ignore-if-exists flags are mutually exclusive and cannot be specified together." assert.Contains(t, stderr.String(), expectedStderr) } + +func TestInitRepoFileNotFound(t *testing.T) { + t.Log("Test init action when repo file not found") + + const ( + repoName = "test-init-repo-file-not-found" + repoDir = "charts" + indexObjectName = repoDir + "/index.yaml" + uri = "s3://" + repoName + "/" + repoDir + ) + + setupBucket(t, repoName) + defer teardownBucket(t, repoName) + + // Run init. + + t.Setenv("HELM_REPOSITORY_CONFIG", "testdata/file-not-exists.yaml") + + cmd, stdout, stderr := command(fmt.Sprintf("helm s3 init %s", uri)) + err := cmd.Run() + assert.NoError(t, err) + assertEmptyOutput(t, nil, stderr) + assert.Contains(t, stdout.String(), "Initialized empty repository at "+uri) + + // Skip other checks because they are already covered by TestInit. +}