diff --git a/internal/cli/output/color.go b/internal/cli/output/color.go index 93bbbe56e..e59cdde87 100644 --- a/internal/cli/output/color.go +++ b/internal/cli/output/color.go @@ -19,24 +19,24 @@ package output import ( "github.com/fatih/color" - release "helm.sh/helm/v4/pkg/release/v1" + "helm.sh/helm/v4/pkg/release/common" ) // ColorizeStatus returns a colorized version of the status string based on the status value -func ColorizeStatus(status release.Status, noColor bool) string { +func ColorizeStatus(status common.Status, noColor bool) string { // Disable color if requested if noColor { return status.String() } switch status { - case release.StatusDeployed: + case common.StatusDeployed: return color.GreenString(status.String()) - case release.StatusFailed: + case common.StatusFailed: return color.RedString(status.String()) - case release.StatusPendingInstall, release.StatusPendingUpgrade, release.StatusPendingRollback, release.StatusUninstalling: + case common.StatusPendingInstall, common.StatusPendingUpgrade, common.StatusPendingRollback, common.StatusUninstalling: return color.YellowString(status.String()) - case release.StatusUnknown: + case common.StatusUnknown: return color.RedString(status.String()) default: // For uninstalled, superseded, and any other status diff --git a/internal/cli/output/color_test.go b/internal/cli/output/color_test.go index c84e2c359..3b8de39e8 100644 --- a/internal/cli/output/color_test.go +++ b/internal/cli/output/color_test.go @@ -20,63 +20,63 @@ import ( "strings" "testing" - release "helm.sh/helm/v4/pkg/release/v1" + "helm.sh/helm/v4/pkg/release/common" ) func TestColorizeStatus(t *testing.T) { tests := []struct { name string - status release.Status + status common.Status noColor bool envNoColor string wantColor bool // whether we expect color codes in output }{ { name: "deployed status with color", - status: release.StatusDeployed, + status: common.StatusDeployed, noColor: false, envNoColor: "", wantColor: true, }, { name: "deployed status without color flag", - status: release.StatusDeployed, + status: common.StatusDeployed, noColor: true, envNoColor: "", wantColor: false, }, { name: "deployed status with NO_COLOR env", - status: release.StatusDeployed, + status: common.StatusDeployed, noColor: false, envNoColor: "1", wantColor: false, }, { name: "failed status with color", - status: release.StatusFailed, + status: common.StatusFailed, noColor: false, envNoColor: "", wantColor: true, }, { name: "pending install status with color", - status: release.StatusPendingInstall, + status: common.StatusPendingInstall, noColor: false, envNoColor: "", wantColor: true, }, { name: "unknown status with color", - status: release.StatusUnknown, + status: common.StatusUnknown, noColor: false, envNoColor: "", wantColor: true, }, { name: "superseded status with color", - status: release.StatusSuperseded, + status: common.StatusSuperseded, noColor: false, envNoColor: "", wantColor: false, // superseded doesn't get colored diff --git a/pkg/action/action.go b/pkg/action/action.go index a3db047b4..fd75b85d3 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -47,6 +47,7 @@ import ( "helm.sh/helm/v4/pkg/kube" "helm.sh/helm/v4/pkg/postrenderer" "helm.sh/helm/v4/pkg/registry" + ri "helm.sh/helm/v4/pkg/release" release "helm.sh/helm/v4/pkg/release/v1" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/storage" @@ -412,7 +413,7 @@ func (cfg *Configuration) Now() time.Time { return Timestamper() } -func (cfg *Configuration) releaseContent(name string, version int) (*release.Release, error) { +func (cfg *Configuration) releaseContent(name string, version int) (ri.Releaser, error) { if err := chartutil.ValidateReleaseName(name); err != nil { return nil, fmt.Errorf("releaseContent: Release name is invalid: %s", name) } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index d993da100..06329095e 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -36,6 +36,7 @@ import ( "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" "helm.sh/helm/v4/pkg/registry" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/storage" "helm.sh/helm/v4/pkg/storage/driver" @@ -249,10 +250,10 @@ func withKube(version string) chartOption { // releaseStub creates a release stub, complete with the chartStub as its chart. func releaseStub() *release.Release { - return namedReleaseStub("angry-panda", release.StatusDeployed) + return namedReleaseStub("angry-panda", rcommon.StatusDeployed) } -func namedReleaseStub(name string, status release.Status) *release.Release { +func namedReleaseStub(name string, status rcommon.Status) *release.Release { now := time.Now() return &release.Release{ Name: name, diff --git a/pkg/action/get_metadata.go b/pkg/action/get_metadata.go index 2677a57ad..5312dac7f 100644 --- a/pkg/action/get_metadata.go +++ b/pkg/action/get_metadata.go @@ -17,12 +17,15 @@ limitations under the License. package action import ( + "errors" "log/slog" "sort" "strings" "time" ci "helm.sh/helm/v4/pkg/chart" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release" ) // GetMetadata is the action for checking a given release's metadata. @@ -69,24 +72,40 @@ func (g *GetMetadata) Run(name string) (*Metadata, error) { return nil, err } - ac, err := ci.NewAccessor(rel.Chart) + rac, err := release.NewAccessor(rel) if err != nil { return nil, err } + ac, err := ci.NewAccessor(rac.Chart()) + if err != nil { + return nil, err + } + + charti := rac.Chart() + + var chrt *chart.Chart + switch c := charti.(type) { + case *chart.Chart: + chrt = c + case chart.Chart: + chrt = &c + default: + return nil, errors.New("invalid chart apiVersion") + } return &Metadata{ - Name: rel.Name, - Chart: rel.Chart.Metadata.Name, - Version: rel.Chart.Metadata.Version, - AppVersion: rel.Chart.Metadata.AppVersion, + Name: rac.Name(), + Chart: chrt.Metadata.Name, + Version: chrt.Metadata.Version, + AppVersion: chrt.Metadata.AppVersion, Dependencies: ac.MetaDependencies(), - Annotations: rel.Chart.Metadata.Annotations, - Labels: rel.Labels, - Namespace: rel.Namespace, - Revision: rel.Version, - Status: rel.Info.Status.String(), - DeployedAt: rel.Info.LastDeployed.Format(time.RFC3339), - ApplyMethod: rel.ApplyMethod, + Annotations: chrt.Metadata.Annotations, + Labels: rac.Labels(), + Namespace: rac.Namespace(), + Revision: rac.Version(), + Status: rac.Status(), + DeployedAt: rac.DeployedAt().Format(time.RFC3339), + ApplyMethod: rac.ApplyMethod(), }, nil } diff --git a/pkg/action/get_metadata_test.go b/pkg/action/get_metadata_test.go index 27e9f3777..cd5988d8e 100644 --- a/pkg/action/get_metadata_test.go +++ b/pkg/action/get_metadata_test.go @@ -28,6 +28,7 @@ import ( ci "helm.sh/helm/v4/pkg/chart" chart "helm.sh/helm/v4/pkg/chart/v2" kubefake "helm.sh/helm/v4/pkg/kube/fake" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -50,7 +51,7 @@ func TestGetMetadata_Run_BasicMetadata(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -64,7 +65,8 @@ func TestGetMetadata_Run_BasicMetadata(t *testing.T) { Namespace: "default", } - cfg.Releases.Create(rel) + err := cfg.Releases.Create(rel) + require.NoError(t, err) result, err := client.Run(releaseName) require.NoError(t, err) @@ -104,7 +106,7 @@ func TestGetMetadata_Run_WithDependencies(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -163,7 +165,7 @@ func TestGetMetadata_Run_WithDependenciesAliases(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -234,7 +236,7 @@ func TestGetMetadata_Run_WithMixedDependencies(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -298,7 +300,7 @@ func TestGetMetadata_Run_WithAnnotations(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -337,7 +339,7 @@ func TestGetMetadata_Run_SpecificVersion(t *testing.T) { rel1 := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusSuperseded, + Status: common.StatusSuperseded, LastDeployed: deployedTime.Add(-time.Hour), }, Chart: &chart.Chart{ @@ -354,7 +356,7 @@ func TestGetMetadata_Run_SpecificVersion(t *testing.T) { rel2 := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -388,16 +390,16 @@ func TestGetMetadata_Run_DifferentStatuses(t *testing.T) { testCases := []struct { name string - status release.Status + status common.Status expected string }{ - {"deployed", release.StatusDeployed, "deployed"}, - {"failed", release.StatusFailed, "failed"}, - {"uninstalled", release.StatusUninstalled, "uninstalled"}, - {"pending-install", release.StatusPendingInstall, "pending-install"}, - {"pending-upgrade", release.StatusPendingUpgrade, "pending-upgrade"}, - {"pending-rollback", release.StatusPendingRollback, "pending-rollback"}, - {"superseded", release.StatusSuperseded, "superseded"}, + {"deployed", common.StatusDeployed, "deployed"}, + {"failed", common.StatusFailed, "failed"}, + {"uninstalled", common.StatusUninstalled, "uninstalled"}, + {"pending-install", common.StatusPendingInstall, "pending-install"}, + {"pending-upgrade", common.StatusPendingUpgrade, "pending-upgrade"}, + {"pending-rollback", common.StatusPendingRollback, "pending-rollback"}, + {"superseded", common.StatusSuperseded, "superseded"}, } for _, tc := range testCases { @@ -464,7 +466,7 @@ func TestGetMetadata_Run_EmptyAppVersion(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, LastDeployed: deployedTime, }, Chart: &chart.Chart{ @@ -640,7 +642,7 @@ func TestMetadata_FormattedDepNames_WithAliases(t *testing.T) { func TestGetMetadata_Labels(t *testing.T) { rel := releaseStub() - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed customLabels := map[string]string{"key1": "value1", "key2": "value2"} rel.Labels = customLabels diff --git a/pkg/action/get_values.go b/pkg/action/get_values.go index a0b5d92c1..6475a140b 100644 --- a/pkg/action/get_values.go +++ b/pkg/action/get_values.go @@ -16,7 +16,13 @@ limitations under the License. package action -import "helm.sh/helm/v4/pkg/chart/common/util" +import ( + "fmt" + + "helm.sh/helm/v4/pkg/chart/common/util" + release "helm.sh/helm/v4/pkg/release" + rspb "helm.sh/helm/v4/pkg/release/v1" +) // GetValues is the action for checking a given release's values. // @@ -41,7 +47,12 @@ func (g *GetValues) Run(name string) (map[string]interface{}, error) { return nil, err } - rel, err := g.cfg.releaseContent(name, g.Version) + reli, err := g.cfg.releaseContent(name, g.Version) + if err != nil { + return nil, err + } + + rel, err := releaserToV1Release(reli) if err != nil { return nil, err } @@ -56,3 +67,18 @@ func (g *GetValues) Run(name string) (map[string]interface{}, error) { } return rel.Config, nil } + +// releaserToV1Release is a helper function to convert a v1 release passed by interface +// into the type object. +func releaserToV1Release(rel release.Releaser) (*rspb.Release, error) { + switch r := rel.(type) { + case rspb.Release: + return &r, nil + case *rspb.Release: + return r, nil + case nil: + return nil, nil + default: + return nil, fmt.Errorf("unsupported release type: %T", rel) + } +} diff --git a/pkg/action/get_values_test.go b/pkg/action/get_values_test.go index b8630c322..8e6588454 100644 --- a/pkg/action/get_values_test.go +++ b/pkg/action/get_values_test.go @@ -26,6 +26,7 @@ import ( chart "helm.sh/helm/v4/pkg/chart/v2" kubefake "helm.sh/helm/v4/pkg/kube/fake" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -58,7 +59,7 @@ func TestGetValues_Run_UserConfigOnly(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ @@ -112,7 +113,7 @@ func TestGetValues_Run_AllValues(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ @@ -147,7 +148,7 @@ func TestGetValues_Run_EmptyValues(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ @@ -198,7 +199,7 @@ func TestGetValues_Run_NilConfig(t *testing.T) { rel := &release.Release{ Name: releaseName, Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ diff --git a/pkg/action/history.go b/pkg/action/history.go index d7af1d6a4..dc3ab51d4 100644 --- a/pkg/action/history.go +++ b/pkg/action/history.go @@ -22,7 +22,7 @@ import ( "fmt" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" - release "helm.sh/helm/v4/pkg/release/v1" + release "helm.sh/helm/v4/pkg/release" ) // History is the action for checking the release's ledger. @@ -46,7 +46,7 @@ func NewHistory(cfg *Configuration) *History { } // Run executes 'helm history' against the given release. -func (h *History) Run(name string) ([]*release.Release, error) { +func (h *History) Run(name string) ([]release.Releaser, error) { if err := h.cfg.KubeClient.IsReachable(); err != nil { return nil, err } diff --git a/pkg/action/hooks_test.go b/pkg/action/hooks_test.go index fb7d1b4ec..f43c45d77 100644 --- a/pkg/action/hooks_test.go +++ b/pkg/action/hooks_test.go @@ -33,6 +33,7 @@ import ( "helm.sh/helm/v4/pkg/chart/common" "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/storage" "helm.sh/helm/v4/pkg/storage/driver" @@ -187,7 +188,7 @@ func runInstallForHooksWithSuccess(t *testing.T, manifest, expectedNamespace str res, err := instAction.Run(buildChartWithTemplates(templates), vals) is.NoError(err) is.Equal(expectedOutput, outBuffer.String()) - is.Equal(release.StatusDeployed, res.Info.Status) + is.Equal(rcommon.StatusDeployed, res.Info.Status) } func runInstallForHooksWithFailure(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) { @@ -215,7 +216,7 @@ func runInstallForHooksWithFailure(t *testing.T, manifest, expectedNamespace str is.Error(err) is.Contains(res.Info.Description, "failed pre-install") is.Equal(expectedOutput, outBuffer.String()) - is.Equal(release.StatusFailed, res.Info.Status) + is.Equal(rcommon.StatusFailed, res.Info.Status) } type HookFailedError struct{} diff --git a/pkg/action/install.go b/pkg/action/install.go index 6c5d62d81..49a36b276 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -53,6 +53,8 @@ import ( kubefake "helm.sh/helm/v4/pkg/kube/fake" "helm.sh/helm/v4/pkg/postrenderer" "helm.sh/helm/v4/pkg/registry" + ri "helm.sh/helm/v4/pkg/release" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/repo/v1" @@ -353,13 +355,13 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st } // Check error from render if err != nil { - rel.SetStatus(release.StatusFailed, fmt.Sprintf("failed to render resource: %s", err.Error())) + rel.SetStatus(rcommon.StatusFailed, fmt.Sprintf("failed to render resource: %s", err.Error())) // Return a release with partial data so that the client can show debugging information. return rel, err } // Mark this release as in-progress - rel.SetStatus(release.StatusPendingInstall, "Initial install underway") + rel.SetStatus(rcommon.StatusPendingInstall, "Initial install underway") var toBeAdopted kube.ResourceList resources, err := i.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), !i.DisableOpenAPIValidation) @@ -524,9 +526,9 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource } if len(i.Description) > 0 { - rel.SetStatus(release.StatusDeployed, i.Description) + rel.SetStatus(rcommon.StatusDeployed, i.Description) } else { - rel.SetStatus(release.StatusDeployed, "Install complete") + rel.SetStatus(rcommon.StatusDeployed, "Install complete") } // This is a tricky case. The release has been created, but the result @@ -544,7 +546,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource } func (i *Install) failRelease(rel *release.Release, err error) (*release.Release, error) { - rel.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", i.ReleaseName, err.Error())) + rel.SetStatus(rcommon.StatusFailed, fmt.Sprintf("Release %q failed: %s", i.ReleaseName, err.Error())) if i.RollbackOnFailure { slog.Debug("install failed and rollback-on-failure is set, uninstalling release", "release", i.ReleaseName) uninstall := NewUninstall(i.cfg) @@ -583,15 +585,43 @@ func (i *Install) availableName() error { if err != nil || len(h) < 1 { return nil } - releaseutil.Reverse(h, releaseutil.SortByRevision) - rel := h[0] - if st := rel.Info.Status; i.Replace && (st == release.StatusUninstalled || st == release.StatusFailed) { + hl, err := releaseListToV1List(h) + if err != nil { + return err + } + + releaseutil.Reverse(hl, releaseutil.SortByRevision) + rel := hl[0] + + if st := rel.Info.Status; i.Replace && (st == rcommon.StatusUninstalled || st == rcommon.StatusFailed) { return nil } return errors.New("cannot reuse a name that is still in use") } +func releaseListToV1List(ls []ri.Releaser) ([]*release.Release, error) { + rls := make([]*release.Release, 0, len(ls)) + for _, val := range ls { + rel, err := releaserToV1Release(val) + if err != nil { + return nil, err + } + rls = append(rls, rel) + } + + return rls, nil +} + +func releaseV1ListToReleaserList(ls []*release.Release) ([]ri.Releaser, error) { + rls := make([]ri.Releaser, 0, len(ls)) + for _, val := range ls { + rls = append(rls, val) + } + + return rls, nil +} + // createRelease creates a new release object func (i *Install) createRelease(chrt *chart.Chart, rawVals map[string]interface{}, labels map[string]string) *release.Release { ts := i.cfg.Now() @@ -604,7 +634,7 @@ func (i *Install) createRelease(chrt *chart.Chart, rawVals map[string]interface{ Info: &release.Info{ FirstDeployed: ts, LastDeployed: ts, - Status: release.StatusUnknown, + Status: rcommon.StatusUnknown, }, Version: 1, Labels: labels, @@ -630,20 +660,24 @@ func (i *Install) replaceRelease(rel *release.Release) error { // No releases exist for this name, so we can return early return nil } + hl, err := releaseListToV1List(hist) + if err != nil { + return err + } - releaseutil.Reverse(hist, releaseutil.SortByRevision) - last := hist[0] + releaseutil.Reverse(hl, releaseutil.SortByRevision) + last := hl[0] // Update version to the next available rel.Version = last.Version + 1 // Do not change the status of a failed release. - if last.Info.Status == release.StatusFailed { + if last.Info.Status == rcommon.StatusFailed { return nil } // For any other status, mark it as superseded and store the old record - last.SetStatus(release.StatusSuperseded, "superseded by new release") + last.SetStatus(rcommon.StatusSuperseded, "superseded by new release") return i.recordRelease(last) } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 8d1fc149d..535b14559 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -47,6 +47,7 @@ import ( "helm.sh/helm/v4/pkg/chart/common" "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/storage/driver" ) @@ -137,7 +138,10 @@ func TestInstallRelease(t *testing.T) { is.Equal(res.Name, "test-install-release", "Expected release name.") is.Equal(res.Namespace, "spaced") - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + + rel, err := releaserToV1Release(r) is.NoError(err) is.Len(rel.Hooks, 1) @@ -156,7 +160,9 @@ func TestInstallRelease(t *testing.T) { time.Sleep(time.Millisecond * 100) lastRelease, err := instAction.cfg.Releases.Last(rel.Name) req.NoError(err) - is.Equal(lastRelease.Info.Status, release.StatusDeployed) + lrel, err := releaserToV1Release(lastRelease) + is.NoError(err) + is.Equal(lrel.Info.Status, rcommon.StatusDeployed) } func TestInstallReleaseWithTakeOwnership_ResourceNotOwned(t *testing.T) { @@ -180,7 +186,10 @@ func TestInstallReleaseWithTakeOwnership_ResourceNotOwned(t *testing.T) { t.Fatalf("Failed install: %s", err) } - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + + rel, err := releaserToV1Release(r) is.NoError(err) is.Equal(rel.Info.Description, "Install complete") @@ -197,7 +206,10 @@ func TestInstallReleaseWithTakeOwnership_ResourceOwned(t *testing.T) { if err != nil { t.Fatalf("Failed install: %s", err) } - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + + rel, err := releaserToV1Release(r) is.NoError(err) is.Equal(rel.Info.Description, "Install complete") @@ -234,7 +246,10 @@ func TestInstallReleaseWithValues(t *testing.T) { is.Equal(res.Name, "test-install-release", "Expected release name.") is.Equal(res.Namespace, "spaced") - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + + rel, err := releaserToV1Release(r) is.NoError(err) is.Len(rel.Hooks, 1) @@ -273,7 +288,9 @@ func TestInstallRelease_WithNotes(t *testing.T) { is.Equal(res.Name, "with-notes") is.Equal(res.Namespace, "spaced") - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + rel, err := releaserToV1Release(r) is.NoError(err) is.Len(rel.Hooks, 1) is.Equal(rel.Hooks[0].Manifest, manifestWithHook) @@ -297,7 +314,9 @@ func TestInstallRelease_WithNotesRendered(t *testing.T) { t.Fatalf("Failed install: %s", err) } - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + rel, err := releaserToV1Release(r) is.NoError(err) expectedNotes := fmt.Sprintf("got-%s", res.Name) @@ -316,7 +335,9 @@ func TestInstallRelease_WithChartAndDependencyParentNotes(t *testing.T) { t.Fatalf("Failed install: %s", err) } - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + rel, err := releaserToV1Release(r) is.NoError(err) is.Equal("with-notes", rel.Name) is.Equal("parent", rel.Info.Notes) @@ -335,7 +356,9 @@ func TestInstallRelease_WithChartAndDependencyAllNotes(t *testing.T) { t.Fatalf("Failed install: %s", err) } - rel, err := instAction.cfg.Releases.Get(res.Name, res.Version) + r, err := instAction.cfg.Releases.Get(res.Name, res.Version) + is.NoError(err) + rel, err := releaserToV1Release(r) is.NoError(err) is.Equal("with-notes", rel.Name) // test run can return as either 'parent\nchild' or 'child\nparent' @@ -478,7 +501,7 @@ func TestInstallRelease_FailedHooks(t *testing.T) { is.Error(err) is.Contains(res.Info.Description, "failed post-install") is.Equal("", outBuffer.String()) - is.Equal(release.StatusFailed, res.Info.Status) + is.Equal(rcommon.StatusFailed, res.Info.Status) } func TestInstallRelease_ReplaceRelease(t *testing.T) { @@ -487,7 +510,7 @@ func TestInstallRelease_ReplaceRelease(t *testing.T) { instAction.Replace = true rel := releaseStub() - rel.Info.Status = release.StatusUninstalled + rel.Info.Status = rcommon.StatusUninstalled instAction.cfg.Releases.Create(rel) instAction.ReleaseName = rel.Name @@ -499,9 +522,11 @@ func TestInstallRelease_ReplaceRelease(t *testing.T) { is.Equal(2, res.Version) is.Equal(res.Name, rel.Name) - getres, err := instAction.cfg.Releases.Get(rel.Name, res.Version) + r, err := instAction.cfg.Releases.Get(rel.Name, res.Version) + is.NoError(err) + getres, err := releaserToV1Release(r) is.NoError(err) - is.Equal(getres.Info.Status, release.StatusDeployed) + is.Equal(getres.Info.Status, rcommon.StatusDeployed) } func TestInstallRelease_KubeVersion(t *testing.T) { @@ -534,7 +559,7 @@ func TestInstallRelease_Wait(t *testing.T) { res, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(res.Info.Description, "I timed out") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(res.Info.Status, rcommon.StatusFailed) is.Equal(goroutines, instAction.getGoroutineCount()) } @@ -575,7 +600,7 @@ func TestInstallRelease_WaitForJobs(t *testing.T) { res, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(res.Info.Description, "I timed out") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(res.Info.Status, rcommon.StatusFailed) } func TestInstallRelease_RollbackOnFailure(t *testing.T) { diff --git a/pkg/action/list.go b/pkg/action/list.go index c6d6f2037..ea743db60 100644 --- a/pkg/action/list.go +++ b/pkg/action/list.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/labels" + ri "helm.sh/helm/v4/pkg/release" release "helm.sh/helm/v4/pkg/release/v1" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" ) @@ -145,7 +146,7 @@ func NewList(cfg *Configuration) *List { } // Run executes the list command, returning a set of matches. -func (l *List) Run() ([]*release.Release, error) { +func (l *List) Run() ([]ri.Releaser, error) { if err := l.cfg.KubeClient.IsReachable(); err != nil { return nil, err } @@ -159,9 +160,13 @@ func (l *List) Run() ([]*release.Release, error) { } } - results, err := l.cfg.Releases.List(func(rel *release.Release) bool { + results, err := l.cfg.Releases.List(func(rel ri.Releaser) bool { + r, err := releaserToV1Release(rel) + if err != nil { + return false + } // Skip anything that doesn't match the filter. - if filter != nil && !filter.MatchString(rel.Name) { + if filter != nil && !filter.MatchString(r.Name) { return false } @@ -176,30 +181,35 @@ func (l *List) Run() ([]*release.Release, error) { return results, nil } + rresults, err := releaseListToV1List(results) + if err != nil { + return nil, err + } + // by definition, superseded releases are never shown if // only the latest releases are returned. so if requested statemask // is _only_ ListSuperseded, skip the latest release filter if l.StateMask != ListSuperseded { - results = filterLatestReleases(results) + rresults = filterLatestReleases(rresults) } // State mask application must occur after filtering to // latest releases, otherwise outdated entries can be returned - results = l.filterStateMask(results) + rresults = l.filterStateMask(rresults) // Skip anything that doesn't match the selector selectorObj, err := labels.Parse(l.Selector) if err != nil { return nil, err } - results = l.filterSelector(results, selectorObj) + rresults = l.filterSelector(rresults, selectorObj) // Unfortunately, we have to sort before truncating, which can incur substantial overhead - l.sort(results) + l.sort(rresults) // Guard on offset - if l.Offset >= len(results) { - return []*release.Release{}, nil + if l.Offset >= len(rresults) { + return releaseV1ListToReleaserList([]*release.Release{}) } // Calculate the limit and offset, and then truncate results if necessary. @@ -208,12 +218,12 @@ func (l *List) Run() ([]*release.Release, error) { limit = l.Limit } last := l.Offset + limit - if l := len(results); l < last { + if l := len(rresults); l < last { last = l } - results = results[l.Offset:last] + rresults = rresults[l.Offset:last] - return results, err + return releaseV1ListToReleaserList(rresults) } // sort is an in-place sort where order is based on the value of a.Sort diff --git a/pkg/action/list_test.go b/pkg/action/list_test.go index 75737d635..bf34b0ba1 100644 --- a/pkg/action/list_test.go +++ b/pkg/action/list_test.go @@ -24,6 +24,8 @@ import ( "github.com/stretchr/testify/assert" kubefake "helm.sh/helm/v4/pkg/kube/fake" + ri "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/storage" ) @@ -96,8 +98,11 @@ func TestList_Sort(t *testing.T) { lister := newListFixture(t) lister.Sort = ByNameDesc // Other sorts are tested elsewhere makeMeSomeReleases(t, lister.cfg.Releases) - list, err := lister.Run() + l, err := lister.Run() + is.NoError(err) + list, err := releaseListToV1List(l) is.NoError(err) + is.Len(list, 3) is.Equal("two", list[0].Name) is.Equal("three", list[1].Name) @@ -109,7 +114,9 @@ func TestList_Limit(t *testing.T) { lister := newListFixture(t) lister.Limit = 2 makeMeSomeReleases(t, lister.cfg.Releases) - list, err := lister.Run() + l, err := lister.Run() + is.NoError(err) + list, err := releaseListToV1List(l) is.NoError(err) is.Len(list, 2) // Lex order means one, three, two @@ -122,7 +129,9 @@ func TestList_BigLimit(t *testing.T) { lister := newListFixture(t) lister.Limit = 20 makeMeSomeReleases(t, lister.cfg.Releases) - list, err := lister.Run() + l, err := lister.Run() + is.NoError(err) + list, err := releaseListToV1List(l) is.NoError(err) is.Len(list, 3) @@ -138,7 +147,9 @@ func TestList_LimitOffset(t *testing.T) { lister.Limit = 2 lister.Offset = 1 makeMeSomeReleases(t, lister.cfg.Releases) - list, err := lister.Run() + l, err := lister.Run() + is.NoError(err) + list, err := releaseListToV1List(l) is.NoError(err) is.Len(list, 2) @@ -168,23 +179,42 @@ func TestList_StateMask(t *testing.T) { is := assert.New(t) lister := newListFixture(t) makeMeSomeReleases(t, lister.cfg.Releases) - one, err := lister.cfg.Releases.Get("one", 1) + oner, err := lister.cfg.Releases.Get("one", 1) is.NoError(err) - one.SetStatus(release.StatusUninstalled, "uninstalled") + + var one release.Release + switch v := oner.(type) { + case release.Release: + one = v + case *release.Release: + one = *v + default: + t.Fatal("unsupported release type") + } + + one.SetStatus(common.StatusUninstalled, "uninstalled") err = lister.cfg.Releases.Update(one) is.NoError(err) res, err := lister.Run() is.NoError(err) is.Len(res, 2) - is.Equal("three", res[0].Name) - is.Equal("two", res[1].Name) + + ac0, err := ri.NewAccessor(res[0]) + is.NoError(err) + ac1, err := ri.NewAccessor(res[1]) + is.NoError(err) + + is.Equal("three", ac0.Name()) + is.Equal("two", ac1.Name()) lister.StateMask = ListUninstalled res, err = lister.Run() is.NoError(err) is.Len(res, 1) - is.Equal("one", res[0].Name) + ac0, err = ri.NewAccessor(res[0]) + is.NoError(err) + is.Equal("one", ac0.Name()) lister.StateMask |= ListDeployed res, err = lister.Run() @@ -206,28 +236,30 @@ func TestList_StateMaskWithStaleRevisions(t *testing.T) { // "dirty" release should _not_ be present as most recent // release is deployed despite failed release in past - is.Equal("failed", res[0].Name) + ac0, err := ri.NewAccessor(res[0]) + is.NoError(err) + is.Equal("failed", ac0.Name()) } func makeMeSomeReleasesWithStaleFailure(t *testing.T, store *storage.Storage) { t.Helper() - one := namedReleaseStub("clean", release.StatusDeployed) + one := namedReleaseStub("clean", common.StatusDeployed) one.Namespace = "default" one.Version = 1 - two := namedReleaseStub("dirty", release.StatusDeployed) + two := namedReleaseStub("dirty", common.StatusDeployed) two.Namespace = "default" two.Version = 1 - three := namedReleaseStub("dirty", release.StatusFailed) + three := namedReleaseStub("dirty", common.StatusFailed) three.Namespace = "default" three.Version = 2 - four := namedReleaseStub("dirty", release.StatusDeployed) + four := namedReleaseStub("dirty", common.StatusDeployed) four.Namespace = "default" four.Version = 3 - five := namedReleaseStub("failed", release.StatusFailed) + five := namedReleaseStub("failed", common.StatusFailed) five.Namespace = "default" five.Version = 1 @@ -251,7 +283,9 @@ func TestList_Filter(t *testing.T) { res, err := lister.Run() is.NoError(err) is.Len(res, 1) - is.Equal("three", res[0].Name) + ac0, err := ri.NewAccessor(res[0]) + is.NoError(err) + is.Equal("three", ac0.Name()) } func TestList_FilterFailsCompile(t *testing.T) { diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index d00c8d8c0..b649579f4 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -28,6 +28,7 @@ import ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" "helm.sh/helm/v4/pkg/kube" + ri "helm.sh/helm/v4/pkg/release" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -56,7 +57,7 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting { } // Run executes 'helm test' against the given release. -func (r *ReleaseTesting) Run(name string) (*release.Release, error) { +func (r *ReleaseTesting) Run(name string) (ri.Releaser, error) { if err := r.cfg.KubeClient.IsReachable(); err != nil { return nil, err } @@ -66,7 +67,12 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { } // finds the non-deleted release with the given name - rel, err := r.cfg.Releases.Last(name) + reli, err := r.cfg.Releases.Last(name) + if err != nil { + return reli, err + } + + rel, err := releaserToV1Release(reli) if err != nil { return rel, err } diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index ade3ab233..a146e0e73 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -25,6 +25,7 @@ import ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" "helm.sh/helm/v4/pkg/kube" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -111,7 +112,12 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele return nil, nil, false, errInvalidRevision } - currentRelease, err := r.cfg.Releases.Last(name) + currentReleasei, err := r.cfg.Releases.Last(name) + if err != nil { + return nil, nil, false, err + } + + currentRelease, err := releaserToV1Release(currentReleasei) if err != nil { return nil, nil, false, err } @@ -128,7 +134,11 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele // Check if the history version to be rolled back exists previousVersionExist := false - for _, historyRelease := range historyReleases { + for _, historyReleasei := range historyReleases { + historyRelease, err := releaserToV1Release(historyReleasei) + if err != nil { + return nil, nil, false, err + } version := historyRelease.Version if previousVersion == version { previousVersionExist = true @@ -141,7 +151,11 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele slog.Debug("rolling back", "name", name, "currentVersion", currentRelease.Version, "targetVersion", previousVersion) - previousRelease, err := r.cfg.Releases.Get(name, previousVersion) + previousReleasei, err := r.cfg.Releases.Get(name, previousVersion) + if err != nil { + return nil, nil, false, err + } + previousRelease, err := releaserToV1Release(previousReleasei) if err != nil { return nil, nil, false, err } @@ -160,7 +174,7 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele Info: &release.Info{ FirstDeployed: currentRelease.Info.FirstDeployed, LastDeployed: time.Now(), - Status: release.StatusPendingRollback, + Status: common.StatusPendingRollback, 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. @@ -217,8 +231,8 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas if err != nil { msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) slog.Warn(msg) - currentRelease.Info.Status = release.StatusSuperseded - targetRelease.Info.Status = release.StatusFailed + currentRelease.Info.Status = common.StatusSuperseded + targetRelease.Info.Status = common.StatusFailed targetRelease.Info.Description = msg r.cfg.recordRelease(currentRelease) r.cfg.recordRelease(targetRelease) @@ -241,14 +255,14 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas } if r.WaitForJobs { if err := waiter.WaitWithJobs(target, r.Timeout); err != nil { - targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + targetRelease.SetStatus(common.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) r.cfg.recordRelease(currentRelease) r.cfg.recordRelease(targetRelease) return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err) } } else { if err := waiter.Wait(target, r.Timeout); err != nil { - targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + targetRelease.SetStatus(common.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) r.cfg.recordRelease(currentRelease) r.cfg.recordRelease(targetRelease) return targetRelease, fmt.Errorf("release %s failed: %w", targetRelease.Name, err) @@ -267,13 +281,17 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas return nil, err } // Supersede all previous deployments, see issue #2941. - for _, rel := range deployed { + for _, reli := range deployed { + rel, err := releaserToV1Release(reli) + if err != nil { + return nil, err + } slog.Debug("superseding previous deployment", "version", rel.Version) - rel.Info.Status = release.StatusSuperseded + rel.Info.Status = common.StatusSuperseded r.cfg.recordRelease(rel) } - targetRelease.Info.Status = release.StatusDeployed + targetRelease.Info.Status = common.StatusDeployed return targetRelease, nil } diff --git a/pkg/action/status.go b/pkg/action/status.go index 509c52cd9..a9f2dcd4f 100644 --- a/pkg/action/status.go +++ b/pkg/action/status.go @@ -50,7 +50,12 @@ func (s *Status) Run(name string) (*release.Release, error) { return nil, err } - rel, err := s.cfg.releaseContent(name, s.Version) + reli, err := s.cfg.releaseContent(name, s.Version) + if err != nil { + return nil, err + } + + rel, err := releaserToV1Release(reli) if err != nil { return nil, err } diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 057c2118f..669431741 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -27,6 +27,7 @@ import ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" "helm.sh/helm/v4/pkg/kube" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/storage/driver" @@ -67,13 +68,18 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) } if u.DryRun { - r, err := u.cfg.releaseContent(name, 0) + ri, err := u.cfg.releaseContent(name, 0) + if err != nil { if u.IgnoreNotFound && errors.Is(err, driver.ErrReleaseNotFound) { return nil, nil } return &release.UninstallReleaseResponse{}, err } + r, err := releaserToV1Release(ri) + if err != nil { + return nil, err + } return &release.UninstallReleaseResponse{Release: r}, nil } @@ -81,23 +87,28 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) return nil, fmt.Errorf("uninstall: Release name is invalid: %s", name) } - rels, err := u.cfg.Releases.History(name) + relsi, err := u.cfg.Releases.History(name) if err != nil { if u.IgnoreNotFound { return nil, nil } return nil, fmt.Errorf("uninstall: Release not loaded: %s: %w", name, err) } - if len(rels) < 1 { + if len(relsi) < 1 { return nil, errMissingRelease } + rels, err := releaseListToV1List(relsi) + if err != nil { + return nil, err + } + releaseutil.SortByRevision(rels) rel := rels[len(rels)-1] // TODO: Are there any cases where we want to force a delete even if it's // already marked deleted? - if rel.Info.Status == release.StatusUninstalled { + if rel.Info.Status == common.StatusUninstalled { if !u.KeepHistory { if err := u.purgeReleases(rels...); err != nil { return nil, fmt.Errorf("uninstall: Failed to purge the release: %w", err) @@ -108,7 +119,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) } slog.Debug("uninstall: deleting release", "name", name) - rel.Info.Status = release.StatusUninstalling + rel.Info.Status = common.StatusUninstalling rel.Info.Deleted = time.Now() rel.Info.Description = "Deletion in progress (or silently failed)" res := &release.UninstallReleaseResponse{Release: rel} @@ -150,7 +161,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) } } - rel.Info.Status = release.StatusUninstalled + rel.Info.Status = common.StatusUninstalled if len(u.Description) > 0 { rel.Info.Description = u.Description } else { diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index 7c7344383..667e96820 100644 --- a/pkg/action/uninstall_test.go +++ b/pkg/action/uninstall_test.go @@ -27,7 +27,7 @@ import ( "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" - release "helm.sh/helm/v4/pkg/release/v1" + "helm.sh/helm/v4/pkg/release/common" ) func uninstallAction(t *testing.T) *Uninstall { @@ -119,7 +119,7 @@ func TestUninstallRelease_Wait(t *testing.T) { res, err := unAction.Run(rel.Name) is.Error(err) is.Contains(err.Error(), "U timed out") - is.Equal(res.Release.Info.Status, release.StatusUninstalled) + is.Equal(res.Release.Info.Status, common.StatusUninstalled) } func TestUninstallRelease_Cascade(t *testing.T) { diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 8dfc8b206..dc3761761 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -36,6 +36,7 @@ import ( "helm.sh/helm/v4/pkg/kube" "helm.sh/helm/v4/pkg/postrenderer" "helm.sh/helm/v4/pkg/registry" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/storage/driver" @@ -219,7 +220,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str } // finds the last non-deleted release with the given name - lastRelease, err := u.cfg.Releases.Last(name) + lastReleasei, err := u.cfg.Releases.Last(name) if err != nil { // to keep existing behavior of returning the "%q has no deployed releases" error when an existing release does not exist if errors.Is(err, driver.ErrReleaseNotFound) { @@ -228,26 +229,37 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str return nil, nil, false, err } + lastRelease, err := releaserToV1Release(lastReleasei) + if err != nil { + return nil, nil, false, err + } + // Concurrent `helm upgrade`s will either fail here with `errPending` or when creating the release with "already exists". This should act as a pessimistic lock. if lastRelease.Info.Status.IsPending() { return nil, nil, false, errPending } var currentRelease *release.Release - if lastRelease.Info.Status == release.StatusDeployed { + if lastRelease.Info.Status == rcommon.StatusDeployed { // no need to retrieve the last deployed release from storage as the last release is deployed currentRelease = lastRelease } else { // finds the deployed release with the given name - currentRelease, err = u.cfg.Releases.Deployed(name) + currentReleasei, err := u.cfg.Releases.Deployed(name) + var cerr error + currentRelease, cerr = releaserToV1Release(currentReleasei) + if cerr != nil { + return nil, nil, false, err + } if err != nil { if errors.Is(err, driver.ErrNoDeployedReleases) && - (lastRelease.Info.Status == release.StatusFailed || lastRelease.Info.Status == release.StatusSuperseded) { + (lastRelease.Info.Status == rcommon.StatusFailed || lastRelease.Info.Status == rcommon.StatusSuperseded) { currentRelease = lastRelease } else { return nil, nil, false, err } } + } // determine if values will be reused @@ -305,7 +317,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str Info: &release.Info{ FirstDeployed: currentRelease.Info.FirstDeployed, LastDeployed: Timestamper(), - Status: release.StatusPendingUpgrade, + Status: rcommon.StatusPendingUpgrade, Description: "Preparing upgrade", // This should be overwritten later. }, Version: revision, @@ -487,10 +499,10 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele } } - originalRelease.Info.Status = release.StatusSuperseded + originalRelease.Info.Status = rcommon.StatusSuperseded u.cfg.recordRelease(originalRelease) - upgradedRelease.Info.Status = release.StatusDeployed + upgradedRelease.Info.Status = rcommon.StatusDeployed if len(u.Description) > 0 { upgradedRelease.Info.Description = u.Description } else { @@ -503,7 +515,7 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e msg := fmt.Sprintf("Upgrade %q failed: %s", rel.Name, err) slog.Warn("upgrade failed", "name", rel.Name, slog.Any("error", err)) - rel.Info.Status = release.StatusFailed + rel.Info.Status = rcommon.StatusFailed rel.Info.Description = msg u.cfg.recordRelease(rel) if u.CleanupOnFail && len(created) > 0 { @@ -533,12 +545,16 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e return rel, fmt.Errorf("an error occurred while finding last successful release. original upgrade error: %w: %w", err, herr) } + fullHistoryV1, herr := releaseListToV1List(fullHistory) + if herr != nil { + return nil, herr + } // There isn't a way to tell if a previous release was successful, but // generally failed releases do not get superseded unless the next // release is successful, so this should be relatively safe filteredHistory := releaseutil.FilterFunc(func(r *release.Release) bool { - return r.Info.Status == release.StatusSuperseded || r.Info.Status == release.StatusDeployed - }).Filter(fullHistory) + return r.Info.Status == rcommon.StatusSuperseded || r.Info.Status == rcommon.StatusDeployed + }).Filter(fullHistoryV1) if len(filteredHistory) == 0 { return rel, fmt.Errorf("unable to find a previously successful release when attempting to rollback. original upgrade error: %w", err) } diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index c7930c769..67320b207 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -33,6 +33,7 @@ import ( "github.com/stretchr/testify/require" kubefake "helm.sh/helm/v4/pkg/kube/fake" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -52,7 +53,7 @@ func TestUpgradeRelease_Success(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "previous-release" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed req.NoError(upAction.cfg.Releases.Create(rel)) upAction.WaitStrategy = kube.StatusWatcherStrategy @@ -61,15 +62,17 @@ func TestUpgradeRelease_Success(t *testing.T) { ctx, done := context.WithCancel(t.Context()) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) req.NoError(err) - is.Equal(res.Info.Status, release.StatusDeployed) + is.Equal(res.Info.Status, common.StatusDeployed) done() // Detecting previous bug where context termination after successful release // caused release to fail. time.Sleep(time.Millisecond * 100) - lastRelease, err := upAction.cfg.Releases.Last(rel.Name) + lastReleasei, err := upAction.cfg.Releases.Last(rel.Name) req.NoError(err) - is.Equal(lastRelease.Info.Status, release.StatusDeployed) + lastRelease, err := releaserToV1Release(lastReleasei) + req.NoError(err) + is.Equal(lastRelease.Info.Status, common.StatusDeployed) } func TestUpgradeRelease_Wait(t *testing.T) { @@ -79,7 +82,7 @@ func TestUpgradeRelease_Wait(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "come-fail-away" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -91,7 +94,7 @@ func TestUpgradeRelease_Wait(t *testing.T) { res, err := upAction.Run(rel.Name, buildChart(), vals) req.Error(err) is.Contains(res.Info.Description, "I timed out") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(res.Info.Status, common.StatusFailed) } func TestUpgradeRelease_WaitForJobs(t *testing.T) { @@ -101,7 +104,7 @@ func TestUpgradeRelease_WaitForJobs(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "come-fail-away" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -114,7 +117,7 @@ func TestUpgradeRelease_WaitForJobs(t *testing.T) { res, err := upAction.Run(rel.Name, buildChart(), vals) req.Error(err) is.Contains(res.Info.Description, "I timed out") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(res.Info.Status, common.StatusFailed) } func TestUpgradeRelease_CleanupOnFail(t *testing.T) { @@ -124,7 +127,7 @@ func TestUpgradeRelease_CleanupOnFail(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "come-fail-away" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -139,7 +142,7 @@ func TestUpgradeRelease_CleanupOnFail(t *testing.T) { req.Error(err) is.NotContains(err.Error(), "unable to cleanup resources") is.Contains(res.Info.Description, "I timed out") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(res.Info.Status, common.StatusFailed) } func TestUpgradeRelease_RollbackOnFailure(t *testing.T) { @@ -151,7 +154,7 @@ func TestUpgradeRelease_RollbackOnFailure(t *testing.T) { rel := releaseStub() rel.Name = "nuketown" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -167,17 +170,19 @@ func TestUpgradeRelease_RollbackOnFailure(t *testing.T) { is.Contains(err.Error(), "rollback-on-failure") // Now make sure it is actually upgraded - updatedRes, err := upAction.cfg.Releases.Get(res.Name, 3) + updatedResi, err := upAction.cfg.Releases.Get(res.Name, 3) + is.NoError(err) + updatedRes, err := releaserToV1Release(updatedResi) is.NoError(err) // Should have rolled back to the previous - is.Equal(updatedRes.Info.Status, release.StatusDeployed) + is.Equal(updatedRes.Info.Status, common.StatusDeployed) }) t.Run("rollback-on-failure uninstall fails", func(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "fallout" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -218,7 +223,7 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { rel := releaseStub() rel.Name = "nuketown" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed rel.Config = existingValues err := upAction.cfg.Releases.Create(rel) @@ -230,14 +235,17 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { is.NoError(err) // Now make sure it is actually upgraded - updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2) + updatedResi, err := upAction.cfg.Releases.Get(res.Name, 2) is.NoError(err) - if updatedRes == nil { + if updatedResi == nil { is.Fail("Updated Release is nil") return } - is.Equal(release.StatusDeployed, updatedRes.Info.Status) + updatedRes, err := releaserToV1Release(updatedResi) + is.NoError(err) + + is.Equal(common.StatusDeployed, updatedRes.Info.Status) is.Equal(expectedValues, updatedRes.Config) }) @@ -270,7 +278,7 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { Info: &release.Info{ FirstDeployed: now, LastDeployed: now, - Status: release.StatusDeployed, + Status: common.StatusDeployed, Description: "Named Release Stub", }, Chart: sampleChart, @@ -292,14 +300,17 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { is.NoError(err) // Now get the upgraded release - updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2) + updatedResi, err := upAction.cfg.Releases.Get(res.Name, 2) is.NoError(err) - if updatedRes == nil { + if updatedResi == nil { is.Fail("Updated Release is nil") return } - is.Equal(release.StatusDeployed, updatedRes.Info.Status) + updatedRes, err := releaserToV1Release(updatedResi) + is.NoError(err) + + is.Equal(common.StatusDeployed, updatedRes.Info.Status) is.Equal(0, len(updatedRes.Chart.Dependencies()), "expected 0 dependencies") expectedValues := map[string]interface{}{ @@ -339,7 +350,7 @@ func TestUpgradeRelease_ResetThenReuseValues(t *testing.T) { rel := releaseStub() rel.Name = "nuketown" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed rel.Config = existingValues err := upAction.cfg.Releases.Create(rel) @@ -351,14 +362,17 @@ func TestUpgradeRelease_ResetThenReuseValues(t *testing.T) { is.NoError(err) // Now make sure it is actually upgraded - updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2) + updatedResi, err := upAction.cfg.Releases.Get(res.Name, 2) is.NoError(err) - if updatedRes == nil { + if updatedResi == nil { is.Fail("Updated Release is nil") return } - is.Equal(release.StatusDeployed, updatedRes.Info.Status) + updatedRes, err := releaserToV1Release(updatedResi) + is.NoError(err) + + is.Equal(common.StatusDeployed, updatedRes.Info.Status) is.Equal(expectedValues, updatedRes.Config) is.Equal(newChartValues, updatedRes.Chart.Values) }) @@ -370,11 +384,11 @@ func TestUpgradeRelease_Pending(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "come-fail-away" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) rel2 := releaseStub() rel2.Name = "come-fail-away" - rel2.Info.Status = release.StatusPendingUpgrade + rel2.Info.Status = common.StatusPendingUpgrade rel2.Version = 2 upAction.cfg.Releases.Create(rel2) @@ -391,7 +405,7 @@ func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "interrupted-release" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -407,7 +421,7 @@ func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { req.Error(err) is.Contains(res.Info.Description, "Upgrade \"interrupted-release\" failed: context canceled") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(res.Info.Status, common.StatusFailed) } func TestUpgradeRelease_Interrupted_RollbackOnFailure(t *testing.T) { @@ -418,7 +432,7 @@ func TestUpgradeRelease_Interrupted_RollbackOnFailure(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "interrupted-release" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed upAction.cfg.Releases.Create(rel) failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) @@ -436,10 +450,12 @@ func TestUpgradeRelease_Interrupted_RollbackOnFailure(t *testing.T) { is.Contains(err.Error(), "release interrupted-release failed, and has been rolled back due to rollback-on-failure being set: context canceled") // Now make sure it is actually upgraded - updatedRes, err := upAction.cfg.Releases.Get(res.Name, 3) + updatedResi, err := upAction.cfg.Releases.Get(res.Name, 3) + is.NoError(err) + updatedRes, err := releaserToV1Release(updatedResi) is.NoError(err) // Should have rolled back to the previous - is.Equal(updatedRes.Info.Status, release.StatusDeployed) + is.Equal(updatedRes.Info.Status, common.StatusDeployed) } func TestMergeCustomLabels(t *testing.T) { @@ -468,7 +484,7 @@ func TestUpgradeRelease_Labels(t *testing.T) { "key1": "val1", "key2": "val2.1", } - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed err := upAction.cfg.Releases.Create(rel) is.NoError(err) @@ -483,25 +499,29 @@ func TestUpgradeRelease_Labels(t *testing.T) { is.NoError(err) // Now make sure it is actually upgraded and labels were merged - updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2) + updatedResi, err := upAction.cfg.Releases.Get(res.Name, 2) is.NoError(err) - if updatedRes == nil { + if updatedResi == nil { is.Fail("Updated Release is nil") return } - is.Equal(release.StatusDeployed, updatedRes.Info.Status) + updatedRes, err := releaserToV1Release(updatedResi) + is.NoError(err) + is.Equal(common.StatusDeployed, updatedRes.Info.Status) is.Equal(mergeCustomLabels(rel.Labels, upAction.Labels), updatedRes.Labels) // Now make sure it is suppressed release still contains original labels - initialRes, err := upAction.cfg.Releases.Get(res.Name, 1) + initialResi, err := upAction.cfg.Releases.Get(res.Name, 1) is.NoError(err) - if initialRes == nil { + if initialResi == nil { is.Fail("Updated Release is nil") return } - is.Equal(initialRes.Info.Status, release.StatusSuperseded) + initialRes, err := releaserToV1Release(initialResi) + is.NoError(err) + is.Equal(initialRes.Info.Status, common.StatusSuperseded) is.Equal(initialRes.Labels, rel.Labels) } @@ -516,7 +536,7 @@ func TestUpgradeRelease_SystemLabels(t *testing.T) { "key1": "val1", "key2": "val2.1", } - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed err := upAction.cfg.Releases.Create(rel) is.NoError(err) @@ -542,7 +562,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction := upgradeAction(t) rel := releaseStub() rel.Name = "previous-release" - rel.Info.Status = release.StatusDeployed + rel.Info.Status = common.StatusDeployed req.NoError(upAction.cfg.Releases.Create(rel)) upAction.DryRunStrategy = DryRunClient @@ -552,12 +572,14 @@ func TestUpgradeRelease_DryRun(t *testing.T) { res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.NoError(err) - is.Equal(release.StatusPendingUpgrade, res.Info.Status) + is.Equal(common.StatusPendingUpgrade, res.Info.Status) is.Contains(res.Manifest, "kind: Secret") - lastRelease, err := upAction.cfg.Releases.Last(rel.Name) + lastReleasei, err := upAction.cfg.Releases.Last(rel.Name) + req.NoError(err) + lastRelease, err := releaserToV1Release(lastReleasei) req.NoError(err) - is.Equal(lastRelease.Info.Status, release.StatusDeployed) + is.Equal(lastRelease.Info.Status, common.StatusDeployed) is.Equal(1, lastRelease.Version) // Test the case for hiding the secret to ensure it is not displayed @@ -568,12 +590,14 @@ func TestUpgradeRelease_DryRun(t *testing.T) { res, err = upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.NoError(err) - is.Equal(release.StatusPendingUpgrade, res.Info.Status) + is.Equal(common.StatusPendingUpgrade, res.Info.Status) is.NotContains(res.Manifest, "kind: Secret") - lastRelease, err = upAction.cfg.Releases.Last(rel.Name) + lastReleasei, err = upAction.cfg.Releases.Last(rel.Name) + req.NoError(err) + lastRelease, err = releaserToV1Release(lastReleasei) req.NoError(err) - is.Equal(lastRelease.Info.Status, release.StatusDeployed) + is.Equal(lastRelease.Info.Status, common.StatusDeployed) is.Equal(1, lastRelease.Version) // Ensure in a dry run mode when using HideSecret diff --git a/pkg/cmd/completion_test.go b/pkg/cmd/completion_test.go index 375a9a97d..81c1ee2ad 100644 --- a/pkg/cmd/completion_test.go +++ b/pkg/cmd/completion_test.go @@ -22,6 +22,7 @@ import ( "testing" chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -31,7 +32,7 @@ func checkFileCompletion(t *testing.T, cmdName string, shouldBePerformed bool) { storage := storageFixture() storage.Create(&release.Release{ Name: "myrelease", - Info: &release.Info{Status: release.StatusDeployed}, + Info: &release.Info{Status: common.StatusDeployed}, Chart: &chart.Chart{ Metadata: &chart.Metadata{ Name: "Myrelease-Chart", diff --git a/pkg/cmd/flags_test.go b/pkg/cmd/flags_test.go index 8d79716f0..614970252 100644 --- a/pkg/cmd/flags_test.go +++ b/pkg/cmd/flags_test.go @@ -25,6 +25,7 @@ import ( "helm.sh/helm/v4/pkg/action" chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -64,35 +65,35 @@ func outputFlagCompletionTest(t *testing.T, cmdName string) { cmd: fmt.Sprintf("__complete %s --output ''", cmdName), golden: "output/output-comp.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }), }, { name: "completion for output flag long and after arg", cmd: fmt.Sprintf("__complete %s aramis --output ''", cmdName), golden: "output/output-comp.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }), }, { name: "completion for output flag short and before arg", cmd: fmt.Sprintf("__complete %s -o ''", cmdName), golden: "output/output-comp.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }), }, { name: "completion for output flag short and after arg", cmd: fmt.Sprintf("__complete %s aramis -o ''", cmdName), golden: "output/output-comp.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }), }, { name: "completion for output flag, no filter", cmd: fmt.Sprintf("__complete %s --output jso", cmdName), golden: "output/output-comp.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }), }} runTestCmd(t, tests) diff --git a/pkg/cmd/history.go b/pkg/cmd/history.go index f4dde95e4..fc3c26b02 100644 --- a/pkg/cmd/history.go +++ b/pkg/cmd/history.go @@ -111,7 +111,11 @@ func (r releaseHistory) WriteTable(out io.Writer) error { } func getHistory(client *action.History, name string) (releaseHistory, error) { - hist, err := client.Run(name) + histi, err := client.Run(name) + if err != nil { + return nil, err + } + hist, err := releaseListToV1List(histi) if err != nil { return nil, err } @@ -180,7 +184,11 @@ func compListRevisions(_ string, cfg *action.Configuration, releaseName string) client := action.NewHistory(cfg) var revisions []string - if hist, err := client.Run(releaseName); err == nil { + if histi, err := client.Run(releaseName); err == nil { + hist, err := releaseListToV1List(histi) + if err != nil { + return nil, cobra.ShellCompDirectiveError + } for _, version := range hist { appVersion := fmt.Sprintf("App: %s", version.Chart.Metadata.AppVersion) chartDesc := fmt.Sprintf("Chart: %s-%s", version.Chart.Metadata.Name, version.Chart.Metadata.Version) diff --git a/pkg/cmd/history_test.go b/pkg/cmd/history_test.go index d26ed9ecf..a324e8bdd 100644 --- a/pkg/cmd/history_test.go +++ b/pkg/cmd/history_test.go @@ -20,11 +20,12 @@ import ( "fmt" "testing" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) func TestHistoryCmd(t *testing.T) { - mk := func(name string, vers int, status release.Status) *release.Release { + mk := func(name string, vers int, status common.Status) *release.Release { return release.Mock(&release.MockReleaseOptions{ Name: name, Version: vers, @@ -36,34 +37,34 @@ func TestHistoryCmd(t *testing.T) { name: "get history for release", cmd: "history angry-bird", rels: []*release.Release{ - mk("angry-bird", 4, release.StatusDeployed), - mk("angry-bird", 3, release.StatusSuperseded), - mk("angry-bird", 2, release.StatusSuperseded), - mk("angry-bird", 1, release.StatusSuperseded), + mk("angry-bird", 4, common.StatusDeployed), + mk("angry-bird", 3, common.StatusSuperseded), + mk("angry-bird", 2, common.StatusSuperseded), + mk("angry-bird", 1, common.StatusSuperseded), }, golden: "output/history.txt", }, { name: "get history with max limit set", cmd: "history angry-bird --max 2", rels: []*release.Release{ - mk("angry-bird", 4, release.StatusDeployed), - mk("angry-bird", 3, release.StatusSuperseded), + mk("angry-bird", 4, common.StatusDeployed), + mk("angry-bird", 3, common.StatusSuperseded), }, golden: "output/history-limit.txt", }, { name: "get history with yaml output format", cmd: "history angry-bird --output yaml", rels: []*release.Release{ - mk("angry-bird", 4, release.StatusDeployed), - mk("angry-bird", 3, release.StatusSuperseded), + mk("angry-bird", 4, common.StatusDeployed), + mk("angry-bird", 3, common.StatusSuperseded), }, golden: "output/history.yaml", }, { name: "get history with json output format", cmd: "history angry-bird --output json", rels: []*release.Release{ - mk("angry-bird", 4, release.StatusDeployed), - mk("angry-bird", 3, release.StatusSuperseded), + mk("angry-bird", 4, common.StatusDeployed), + mk("angry-bird", 3, common.StatusSuperseded), }, golden: "output/history.json", }} @@ -76,7 +77,7 @@ func TestHistoryOutputCompletion(t *testing.T) { func revisionFlagCompletionTest(t *testing.T, cmdName string) { t.Helper() - mk := func(name string, vers int, status release.Status) *release.Release { + mk := func(name string, vers int, status common.Status) *release.Release { return release.Mock(&release.MockReleaseOptions{ Name: name, Version: vers, @@ -85,10 +86,10 @@ func revisionFlagCompletionTest(t *testing.T, cmdName string) { } releases := []*release.Release{ - mk("musketeers", 11, release.StatusDeployed), - mk("musketeers", 10, release.StatusSuperseded), - mk("musketeers", 9, release.StatusSuperseded), - mk("musketeers", 8, release.StatusSuperseded), + mk("musketeers", 11, common.StatusDeployed), + mk("musketeers", 10, common.StatusSuperseded), + mk("musketeers", 9, common.StatusSuperseded), + mk("musketeers", 8, common.StatusSuperseded), } tests := []cmdTestCase{{ diff --git a/pkg/cmd/list.go b/pkg/cmd/list.go index 55d828036..99e0bc9b8 100644 --- a/pkg/cmd/list.go +++ b/pkg/cmd/list.go @@ -30,6 +30,7 @@ import ( "helm.sh/helm/v4/pkg/action" "helm.sh/helm/v4/pkg/cli/output" "helm.sh/helm/v4/pkg/cmd/require" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -79,7 +80,11 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetStateMask() - results, err := client.Run() + resultsi, err := client.Run() + if err != nil { + return err + } + results, err := releaseListToV1List(resultsi) if err != nil { return err } @@ -193,28 +198,28 @@ func (w *releaseListWriter) WriteTable(out io.Writer) error { } for _, r := range w.releases { // Parse the status string back to a release.Status to use color - var status release.Status + var status common.Status switch r.Status { case "deployed": - status = release.StatusDeployed + status = common.StatusDeployed case "failed": - status = release.StatusFailed + status = common.StatusFailed case "pending-install": - status = release.StatusPendingInstall + status = common.StatusPendingInstall case "pending-upgrade": - status = release.StatusPendingUpgrade + status = common.StatusPendingUpgrade case "pending-rollback": - status = release.StatusPendingRollback + status = common.StatusPendingRollback case "uninstalling": - status = release.StatusUninstalling + status = common.StatusUninstalling case "uninstalled": - status = release.StatusUninstalled + status = common.StatusUninstalled case "superseded": - status = release.StatusSuperseded + status = common.StatusSuperseded case "unknown": - status = release.StatusUnknown + status = common.StatusUnknown default: - status = release.Status(r.Status) + status = common.Status(r.Status) } table.AddRow(r.Name, coloroutput.ColorizeNamespace(r.Namespace, w.noColor), r.Revision, r.Updated, coloroutput.ColorizeStatus(status, w.noColor), r.Chart, r.AppVersion) } @@ -264,7 +269,11 @@ func compListReleases(toComplete string, ignoredReleaseNames []string, cfg *acti // client.Filter = fmt.Sprintf("^%s", toComplete) client.SetStateMask() - releases, err := client.Run() + releasesi, err := client.Run() + if err != nil { + return nil, cobra.ShellCompDirectiveDefault + } + releases, err := releaseListToV1List(releasesi) if err != nil { return nil, cobra.ShellCompDirectiveDefault } diff --git a/pkg/cmd/list_test.go b/pkg/cmd/list_test.go index 22a948fff..42e9e2083 100644 --- a/pkg/cmd/list_test.go +++ b/pkg/cmd/list_test.go @@ -21,6 +21,7 @@ import ( "time" chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -47,7 +48,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusSuperseded, + Status: common.StatusSuperseded, }, Chart: chartInfo, }, @@ -57,7 +58,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: chartInfo, }, @@ -67,7 +68,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusUninstalled, + Status: common.StatusUninstalled, }, Chart: chartInfo, }, @@ -77,7 +78,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusSuperseded, + Status: common.StatusSuperseded, }, Chart: chartInfo, }, @@ -87,7 +88,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp2, - Status: release.StatusFailed, + Status: common.StatusFailed, }, Chart: chartInfo, }, @@ -97,7 +98,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusUninstalling, + Status: common.StatusUninstalling, }, Chart: chartInfo, }, @@ -107,7 +108,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusPendingInstall, + Status: common.StatusPendingInstall, }, Chart: chartInfo, }, @@ -117,7 +118,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp3, - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: chartInfo, }, @@ -127,7 +128,7 @@ func TestListCmd(t *testing.T) { Namespace: defaultNamespace, Info: &release.Info{ LastDeployed: timestamp4, - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: chartInfo, }, @@ -137,7 +138,7 @@ func TestListCmd(t *testing.T) { Namespace: "milano", Info: &release.Info{ LastDeployed: timestamp1, - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: chartInfo, }, diff --git a/pkg/cmd/release_testing.go b/pkg/cmd/release_testing.go index e02604f63..88a6f351f 100644 --- a/pkg/cmd/release_testing.go +++ b/pkg/cmd/release_testing.go @@ -65,13 +65,17 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command client.Filters[action.ExcludeNameFilter] = append(client.Filters[action.ExcludeNameFilter], notName.ReplaceAllLiteralString(f, "")) } } - rel, runErr := client.Run(args[0]) + reli, runErr := client.Run(args[0]) // We only return an error if we weren't even able to get the // release, otherwise we keep going so we can print status and logs // if requested - if runErr != nil && rel == nil { + if runErr != nil && reli == nil { return runErr } + rel, err := releaserToV1Release(reli) + if err != nil { + return err + } if err := outfmt.Write(out, &statusPrinter{ release: rel, diff --git a/pkg/cmd/release_testing_test.go b/pkg/cmd/release_testing_test.go index 0c46755ac..fdb5df1e9 100644 --- a/pkg/cmd/release_testing_test.go +++ b/pkg/cmd/release_testing_test.go @@ -26,6 +26,7 @@ import ( "helm.sh/helm/v4/pkg/chart/common" chart "helm.sh/helm/v4/pkg/chart/v2" kubefake "helm.sh/helm/v4/pkg/kube/fake" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -46,7 +47,7 @@ func TestReleaseTestNotesHandling(t *testing.T) { Name: "test-release", Namespace: "default", Info: &release.Info{ - Status: release.StatusDeployed, + Status: rcommon.StatusDeployed, Notes: "Some important notes that should be hidden by default", }, Chart: &chart.Chart{Metadata: &chart.Metadata{Name: "test", Version: "1.0.0"}}, diff --git a/pkg/cmd/rollback_test.go b/pkg/cmd/rollback_test.go index 53c63613e..116e158fd 100644 --- a/pkg/cmd/rollback_test.go +++ b/pkg/cmd/rollback_test.go @@ -22,6 +22,7 @@ import ( "testing" chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -29,13 +30,13 @@ func TestRollbackCmd(t *testing.T) { rels := []*release.Release{ { Name: "funny-honey", - Info: &release.Info{Status: release.StatusSuperseded}, + Info: &release.Info{Status: common.StatusSuperseded}, Chart: &chart.Chart{}, Version: 1, }, { Name: "funny-honey", - Info: &release.Info{Status: release.StatusDeployed}, + Info: &release.Info{Status: common.StatusDeployed}, Chart: &chart.Chart{}, Version: 2, }, @@ -83,7 +84,7 @@ func TestRollbackCmd(t *testing.T) { } func TestRollbackRevisionCompletion(t *testing.T) { - mk := func(name string, vers int, status release.Status) *release.Release { + mk := func(name string, vers int, status common.Status) *release.Release { return release.Mock(&release.MockReleaseOptions{ Name: name, Version: vers, @@ -92,11 +93,11 @@ func TestRollbackRevisionCompletion(t *testing.T) { } releases := []*release.Release{ - mk("musketeers", 11, release.StatusDeployed), - mk("musketeers", 10, release.StatusSuperseded), - mk("musketeers", 9, release.StatusSuperseded), - mk("musketeers", 8, release.StatusSuperseded), - mk("carabins", 1, release.StatusSuperseded), + mk("musketeers", 11, common.StatusDeployed), + mk("musketeers", 10, common.StatusSuperseded), + mk("musketeers", 9, common.StatusSuperseded), + mk("musketeers", 8, common.StatusSuperseded), + mk("carabins", 1, common.StatusSuperseded), } tests := []cmdTestCase{{ @@ -132,14 +133,14 @@ func TestRollbackWithLabels(t *testing.T) { rels := []*release.Release{ { Name: releaseName, - Info: &release.Info{Status: release.StatusSuperseded}, + Info: &release.Info{Status: common.StatusSuperseded}, Chart: &chart.Chart{}, Version: 1, Labels: labels1, }, { Name: releaseName, - Info: &release.Info{Status: release.StatusDeployed}, + Info: &release.Info{Status: common.StatusDeployed}, Chart: &chart.Chart{}, Version: 2, Labels: labels2, @@ -155,7 +156,11 @@ func TestRollbackWithLabels(t *testing.T) { if err != nil { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := storage.Get(releaseName, 3) + updatedReli, err := storage.Get(releaseName, 3) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 4f1be88d6..48dbd760d 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -39,6 +39,7 @@ import ( "helm.sh/helm/v4/pkg/cli" kubefake "helm.sh/helm/v4/pkg/kube/fake" "helm.sh/helm/v4/pkg/registry" + ri "helm.sh/helm/v4/pkg/release" release "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/repo/v1" "helm.sh/helm/v4/pkg/storage/driver" @@ -465,3 +466,31 @@ type CommandError struct { error ExitCode int } + +// releaserToV1Release is a helper function to convert a v1 release passed by interface +// into the type object. +func releaserToV1Release(rel ri.Releaser) (*release.Release, error) { + switch r := rel.(type) { + case release.Release: + return &r, nil + case *release.Release: + return r, nil + case nil: + return nil, nil + default: + return nil, fmt.Errorf("unsupported release type: %T", rel) + } +} + +func releaseListToV1List(ls []ri.Releaser) ([]*release.Release, error) { + rls := make([]*release.Release, 0, len(ls)) + for _, val := range ls { + rel, err := releaserToV1Release(val) + if err != nil { + return nil, err + } + rls = append(rls, rel) + } + + return rls, nil +} diff --git a/pkg/cmd/status_test.go b/pkg/cmd/status_test.go index 8c251b76b..b96a0d19a 100644 --- a/pkg/cmd/status_test.go +++ b/pkg/cmd/status_test.go @@ -21,6 +21,7 @@ import ( "time" chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -41,14 +42,14 @@ func TestStatusCmd(t *testing.T) { cmd: "status flummoxed-chickadee", golden: "output/status.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }), }, { name: "get status of a deployed release, with desc", cmd: "status flummoxed-chickadee", golden: "output/status-with-desc.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, Description: "Mock description", }), }, { @@ -56,7 +57,7 @@ func TestStatusCmd(t *testing.T) { cmd: "status flummoxed-chickadee", golden: "output/status-with-notes.txt", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, Notes: "release notes", }), }, { @@ -64,7 +65,7 @@ func TestStatusCmd(t *testing.T) { cmd: "status flummoxed-chickadee -o json", golden: "output/status.json", rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, Notes: "release notes", }), }, { @@ -73,7 +74,7 @@ func TestStatusCmd(t *testing.T) { golden: "output/status-with-resources.txt", rels: releasesMockWithStatus( &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, ), }, { @@ -82,7 +83,7 @@ func TestStatusCmd(t *testing.T) { golden: "output/status-with-resources.json", rels: releasesMockWithStatus( &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, ), }, { @@ -91,7 +92,7 @@ func TestStatusCmd(t *testing.T) { golden: "output/status-with-test-suite.txt", rels: releasesMockWithStatus( &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, &release.Hook{ Name: "never-run-test", @@ -140,7 +141,7 @@ func TestStatusCompletion(t *testing.T) { Name: "athos", Namespace: "default", Info: &release.Info{ - Status: release.StatusDeployed, + Status: common.StatusDeployed, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ @@ -152,7 +153,7 @@ func TestStatusCompletion(t *testing.T) { Name: "porthos", Namespace: "default", Info: &release.Info{ - Status: release.StatusFailed, + Status: common.StatusFailed, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ @@ -164,7 +165,7 @@ func TestStatusCompletion(t *testing.T) { Name: "aramis", Namespace: "default", Info: &release.Info{ - Status: release.StatusUninstalled, + Status: common.StatusUninstalled, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ @@ -176,7 +177,7 @@ func TestStatusCompletion(t *testing.T) { Name: "dartagnan", Namespace: "gascony", Info: &release.Info{ - Status: release.StatusUnknown, + Status: common.StatusUnknown, }, Chart: &chart.Chart{ Metadata: &chart.Metadata{ diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index a9f834ebb..f32493e87 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -37,7 +37,8 @@ import ( "helm.sh/helm/v4/pkg/cmd/require" "helm.sh/helm/v4/pkg/downloader" "helm.sh/helm/v4/pkg/getter" - release "helm.sh/helm/v4/pkg/release/v1" + ri "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" "helm.sh/helm/v4/pkg/storage/driver" ) @@ -318,6 +319,11 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return cmd } -func isReleaseUninstalled(versions []*release.Release) bool { - return len(versions) > 0 && versions[len(versions)-1].Info.Status == release.StatusUninstalled +func isReleaseUninstalled(versionsi []ri.Releaser) bool { + versions, err := releaseListToV1List(versionsi) + if err != nil { + slog.Error("cannot convert release list to v1 release list", "error", err) + return false + } + return len(versions) > 0 && versions[len(versions)-1].Info.Status == common.StatusUninstalled } diff --git a/pkg/cmd/upgrade_test.go b/pkg/cmd/upgrade_test.go index 9b17f187d..fd715a1fa 100644 --- a/pkg/cmd/upgrade_test.go +++ b/pkg/cmd/upgrade_test.go @@ -28,6 +28,7 @@ import ( chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/loader" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + rcommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -82,7 +83,7 @@ func TestUpgradeCmd(t *testing.T) { badDepsPath := "testdata/testcharts/chart-bad-requirements" presentDepsPath := "testdata/testcharts/chart-with-subchart-update" - relWithStatusMock := func(n string, v int, ch *chart.Chart, status release.Status) *release.Release { + relWithStatusMock := func(n string, v int, ch *chart.Chart, status rcommon.Status) *release.Release { return release.Mock(&release.MockReleaseOptions{Name: n, Version: v, Chart: ch, Status: status}) } @@ -173,20 +174,20 @@ func TestUpgradeCmd(t *testing.T) { name: "upgrade a failed release", cmd: fmt.Sprintf("upgrade funny-bunny '%s'", chartPath), golden: "output/upgrade.txt", - rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, release.StatusFailed)}, + rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, rcommon.StatusFailed)}, }, { name: "upgrade a pending install release", cmd: fmt.Sprintf("upgrade funny-bunny '%s'", chartPath), golden: "output/upgrade-with-pending-install.txt", wantError: true, - rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, release.StatusPendingInstall)}, + rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, rcommon.StatusPendingInstall)}, }, { name: "install a previously uninstalled release with '--keep-history' using 'upgrade --install'", cmd: fmt.Sprintf("upgrade funny-bunny -i '%s'", chartPath), golden: "output/upgrade-uninstalled-with-keep-history.txt", - rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, release.StatusUninstalled)}, + rels: []*release.Release{relWithStatusMock("funny-bunny", 2, ch, rcommon.StatusUninstalled)}, }, } runTestCmd(t, tests) @@ -208,7 +209,11 @@ func TestUpgradeWithValue(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := store.Get(releaseName, 4) + updatedReli, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } @@ -235,7 +240,11 @@ func TestUpgradeWithStringValue(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := store.Get(releaseName, 4) + updatedReli, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } @@ -263,7 +272,11 @@ func TestUpgradeInstallWithSubchartNotes(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - upgradedRel, err := store.Get(releaseName, 2) + upgradedReli, err := store.Get(releaseName, 2) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + upgradedRel, err := releaserToV1Release(upgradedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } @@ -295,7 +308,11 @@ func TestUpgradeWithValuesFile(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := store.Get(releaseName, 4) + updatedReli, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } @@ -328,7 +345,11 @@ func TestUpgradeWithValuesFromStdin(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := store.Get(releaseName, 4) + updatedReli, err := store.Get(releaseName, 4) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } @@ -358,7 +379,11 @@ func TestUpgradeInstallWithValuesFromStdin(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := store.Get(releaseName, 1) + updatedReli, err := store.Get(releaseName, 1) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } @@ -463,7 +488,11 @@ func TestUpgradeInstallWithLabels(t *testing.T) { t.Errorf("unexpected error, got '%v'", err) } - updatedRel, err := store.Get(releaseName, 1) + updatedReli, err := store.Get(releaseName, 1) + if err != nil { + t.Errorf("unexpected error, got '%v'", err) + } + updatedRel, err := releaserToV1Release(updatedReli) if err != nil { t.Errorf("unexpected error, got '%v'", err) } diff --git a/pkg/release/common.go b/pkg/release/common.go index 5cdcf9b88..9e6e1050e 100644 --- a/pkg/release/common.go +++ b/pkg/release/common.go @@ -18,7 +18,10 @@ package release import ( "errors" + "fmt" + "time" + "helm.sh/helm/v4/pkg/chart" v1release "helm.sh/helm/v4/pkg/release/v1" ) @@ -33,7 +36,7 @@ func NewDefaultAccessor(rel Releaser) (Accessor, error) { case *v1release.Release: return &v1Accessor{v}, nil default: - return nil, errors.New("unsupported release type") + return nil, fmt.Errorf("unsupported release type: %T", rel) } } @@ -52,6 +55,18 @@ type v1Accessor struct { rel *v1release.Release } +func (a *v1Accessor) Name() string { + return a.rel.Name +} + +func (a *v1Accessor) Namespace() string { + return a.rel.Namespace +} + +func (a *v1Accessor) Version() int { + return a.rel.Version +} + func (a *v1Accessor) Hooks() []Hook { var hooks = make([]Hook, len(a.rel.Hooks)) for i, h := range a.rel.Hooks { @@ -68,6 +83,26 @@ func (a *v1Accessor) Notes() string { return a.rel.Info.Notes } +func (a *v1Accessor) Labels() map[string]string { + return a.rel.Labels +} + +func (a *v1Accessor) Chart() chart.Charter { + return a.rel.Chart +} + +func (a *v1Accessor) Status() string { + return a.rel.Info.Status.String() +} + +func (a *v1Accessor) ApplyMethod() string { + return a.rel.ApplyMethod +} + +func (a *v1Accessor) DeployedAt() time.Time { + return a.rel.Info.LastDeployed +} + type v1HookAccessor struct { hook *v1release.Hook } diff --git a/pkg/release/v1/status.go b/pkg/release/common/status.go similarity index 99% rename from pkg/release/v1/status.go rename to pkg/release/common/status.go index 8d6459013..fd5010301 100644 --- a/pkg/release/v1/status.go +++ b/pkg/release/common/status.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1 +package common // Status is the status of a release type Status string diff --git a/pkg/release/interfaces.go b/pkg/release/interfaces.go index 9d6987625..aaa5a756f 100644 --- a/pkg/release/interfaces.go +++ b/pkg/release/interfaces.go @@ -16,14 +16,28 @@ limitations under the License. package release +import ( + "time" + + "helm.sh/helm/v4/pkg/chart" +) + type Releaser interface{} type Hook interface{} type Accessor interface { + Name() string + Namespace() string + Version() int Hooks() []Hook Manifest() string Notes() string + Labels() map[string]string + Chart() chart.Charter + Status() string + ApplyMethod() string + DeployedAt() time.Time } type HookAccessor interface { diff --git a/pkg/release/v1/info.go b/pkg/release/v1/info.go index cef7e9960..d5b5089dc 100644 --- a/pkg/release/v1/info.go +++ b/pkg/release/v1/info.go @@ -18,6 +18,7 @@ package v1 import ( "time" + "helm.sh/helm/v4/pkg/release/common" "k8s.io/apimachinery/pkg/runtime" ) @@ -32,7 +33,7 @@ type Info struct { // Description is human-friendly "log entry" about this release. Description string `json:"description,omitempty"` // Status is the current state of the release - Status Status `json:"status,omitempty"` + Status common.Status `json:"status,omitempty"` // Contains the rendered templates/NOTES.txt if available Notes string `json:"notes,omitempty"` // Contains the deployed resources information diff --git a/pkg/release/v1/mock.go b/pkg/release/v1/mock.go index 818cd777e..06ad90e8f 100644 --- a/pkg/release/v1/mock.go +++ b/pkg/release/v1/mock.go @@ -23,6 +23,7 @@ import ( "helm.sh/helm/v4/pkg/chart/common" chart "helm.sh/helm/v4/pkg/chart/v2" + rcommon "helm.sh/helm/v4/pkg/release/common" ) // MockHookTemplate is the hook template used for all mock release objects. @@ -45,7 +46,7 @@ type MockReleaseOptions struct { Name string Version int Chart *chart.Chart - Status Status + Status rcommon.Status Namespace string Labels map[string]string } @@ -105,7 +106,7 @@ func Mock(opts *MockReleaseOptions) *Release { } } - scode := StatusDeployed + scode := rcommon.StatusDeployed if len(opts.Status) > 0 { scode = opts.Status } diff --git a/pkg/release/v1/release.go b/pkg/release/v1/release.go index a7f076e04..454ee6eb7 100644 --- a/pkg/release/v1/release.go +++ b/pkg/release/v1/release.go @@ -17,6 +17,7 @@ package v1 import ( chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/release/common" ) type ApplyMethod string @@ -53,7 +54,7 @@ type Release struct { } // SetStatus is a helper for setting the status on a release. -func (r *Release) SetStatus(status Status, msg string) { +func (r *Release) SetStatus(status common.Status, msg string) { r.Info.Status = status r.Info.Description = msg } diff --git a/pkg/release/v1/util/filter.go b/pkg/release/v1/util/filter.go index f818a6196..dc60195cf 100644 --- a/pkg/release/v1/util/filter.go +++ b/pkg/release/v1/util/filter.go @@ -16,7 +16,10 @@ limitations under the License. package util // import "helm.sh/helm/v4/pkg/release/v1/util" -import rspb "helm.sh/helm/v4/pkg/release/v1" +import ( + "helm.sh/helm/v4/pkg/release/common" + rspb "helm.sh/helm/v4/pkg/release/v1" +) // FilterFunc returns true if the release object satisfies // the predicate of the underlying filter func. @@ -68,7 +71,7 @@ func All(filters ...FilterFunc) FilterFunc { } // StatusFilter filters a set of releases by status code. -func StatusFilter(status rspb.Status) FilterFunc { +func StatusFilter(status common.Status) FilterFunc { return FilterFunc(func(rls *rspb.Release) bool { if rls == nil { return true diff --git a/pkg/release/v1/util/filter_test.go b/pkg/release/v1/util/filter_test.go index c8b23d526..1004a4c57 100644 --- a/pkg/release/v1/util/filter_test.go +++ b/pkg/release/v1/util/filter_test.go @@ -19,20 +19,21 @@ package util // import "helm.sh/helm/v4/pkg/release/v1/util" import ( "testing" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) func TestFilterAny(t *testing.T) { - ls := Any(StatusFilter(rspb.StatusUninstalled)).Filter(releases) + ls := Any(StatusFilter(common.StatusUninstalled)).Filter(releases) if len(ls) != 2 { t.Fatalf("expected 2 results, got '%d'", len(ls)) } r0, r1 := ls[0], ls[1] switch { - case r0.Info.Status != rspb.StatusUninstalled: + case r0.Info.Status != common.StatusUninstalled: t.Fatalf("expected UNINSTALLED result, got '%s'", r1.Info.Status.String()) - case r1.Info.Status != rspb.StatusUninstalled: + case r1.Info.Status != common.StatusUninstalled: t.Fatalf("expected UNINSTALLED result, got '%s'", r1.Info.Status.String()) } } @@ -40,7 +41,7 @@ func TestFilterAny(t *testing.T) { func TestFilterAll(t *testing.T) { fn := FilterFunc(func(rls *rspb.Release) bool { // true if not uninstalled and version < 4 - v0 := !StatusFilter(rspb.StatusUninstalled).Check(rls) + v0 := !StatusFilter(common.StatusUninstalled).Check(rls) v1 := rls.Version < 4 return v0 && v1 }) @@ -53,7 +54,7 @@ func TestFilterAll(t *testing.T) { switch r0 := ls[0]; { case r0.Version == 4: t.Fatal("got release with status revision 4") - case r0.Info.Status == rspb.StatusUninstalled: + case r0.Info.Status == common.StatusUninstalled: t.Fatal("got release with status UNINSTALLED") } } diff --git a/pkg/release/v1/util/sorter_test.go b/pkg/release/v1/util/sorter_test.go index 0889ddb94..f47db7db8 100644 --- a/pkg/release/v1/util/sorter_test.go +++ b/pkg/release/v1/util/sorter_test.go @@ -20,19 +20,20 @@ import ( "testing" "time" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) // note: this test data is shared with filter_test.go. var releases = []*rspb.Release{ - tsRelease("quiet-bear", 2, 2000, rspb.StatusSuperseded), - tsRelease("angry-bird", 4, 3000, rspb.StatusDeployed), - tsRelease("happy-cats", 1, 4000, rspb.StatusUninstalled), - tsRelease("vocal-dogs", 3, 6000, rspb.StatusUninstalled), + tsRelease("quiet-bear", 2, 2000, common.StatusSuperseded), + tsRelease("angry-bird", 4, 3000, common.StatusDeployed), + tsRelease("happy-cats", 1, 4000, common.StatusUninstalled), + tsRelease("vocal-dogs", 3, 6000, common.StatusUninstalled), } -func tsRelease(name string, vers int, dur time.Duration, status rspb.Status) *rspb.Release { +func tsRelease(name string, vers int, dur time.Duration, status common.Status) *rspb.Release { info := &rspb.Info{Status: status, LastDeployed: time.Now().Add(dur)} return &rspb.Release{ Name: name, diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index de097f294..ada148158 100644 --- a/pkg/storage/driver/cfgmaps.go +++ b/pkg/storage/driver/cfgmaps.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" + "helm.sh/helm/v4/pkg/release" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -60,7 +61,7 @@ func (cfgmaps *ConfigMaps) Name() string { // Get fetches the release named by key. The corresponding release is returned // or error if not found. -func (cfgmaps *ConfigMaps) Get(key string) (*rspb.Release, error) { +func (cfgmaps *ConfigMaps) Get(key string) (release.Releaser, error) { // fetch the configmap holding the release named by key obj, err := cfgmaps.impl.Get(context.Background(), key, metav1.GetOptions{}) if err != nil { @@ -85,7 +86,7 @@ func (cfgmaps *ConfigMaps) Get(key string) (*rspb.Release, error) { // List fetches all releases and returns the list releases such // that filter(release) == true. An error is returned if the // configmap fails to retrieve the releases. -func (cfgmaps *ConfigMaps) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { +func (cfgmaps *ConfigMaps) List(filter func(release.Releaser) bool) ([]release.Releaser, error) { lsel := kblabels.Set{"owner": "helm"}.AsSelector() opts := metav1.ListOptions{LabelSelector: lsel.String()} @@ -95,7 +96,7 @@ func (cfgmaps *ConfigMaps) List(filter func(*rspb.Release) bool) ([]*rspb.Releas return nil, err } - var results []*rspb.Release + var results []release.Releaser // iterate over the configmaps object list // and decode each release @@ -117,7 +118,7 @@ func (cfgmaps *ConfigMaps) List(filter func(*rspb.Release) bool) ([]*rspb.Releas // Query fetches all releases that match the provided map of labels. // An error is returned if the configmap fails to retrieve the releases. -func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]*rspb.Release, error) { +func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]release.Releaser, error) { ls := kblabels.Set{} for k, v := range labels { if errs := validation.IsValidLabelValue(v); len(errs) != 0 { @@ -138,7 +139,7 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]*rspb.Release, err return nil, ErrReleaseNotFound } - var results []*rspb.Release + var results []release.Releaser for _, item := range list.Items { rls, err := decodeRelease(item.Data["release"]) if err != nil { @@ -153,18 +154,28 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]*rspb.Release, err // Create creates a new ConfigMap holding the release. If the // ConfigMap already exists, ErrReleaseExists is returned. -func (cfgmaps *ConfigMaps) Create(key string, rls *rspb.Release) error { +func (cfgmaps *ConfigMaps) Create(key string, rls release.Releaser) error { // set labels for configmaps object meta data var lbs labels + rac, err := release.NewAccessor(rls) + if err != nil { + return err + } + lbs.init() - lbs.fromMap(rls.Labels) + lbs.fromMap(rac.Labels()) lbs.set("createdAt", fmt.Sprintf("%v", time.Now().Unix())) + rel, err := releaserToV1Release(rls) + if err != nil { + return err + } + // create a new configmap to hold the release - obj, err := newConfigMapsObject(key, rls, lbs) + obj, err := newConfigMapsObject(key, rel, lbs) if err != nil { - slog.Debug("failed to encode release", "name", rls.Name, slog.Any("error", err)) + slog.Debug("failed to encode release", "name", rac.Name(), slog.Any("error", err)) return err } // push the configmap object out into the kubiverse @@ -181,10 +192,15 @@ func (cfgmaps *ConfigMaps) Create(key string, rls *rspb.Release) error { // Update updates the ConfigMap holding the release. If not found // the ConfigMap is created to hold the release. -func (cfgmaps *ConfigMaps) Update(key string, rls *rspb.Release) error { +func (cfgmaps *ConfigMaps) Update(key string, rel release.Releaser) error { // set labels for configmaps object meta data var lbs labels + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } + lbs.init() lbs.fromMap(rls.Labels) lbs.set("modifiedAt", fmt.Sprintf("%v", time.Now().Unix())) @@ -205,7 +221,7 @@ func (cfgmaps *ConfigMaps) Update(key string, rls *rspb.Release) error { } // Delete deletes the ConfigMap holding the release named by key. -func (cfgmaps *ConfigMaps) Delete(key string) (rls *rspb.Release, err error) { +func (cfgmaps *ConfigMaps) Delete(key string) (rls release.Releaser, err error) { // fetch the release to check existence if rls, err = cfgmaps.Get(key); err != nil { return nil, err diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index a563eb7d9..8beb45547 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -22,6 +22,8 @@ import ( v1 "k8s.io/api/core/v1" + "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -37,7 +39,7 @@ func TestConfigMapGet(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{rel}...) @@ -57,7 +59,7 @@ func TestUncompressedConfigMapGet(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) // Create a test fixture which contains an uncompressed release cfgmap, err := newConfigMapsObject(key, rel, nil) @@ -84,19 +86,35 @@ func TestUncompressedConfigMapGet(t *testing.T) { } } +func convertReleaserToV1(t *testing.T, rel release.Releaser) *rspb.Release { + t.Helper() + switch r := rel.(type) { + case rspb.Release: + return &r + case *rspb.Release: + return r + case nil: + return nil + } + + t.Fatalf("Unsupported release type: %T", rel) + return nil +} + func TestConfigMapList(t *testing.T) { cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{ - releaseStub("key-1", 1, "default", rspb.StatusUninstalled), - releaseStub("key-2", 1, "default", rspb.StatusUninstalled), - releaseStub("key-3", 1, "default", rspb.StatusDeployed), - releaseStub("key-4", 1, "default", rspb.StatusDeployed), - releaseStub("key-5", 1, "default", rspb.StatusSuperseded), - releaseStub("key-6", 1, "default", rspb.StatusSuperseded), + releaseStub("key-1", 1, "default", common.StatusUninstalled), + releaseStub("key-2", 1, "default", common.StatusUninstalled), + releaseStub("key-3", 1, "default", common.StatusDeployed), + releaseStub("key-4", 1, "default", common.StatusDeployed), + releaseStub("key-5", 1, "default", common.StatusSuperseded), + releaseStub("key-6", 1, "default", common.StatusSuperseded), }...) // list all deleted releases - del, err := cfgmaps.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusUninstalled + del, err := cfgmaps.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusUninstalled }) // check if err != nil { @@ -107,8 +125,9 @@ func TestConfigMapList(t *testing.T) { } // list all deployed releases - dpl, err := cfgmaps.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusDeployed + dpl, err := cfgmaps.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusDeployed }) // check if err != nil { @@ -119,8 +138,9 @@ func TestConfigMapList(t *testing.T) { } // list all superseded releases - ssd, err := cfgmaps.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusSuperseded + ssd, err := cfgmaps.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusSuperseded }) // check if err != nil { @@ -130,7 +150,7 @@ func TestConfigMapList(t *testing.T) { t.Errorf("Expected 2 superseded, got %d", len(ssd)) } // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. - rls := ssd[0] + rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] if !ok { t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) @@ -143,12 +163,12 @@ func TestConfigMapList(t *testing.T) { func TestConfigMapQuery(t *testing.T) { cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{ - releaseStub("key-1", 1, "default", rspb.StatusUninstalled), - releaseStub("key-2", 1, "default", rspb.StatusUninstalled), - releaseStub("key-3", 1, "default", rspb.StatusDeployed), - releaseStub("key-4", 1, "default", rspb.StatusDeployed), - releaseStub("key-5", 1, "default", rspb.StatusSuperseded), - releaseStub("key-6", 1, "default", rspb.StatusSuperseded), + releaseStub("key-1", 1, "default", common.StatusUninstalled), + releaseStub("key-2", 1, "default", common.StatusUninstalled), + releaseStub("key-3", 1, "default", common.StatusDeployed), + releaseStub("key-4", 1, "default", common.StatusDeployed), + releaseStub("key-5", 1, "default", common.StatusSuperseded), + releaseStub("key-6", 1, "default", common.StatusSuperseded), }...) rls, err := cfgmaps.Query(map[string]string{"status": "deployed"}) @@ -172,7 +192,7 @@ func TestConfigMapCreate(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) // store the release in a configmap if err := cfgmaps.Create(key, rel); err != nil { @@ -196,12 +216,12 @@ func TestConfigMapUpdate(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{rel}...) // modify release status code - rel.Info.Status = rspb.StatusSuperseded + rel.Info.Status = common.StatusSuperseded // perform the update if err := cfgmaps.Update(key, rel); err != nil { @@ -209,10 +229,11 @@ func TestConfigMapUpdate(t *testing.T) { } // fetch the updated release - got, err := cfgmaps.Get(key) + goti, err := cfgmaps.Get(key) if err != nil { t.Fatalf("Failed to get release with key %q: %s", key, err) } + got := convertReleaserToV1(t, goti) // check release has actually been updated by comparing modified fields if rel.Info.Status != got.Info.Status { @@ -225,7 +246,7 @@ func TestConfigMapDelete(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) cfgmaps := newTestFixtureCfgMaps(t, []*rspb.Release{rel}...) diff --git a/pkg/storage/driver/driver.go b/pkg/storage/driver/driver.go index 4f9d63928..6efd1dbaa 100644 --- a/pkg/storage/driver/driver.go +++ b/pkg/storage/driver/driver.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" + "helm.sh/helm/v4/pkg/release" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -58,7 +59,7 @@ func NewErrNoDeployedReleases(releaseName string) error { // Create stores the release or returns ErrReleaseExists // if an identical release already exists. type Creator interface { - Create(key string, rls *rspb.Release) error + Create(key string, rls release.Releaser) error } // Updator is the interface that wraps the Update method. @@ -66,7 +67,7 @@ type Creator interface { // Update updates an existing release or returns // ErrReleaseNotFound if the release does not exist. type Updator interface { - Update(key string, rls *rspb.Release) error + Update(key string, rls release.Releaser) error } // Deletor is the interface that wraps the Delete method. @@ -74,7 +75,7 @@ type Updator interface { // Delete deletes the release named by key or returns // ErrReleaseNotFound if the release does not exist. type Deletor interface { - Delete(key string) (*rspb.Release, error) + Delete(key string) (release.Releaser, error) } // Queryor is the interface that wraps the Get and List methods. @@ -86,9 +87,9 @@ type Deletor interface { // // Query returns the set of all releases that match the provided label set. type Queryor interface { - Get(key string) (*rspb.Release, error) - List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) - Query(labels map[string]string) ([]*rspb.Release, error) + Get(key string) (release.Releaser, error) + List(filter func(release.Releaser) bool) ([]release.Releaser, error) + Query(labels map[string]string) ([]release.Releaser, error) } // Driver is the interface composed of Creator, Updator, Deletor, and Queryor @@ -102,3 +103,18 @@ type Driver interface { Queryor Name() string } + +// releaserToV1Release is a helper function to convert a v1 release passed by interface +// into the type object. +func releaserToV1Release(rel release.Releaser) (*rspb.Release, error) { + switch r := rel.(type) { + case rspb.Release: + return &r, nil + case *rspb.Release: + return r, nil + case nil: + return nil, nil + default: + return nil, fmt.Errorf("unsupported release type: %T", rel) + } +} diff --git a/pkg/storage/driver/memory.go b/pkg/storage/driver/memory.go index 79e7f090e..352fe2c6a 100644 --- a/pkg/storage/driver/memory.go +++ b/pkg/storage/driver/memory.go @@ -21,7 +21,7 @@ import ( "strings" "sync" - rspb "helm.sh/helm/v4/pkg/release/v1" + "helm.sh/helm/v4/pkg/release" ) var _ Driver = (*Memory)(nil) @@ -61,7 +61,7 @@ func (mem *Memory) Name() string { } // Get returns the release named by key or returns ErrReleaseNotFound. -func (mem *Memory) Get(key string) (*rspb.Release, error) { +func (mem *Memory) Get(key string) (release.Releaser, error) { defer unlock(mem.rlock()) keyWithoutPrefix := strings.TrimPrefix(key, "sh.helm.release.v1.") @@ -83,10 +83,10 @@ func (mem *Memory) Get(key string) (*rspb.Release, error) { } // List returns the list of all releases such that filter(release) == true -func (mem *Memory) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { +func (mem *Memory) List(filter func(release.Releaser) bool) ([]release.Releaser, error) { defer unlock(mem.rlock()) - var ls []*rspb.Release + var ls []release.Releaser for namespace := range mem.cache { if mem.namespace != "" { // Should only list releases of this namespace @@ -109,7 +109,7 @@ func (mem *Memory) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error } // Query returns the set of releases that match the provided set of labels -func (mem *Memory) Query(keyvals map[string]string) ([]*rspb.Release, error) { +func (mem *Memory) Query(keyvals map[string]string) ([]release.Releaser, error) { defer unlock(mem.rlock()) var lbs labels @@ -117,7 +117,7 @@ func (mem *Memory) Query(keyvals map[string]string) ([]*rspb.Release, error) { lbs.init() lbs.fromMap(keyvals) - var ls []*rspb.Release + var ls []release.Releaser for namespace := range mem.cache { if mem.namespace != "" { // Should only query releases of this namespace @@ -150,9 +150,13 @@ func (mem *Memory) Query(keyvals map[string]string) ([]*rspb.Release, error) { } // Create creates a new release or returns ErrReleaseExists. -func (mem *Memory) Create(key string, rls *rspb.Release) error { +func (mem *Memory) Create(key string, rel release.Releaser) error { defer unlock(mem.wlock()) + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } // For backwards compatibility, we protect against an unset namespace namespace := rls.Namespace if namespace == "" { @@ -176,9 +180,14 @@ func (mem *Memory) Create(key string, rls *rspb.Release) error { } // Update updates a release or returns ErrReleaseNotFound. -func (mem *Memory) Update(key string, rls *rspb.Release) error { +func (mem *Memory) Update(key string, rel release.Releaser) error { defer unlock(mem.wlock()) + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } + // For backwards compatibility, we protect against an unset namespace namespace := rls.Namespace if namespace == "" { @@ -196,7 +205,7 @@ func (mem *Memory) Update(key string, rls *rspb.Release) error { } // Delete deletes a release or returns ErrReleaseNotFound. -func (mem *Memory) Delete(key string) (*rspb.Release, error) { +func (mem *Memory) Delete(key string) (release.Releaser, error) { defer unlock(mem.wlock()) keyWithoutPrefix := strings.TrimPrefix(key, "sh.helm.release.v1.") diff --git a/pkg/storage/driver/memory_test.go b/pkg/storage/driver/memory_test.go index ee547b58b..18786c004 100644 --- a/pkg/storage/driver/memory_test.go +++ b/pkg/storage/driver/memory_test.go @@ -21,6 +21,9 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -38,22 +41,22 @@ func TestMemoryCreate(t *testing.T) { }{ { "create should succeed", - releaseStub("rls-c", 1, "default", rspb.StatusDeployed), + releaseStub("rls-c", 1, "default", common.StatusDeployed), false, }, { "create should fail (release already exists)", - releaseStub("rls-a", 1, "default", rspb.StatusDeployed), + releaseStub("rls-a", 1, "default", common.StatusDeployed), true, }, { "create in namespace should succeed", - releaseStub("rls-a", 1, "mynamespace", rspb.StatusDeployed), + releaseStub("rls-a", 1, "mynamespace", common.StatusDeployed), false, }, { "create in other namespace should fail (release already exists)", - releaseStub("rls-c", 1, "mynamespace", rspb.StatusDeployed), + releaseStub("rls-c", 1, "mynamespace", common.StatusDeployed), true, }, } @@ -104,8 +107,9 @@ func TestMemoryList(t *testing.T) { ts.SetNamespace("default") // list all deployed releases - dpl, err := ts.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusDeployed + dpl, err := ts.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusDeployed }) // check if err != nil { @@ -116,8 +120,9 @@ func TestMemoryList(t *testing.T) { } // list all superseded releases - ssd, err := ts.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusSuperseded + ssd, err := ts.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusSuperseded }) // check if err != nil { @@ -128,8 +133,9 @@ func TestMemoryList(t *testing.T) { } // list all deleted releases - del, err := ts.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusUninstalled + del, err := ts.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusUninstalled }) // check if err != nil { @@ -185,25 +191,25 @@ func TestMemoryUpdate(t *testing.T) { { "update release status", "rls-a.v4", - releaseStub("rls-a", 4, "default", rspb.StatusSuperseded), + releaseStub("rls-a", 4, "default", common.StatusSuperseded), false, }, { "update release does not exist", "rls-c.v1", - releaseStub("rls-c", 1, "default", rspb.StatusUninstalled), + releaseStub("rls-c", 1, "default", common.StatusUninstalled), true, }, { "update release status in namespace", "rls-c.v4", - releaseStub("rls-c", 4, "mynamespace", rspb.StatusSuperseded), + releaseStub("rls-c", 4, "mynamespace", common.StatusSuperseded), false, }, { "update release in namespace does not exist", "rls-a.v1", - releaseStub("rls-a", 1, "mynamespace", rspb.StatusUninstalled), + releaseStub("rls-a", 1, "mynamespace", common.StatusUninstalled), true, }, } @@ -255,17 +261,23 @@ func TestMemoryDelete(t *testing.T) { startLen := len(start) for _, tt := range tests { ts.SetNamespace(tt.namespace) - if rel, err := ts.Delete(tt.key); err != nil { + + rel, err := ts.Delete(tt.key) + var rls *rspb.Release + if err == nil { + rls = convertReleaserToV1(t, rel) + } + if err != nil { if !tt.err { t.Fatalf("Failed %q to get '%s': %q\n", tt.desc, tt.key, err) } continue } else if tt.err { t.Fatalf("Did not get expected error for %q '%s'\n", tt.desc, tt.key) - } else if fmt.Sprintf("%s.v%d", rel.Name, rel.Version) != tt.key { - t.Fatalf("Asked for delete on %s, but deleted %d", tt.key, rel.Version) + } else if fmt.Sprintf("%s.v%d", rls.Name, rls.Version) != tt.key { + t.Fatalf("Asked for delete on %s, but deleted %d", tt.key, rls.Version) } - _, err := ts.Get(tt.key) + _, err = ts.Get(tt.key) if err == nil { t.Errorf("Expected an error when asking for a deleted key") } @@ -282,7 +294,9 @@ func TestMemoryDelete(t *testing.T) { if startLen-2 != endLen { t.Errorf("expected end to be %d instead of %d", startLen-2, endLen) for _, ee := range end { - t.Logf("Name: %s, Version: %d", ee.Name, ee.Version) + rac, err := release.NewAccessor(ee) + assert.NoError(t, err, "unable to get release accessor") + t.Logf("Name: %s, Version: %d", rac.Name(), rac.Version()) } } diff --git a/pkg/storage/driver/mock_test.go b/pkg/storage/driver/mock_test.go index 7dba5fea2..e62b02f43 100644 --- a/pkg/storage/driver/mock_test.go +++ b/pkg/storage/driver/mock_test.go @@ -31,10 +31,11 @@ import ( kblabels "k8s.io/apimachinery/pkg/labels" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) -func releaseStub(name string, vers int, namespace string, status rspb.Status) *rspb.Release { +func releaseStub(name string, vers int, namespace string, status common.Status) *rspb.Release { return &rspb.Release{ Name: name, Version: vers, @@ -55,20 +56,20 @@ func tsFixtureMemory(t *testing.T) *Memory { t.Helper() hs := []*rspb.Release{ // rls-a - releaseStub("rls-a", 4, "default", rspb.StatusDeployed), - releaseStub("rls-a", 1, "default", rspb.StatusSuperseded), - releaseStub("rls-a", 3, "default", rspb.StatusSuperseded), - releaseStub("rls-a", 2, "default", rspb.StatusSuperseded), + releaseStub("rls-a", 4, "default", common.StatusDeployed), + releaseStub("rls-a", 1, "default", common.StatusSuperseded), + releaseStub("rls-a", 3, "default", common.StatusSuperseded), + releaseStub("rls-a", 2, "default", common.StatusSuperseded), // rls-b - releaseStub("rls-b", 4, "default", rspb.StatusDeployed), - releaseStub("rls-b", 1, "default", rspb.StatusSuperseded), - releaseStub("rls-b", 3, "default", rspb.StatusSuperseded), - releaseStub("rls-b", 2, "default", rspb.StatusSuperseded), + releaseStub("rls-b", 4, "default", common.StatusDeployed), + releaseStub("rls-b", 1, "default", common.StatusSuperseded), + releaseStub("rls-b", 3, "default", common.StatusSuperseded), + releaseStub("rls-b", 2, "default", common.StatusSuperseded), // rls-c in other namespace - releaseStub("rls-c", 4, "mynamespace", rspb.StatusDeployed), - releaseStub("rls-c", 1, "mynamespace", rspb.StatusSuperseded), - releaseStub("rls-c", 3, "mynamespace", rspb.StatusSuperseded), - releaseStub("rls-c", 2, "mynamespace", rspb.StatusSuperseded), + releaseStub("rls-c", 4, "mynamespace", common.StatusDeployed), + releaseStub("rls-c", 1, "mynamespace", common.StatusSuperseded), + releaseStub("rls-c", 3, "mynamespace", common.StatusSuperseded), + releaseStub("rls-c", 2, "mynamespace", common.StatusSuperseded), } mem := NewMemory() diff --git a/pkg/storage/driver/records_test.go b/pkg/storage/driver/records_test.go index 34b2fb80c..24e4ccb4e 100644 --- a/pkg/storage/driver/records_test.go +++ b/pkg/storage/driver/records_test.go @@ -20,13 +20,13 @@ import ( "reflect" "testing" - rspb "helm.sh/helm/v4/pkg/release/v1" + "helm.sh/helm/v4/pkg/release/common" ) func TestRecordsAdd(t *testing.T) { rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) var tests = []struct { @@ -39,13 +39,13 @@ func TestRecordsAdd(t *testing.T) { "add valid key", "rls-a.v3", false, - newRecord("rls-a.v3", releaseStub("rls-a", 3, "default", rspb.StatusSuperseded)), + newRecord("rls-a.v3", releaseStub("rls-a", 3, "default", common.StatusSuperseded)), }, { "add already existing key", "rls-a.v1", true, - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusDeployed)), }, } @@ -70,8 +70,8 @@ func TestRecordsRemove(t *testing.T) { } rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) startLen := rs.Len() @@ -98,8 +98,8 @@ func TestRecordsRemove(t *testing.T) { func TestRecordsRemoveAt(t *testing.T) { rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) if len(rs) != 2 { @@ -114,8 +114,8 @@ func TestRecordsRemoveAt(t *testing.T) { func TestRecordsGet(t *testing.T) { rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) var tests = []struct { @@ -126,7 +126,7 @@ func TestRecordsGet(t *testing.T) { { "get valid key", "rls-a.v1", - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), }, { "get invalid key", @@ -145,8 +145,8 @@ func TestRecordsGet(t *testing.T) { func TestRecordsIndex(t *testing.T) { rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) var tests = []struct { @@ -176,8 +176,8 @@ func TestRecordsIndex(t *testing.T) { func TestRecordsExists(t *testing.T) { rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) var tests = []struct { @@ -207,8 +207,8 @@ func TestRecordsExists(t *testing.T) { func TestRecordsReplace(t *testing.T) { rs := records([]*record{ - newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v1", releaseStub("rls-a", 1, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }) var tests = []struct { @@ -220,13 +220,13 @@ func TestRecordsReplace(t *testing.T) { { "replace with existing key", "rls-a.v2", - newRecord("rls-a.v3", releaseStub("rls-a", 3, "default", rspb.StatusSuperseded)), - newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", rspb.StatusDeployed)), + newRecord("rls-a.v3", releaseStub("rls-a", 3, "default", common.StatusSuperseded)), + newRecord("rls-a.v2", releaseStub("rls-a", 2, "default", common.StatusDeployed)), }, { "replace with non existing key", "rls-a.v4", - newRecord("rls-a.v4", releaseStub("rls-a", 4, "default", rspb.StatusDeployed)), + newRecord("rls-a.v4", releaseStub("rls-a", 4, "default", common.StatusDeployed)), nil, }, } diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go index 23a8f5cab..1f5ce75ac 100644 --- a/pkg/storage/driver/secrets.go +++ b/pkg/storage/driver/secrets.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" + "helm.sh/helm/v4/pkg/release" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -60,7 +61,7 @@ func (secrets *Secrets) Name() string { // Get fetches the release named by key. The corresponding release is returned // or error if not found. -func (secrets *Secrets) Get(key string) (*rspb.Release, error) { +func (secrets *Secrets) Get(key string) (release.Releaser, error) { // fetch the secret holding the release named by key obj, err := secrets.impl.Get(context.Background(), key, metav1.GetOptions{}) if err != nil { @@ -81,7 +82,7 @@ func (secrets *Secrets) Get(key string) (*rspb.Release, error) { // List fetches all releases and returns the list releases such // that filter(release) == true. An error is returned if the // secret fails to retrieve the releases. -func (secrets *Secrets) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { +func (secrets *Secrets) List(filter func(release.Releaser) bool) ([]release.Releaser, error) { lsel := kblabels.Set{"owner": "helm"}.AsSelector() opts := metav1.ListOptions{LabelSelector: lsel.String()} @@ -90,7 +91,7 @@ func (secrets *Secrets) List(filter func(*rspb.Release) bool) ([]*rspb.Release, return nil, fmt.Errorf("list: failed to list: %w", err) } - var results []*rspb.Release + var results []release.Releaser // iterate over the secrets object list // and decode each release @@ -112,7 +113,7 @@ func (secrets *Secrets) List(filter func(*rspb.Release) bool) ([]*rspb.Release, // Query fetches all releases that match the provided map of labels. // An error is returned if the secret fails to retrieve the releases. -func (secrets *Secrets) Query(labels map[string]string) ([]*rspb.Release, error) { +func (secrets *Secrets) Query(labels map[string]string) ([]release.Releaser, error) { ls := kblabels.Set{} for k, v := range labels { if errs := validation.IsValidLabelValue(v); len(errs) != 0 { @@ -132,7 +133,7 @@ func (secrets *Secrets) Query(labels map[string]string) ([]*rspb.Release, error) return nil, ErrReleaseNotFound } - var results []*rspb.Release + var results []release.Releaser for _, item := range list.Items { rls, err := decodeRelease(string(item.Data["release"])) if err != nil { @@ -147,10 +148,15 @@ func (secrets *Secrets) Query(labels map[string]string) ([]*rspb.Release, error) // Create creates a new Secret holding the release. If the // Secret already exists, ErrReleaseExists is returned. -func (secrets *Secrets) Create(key string, rls *rspb.Release) error { +func (secrets *Secrets) Create(key string, rel release.Releaser) error { // set labels for secrets object meta data var lbs labels + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } + lbs.init() lbs.fromMap(rls.Labels) lbs.set("createdAt", fmt.Sprintf("%v", time.Now().Unix())) @@ -173,10 +179,15 @@ func (secrets *Secrets) Create(key string, rls *rspb.Release) error { // Update updates the Secret holding the release. If not found // the Secret is created to hold the release. -func (secrets *Secrets) Update(key string, rls *rspb.Release) error { +func (secrets *Secrets) Update(key string, rel release.Releaser) error { // set labels for secrets object meta data var lbs labels + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } + lbs.init() lbs.fromMap(rls.Labels) lbs.set("modifiedAt", fmt.Sprintf("%v", time.Now().Unix())) @@ -195,7 +206,7 @@ func (secrets *Secrets) Update(key string, rls *rspb.Release) error { } // Delete deletes the Secret holding the release named by key. -func (secrets *Secrets) Delete(key string) (rls *rspb.Release, err error) { +func (secrets *Secrets) Delete(key string) (rls release.Releaser, err error) { // fetch the release to check existence if rls, err = secrets.Get(key); err != nil { return nil, err diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go index 9e45bae67..f4aa1176c 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -22,6 +22,8 @@ import ( v1 "k8s.io/api/core/v1" + "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -37,7 +39,7 @@ func TestSecretGet(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) secrets := newTestFixtureSecrets(t, []*rspb.Release{rel}...) @@ -57,7 +59,7 @@ func TestUNcompressedSecretGet(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) // Create a test fixture which contains an uncompressed release secret, err := newSecretsObject(key, rel, nil) @@ -86,17 +88,18 @@ func TestUNcompressedSecretGet(t *testing.T) { func TestSecretList(t *testing.T) { secrets := newTestFixtureSecrets(t, []*rspb.Release{ - releaseStub("key-1", 1, "default", rspb.StatusUninstalled), - releaseStub("key-2", 1, "default", rspb.StatusUninstalled), - releaseStub("key-3", 1, "default", rspb.StatusDeployed), - releaseStub("key-4", 1, "default", rspb.StatusDeployed), - releaseStub("key-5", 1, "default", rspb.StatusSuperseded), - releaseStub("key-6", 1, "default", rspb.StatusSuperseded), + releaseStub("key-1", 1, "default", common.StatusUninstalled), + releaseStub("key-2", 1, "default", common.StatusUninstalled), + releaseStub("key-3", 1, "default", common.StatusDeployed), + releaseStub("key-4", 1, "default", common.StatusDeployed), + releaseStub("key-5", 1, "default", common.StatusSuperseded), + releaseStub("key-6", 1, "default", common.StatusSuperseded), }...) // list all deleted releases - del, err := secrets.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusUninstalled + del, err := secrets.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusUninstalled }) // check if err != nil { @@ -107,8 +110,9 @@ func TestSecretList(t *testing.T) { } // list all deployed releases - dpl, err := secrets.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusDeployed + dpl, err := secrets.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusDeployed }) // check if err != nil { @@ -119,8 +123,9 @@ func TestSecretList(t *testing.T) { } // list all superseded releases - ssd, err := secrets.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusSuperseded + ssd, err := secrets.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusSuperseded }) // check if err != nil { @@ -130,7 +135,7 @@ func TestSecretList(t *testing.T) { t.Errorf("Expected 2 superseded, got %d", len(ssd)) } // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. - rls := ssd[0] + rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] if !ok { t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) @@ -143,12 +148,12 @@ func TestSecretList(t *testing.T) { func TestSecretQuery(t *testing.T) { secrets := newTestFixtureSecrets(t, []*rspb.Release{ - releaseStub("key-1", 1, "default", rspb.StatusUninstalled), - releaseStub("key-2", 1, "default", rspb.StatusUninstalled), - releaseStub("key-3", 1, "default", rspb.StatusDeployed), - releaseStub("key-4", 1, "default", rspb.StatusDeployed), - releaseStub("key-5", 1, "default", rspb.StatusSuperseded), - releaseStub("key-6", 1, "default", rspb.StatusSuperseded), + releaseStub("key-1", 1, "default", common.StatusUninstalled), + releaseStub("key-2", 1, "default", common.StatusUninstalled), + releaseStub("key-3", 1, "default", common.StatusDeployed), + releaseStub("key-4", 1, "default", common.StatusDeployed), + releaseStub("key-5", 1, "default", common.StatusSuperseded), + releaseStub("key-6", 1, "default", common.StatusSuperseded), }...) rls, err := secrets.Query(map[string]string{"status": "deployed"}) @@ -172,7 +177,7 @@ func TestSecretCreate(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) // store the release in a secret if err := secrets.Create(key, rel); err != nil { @@ -196,12 +201,12 @@ func TestSecretUpdate(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) secrets := newTestFixtureSecrets(t, []*rspb.Release{rel}...) // modify release status code - rel.Info.Status = rspb.StatusSuperseded + rel.Info.Status = common.StatusSuperseded // perform the update if err := secrets.Update(key, rel); err != nil { @@ -209,10 +214,11 @@ func TestSecretUpdate(t *testing.T) { } // fetch the updated release - got, err := secrets.Get(key) + goti, err := secrets.Get(key) if err != nil { t.Fatalf("Failed to get release with key %q: %s", key, err) } + got := convertReleaserToV1(t, goti) // check release has actually been updated by comparing modified fields if rel.Info.Status != got.Info.Status { @@ -225,7 +231,7 @@ func TestSecretDelete(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) secrets := newTestFixtureSecrets(t, []*rspb.Release{rel}...) diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index 46f6c6b2e..0020d2436 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -32,6 +32,7 @@ import ( // Import pq for postgres dialect _ "github.com/lib/pq" + "helm.sh/helm/v4/pkg/release" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -297,7 +298,7 @@ func NewSQL(connectionString string, namespace string) (*SQL, error) { } // Get returns the release named by key. -func (s *SQL) Get(key string) (*rspb.Release, error) { +func (s *SQL) Get(key string) (release.Releaser, error) { var record SQLReleaseWrapper qb := s.statementBuilder. @@ -333,7 +334,7 @@ func (s *SQL) Get(key string) (*rspb.Release, error) { } // List returns the list of all releases such that filter(release) == true -func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { +func (s *SQL) List(filter func(release.Releaser) bool) ([]release.Releaser, error) { sb := s.statementBuilder. Select(sqlReleaseTableKeyColumn, sqlReleaseTableNamespaceColumn, sqlReleaseTableBodyColumn). From(sqlReleaseTableName). @@ -356,7 +357,7 @@ func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { return nil, err } - var releases []*rspb.Release + var releases []release.Releaser for _, record := range records { release, err := decodeRelease(record.Body) if err != nil { @@ -379,7 +380,7 @@ func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { } // Query returns the set of releases that match the provided set of labels. -func (s *SQL) Query(labels map[string]string) ([]*rspb.Release, error) { +func (s *SQL) Query(labels map[string]string) ([]release.Releaser, error) { sb := s.statementBuilder. Select(sqlReleaseTableKeyColumn, sqlReleaseTableNamespaceColumn, sqlReleaseTableBodyColumn). From(sqlReleaseTableName) @@ -420,7 +421,7 @@ func (s *SQL) Query(labels map[string]string) ([]*rspb.Release, error) { return nil, ErrReleaseNotFound } - var releases []*rspb.Release + var releases []release.Releaser for _, record := range records { release, err := decodeRelease(record.Body) if err != nil { @@ -444,7 +445,12 @@ func (s *SQL) Query(labels map[string]string) ([]*rspb.Release, error) { } // Create creates a new release. -func (s *SQL) Create(key string, rls *rspb.Release) error { +func (s *SQL) Create(key string, rel release.Releaser) error { + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } + namespace := rls.Namespace if namespace == "" { namespace = defaultNamespace @@ -551,7 +557,11 @@ func (s *SQL) Create(key string, rls *rspb.Release) error { } // Update updates a release. -func (s *SQL) Update(key string, rls *rspb.Release) error { +func (s *SQL) Update(key string, rel release.Releaser) error { + rls, err := releaserToV1Release(rel) + if err != nil { + return err + } namespace := rls.Namespace if namespace == "" { namespace = defaultNamespace @@ -590,7 +600,7 @@ func (s *SQL) Update(key string, rls *rspb.Release) error { } // Delete deletes a release or returns ErrReleaseNotFound. -func (s *SQL) Delete(key string) (*rspb.Release, error) { +func (s *SQL) Delete(key string) (release.Releaser, error) { transaction, err := s.db.Beginx() if err != nil { slog.Debug("failed to start SQL transaction", slog.Any("error", err)) diff --git a/pkg/storage/driver/sql_test.go b/pkg/storage/driver/sql_test.go index fe147816c..d85691a6f 100644 --- a/pkg/storage/driver/sql_test.go +++ b/pkg/storage/driver/sql_test.go @@ -24,6 +24,8 @@ import ( sqlmock "github.com/DATA-DOG/go-sqlmock" migrate "github.com/rubenv/sql-migrate" + "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -66,7 +68,7 @@ func TestSQLGet(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) body, _ := encodeRelease(rel) @@ -109,12 +111,12 @@ func TestSQLGet(t *testing.T) { func TestSQLList(t *testing.T) { releases := []*rspb.Release{} - releases = append(releases, releaseStub("key-1", 1, "default", rspb.StatusUninstalled)) - releases = append(releases, releaseStub("key-2", 1, "default", rspb.StatusUninstalled)) - releases = append(releases, releaseStub("key-3", 1, "default", rspb.StatusDeployed)) - releases = append(releases, releaseStub("key-4", 1, "default", rspb.StatusDeployed)) - releases = append(releases, releaseStub("key-5", 1, "default", rspb.StatusSuperseded)) - releases = append(releases, releaseStub("key-6", 1, "default", rspb.StatusSuperseded)) + releases = append(releases, releaseStub("key-1", 1, "default", common.StatusUninstalled)) + releases = append(releases, releaseStub("key-2", 1, "default", common.StatusUninstalled)) + releases = append(releases, releaseStub("key-3", 1, "default", common.StatusDeployed)) + releases = append(releases, releaseStub("key-4", 1, "default", common.StatusDeployed)) + releases = append(releases, releaseStub("key-5", 1, "default", common.StatusSuperseded)) + releases = append(releases, releaseStub("key-6", 1, "default", common.StatusSuperseded)) sqlDriver, mock := newTestFixtureSQL(t) @@ -147,8 +149,9 @@ func TestSQLList(t *testing.T) { } // list all deleted releases - del, err := sqlDriver.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusUninstalled + del, err := sqlDriver.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusUninstalled }) // check if err != nil { @@ -159,8 +162,9 @@ func TestSQLList(t *testing.T) { } // list all deployed releases - dpl, err := sqlDriver.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusDeployed + dpl, err := sqlDriver.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusDeployed }) // check if err != nil { @@ -171,8 +175,9 @@ func TestSQLList(t *testing.T) { } // list all superseded releases - ssd, err := sqlDriver.List(func(rel *rspb.Release) bool { - return rel.Info.Status == rspb.StatusSuperseded + ssd, err := sqlDriver.List(func(rel release.Releaser) bool { + rls := convertReleaserToV1(t, rel) + return rls.Info.Status == common.StatusSuperseded }) // check if err != nil { @@ -187,7 +192,7 @@ func TestSQLList(t *testing.T) { } // Check if release having both system and custom labels, this is needed to ensure that selector filtering would work. - rls := ssd[0] + rls := convertReleaserToV1(t, ssd[0]) _, ok := rls.Labels["name"] if !ok { t.Fatalf("Expected 'name' label in results, actual %v", rls.Labels) @@ -203,7 +208,7 @@ func TestSqlCreate(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) sqlDriver, mock := newTestFixtureSQL(t) body, _ := encodeRelease(rel) @@ -260,7 +265,7 @@ func TestSqlCreateAlreadyExists(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) sqlDriver, mock := newTestFixtureSQL(t) body, _ := encodeRelease(rel) @@ -321,7 +326,7 @@ func TestSqlUpdate(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) sqlDriver, mock := newTestFixtureSQL(t) body, _ := encodeRelease(rel) @@ -370,9 +375,9 @@ func TestSqlQuery(t *testing.T) { "owner": sqlReleaseDefaultOwner, } - supersededRelease := releaseStub("smug-pigeon", 1, "default", rspb.StatusSuperseded) + supersededRelease := releaseStub("smug-pigeon", 1, "default", common.StatusSuperseded) supersededReleaseBody, _ := encodeRelease(supersededRelease) - deployedRelease := releaseStub("smug-pigeon", 2, "default", rspb.StatusDeployed) + deployedRelease := releaseStub("smug-pigeon", 2, "default", common.StatusDeployed) deployedReleaseBody, _ := encodeRelease(deployedRelease) // Let's actually start our test @@ -482,7 +487,7 @@ func TestSqlDelete(t *testing.T) { name := "smug-pigeon" namespace := "default" key := testKey(name, vers) - rel := releaseStub(name, vers, namespace, rspb.StatusDeployed) + rel := releaseStub(name, vers, namespace, common.StatusDeployed) body, _ := encodeRelease(rel) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index f086309bb..4603a1de6 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -22,6 +22,8 @@ import ( "log/slog" "strings" + "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" relutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/storage/driver" @@ -47,7 +49,7 @@ type Storage struct { // Get retrieves the release from storage. An error is returned // if the storage driver failed to fetch the release, or the // release identified by the key, version pair does not exist. -func (s *Storage) Get(name string, version int) (*rspb.Release, error) { +func (s *Storage) Get(name string, version int) (release.Releaser, error) { slog.Debug("getting release", "key", makeKey(name, version)) return s.Driver.Get(makeKey(name, version)) } @@ -55,62 +57,99 @@ func (s *Storage) Get(name string, version int) (*rspb.Release, error) { // Create creates a new storage entry holding the release. An // error is returned if the storage driver fails to store the // release, or a release with an identical key already exists. -func (s *Storage) Create(rls *rspb.Release) error { - slog.Debug("creating release", "key", makeKey(rls.Name, rls.Version)) +func (s *Storage) Create(rls release.Releaser) error { + rac, err := release.NewAccessor(rls) + if err != nil { + return err + } + slog.Debug("creating release", "key", makeKey(rac.Name(), rac.Version())) if s.MaxHistory > 0 { // Want to make space for one more release. - if err := s.removeLeastRecent(rls.Name, s.MaxHistory-1); err != nil && + if err := s.removeLeastRecent(rac.Name(), s.MaxHistory-1); err != nil && !errors.Is(err, driver.ErrReleaseNotFound) { return err } } - return s.Driver.Create(makeKey(rls.Name, rls.Version), rls) + return s.Driver.Create(makeKey(rac.Name(), rac.Version()), rls) } // Update updates the release in storage. An error is returned if the // storage backend fails to update the release or if the release // does not exist. -func (s *Storage) Update(rls *rspb.Release) error { - slog.Debug("updating release", "key", makeKey(rls.Name, rls.Version)) - return s.Driver.Update(makeKey(rls.Name, rls.Version), rls) +func (s *Storage) Update(rls release.Releaser) error { + rac, err := release.NewAccessor(rls) + if err != nil { + return err + } + slog.Debug("updating release", "key", makeKey(rac.Name(), rac.Version())) + return s.Driver.Update(makeKey(rac.Name(), rac.Version()), rls) } // Delete deletes the release from storage. An error is returned if // the storage backend fails to delete the release or if the release // does not exist. -func (s *Storage) Delete(name string, version int) (*rspb.Release, error) { +func (s *Storage) Delete(name string, version int) (release.Releaser, error) { slog.Debug("deleting release", "key", makeKey(name, version)) return s.Driver.Delete(makeKey(name, version)) } // ListReleases returns all releases from storage. An error is returned if the // storage backend fails to retrieve the releases. -func (s *Storage) ListReleases() ([]*rspb.Release, error) { +func (s *Storage) ListReleases() ([]release.Releaser, error) { slog.Debug("listing all releases in storage") - return s.List(func(_ *rspb.Release) bool { return true }) + return s.List(func(_ release.Releaser) bool { return true }) +} + +// releaserToV1Release is a helper function to convert a v1 release passed by interface +// into the type object. +func releaserToV1Release(rel release.Releaser) (*rspb.Release, error) { + switch r := rel.(type) { + case rspb.Release: + return &r, nil + case *rspb.Release: + return r, nil + case nil: + return nil, nil + default: + return nil, fmt.Errorf("unsupported release type: %T", rel) + } } // ListUninstalled returns all releases with Status == UNINSTALLED. An error is returned // if the storage backend fails to retrieve the releases. -func (s *Storage) ListUninstalled() ([]*rspb.Release, error) { +func (s *Storage) ListUninstalled() ([]release.Releaser, error) { slog.Debug("listing uninstalled releases in storage") - return s.List(func(rls *rspb.Release) bool { - return relutil.StatusFilter(rspb.StatusUninstalled).Check(rls) + return s.List(func(rls release.Releaser) bool { + rel, err := releaserToV1Release(rls) + if err != nil { + // This will only happen if calling code does not pass the proper types. This is + // a problem with the application and not user data. + slog.Error("unable to convert release to typed release", slog.Any("error", err)) + panic(fmt.Sprintf("unable to convert release to typed release: %s", err)) + } + return relutil.StatusFilter(common.StatusUninstalled).Check(rel) }) } // ListDeployed returns all releases with Status == DEPLOYED. An error is returned // if the storage backend fails to retrieve the releases. -func (s *Storage) ListDeployed() ([]*rspb.Release, error) { +func (s *Storage) ListDeployed() ([]release.Releaser, error) { slog.Debug("listing all deployed releases in storage") - return s.List(func(rls *rspb.Release) bool { - return relutil.StatusFilter(rspb.StatusDeployed).Check(rls) + return s.List(func(rls release.Releaser) bool { + rel, err := releaserToV1Release(rls) + if err != nil { + // This will only happen if calling code does not pass the proper types. This is + // a problem with the application and not user data. + slog.Error("unable to convert release to typed release", slog.Any("error", err)) + panic(fmt.Sprintf("unable to convert release to typed release: %s", err)) + } + return relutil.StatusFilter(common.StatusDeployed).Check(rel) }) } // Deployed returns the last deployed release with the provided release name, or // returns driver.NewErrNoDeployedReleases if not found. -func (s *Storage) Deployed(name string) (*rspb.Release, error) { +func (s *Storage) Deployed(name string) (release.Releaser, error) { ls, err := s.DeployedAll(name) if err != nil { return nil, err @@ -120,16 +159,34 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) { return nil, driver.NewErrNoDeployedReleases(name) } + rls, err := releaseListToV1List(ls) + if err != nil { + return nil, err + } + // If executed concurrently, Helm's database gets corrupted // and multiple releases are DEPLOYED. Take the latest. - relutil.Reverse(ls, relutil.SortByRevision) + relutil.Reverse(rls, relutil.SortByRevision) - return ls[0], nil + return rls[0], nil +} + +func releaseListToV1List(ls []release.Releaser) ([]*rspb.Release, error) { + rls := make([]*rspb.Release, 0, len(ls)) + for _, val := range ls { + rel, err := releaserToV1Release(val) + if err != nil { + return nil, err + } + rls = append(rls, rel) + } + + return rls, nil } // DeployedAll returns all deployed releases with the provided name, or // returns driver.NewErrNoDeployedReleases if not found. -func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) { +func (s *Storage) DeployedAll(name string) ([]release.Releaser, error) { slog.Debug("getting deployed releases", "name", name) ls, err := s.Query(map[string]string{ @@ -148,7 +205,7 @@ func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) { // History returns the revision history for the release with the provided name, or // returns driver.ErrReleaseNotFound if no such release name exists. -func (s *Storage) History(name string) ([]*rspb.Release, error) { +func (s *Storage) History(name string) ([]release.Releaser, error) { slog.Debug("getting release history", "name", name) return s.Query(map[string]string{"name": name, "owner": "helm"}) @@ -170,23 +227,31 @@ func (s *Storage) removeLeastRecent(name string, maximum int) error { if len(h) <= maximum { return nil } + rls, err := releaseListToV1List(h) + if err != nil { + return err + } // We want oldest to newest - relutil.SortByRevision(h) + relutil.SortByRevision(rls) lastDeployed, err := s.Deployed(name) if err != nil && !errors.Is(err, driver.ErrNoDeployedReleases) { return err } - var toDelete []*rspb.Release - for _, rel := range h { + var toDelete []release.Releaser + for _, rel := range rls { // once we have enough releases to delete to reach the maximum, stop - if len(h)-len(toDelete) == maximum { + if len(rls)-len(toDelete) == maximum { break } if lastDeployed != nil { - if rel.Version != lastDeployed.Version { + ldac, err := release.NewAccessor(lastDeployed) + if err != nil { + return err + } + if rel.Version != ldac.Version() { toDelete = append(toDelete, rel) } } else { @@ -198,7 +263,12 @@ func (s *Storage) removeLeastRecent(name string, maximum int) error { // multiple invocations of this function will eventually delete them all. errs := []error{} for _, rel := range toDelete { - err = s.deleteReleaseVersion(name, rel.Version) + rac, err := release.NewAccessor(rel) + if err != nil { + errs = append(errs, err) + continue + } + err = s.deleteReleaseVersion(name, rac.Version()) if err != nil { errs = append(errs, err) } @@ -226,7 +296,7 @@ func (s *Storage) deleteReleaseVersion(name string, version int) error { } // Last fetches the last revision of the named release. -func (s *Storage) Last(name string) (*rspb.Release, error) { +func (s *Storage) Last(name string) (release.Releaser, error) { slog.Debug("getting last revision", "name", name) h, err := s.History(name) if err != nil { @@ -235,9 +305,13 @@ func (s *Storage) Last(name string) (*rspb.Release, error) { if len(h) == 0 { return nil, fmt.Errorf("no revision for release %q", name) } + rls, err := releaseListToV1List(h) + if err != nil { + return nil, err + } - relutil.Reverse(h, relutil.SortByRevision) - return h[0], nil + relutil.Reverse(rls, relutil.SortByRevision) + return rls[0], nil } // makeKey concatenates the Kubernetes storage object type, a release name and version diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index d3025eca3..283bd8199 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -22,6 +22,9 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/release" + "helm.sh/helm/v4/pkg/release/common" rspb "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/storage/driver" ) @@ -56,13 +59,13 @@ func TestStorageUpdate(t *testing.T) { rls := ReleaseTestData{ Name: "angry-beaver", Version: 1, - Status: rspb.StatusDeployed, + Status: common.StatusDeployed, }.ToRelease() assertErrNil(t.Fatal, storage.Create(rls), "StoreRelease") // modify the release - rls.Info.Status = rspb.StatusUninstalled + rls.Info.Status = common.StatusUninstalled assertErrNil(t.Fatal, storage.Update(rls), "UpdateRelease") // retrieve the updated release @@ -106,13 +109,16 @@ func TestStorageDelete(t *testing.T) { t.Errorf("unexpected error: %s", err) } + rhist, err := releaseListToV1List(hist) + assert.NoError(t, err) + // We have now deleted one of the two records. - if len(hist) != 1 { + if len(rhist) != 1 { t.Errorf("expected 1 record for deleted release version, got %d", len(hist)) } - if hist[0].Version != 2 { - t.Errorf("Expected version to be 2, got %d", hist[0].Version) + if rhist[0].Version != 2 { + t.Errorf("Expected version to be 2, got %d", rhist[0].Version) } } @@ -123,13 +129,13 @@ func TestStorageList(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls0 := ReleaseTestData{Name: "happy-catdog", Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: "livid-human", Status: rspb.StatusSuperseded}.ToRelease() - rls2 := ReleaseTestData{Name: "relaxed-cat", Status: rspb.StatusSuperseded}.ToRelease() - rls3 := ReleaseTestData{Name: "hungry-hippo", Status: rspb.StatusDeployed}.ToRelease() - rls4 := ReleaseTestData{Name: "angry-beaver", Status: rspb.StatusDeployed}.ToRelease() - rls5 := ReleaseTestData{Name: "opulent-frog", Status: rspb.StatusUninstalled}.ToRelease() - rls6 := ReleaseTestData{Name: "happy-liger", Status: rspb.StatusUninstalled}.ToRelease() + rls0 := ReleaseTestData{Name: "happy-catdog", Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: "livid-human", Status: common.StatusSuperseded}.ToRelease() + rls2 := ReleaseTestData{Name: "relaxed-cat", Status: common.StatusSuperseded}.ToRelease() + rls3 := ReleaseTestData{Name: "hungry-hippo", Status: common.StatusDeployed}.ToRelease() + rls4 := ReleaseTestData{Name: "angry-beaver", Status: common.StatusDeployed}.ToRelease() + rls5 := ReleaseTestData{Name: "opulent-frog", Status: common.StatusUninstalled}.ToRelease() + rls6 := ReleaseTestData{Name: "happy-liger", Status: common.StatusUninstalled}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'rls0'") @@ -144,7 +150,7 @@ func TestStorageList(t *testing.T) { var listTests = []struct { Description string NumExpected int - ListFunc func() ([]*rspb.Release, error) + ListFunc func() ([]release.Releaser, error) }{ {"ListDeployed", 2, storage.ListDeployed}, {"ListReleases", 7, storage.ListReleases}, @@ -175,10 +181,10 @@ func TestStorageDeployed(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusSuperseded}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusDeployed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusSuperseded}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusSuperseded}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusDeployed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -194,15 +200,18 @@ func TestStorageDeployed(t *testing.T) { t.Fatalf("Failed to query for deployed release: %s\n", err) } + rel, err := releaserToV1Release(rls) + assert.NoError(t, err) + switch { case rls == nil: t.Fatalf("Release is nil") - case rls.Name != name: - t.Fatalf("Expected release name %q, actual %q\n", name, rls.Name) - case rls.Version != vers: - t.Fatalf("Expected release version %d, actual %d\n", vers, rls.Version) - case rls.Info.Status != rspb.StatusDeployed: - t.Fatalf("Expected release status 'DEPLOYED', actual %s\n", rls.Info.Status.String()) + case rel.Name != name: + t.Fatalf("Expected release name %q, actual %q\n", name, rel.Name) + case rel.Version != vers: + t.Fatalf("Expected release version %d, actual %d\n", vers, rel.Version) + case rel.Info.Status != common.StatusDeployed: + t.Fatalf("Expected release status 'DEPLOYED', actual %s\n", rel.Info.Status.String()) } } @@ -215,10 +224,10 @@ func TestStorageDeployedWithCorruption(t *testing.T) { // setup storage with test releases setup := func() { // release records (notice odd order and corruption) - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusDeployed}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusSuperseded}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusDeployed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusDeployed}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusSuperseded}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusDeployed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -234,15 +243,18 @@ func TestStorageDeployedWithCorruption(t *testing.T) { t.Fatalf("Failed to query for deployed release: %s\n", err) } + rel, err := releaserToV1Release(rls) + assert.NoError(t, err) + switch { case rls == nil: t.Fatalf("Release is nil") - case rls.Name != name: - t.Fatalf("Expected release name %q, actual %q\n", name, rls.Name) - case rls.Version != vers: - t.Fatalf("Expected release version %d, actual %d\n", vers, rls.Version) - case rls.Info.Status != rspb.StatusDeployed: - t.Fatalf("Expected release status 'DEPLOYED', actual %s\n", rls.Info.Status.String()) + case rel.Name != name: + t.Fatalf("Expected release name %q, actual %q\n", name, rel.Name) + case rel.Version != vers: + t.Fatalf("Expected release version %d, actual %d\n", vers, rel.Version) + case rel.Info.Status != common.StatusDeployed: + t.Fatalf("Expected release status 'DEPLOYED', actual %s\n", rel.Info.Status.String()) } } @@ -254,10 +266,10 @@ func TestStorageHistory(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusSuperseded}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusDeployed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusSuperseded}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusSuperseded}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusDeployed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -286,22 +298,22 @@ type MaxHistoryMockDriver struct { func NewMaxHistoryMockDriver(d driver.Driver) *MaxHistoryMockDriver { return &MaxHistoryMockDriver{Driver: d} } -func (d *MaxHistoryMockDriver) Create(key string, rls *rspb.Release) error { +func (d *MaxHistoryMockDriver) Create(key string, rls release.Releaser) error { return d.Driver.Create(key, rls) } -func (d *MaxHistoryMockDriver) Update(key string, rls *rspb.Release) error { +func (d *MaxHistoryMockDriver) Update(key string, rls release.Releaser) error { return d.Driver.Update(key, rls) } -func (d *MaxHistoryMockDriver) Delete(_ string) (*rspb.Release, error) { +func (d *MaxHistoryMockDriver) Delete(_ string) (release.Releaser, error) { return nil, errMaxHistoryMockDriverSomethingHappened } -func (d *MaxHistoryMockDriver) Get(key string) (*rspb.Release, error) { +func (d *MaxHistoryMockDriver) Get(key string) (release.Releaser, error) { return d.Driver.Get(key) } -func (d *MaxHistoryMockDriver) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { +func (d *MaxHistoryMockDriver) List(filter func(release.Releaser) bool) ([]release.Releaser, error) { return d.Driver.List(filter) } -func (d *MaxHistoryMockDriver) Query(labels map[string]string) ([]*rspb.Release, error) { +func (d *MaxHistoryMockDriver) Query(labels map[string]string) ([]release.Releaser, error) { return d.Driver.Query(labels) } func (d *MaxHistoryMockDriver) Name() string { @@ -319,14 +331,14 @@ func TestMaxHistoryErrorHandling(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls1 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Driver.Create(makeKey(rls1.Name, rls1.Version), rls1), "Storing release 'angry-bird' (v1)") } setup() - rls2 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusSuperseded}.ToRelease() wantErr := errMaxHistoryMockDriverSomethingHappened gotErr := storage.Create(rls2) if !errors.Is(gotErr, wantErr) { @@ -345,10 +357,10 @@ func TestStorageRemoveLeastRecent(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusSuperseded}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusDeployed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusSuperseded}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusSuperseded}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusDeployed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -367,22 +379,24 @@ func TestStorageRemoveLeastRecent(t *testing.T) { } storage.MaxHistory = 3 - rls5 := ReleaseTestData{Name: name, Version: 5, Status: rspb.StatusDeployed}.ToRelease() + rls5 := ReleaseTestData{Name: name, Version: 5, Status: common.StatusDeployed}.ToRelease() assertErrNil(t.Fatal, storage.Create(rls5), "Storing release 'angry-bird' (v5)") // On inserting the 5th record, we expect two records to be pruned from history. hist, err := storage.History(name) + rhist, err := releaseListToV1List(hist) + assert.NoError(t, err) if err != nil { t.Fatal(err) - } else if len(hist) != storage.MaxHistory { - for _, item := range hist { + } else if len(rhist) != storage.MaxHistory { + for _, item := range rhist { t.Logf("%s %v", item.Name, item.Version) } - t.Fatalf("expected %d items in history, got %d", storage.MaxHistory, len(hist)) + t.Fatalf("expected %d items in history, got %d", storage.MaxHistory, len(rhist)) } // We expect the existing records to be 3, 4, and 5. - for i, item := range hist { + for i, item := range rhist { v := item.Version if expect := i + 3; v != expect { t.Errorf("Expected release %d, got %d", expect, v) @@ -399,10 +413,10 @@ func TestStorageDoNotDeleteDeployed(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusDeployed}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusFailed}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusFailed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusDeployed}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusFailed}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusFailed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -412,7 +426,7 @@ func TestStorageDoNotDeleteDeployed(t *testing.T) { } setup() - rls5 := ReleaseTestData{Name: name, Version: 5, Status: rspb.StatusFailed}.ToRelease() + rls5 := ReleaseTestData{Name: name, Version: 5, Status: common.StatusFailed}.ToRelease() assertErrNil(t.Fatal, storage.Create(rls5), "Storing release 'angry-bird' (v5)") // On inserting the 5th record, we expect a total of 3 releases, but we expect version 2 @@ -421,10 +435,12 @@ func TestStorageDoNotDeleteDeployed(t *testing.T) { if err != nil { t.Fatal(err) } else if len(hist) != storage.MaxHistory { - for _, item := range hist { + rhist, err := releaseListToV1List(hist) + assert.NoError(t, err) + for _, item := range rhist { t.Logf("%s %v", item.Name, item.Version) } - t.Fatalf("expected %d items in history, got %d", storage.MaxHistory, len(hist)) + t.Fatalf("expected %d items in history, got %d", storage.MaxHistory, len(rhist)) } expectedVersions := map[int]bool{ @@ -433,7 +449,9 @@ func TestStorageDoNotDeleteDeployed(t *testing.T) { 5: true, } - for _, item := range hist { + rhist, err := releaseListToV1List(hist) + assert.NoError(t, err) + for _, item := range rhist { if !expectedVersions[item.Version] { t.Errorf("Release version %d, found when not expected", item.Version) } @@ -448,10 +466,10 @@ func TestStorageLast(t *testing.T) { // Set up storage with test releases. setup := func() { // release records - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusSuperseded}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusSuperseded}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusFailed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusSuperseded}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusSuperseded}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusSuperseded}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusFailed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -467,8 +485,11 @@ func TestStorageLast(t *testing.T) { t.Fatalf("Failed to query for release history (%q): %s\n", name, err) } - if h.Version != 4 { - t.Errorf("Expected revision 4, got %d", h.Version) + rel, err := releaserToV1Release(h) + assert.NoError(t, err) + + if rel.Version != 4 { + t.Errorf("Expected revision 4, got %d", rel.Version) } } @@ -483,10 +504,10 @@ func TestUpgradeInitiallyFailedReleaseWithHistoryLimit(t *testing.T) { // setup storage with test releases setup := func() { // release records - rls0 := ReleaseTestData{Name: name, Version: 1, Status: rspb.StatusFailed}.ToRelease() - rls1 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusFailed}.ToRelease() - rls2 := ReleaseTestData{Name: name, Version: 3, Status: rspb.StatusFailed}.ToRelease() - rls3 := ReleaseTestData{Name: name, Version: 4, Status: rspb.StatusFailed}.ToRelease() + rls0 := ReleaseTestData{Name: name, Version: 1, Status: common.StatusFailed}.ToRelease() + rls1 := ReleaseTestData{Name: name, Version: 2, Status: common.StatusFailed}.ToRelease() + rls2 := ReleaseTestData{Name: name, Version: 3, Status: common.StatusFailed}.ToRelease() + rls3 := ReleaseTestData{Name: name, Version: 4, Status: common.StatusFailed}.ToRelease() // create the release records in the storage assertErrNil(t.Fatal, storage.Create(rls0), "Storing release 'angry-bird' (v1)") @@ -507,7 +528,7 @@ func TestUpgradeInitiallyFailedReleaseWithHistoryLimit(t *testing.T) { setup() - rls5 := ReleaseTestData{Name: name, Version: 5, Status: rspb.StatusFailed}.ToRelease() + rls5 := ReleaseTestData{Name: name, Version: 5, Status: common.StatusFailed}.ToRelease() err := storage.Create(rls5) if err != nil { t.Fatalf("Failed to create a new release version: %s", err) @@ -518,13 +539,15 @@ func TestUpgradeInitiallyFailedReleaseWithHistoryLimit(t *testing.T) { t.Fatalf("unexpected error: %s", err) } - for i, rel := range hist { + rhist, err := releaseListToV1List(hist) + assert.NoError(t, err) + for i, rel := range rhist { wantVersion := i + 2 if rel.Version != wantVersion { t.Fatalf("Expected history release %d version to equal %d, got %d", i+1, wantVersion, rel.Version) } - wantStatus := rspb.StatusFailed + wantStatus := common.StatusFailed if rel.Info.Status != wantStatus { t.Fatalf("Expected history release %d status to equal %q, got %q", i+1, wantStatus, rel.Info.Status) } @@ -536,7 +559,7 @@ type ReleaseTestData struct { Version int Manifest string Namespace string - Status rspb.Status + Status common.Status } func (test ReleaseTestData) ToRelease() *rspb.Release {