Merge 1e8ef74efe into 72c0c28207

pkg/action/rollback.go

@@ -59,6 +59,8 @@ type Rollback struct {
 	ServerSideApply string
 	CleanupOnFail   bool
 	MaxHistory      int // MaxHistory limits the maximum number of revisions saved per release
+	// Description is the description of this rollback operation
+	Description string
 }
 
 // NewRollback creates a new Rollback object with the given configuration.
@@ -169,6 +171,12 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele
 		return nil, nil, false, err
 	}
 
+	// Determine the description for this rollback
+	description := fmt.Sprintf("Rollback to %d", previousVersion)
+	if r.Description != "" {
+		description = r.Description
+	}
+
 	// Store a new release object with previous release's configuration
 	targetRelease := &release.Release{
 		Name:      name,
@@ -182,7 +190,7 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele
 			Notes:         previousRelease.Info.Notes,
 			// Because we lose the reference to previous version elsewhere, we set the
 			// message here, and only override it later if we experience failure.
-			Description: fmt.Sprintf("Rollback to %d", previousVersion),
+			Description: description,
 		},
 		Version:     currentRelease.Version + 1,
 		Labels:      previousRelease.Labels,

pkg/action/rollback_test.go

@@ -27,14 +27,26 @@ import (
 
 	"helm.sh/helm/v4/pkg/kube"
 	kubefake "helm.sh/helm/v4/pkg/kube/fake"
+	"helm.sh/helm/v4/pkg/release/common"
 )
 
