Repository: helm/helm
Pull request #12539 (2023-11-03 16:00:56)

Fixing release labelling in rollback
**What this PR does / why we need it**:

refs https://github.com/helm/helm/issues/12521

This is a proposed solution for issue https://github.com/helm/helm/issues/12521:

Fix the problem of missing labeling in the rollback release secret. The labels are copied from the rollback target release.


**Special notes for your reviewer**:

**If applicable**:
- [ ] this PR contains documentation
- [*] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility



Patch:
diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go
index f4ae896e3..b0be17d13 100644
--- a/pkg/action/rollback.go
+++ b/pkg/action/rollback.go
@@ -151,6 +151,7 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele
 			Description: fmt.Sprintf("Rollback to %d", previousVersion),
 		},
 		Version:  currentRelease.Version + 1,
+		Labels:   previousRelease.Labels,
 		Manifest: previousRelease.Manifest,
 		Hooks:    previousRelease.Hooks,
 	}
diff --git a/pkg/registry/client.go b/pkg/registry/client.go
index 7538cf69b..829f6b494 100644
--- a/pkg/registry/client.go
+++ b/pkg/registry/client.go
@@ -96,8 +96,18 @@ func NewClient(options ...ClientOption) (*Client, error) {
 				return resolver, nil
 			}
 		}
+
 		headers := http.Header{}
 		headers.Set("User-Agent", version.GetUserAgent())
