add support for adding relative URLs to the index (for replicated buc…

…kets)

Signed-off-by: Benjamin Ash <bash@intelerad.com>

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

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)
 

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

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

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

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

hack/integration-tests-local.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-set -uo pipefail
+set -x -e -uo pipefail
 
 ## Set up
 
@@ -9,32 +9,45 @@ 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}" || :
+  fi
+}
+
+cleanup
+
+on_exit() {
+  if [ -z "${SKIP_CLEANUP}" ]; then
+    cleanup
+  fi
+}
+trap on_exit EXIT
+
+docker container run -d --rm --name helm-s3-minio \
     -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}
+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
 
 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
-

hack/integration-tests.sh

@@ -5,7 +5,8 @@ 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
@@ -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

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,17 +182,70 @@ 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) {
 		t.Errorf("Expected stderr to begin with %q, but got %q", expectedErrorMessage, stderr.String())
 	}
 }
 
+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)
+	}
+
+	os.Chdir(tmpdir)
+	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 +261,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 {