+func rollbackAction(t *testing.T) *Rollback {
+	t.Helper()
+	config := actionConfigFixture(t)
+	rollAction := NewRollback(config)
+	return rollAction
+}
+
 func TestNewRollback(t *testing.T) {
+	is := assert.New(t)
 	config := actionConfigFixture(t)
-	client := NewRollback(config)
 
-	assert.NotNil(t, client)
-	assert.Equal(t, config, client.cfg)
+	rollback := NewRollback(config)
+
+	is.NotNil(rollback)
+	is.Equal(config, rollback.cfg)
+	is.Equal(DryRunNone, rollback.DryRunStrategy)
+	is.Empty(rollback.Description)
 }
 
 func TestRollbackRun_UnreachableKubeClient(t *testing.T) {
@@ -83,3 +95,120 @@ func TestRollback_WaitOptionsPassedDownstream(t *testing.T) {
 	// Verify that WaitOptions were passed to GetWaiter
 	is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter")
 }
+
+func TestRollback_WithDescription(t *testing.T) {
+	is := assert.New(t)
+	req := require.New(t)
+
+	rollAction := rollbackAction(t)
+
+	// Create two releases - version 1 (superseded) and version 2 (deployed)
+	rel1 := releaseStub()
+	rel1.Name = "test-release"
+	rel1.Version = 1
+	rel1.Info.Status = common.StatusSuperseded
+	rel1.ApplyMethod = "csa" // client-side apply
+	req.NoError(rollAction.cfg.Releases.Create(rel1))
+
+	rel2 := releaseStub()
+	rel2.Name = "test-release"
+	rel2.Version = 2
+	rel2.Info.Status = common.StatusDeployed
+	rel2.ApplyMethod = "csa" // client-side apply
+	req.NoError(rollAction.cfg.Releases.Create(rel2))
+
+	// Set custom description
+	customDescription := "Rollback due to critical bug in version 2"
+	rollAction.Description = customDescription
+	rollAction.Version = 1
+	rollAction.ServerSideApply = "false" // Disable server-side apply for testing
+
+	err := rollAction.Run("test-release")
+	req.NoError(err)
+
+	// Get the new release (version 3)
+	newReleasei, err := rollAction.cfg.Releases.Get("test-release", 3)
+	req.NoError(err)
+	newRelease, err := releaserToV1Release(newReleasei)
+	req.NoError(err)
+
+	// Verify the custom description was set
+	is.Equal(customDescription, newRelease.Info.Description)
+}
+
+func TestRollback_DefaultDescription(t *testing.T) {
+	is := assert.New(t)
+	req := require.New(t)
+
+	rollAction := rollbackAction(t)
+
+	// Create two releases - version 1 (superseded) and version 2 (deployed)
+	rel1 := releaseStub()
+	rel1.Name = "test-release-default"
+	rel1.Version = 1
+	rel1.Info.Status = common.StatusSuperseded
+	rel1.ApplyMethod = "csa" // client-side apply
+	req.NoError(rollAction.cfg.Releases.Create(rel1))
+
+	rel2 := releaseStub()
+	rel2.Name = "test-release-default"
+	rel2.Version = 2
+	rel2.Info.Status = common.StatusDeployed
+	rel2.ApplyMethod = "csa" // client-side apply
+	req.NoError(rollAction.cfg.Releases.Create(rel2))
+
+	// Don't set a description, rely on default
+	rollAction.Version = 1
+	rollAction.ServerSideApply = "false" // Disable server-side apply for testing
+
+	err := rollAction.Run("test-release-default")
+	req.NoError(err)
+
+	// Get the new release (version 3)
+	newReleasei, err := rollAction.cfg.Releases.Get("test-release-default", 3)
+	req.NoError(err)
+	newRelease, err := releaserToV1Release(newReleasei)
+	req.NoError(err)
+
+	// Verify the default description was set
+	is.Equal("Rollback to 1", newRelease.Info.Description)
+}
+
+func TestRollback_EmptyDescription(t *testing.T) {
+	is := assert.New(t)
+	req := require.New(t)
+
+	rollAction := rollbackAction(t)
+
+	// Create two releases - version 1 (superseded) and version 2 (deployed)
+	rel1 := releaseStub()
+	rel1.Name = "test-release-empty"
+	rel1.Version = 1
+	rel1.Info.Status = common.StatusSuperseded
+	rel1.ApplyMethod = "csa" // client-side apply
+	req.NoError(rollAction.cfg.Releases.Create(rel1))
+
+	rel2 := releaseStub()
+	rel2.Name = "test-release-empty"
+	rel2.Version = 2
+	rel2.Info.Status = common.StatusDeployed
+	rel2.ApplyMethod = "csa" // client-side apply
+	req.NoError(rollAction.cfg.Releases.Create(rel2))
+
+	// Set empty description (should use default)
+	rollAction.Description = ""
+	rollAction.Version = 1
+	rollAction.ServerSideApply = "false" // Disable server-side apply for testing
+
+	err := rollAction.Run("test-release-empty")
+	req.NoError(err)
+
+	// Get the new release (version 3)
+	newReleasei, err := rollAction.cfg.Releases.Get("test-release-empty", 3)
+	req.NoError(err)
+	newRelease, err := releaserToV1Release(newReleasei)
+	req.NoError(err)
+
+	// Verify the default description was used for empty string
+	is.Equal("Rollback to 1", newRelease.Info.Description)
+}

pkg/cmd/rollback.go

@@ -38,6 +38,9 @@ second is a revision (version) number. If this argument is omitted or set to
 To see revision numbers, run 'helm history RELEASE'.
 `
 
+// maxDescriptionLength is the maximum length allowed for a rollback description
+const maxDescriptionLength = 512
+
 func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	client := action.NewRollback(cfg)
 
@@ -66,6 +69,11 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 				client.Version = ver
 			}
 
+			// Validate description length
+			if len(client.Description) > maxDescriptionLength {
+				return fmt.Errorf("description must be %d characters or less, got %d", maxDescriptionLength, len(client.Description))
+			}
+
 			dryRunStrategy, err := cmdGetDryRunFlagStrategy(cmd, false)
 			if err != nil {
 				return err
@@ -82,6 +90,7 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	}
 
 	f := cmd.Flags()
+	f.StringVar(&client.Description, "description", "", fmt.Sprintf("add a custom description for the rollback (max %d characters)", maxDescriptionLength))
 	f.BoolVar(&client.ForceReplace, "force-replace", false, "force resource updates by replacement")
 	f.BoolVar(&client.ForceReplace, "force", false, "deprecated")
 	f.MarkDeprecated("force", "use --force-replace instead")

pkg/cmd/rollback_test.go

@@ -19,6 +19,7 @@ package cmd
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 
 	chart "helm.sh/helm/v4/pkg/chart/v2"
@@ -79,6 +80,11 @@ func TestRollbackCmd(t *testing.T) {
 		golden:    "output/rollback-no-args.txt",
 		rels:      rels,
 		wantError: true,
+	}, {
+		name:   "rollback a release with description",
+		cmd:    "rollback funny-honey 1 --description 'Reverting due to bug in version 2'",
+		golden: "output/rollback-with-description.txt",
+		rels:   rels,
 	}}
 	runTestCmd(t, tests)
 }
@@ -125,6 +131,84 @@ func TestRollbackFileCompletion(t *testing.T) {
 	checkFileCompletion(t, "rollback myrelease 1", false)
 }
 
+func TestRollbackWithDescription(t *testing.T) {
+	releaseName := "funny-bunny-desc"
+	rels := []*release.Release{
+		{
+			Name:    releaseName,
+			Info:    &release.Info{Status: common.StatusSuperseded},
+			Chart:   &chart.Chart{},
+			Version: 1,
+		},
+		{
+			Name:    releaseName,
+			Info:    &release.Info{Status: common.StatusDeployed},
+			Chart:   &chart.Chart{},
+			Version: 2,
+		},
+	}
+	storage := storageFixture()
+	for _, rel := range rels {
+		if err := storage.Create(rel); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	customDescription := "Rollback due to critical bug in version 2"
+	_, _, err := executeActionCommandC(storage, fmt.Sprintf("rollback %s 1 --description '%s'", releaseName, customDescription))
+	if err != nil {
+		t.Fatalf("unexpected error, got '%v'", err)
+	}
+
+	// Verify the description was stored correctly
+	updatedReli, err := storage.Get(releaseName, 3)
+	if err != nil {
+		t.Fatalf("unexpected error getting release, got '%v'", err)
+	}
+	updatedRel, err := releaserToV1Release(updatedReli)
+	if err != nil {
+		t.Fatalf("unexpected error converting release, got '%v'", err)
+	}
+
+	if updatedRel.Info.Description != customDescription {
+		t.Errorf("Expected description '%s', got '%s'", customDescription, updatedRel.Info.Description)
+	}
+}
+
+func TestRollbackDescriptionTooLong(t *testing.T) {
+	releaseName := "funny-bunny-long-desc"
+	rels := []*release.Release{
+		{
+			Name:    releaseName,
+			Info:    &release.Info{Status: common.StatusSuperseded},
+			Chart:   &chart.Chart{},
+			Version: 1,
+		},
+		{
+			Name:    releaseName,
+			Info:    &release.Info{Status: common.StatusDeployed},
+			Chart:   &chart.Chart{},
+			Version: 2,
+		},
+	}
+	storage := storageFixture()
+	for _, rel := range rels {
+		if err := storage.Create(rel); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// Create a description that exceeds the 512 character limit
+	longDescription := strings.Repeat("a", 513)
+	_, _, err := executeActionCommandC(storage, fmt.Sprintf("rollback %s 1 --description '%s'", releaseName, longDescription))
+	if err == nil {
+		t.Error("expected error for description exceeding max length, got success")
+	}
+	if err != nil && !strings.Contains(err.Error(), "description must be 512 characters or less") {
+		t.Errorf("expected error about description length, got: %v", err)
+	}
+}
+
 func TestRollbackWithLabels(t *testing.T) {
 	labels1 := map[string]string{"operation": "install", "firstLabel": "firstValue"}
 	labels2 := map[string]string{"operation": "upgrade", "secondLabel": "secondValue"}

pkg/cmd/testdata/output/rollback-with-description.txt

@@ -0,0 +1,2 @@
+Rollback was a success! Happy Helming!
+