Helm release tool: Add an option to force upload (#8771) (#8772)

* Helm release tool: Add an option to force upload

* Read FORCE in hack/helm/release/trigger-helm-release.sh

---------

Co-authored-by: Michael Montgomery <mmontg1@gmail.com>
Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>

Author: 3 people

.buildkite/pipeline-release-helm.yml

@@ -1,5 +1,6 @@
 env:
   HELM_DRY_RUN: ${HELM_DRY_RUN:-true}
+  HELM_FORCE: ${HELM_FORCE:-false}
 
 steps:
 

hack/helm/release/README.md

@@ -13,6 +13,7 @@ Flags:
       --charts-dir string         Directory which contains Helm charts to release (env: HELM_CHARTS_DIR) (default "./deploy")
       --credentials-file string   Path to GCS credentials JSON file (env: HELM_CREDENTIALS_FILE) (default "/tmp/credentials.json")
   -d, --dry-run                   Do not upload files to bucket, or update Helm index (env: HELM_DRY_RUN) (default true)
+  -f, --force                     Force an upload for non dev/snapshot  (env: HELM_FORCE) (default false)
       --enable-vault              Read 'credentials-file' from Vault (requires VAULT_ADDR and VAULT_TOKEN) (env: HELM_ENABLE_VAULT) (default true)
       --env string                Environment in which to release Helm charts ('dev' or 'prod') (env: HELM_ENV) (default "dev")
   -h, --help                      help for release

hack/helm/release/cmd/root.go

@@ -22,6 +22,7 @@ const (
 	chartsDirFlag       = "charts-dir"
 	credentialsFileFlag = "credentials-file"
 	dryRunFlag          = "dry-run"
+	forceFlag           = "force"
 	keepTmpDirFlag      = "keep-tmp-dir"
 	envFlag             = "env"
 	enableVaultFlag     = "enable-vault"
@@ -67,6 +68,7 @@ func releaseCmd() *cobra.Command {
 					ChartsRepoURL:       chartsRepoURL,
 					CredentialsFilePath: viper.GetString(credentialsFileFlag),
 					DryRun:              viper.GetBool(dryRunFlag),
+					Force:               viper.GetBool(forceFlag),
 					KeepTmpDir:          viper.GetBool(keepTmpDirFlag),
 				})
 		},
@@ -82,6 +84,14 @@ func releaseCmd() *cobra.Command {
 	)
 	_ = viper.BindPFlag(dryRunFlag, flags.Lookup(dryRunFlag))
 
