diff --git a/README.md b/README.md index d3372b40..f6e062b0 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,10 @@ Now you can push your chart to this repo: $ helm s3 push ./epicservice-0.7.2.tgz mynewrepo +When the bucket is replicated you should make the index's URLs relative so that the charts can be accessed from a replica bucket. + + $ helm s3 push --relative ./epicservice-0.7.2.tgz mynewrepo + On push, both remote and local repo indexes are automatically updated (that means you don't need to run `helm repo update`). @@ -223,6 +227,10 @@ the index in accordance with the charts in the repository. $ helm s3 reindex mynewrepo +When the bucket is replicated you should make the index's URLs relative so that the charts can be accessed from a replica bucket. + + $ helm s3 reindex --relative mynewrepo + ## Uninstall $ helm plugin remove s3 diff --git a/cmd/helms3/main.go b/cmd/helms3/main.go index 30dce800..b0abd765 100644 --- a/cmd/helms3/main.go +++ b/cmd/helms3/main.go @@ -42,6 +42,8 @@ In opposite, in cases where you want to reindex big repository For more information on S3 ACLs please see https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl ` + relativeFlag = "relative" + helpRelativeFlag = "Index using relative URLs (useful when S3 buckets are replicated)" ) // Action describes plugin action that can be run. @@ -100,11 +102,13 @@ func main() { Default(defaultChartsContentType). OverrideDefaultFromEnvar("S3_CHART_CONTENT_TYPE"). String() + pushRelative := pushCmd.Flag(relativeFlag, helpRelativeFlag).Bool() reindexCmd := cli.Command(actionReindex, "Reindex the repository.") reindexTargetRepository := reindexCmd.Arg("repo", "Target repository to reindex"). Required(). String() + reindexRelative := reindexCmd.Flag(relativeFlag, helpRelativeFlag).Bool() deleteCmd := cli.Command(actionDelete, "Delete chart from the repository.").Alias("del") deleteChartName := deleteCmd.Arg("chartName", "Name of chart to delete"). @@ -154,12 +158,14 @@ func main() { ignoreIfExists: *pushIgnoreIfExists, acl: *acl, contentType: *pushContentType, + relative: *pushRelative, } case actionReindex: act = reindexAction{ repoName: *reindexTargetRepository, acl: *acl, + relative: *reindexRelative, } defer fmt.Printf("Repository %s was successfully reindexed.\n", *reindexTargetRepository) diff --git a/cmd/helms3/proxy.go b/cmd/helms3/proxy.go index 70154997..86a0b901 100644 --- a/cmd/helms3/proxy.go +++ b/cmd/helms3/proxy.go @@ -34,7 +34,7 @@ func (act proxyCmd) Run(ctx context.Context) error { strings.TrimSuffix(strings.TrimSuffix(act.uri, indexYaml), "/"), ) } - return errors.WithMessage(err, "fetch from s3") + return errors.WithMessage(err, fmt.Sprintf("fetch from s3 uri=%s", act.uri)) } fmt.Print(string(b)) diff --git a/cmd/helms3/push.go b/cmd/helms3/push.go index c3994fb5..0f9b6f4c 100644 --- a/cmd/helms3/push.go +++ b/cmd/helms3/push.go @@ -35,6 +35,7 @@ type pushAction struct { ignoreIfExists bool acl string contentType string + relative bool } func (act pushAction) Run(ctx context.Context) error { @@ -139,7 +140,11 @@ func (act pushAction) Run(ctx context.Context) error { if err := idx.UnmarshalBinary(b); err != nil { return errors.WithMessage(err, "load index from downloaded file") } - if err := idx.AddOrReplace(chart.Metadata().Value(), fname, repoEntry.URL(), hash); err != nil { + baseURL := repoEntry.URL() + if act.relative { + baseURL = "" + } + if err := idx.AddOrReplace(chart.Metadata().Value(), fname, baseURL, hash); err != nil { return errors.WithMessage(err, "add/replace chart in the index") } idx.SortEntries() diff --git a/cmd/helms3/reindex.go b/cmd/helms3/reindex.go index 1f803f50..0f7dd8b8 100644 --- a/cmd/helms3/reindex.go +++ b/cmd/helms3/reindex.go @@ -14,6 +14,7 @@ import ( type reindexAction struct { repoName string acl string + relative bool } func (act reindexAction) Run(ctx context.Context) error { @@ -34,7 +35,11 @@ func (act reindexAction) Run(ctx context.Context) error { go func() { idx := helmutil.NewIndex() for item := range items { - if err := idx.Add(item.Meta.Value(), item.Filename, repoEntry.URL(), item.Hash); err != nil { + baseURL := repoEntry.URL() + if act.relative { + baseURL = "" + } + if err := idx.Add(item.Meta.Value(), item.Filename, baseURL, item.Hash); err != nil { log.Printf("[ERROR] failed to add chart to the index: %s", err) } } diff --git a/go.mod b/go.mod index d47ebec5..50405bdf 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/Masterminds/semver/v3 v3.0.1 github.com/aws/aws-sdk-go v1.25.50 github.com/ghodss/yaml v1.0.0 + github.com/google/go-cmp v0.3.1 github.com/minio/minio-go/v6 v6.0.40 github.com/pkg/errors v0.8.1 github.com/smartystreets/goconvey v0.0.0-20190731233626-505e41936337 // indirect diff --git a/hack/integration-tests-local.sh b/hack/integration-tests-local.sh index e1275688..93e66d62 100755 --- a/hack/integration-tests-local.sh +++ b/hack/integration-tests-local.sh @@ -1,5 +1,7 @@ #!/usr/bin/env bash -set -uo pipefail +set -e -uo pipefail + +[ -n "${DEBUG:-}" ] && set -x ## Set up @@ -9,19 +11,38 @@ export AWS_DEFAULT_REGION=us-east-1 export AWS_ENDPOINT=localhost:9000 export AWS_DISABLE_SSL=true -docker container run -d --name helm-s3-minio \ +DOCKER_NAME='helm-s3-minio' + +cleanup() { + if docker container ls | grep -q "${DOCKER_NAME}$" ; then + docker container rm --force --volumes "${DOCKER_NAME}" &>/dev/null || : + fi +} + +cleanup + +on_exit() { + if [ -z "${SKIP_CLEANUP:-}" ]; then + cleanup + fi +} +trap on_exit EXIT + +docker container run -d --rm --name "${DOCKER_NAME}" \ -p 9000:9000 \ -e MINIO_ACCESS_KEY=$AWS_ACCESS_KEY_ID \ -e MINIO_SECRET_KEY=$AWS_SECRET_ACCESS_KEY \ - minio/minio:latest server /data &>/dev/null + minio/minio:latest server /data >/dev/null -MCGOPATH=${GOPATH}/src/github.com/minio/mc +PATH=${GOPATH}/bin:${PATH} if [ ! -x "$(which mc 2>/dev/null)" ]; then - go get -d github.com/minio/mc - (cd ${MCGOPATH} && make) + pushd /tmp > /dev/null + go get github.com/minio/mc + popd > /dev/null fi -PATH=${MCGOPATH}:${PATH} +# give minio time to become service available. +sleep 3 mc config host add helm-s3-minio http://localhost:9000 $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY mc mb helm-s3-minio/test-bucket @@ -29,12 +50,7 @@ go build -o bin/helms3 ./cmd/helms3 ## Test -bash "$(dirname ${BASH_SOURCE[0]})/integration-tests.sh" +$(dirname ${BASH_SOURCE[0]})/integration-tests.sh if [ $? -eq 0 ] ; then echo -e "\nAll tests passed!" fi - -## Tear down - -docker container rm -f helm-s3-minio &>/dev/null - diff --git a/hack/integration-tests.sh b/hack/integration-tests.sh index d8306efa..3edeaa0c 100755 --- a/hack/integration-tests.sh +++ b/hack/integration-tests.sh @@ -5,12 +5,13 @@ set -euo pipefail # For helm v2, the command is `helm search foo/bar` # For helm v3, the command is `helm search repo foo/bar` search_arg="" -IT_HELM_VERSION="${IT_HELM_VERSION:-}" +IT_HELM_VERSION="${IT_HELM_VERSION:-3}" + if [ "${IT_HELM_VERSION:0:1}" == "3" ]; then search_arg="repo" fi -set -x +[ -n "${DEBUG:-}" ] && set -x # # Set up @@ -103,7 +104,8 @@ if [ $? -ne 0 ]; then exit 1 fi -if mc ls -q helm-s3-minio/test-bucket/charts/postgresql-0.8.3.tgz 2>/dev/null ; then +# listing an unknown object no longer seems to exit with a non-zero status. +if mc ls -q helm-s3-minio/test-bucket/charts/ | grep postgresql-0.8.3.tgz; then echo "Chart was not actually deleted" exit 1 fi diff --git a/tests/e2e/push_test.go b/tests/e2e/push_test.go index f7635fbf..2c4da7df 100644 --- a/tests/e2e/push_test.go +++ b/tests/e2e/push_test.go @@ -1,11 +1,17 @@ package e2e import ( + "bytes" "fmt" + "io/ioutil" + "os" + "path/filepath" "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/minio/minio-go/v6" + "helm.sh/helm/v3/pkg/repo" ) const ( @@ -30,12 +36,7 @@ func TestPush(t *testing.T) { if err := cmd.Run(); err != nil { t.Errorf("Unexpected error: %v", err) } - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } - if stderr.String() != "" { - t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) - } + assertEmptyOutput(t, stdout, stderr) // Check that chart was actually pushed obj, err := mc.StatObject(name, key, minio.StatObjectOptions{}) @@ -65,12 +66,7 @@ func TestPushWithContentTypeDefault(t *testing.T) { if err := cmd.Run(); err != nil { t.Errorf("Unexpected error: %v", err) } - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } - if stderr.String() != "" { - t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) - } + assertEmptyOutput(t, stdout, stderr) assertContentType(t, contentType, name, key) } @@ -93,12 +89,7 @@ func TestPushWithContentTypeCustom(t *testing.T) { if err := cmd.Run(); err != nil { t.Errorf("Unexpected error: %v", err) } - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } - if stderr.String() != "" { - t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) - } + assertEmptyOutput(t, stdout, stderr) assertContentType(t, contentType, name, key) } @@ -115,12 +106,7 @@ func TestPushDryRun(t *testing.T) { if err := cmd.Run(); err != nil { t.Errorf("Unexpected error: %v", err) } - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } - if stderr.String() != "" { - t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) - } + assertEmptyOutput(t, stdout, stderr) // Check that actually nothing got pushed @@ -149,12 +135,7 @@ func TestPushIgnoreIfExists(t *testing.T) { if err := cmd.Run(); err != nil { t.Errorf("Unexpected error: %v", err) } - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } - if stderr.String() != "" { - t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) - } + assertEmptyOutput(t, stdout, stderr) // check that chart was actually pushed and remember last modification time @@ -175,12 +156,7 @@ func TestPushIgnoreIfExists(t *testing.T) { if err := cmd.Run(); err != nil { t.Errorf("Unexpected error: %v", err) } - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } - if stderr.String() != "" { - t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) - } + assertEmptyOutput(t, stdout, stderr) // sanity check that chart was not overwritten @@ -206,10 +182,7 @@ func TestPushForceAndIgnoreIfExists(t *testing.T) { if err := cmd.Run(); err == nil { t.Errorf("Expected error") } - - if stdout.String() != "" { - t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) - } + assertEmptyOutput(t, stdout, nil) expectedErrorMessage := "The --force and --ignore-if-exists flags are mutually exclusive and cannot be specified together." if !strings.HasPrefix(stderr.String(), expectedErrorMessage) { @@ -217,6 +190,67 @@ func TestPushForceAndIgnoreIfExists(t *testing.T) { } } +func TestPushRelative(t *testing.T) { + t.Log("Test push action with --relative flag") + + name := "test-push-relative" + dir := "charts" + chartName := "foo" + chartVer := "1.2.3" + filename := fmt.Sprintf("%s-%s.tgz", chartName, chartVer) + + setupRepo(t, name, dir) + defer teardownRepo(t, name) + + // set a cleanup in beforehand + defer removeObject(t, name, dir+"/"+filename) + + cmd, stdout, stderr := command(fmt.Sprintf("helm s3 push --relative testdata/%s %s", filename, name)) + if err := cmd.Run(); err != nil { + t.Errorf("Unexpected error: %v", err) + } + assertEmptyOutput(t, stdout, stderr) + + tmpdir, err := ioutil.TempDir("", t.Name()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + defer os.RemoveAll(tmpdir) + + indexFile := filepath.Join(tmpdir, "index.yaml") + + if err := mc.FGetObject(name, dir+"/index.yaml", indexFile, minio.GetObjectOptions{}); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + idx, err := repo.LoadIndexFile(indexFile) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + v, err := idx.Get(chartName, chartVer) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + expected := []string{filename} + if diff := cmp.Diff(expected, v.URLs); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + + wd, _ := os.Getwd() + if err := os.Chdir(tmpdir); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + defer os.Chdir(wd) + + cmd, stdout, stderr = command(fmt.Sprintf("helm fetch %s/%s --version %s", name, chartName, chartVer)) + if err := cmd.Run(); err != nil { + t.Errorf("Unexpected error: %v", err) + } + assertEmptyOutput(t, stdout, stderr) +} + func assertContentType(t *testing.T, contentType, name, key string) { t.Helper() obj, err := mc.StatObject(name, key, minio.StatObjectOptions{}) @@ -232,6 +266,17 @@ func assertContentType(t *testing.T, contentType, name, key string) { } } +func assertEmptyOutput(t *testing.T, stdout, stderr *bytes.Buffer) { + t.Helper() + if stdout != nil && stdout.String() != "" { + t.Errorf("Expected stdout to be empty, but got %q", stdout.String()) + } + + if stderr != nil && stderr.String() != "" { + t.Errorf("Expected stderr to be empty, but got %q", stderr.String()) + } +} + func removeObject(t *testing.T, name, key string) { t.Helper() if err := mc.RemoveObject(name, key); err != nil {