Skip to content

Commit

Permalink
Merge pull request #301 from hypnoglow/ISSUE-300
Browse files Browse the repository at this point in the history
ISSUE-300 Fix init command when repos file not exist
  • Loading branch information
hypnoglow authored Sep 20, 2023
2 parents 846ffb5 + 470a238 commit 0db704a
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 26 deletions.
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ linters:
- godot
- gofmt
- goheader
- goimports
# - gomnd
- gomodguard
- goprintffuncname
Expand Down
2 changes: 1 addition & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
golang 1.19.3
golang 1.19.13
helm 3.12.3
golangci-lint 1.48.0
45 changes: 32 additions & 13 deletions cmd/helm-s3/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"fmt"
"io/fs"

"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions hack/test-e2e-local.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 10 additions & 1 deletion internal/helmutil/helm_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
4 changes: 4 additions & 0 deletions internal/helmutil/repo_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 <nil> 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)
Expand Down
42 changes: 38 additions & 4 deletions internal/helmutil/repo_entry_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package helmutil

import (
"io/fs"
"os"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
},
},
}

Expand All @@ -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)
})
}
Expand Down
4 changes: 2 additions & 2 deletions internal/helmutil/repo_entry_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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, "/")
Expand Down
4 changes: 2 additions & 2 deletions internal/helmutil/repo_entry_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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, "/")
Expand Down
1 change: 1 addition & 0 deletions internal/helmutil/testing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
26 changes: 26 additions & 0 deletions tests/e2e/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

0 comments on commit 0db704a

Please sign in to comment.