+		dockerClient, ok := client.authorizer.(*dockerauth.Client)
+		if ok {
+			username, password, err := dockerClient.Credential(ref.Registry)
+			if err != nil {
+				return nil, fmt.Errorf("unable to retrieve credentials: %w", err)
+			}
+			authHeader(username, password, &headers)
+		}
+
 		opts := []auth.ResolverOption{auth.WithResolverHeaders(headers)}
 		if client.httpClient != nil {
 			opts = append(opts, auth.WithResolverClient(client.httpClient))
@@ -129,7 +139,6 @@ func NewClient(options ...ClientOption) (*Client, error) {
 				if !ok {
 					return registryauth.EmptyCredential, errors.New("unable to obtain docker client")
 				}
-
 				username, password, err := dockerClient.Credential(reg)
 				if err != nil {
 					return registryauth.EmptyCredential, errors.New("unable to retrieve credentials")
@@ -593,7 +602,6 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu
 	if err := memoryStore.StoreManifest(parsedRef.String(), manifest, manifestData); err != nil {
 		return nil, err
 	}
-
 	remotesResolver, err := c.resolver(parsedRef)
 	if err != nil {
 		return nil, err
diff --git a/pkg/registry/util.go b/pkg/registry/util.go
index 8baf0852a..6fb1d0cda 100644
--- a/pkg/registry/util.go
+++ b/pkg/registry/util.go
@@ -19,6 +19,7 @@ package registry // import "helm.sh/helm/v3/pkg/registry"
 import (
 	"bytes"
 	"context"
+	"encoding/base64"
 	"fmt"
 	"io"
 	"net/http"
@@ -245,3 +246,32 @@ func addToMap(inputMap map[string]string, newKey string, newValue string) map[st
 	return inputMap
 
 }
+
+// See 2 (end of page 4) https://www.ietf.org/rfc/rfc2617.txt
+// "To receive authorization, the client sends the userid and password,
+// separated by a single colon (":") character, within a base64
+// encoded string in the credentials."
+// It is not meant to be urlencoded.
+func basicAuth(username, password string) string {
+	auth := username + ":" + password
+	return base64.StdEncoding.EncodeToString([]byte(auth))
+}
+
+// authHeader generates an HTTP authorization header based on the provided
+// username and password and sets it in the provided HTTP headers pointer.
+//
+// If both username and password are empty, no header is set.
+// If only the password is provided, a "Bearer" token is created and set in
+// the Authorization header.
+// If both username and password are provided, a "Basic" authentication token
+// is created using the basicAuth function, and set in the Authorization header.
+func authHeader(username, password string, headers *http.Header) {
+	if username == "" && password == "" {
+		return
+	}
+	if username == "" {
+		headers.Set("Authorization", fmt.Sprintf("Bearer %s", password))
+		return
+	}
+	headers.Set("Authorization", fmt.Sprintf("Basic %s", basicAuth(username, password)))
+}


Test patch:
diff --git a/cmd/helm/rollback_test.go b/cmd/helm/rollback_test.go
index 6d38e16eb..b58e4c162 100644
--- a/cmd/helm/rollback_test.go
+++ b/cmd/helm/rollback_test.go
@@ -17,6 +17,8 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
+	"reflect"
 	"testing"
 
 	"helm.sh/helm/v3/pkg/chart"
@@ -121,3 +123,44 @@ func TestRollbackFileCompletion(t *testing.T) {
 	checkFileCompletion(t, "rollback myrelease", false)
 	checkFileCompletion(t, "rollback myrelease 1", false)
 }
+
+func TestRollbackWithLabels(t *testing.T) {
+	labels1 := map[string]string{"operation": "install", "firstLabel": "firstValue"}
+	labels2 := map[string]string{"operation": "upgrade", "secondLabel": "secondValue"}
+
+	releaseName := "funny-bunny-labels"
+	rels := []*release.Release{
+		{
+			Name:    releaseName,
+			Info:    &release.Info{Status: release.StatusSuperseded},
+			Chart:   &chart.Chart{},
+			Version: 1,
+			Labels:  labels1,
+		},
+		{
+			Name:    releaseName,
+			Info:    &release.Info{Status: release.StatusDeployed},
+			Chart:   &chart.Chart{},
+			Version: 2,
+			Labels:  labels2,
+		},
+	}
+	storage := storageFixture()
+	for _, rel := range rels {
+		if err := storage.Create(rel); err != nil {
+			t.Fatal(err)
+		}
+	}
+	_, _, err := executeActionCommandC(storage, fmt.Sprintf("rollback %s 1", releaseName))
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+	updatedRel, err := storage.Get(releaseName, 3)
+	if err != nil {
+		t.Errorf("unexpected error, got '%v'", err)
+	}
+
+	if !reflect.DeepEqual(updatedRel.Labels, labels1) {
+		t.Errorf("Expected {%v}, got {%v}", labels1, updatedRel.Labels)
+	}
+}
diff --git a/pkg/registry/util_test.go b/pkg/registry/util_test.go
index fdf09360b..f641801fe 100644
--- a/pkg/registry/util_test.go
+++ b/pkg/registry/util_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package registry // import "helm.sh/helm/v3/pkg/registry"
 
 import (
+	"net/http"
 	"reflect"
 	"testing"
 	"time"
@@ -238,3 +239,77 @@ func TestGenerateOCICreatedAnnotations(t *testing.T) {
 	}
 
 }
+
+func Test_basicAuth(t *testing.T) {
+	type args struct {
+		username string
+		password string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "Basic Auth",
+			args: args{
+				username: "admin",
+				password: "passw0rd",
+			},
+			want: "YWRtaW46cGFzc3cwcmQ=",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := basicAuth(tt.args.username, tt.args.password); got != tt.want {
+				t.Errorf("basicAuth() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_authHeader(t *testing.T) {
+	tests := []struct {
+		name           string
+		username       string
+		password       string
+		expectedHeader http.Header
+	}{
+		{
+			name:     "basic login header with username and password",
+			username: "admin",
+			password: "passw0rd",
+			expectedHeader: func() http.Header {
+				header := http.Header{}
+				header.Set("Authorization", "Basic YWRtaW46cGFzc3cwcmQ=")
+				return header
+			}(),
+		},
+		{
+			name:     "bearer login header with no username and password",
+			username: "",
+			password: "hunter2",
+			expectedHeader: func() http.Header {
+				header := http.Header{}
+				header.Set("Authorization", "Bearer hunter2")
+				return header
+			}(),
+		},
+		{
+			name:           "no change in header with neither username nor password",
+			username:       "",
+			password:       "",
+			expectedHeader: http.Header{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := &http.Header{}
+			authHeader(tt.username, tt.password, got)
+			if !reflect.DeepEqual(*got, tt.expectedHeader) {
+				t.Errorf("authHeader got %#v wanted %#v", *got, tt.expectedHeader)
+			}
+		})
+	}
+}