+	flags.BoolP(
+		forceFlag,
+		"f",
+		false,
+		"Upload artifacts even if they already exist (env: HELM_FORCE)",
+	)
+	_ = viper.BindPFlag(forceFlag, flags.Lookup(forceFlag))
+
 	flags.BoolP(
 		keepTmpDirFlag,
 		"k",

hack/helm/release/internal/helm/helm.go

@@ -41,6 +41,8 @@ type ReleaseConfig struct {
 	CredentialsFilePath string
 	// DryRun determines whether to run the release without making any changes to the GCS bucket or the Helm repository index file.
 	DryRun bool
+	// Force determines if uploading charts should overwrite existing charts in the GCS bucket even if they are not SNAPSHOT versions.
+	Force bool
 	// KeepTmpDir determines whether the temporary directory should be kept or not
 	KeepTmpDir bool
 }
@@ -198,7 +200,8 @@ func copyChartToGCSBucket(ctx context.Context, conf ReleaseConfig, chart chart,
 	// specify that the object must not exist for non-SNAPSHOT chart when publishing to prod Helm repo
 	isNonSnapshot := !strings.HasSuffix(chart.Version, "-SNAPSHOT")
 	isProdHelmRepo := !strings.HasSuffix(conf.Bucket, "-dev")
-	if isNonSnapshot && isProdHelmRepo {
+	shouldNotOverwrite := shouldNotOverwrite(isNonSnapshot, isProdHelmRepo, conf.Force)
+	if shouldNotOverwrite {
 		chartArchiveObj = chartArchiveObj.If(storage.Conditions{DoesNotExist: true})
 	}
 
@@ -215,7 +218,7 @@ func copyChartToGCSBucket(ctx context.Context, conf ReleaseConfig, chart chart,
 	if err := chartArchiveWriter.Close(); err != nil {
 		switch errType := err.(type) {
 		case *googleapi.Error:
-			if errType.Code == http.StatusPreconditionFailed && isNonSnapshot && isProdHelmRepo {
+			if errType.Code == http.StatusPreconditionFailed && shouldNotOverwrite {
 				return fmt.Errorf("file %s already exists in remote bucket; manually remove for this operation to succeed", chartPackagePath)
 			}
 			return fmt.Errorf("while writing data to bucket: %w", err)
@@ -227,6 +230,14 @@ func copyChartToGCSBucket(ctx context.Context, conf ReleaseConfig, chart chart,
 	return nil
 }
 
+// shouldNotOverwrite determines if a chart should not be overwritten in the bucket.
+func shouldNotOverwrite(isNonSnapshot, isProdHelmRepo, force bool) bool {
+	if force {
+		return false
+	}
+	return isNonSnapshot && isProdHelmRepo
+}
+
 // updateIndex updates the Helm repo index by merging the existing index in the bucket
 // with a new version created with the released charts. A 'GenerationMatch' precondition
 // is used when writing to avoid a race condition with another concurrent write.

hack/helm/release/internal/helm/helm_test.go

@@ -14,6 +14,68 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 )
 
+func TestShouldNotOverwrite(t *testing.T) {
+	tests := []struct {
+		name           string
+		isNonSnapshot  bool
+		isProdHelmRepo bool
+		force          bool
+		want           bool
+	}{
+		{
+			name:           "Non-snapshot, prod repo, no force",
+			isNonSnapshot:  true,
+			isProdHelmRepo: true,
+			force:          false,
+			want:           true,
+		},
+		{
+			name:           "Non-snapshot, prod repo, force true",
+			isNonSnapshot:  true,
+			isProdHelmRepo: true,
+			force:          true,
+			want:           false,
+		},
+		{
+			name:           "Snapshot, prod repo, no force",
+			isNonSnapshot:  false,
+			isProdHelmRepo: true,
+			force:          false,
+			want:           false,
+		},
+		{
+			name:           "Non-snapshot, dev repo, no force",
+			isNonSnapshot:  true,
+			isProdHelmRepo: false,
+			force:          false,
+			want:           false,
+		},
+		{
+			name:           "Snapshot, dev repo, no force",
+			isNonSnapshot:  false,
+			isProdHelmRepo: false,
+			force:          false,
+			want:           false,
+		},
+		{
+			name:           "Non-snapshot, dev repo, force true",
+			isNonSnapshot:  true,
+			isProdHelmRepo: false,
+			force:          true,
+			want:           false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := shouldNotOverwrite(tt.isNonSnapshot, tt.isProdHelmRepo, tt.force)
+			if got != tt.want {
+				t.Errorf("shouldNotOverwrite() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func Test_readCharts(t *testing.T) {
 	tests := []struct {
 		name          string

hack/helm/release/trigger-helm-release.sh

@@ -7,14 +7,17 @@
 # Script to call the Buildkite API to trigger the release of the ECK Helm charts.
 #
 # Usage:  BK_TOKEN=$(jq .graphql_token ~/.buildkite/config.json -r) \
-#         BRANCH=2.8 DRY_RUN=true \
-#         ./trigger-bk-release.sh SCOPE
+#         BRANCH=2.8 DRY_RUN=true [FORCE=true] \
+#         ./trigger-helm-release.sh SCOPE
 #
 # Required environment variables:
 #    BK_TOKEN
 #    BRANCH
 #    DRY_RUN
 #
+# Optional environment variable:
+#    FORCE (default: false)
+#
 # Argument:
 #    SCOPE  to select which charts to release ("all", "eck-operator" or "eck-stack")
 #
@@ -24,6 +27,7 @@ set -eu
 : "$BK_TOKEN"
 : "$BRANCH"
 : "$DRY_RUN"
+FORCE=${FORCE:-false}
 
 # properties required to test PRs:
         # "pull_request_base_branch": "main",
@@ -40,7 +44,8 @@ main() {
         "branch": "'"$BRANCH"'",
         "message": "release '"$scope"' helm charts",
         "env": {
-            "HELM_DRY_RUN": "'"$DRY_RUN"'"
+            "HELM_DRY_RUN": "'"$DRY_RUN"'",
+            "HELM_FORCE": "'"$FORCE"'"
         }
     }'
 }