diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index cd160dfec..ec351b09a 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -57,6 +57,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. @@ -166,6 +168,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, @@ -179,7 +187,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, diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go new file mode 100644 index 000000000..2ab1c5048 --- /dev/null +++ b/pkg/action/rollback_test.go @@ -0,0 +1,162 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/release/common" +) + +func rollbackAction(t *testing.T) *Rollback { + t.Helper() + config := actionConfigFixture(t) + rollAction := NewRollback(config) + return rollAction +} + +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) +} + +func TestNewRollback(t *testing.T) { + is := assert.New(t) + config := actionConfigFixture(t) + + rollback := NewRollback(config) + + is.NotNil(rollback) + is.Equal(config, rollback.cfg) + is.Equal(DryRunNone, rollback.DryRunStrategy) + is.Empty(rollback.Description) +} diff --git a/pkg/cmd/rollback.go b/pkg/cmd/rollback.go index 00a2725bc..f402ac536 100644 --- a/pkg/cmd/rollback.go +++ b/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") diff --git a/pkg/cmd/rollback_test.go b/pkg/cmd/rollback_test.go index 116e158fd..adcb07284 100644 --- a/pkg/cmd/rollback_test.go +++ b/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"} diff --git a/pkg/cmd/testdata/output/rollback-with-description.txt b/pkg/cmd/testdata/output/rollback-with-description.txt new file mode 100644 index 000000000..a034dd2df --- /dev/null +++ b/pkg/cmd/testdata/output/rollback-with-description.txt @@ -0,0 +1,2 @@ +Rollback was a success! Happy Helming! +