Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE-300 Fix init command when repos file not exist #301

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@
// 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
}

Check warning on line 22 in internal/helmutil/helm_v2.go

View check run for this annotation

Codecov / codecov/patch

internal/helmutil/helm_v2.go#L16-L22

Added lines #L16 - L22 were not covered by tests

return repo.LoadRepositoriesFile(path)

Check warning on line 24 in internal/helmutil/helm_v2.go

View check run for this annotation

Codecov / codecov/patch

internal/helmutil/helm_v2.go#L24

Added line #L24 was not covered by tests
}
}

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 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 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)

Check warning on line 53 in internal/helmutil/repo_entry_v2.go

View check run for this annotation

Codecov / codecov/patch

internal/helmutil/repo_entry_v2.go#L53

Added line #L53 was not covered by tests
}

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 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 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)

Check warning on line 50 in internal/helmutil/repo_entry_v3.go

View check run for this annotation

Codecov / codecov/patch

internal/helmutil/repo_entry_v3.go#L50

Added line #L50 was not covered by tests
}

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