feat: add --take-ownership flag to helm diff and related config (#1992)

* feat: add --take-ownership flag to helm diff and related config Signed-off-by: yxxhero <aiopsclub@163.com> * fix: nil issue Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * fix more issue Signed-off-by: yxxhero <aiopsclub@163.com> * fix more issues Signed-off-by: yxxhero <aiopsclub@163.com> * fix tests Signed-off-by: yxxhero <aiopsclub@163.com> * add more tests Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com>
2025-04-28 00:57:41 +08:00 · 2025-04-28 00:57:41 +08:00 · c3864a45d3
parent 6b5f1496a8
commit c3864a45d3
11 changed files with 117 additions and 24 deletions
--- a/cmd/diff.go
+++ b/cmd/diff.go
@ -52,6 +52,7 @@ func NewDiffCmd(globalCfg *config.GlobalImpl) *cobra.Command {
 	f.StringArrayVar(&diffOptions.Suppress, "suppress", nil, "suppress specified Kubernetes objects in the output. Can be provided multiple times. For example: --suppress KeycloakClient --suppress VaultSecret")
 	f.BoolVar(&diffOptions.ReuseValues, "reuse-values", false, `Override helmDefaults.reuseValues "helm diff upgrade --install --reuse-values"`)
 	f.BoolVar(&diffOptions.ResetValues, "reset-values", false, `Override helmDefaults.reuseValues "helm diff upgrade --install --reset-values"`)
+	f.BoolVar(&diffOptions.TakeOwnership, "take-ownership", false, "add --take-ownership flag to helm")
 	f.StringVar(&diffOptions.PostRenderer, "post-renderer", "", `pass --post-renderer to "helm template" or "helm upgrade --install"`)
 	f.StringArrayVar(&diffOptions.PostRendererArgs, "post-renderer-args", nil, `pass --post-renderer-args to "helm template" or "helm upgrade --install"`)
 	f.StringArrayVar(&diffOptions.SuppressOutputLineRegex, "suppress-output-line-regex", nil, "a list of regex patterns to suppress output lines from the diff output")
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@ -1390,6 +1390,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) {
 		PostRendererArgs:        c.PostRendererArgs(),
 		SkipSchemaValidation:    c.SkipSchemaValidation(),
 		SuppressOutputLineRegex: c.SuppressOutputLineRegex(),
+		TakeOwnership:           c.TakeOwnership(),
 	}

 	infoMsg, releasesToBeUpdated, releasesToBeDeleted, errs := r.diff(false, detailedExitCode, c, diffOpts)
@ -1634,6 +1635,7 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error)
 			PostRendererArgs:        c.PostRendererArgs(),
 			SkipSchemaValidation:    c.SkipSchemaValidation(),
 			SuppressOutputLineRegex: c.SuppressOutputLineRegex(),
+			TakeOwnership:           c.TakeOwnership(),
 		}

 		filtered := &Run{
--- a/pkg/app/config.go
+++ b/pkg/app/config.go
@ -157,6 +157,7 @@ type DiffConfigProvider interface {
 	NoColor() bool
 	Context() int
 	DiffOutput() string
+	TakeOwnership() bool

 	concurrencyConfig
 	valuesControlMode
--- a/pkg/app/diff_test.go
+++ b/pkg/app/diff_test.go
@ -46,6 +46,7 @@ type diffConfig struct {
 	skipSchemaValidation    bool
 	reuseValues             bool
 	logger                  *zap.SugaredLogger
+	takeOwnership           bool
 }

 func (a diffConfig) Args() string {
@ -183,6 +184,9 @@ func (a diffConfig) SkipSchemaValidation() bool {
 func (a diffConfig) SuppressOutputLineRegex() []string {
 	return a.suppressOutputLineRegex
 }
+func (a diffConfig) TakeOwnership() bool {
+	return a.takeOwnership
+}

 func TestDiff(t *testing.T) {
 	type flags struct {
--- a/pkg/config/apply.go
+++ b/pkg/config/apply.go
@ -269,6 +269,7 @@ func (a *ApplyImpl) TakeOwnership() bool {
 	return a.ApplyOptions.TakeOwnership
 }

+// SyncReleaseLabels returns the SyncReleaseLabels.
 func (a *ApplyImpl) SyncReleaseLabels() bool {
 	return a.ApplyOptions.SyncReleaseLabels
 }
--- a/pkg/config/diff.go
+++ b/pkg/config/diff.go
@ -49,6 +49,8 @@ type DiffOptions struct {
 	// SuppressOutputLineRegex is a list of regexes to suppress output lines
 	SuppressOutputLineRegex []string
 	SkipSchemaValidation    bool
+	// TakeOwnership is true if the ownership should be taken
+	TakeOwnership bool
 }

 // NewDiffOptions creates a new Apply
@ -201,6 +203,12 @@ func (t *DiffImpl) SuppressOutputLineRegex() []string {
 	return t.DiffOptions.SuppressOutputLineRegex
 }

+// SkipSchemaValidation returns the SkipSchemaValidation.
 func (t *DiffImpl) SkipSchemaValidation() bool {
 	return t.DiffOptions.SkipSchemaValidation
 }
+
+// TakeOwnership returns the TakeOwnership.
+func (t *DiffImpl) TakeOwnership() bool {
+	return t.DiffOptions.TakeOwnership
+}
--- a/pkg/state/helmx.go
+++ b/pkg/state/helmx.go
@ -54,7 +54,22 @@ func formatLabels(labels map[string]string) string {

 // append labels flags to helm flags, starting from helm v3.13.0
 func (st *HelmState) appendLabelsFlags(flags []string, helm helmexec.Interface, release *ReleaseSpec, syncReleaseLabels bool) []string {
-	if helm.IsVersionAtLeast("3.13.0") && (syncReleaseLabels || release.SyncReleaseLabels) {
+	if !helm.IsVersionAtLeast("3.13.0") {
+		return flags
+	}
+	isSyncReleaseLabels := false
+	switch {
+	// Check if SyncReleaseLabels is true in the release spec.
+	case release.SyncReleaseLabels != nil && *release.SyncReleaseLabels:
+		isSyncReleaseLabels = true
+	// Check if syncReleaseLabels argument is true.
+	case syncReleaseLabels:
+		isSyncReleaseLabels = true
+	// Check if SyncReleaseLabels is true in HelmDefaults.
+	case st.HelmDefaults.SyncReleaseLabels != nil && *st.HelmDefaults.SyncReleaseLabels:
+		isSyncReleaseLabels = true
+	}
+	if isSyncReleaseLabels {
 		labels := formatLabels(release.Labels)
 		if labels != "" {
 			flags = append(flags, "--labels", labels)
@ -203,13 +218,17 @@ func (st *HelmState) appendHideNotesFlags(flags []string, helm helmexec.Interfac
 }

 // append take-ownership flags to helm flags
-func (st *HelmState) appendTakeOwnershipFlags(flags []string, helm helmexec.Interface, ops *SyncOpts) []string {
+func (st *HelmState) appendTakeOwnershipFlagsForUpgrade(flags []string, helm helmexec.Interface, release *ReleaseSpec, takeOwnership bool) []string {
 	// see https://github.com/helm/helm/releases/tag/v3.17.0
 	if !helm.IsVersionAtLeast("3.17.0") {
 		return flags
 	}
 	switch {
-	case ops.TakeOwnership:
+	case release.TakeOwnership != nil && *release.TakeOwnership:
+		flags = append(flags, "--take-ownership")
+	case takeOwnership:
+		flags = append(flags, "--take-ownership")
+	case st.HelmDefaults.TakeOwnership != nil && *st.HelmDefaults.TakeOwnership:
 		flags = append(flags, "--take-ownership")
 	}
 	return flags
--- a/pkg/state/helmx_test.go
+++ b/pkg/state/helmx_test.go
@ -387,12 +387,13 @@ func TestAppendHideNotesFlags(t *testing.T) {
 	}
 }

-func TestAppendTakeOwnershipFlags(t *testing.T) {
+func TestAppendTakeOwnershipFlagsForUpgrade(t *testing.T) {
 	type args struct {
 		flags    []string
 		helm     helmexec.Interface
 		helmSpec HelmSpec
 		opt      *SyncOpts
+		release  *ReleaseSpec
 		expected []string
 	}
 	tests := []struct {
@ -402,8 +403,9 @@ func TestAppendTakeOwnershipFlags(t *testing.T) {
 		{
 			name: "no take-ownership when helm less than 3.17.0",
 			args: args{
-				flags: []string{},
-				helm:  testutil.NewVersionHelmExec("3.16.0"),
+				flags:   []string{},
+				release: &ReleaseSpec{},
+				helm:    testutil.NewVersionHelmExec("3.16.0"),
 				opt: &SyncOpts{
 					TakeOwnership: true,
 				},
@ -413,21 +415,34 @@ func TestAppendTakeOwnershipFlags(t *testing.T) {
 		{
 			name: "take-ownership from cmd flag",
 			args: args{
-				flags: []string{},
-				helm:  testutil.NewVersionHelmExec("3.17.0"),
+				flags:   []string{},
+				helm:    testutil.NewVersionHelmExec("3.17.0"),
+				release: &ReleaseSpec{},
 				opt: &SyncOpts{
 					TakeOwnership: true,
 				},
 				expected: []string{"--take-ownership"},
 			},
 		},
+		{
+			name: "take-ownership from release",
+			args: args{
+				flags: []string{},
+				helm:  testutil.NewVersionHelmExec("3.17.0"),
+				release: &ReleaseSpec{
+					TakeOwnership: &[]bool{true}[0],
+				},
+				opt:      &SyncOpts{},
+				expected: []string{"--take-ownership"},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			st := &HelmState{}
 			st.HelmDefaults = tt.args.helmSpec
-			got := st.appendTakeOwnershipFlags(tt.args.flags, tt.args.helm, tt.args.opt)
-			require.Equalf(t, tt.args.expected, got, "appendTakeOwnershipFlags() = %v, want %v", got, tt.args.expected)
+			got := st.appendTakeOwnershipFlagsForUpgrade(tt.args.flags, tt.args.helm, tt.args.release, tt.args.opt.TakeOwnership)
+			require.Equalf(t, tt.args.expected, got, "appendTakeOwnershipFlagsForUpgrade() = %v, want %v", got, tt.args.expected)
 		})
 	}
 }
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@ -204,11 +204,13 @@ type HelmSpec struct {
 	// PlainHttp is true if the remote charte should be fetched using HTTP and not HTTPS
 	PlainHttp bool `yaml:"plainHttp,omitempty"`
 	// Wait, if set to true, will wait until all resources are deleted before mark delete command as successful
-	DeleteWait bool `yaml:"deleteWait"`
+	DeleteWait bool `yaml:"deleteWait,omitempty"`
 	// Timeout is the time in seconds to wait for helmfile delete command (default 300)
-	DeleteTimeout int `yaml:"deleteTimeout"`
+	DeleteTimeout int `yaml:"deleteTimeout,omitempty"`
 	// SyncReleaseLabels is true if the release labels should be synced with the helmfile labels
-	SyncReleaseLabels bool `yaml:"syncReleaseLabels"`
+	SyncReleaseLabels *bool `yaml:"syncReleaseLabels,omitempty"`
+	// TakeOwnership is true if the helmfile should take ownership of the release
+	TakeOwnership *bool `yaml:"takeOwnership,omitempty"`
 }

 // RepositorySpec that defines values for a helm repo
@ -412,7 +414,9 @@ type ReleaseSpec struct {
 	// Timeout is the time in seconds to wait for helmfile delete command (default 300)
 	DeleteTimeout *int `yaml:"deleteTimeout,omitempty"`
 	// SyncReleaseLabels is true if the release labels should be synced with the helmfile labels
-	SyncReleaseLabels bool `yaml:"syncReleaseLabels"`
+	SyncReleaseLabels *bool `yaml:"syncReleaseLabels,omitempty"`
+	// TakeOwnership is true if the release should take ownership of the resources
+	TakeOwnership *bool `yaml:"takeOwnership,omitempty"`
 }

 func (r *Inherits) UnmarshalYAML(unmarshal func(any) error) error {
@ -2040,6 +2044,7 @@ type DiffOpts struct {
 	PostRendererArgs        []string
 	SuppressOutputLineRegex []string
 	SkipSchemaValidation    bool
+	TakeOwnership           bool
 }

 func (o *DiffOpts) Apply(opts *DiffOpts) {
@ -2797,9 +2802,11 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp

 	postRenderer := ""
 	syncReleaseLabels := false
+	takeOwnership := false
 	if opt != nil {
 		postRenderer = opt.PostRenderer
 		syncReleaseLabels = opt.SyncReleaseLabels
+		takeOwnership = opt.TakeOwnership
 	}

 	flags = st.appendConnectionFlags(flags, release)
@ -2828,7 +2835,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 	flags = st.appendHideNotesFlags(flags, helm, opt)

 	// append take-ownership flag
-	flags = st.appendTakeOwnershipFlags(flags, helm, opt)
+	flags = st.appendTakeOwnershipFlagsForUpgrade(flags, helm, release, takeOwnership)

 	flags = st.appendExtraSyncFlags(flags, opt)

@ -2969,6 +2976,17 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
 		flags = st.appendSuppressOutputLineRegexFlags(flags, release, suppressOutputLineRegex)
 	}

+	takeOwnership := false
+	if opt != nil {
+		takeOwnership = opt.TakeOwnership
+	}
+
+	var err error
+	flags, err = st.appendTakeOwnershipFlagsForDiff(flags, release, takeOwnership)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex)
 	if err != nil {
 		return nil, files, err
@ -2977,6 +2995,33 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
 	return append(flags, common...), files, nil
 }

+func (st *HelmState) appendTakeOwnershipFlagsForDiff(flags []string, release *ReleaseSpec, takeOwnership bool) ([]string, error) {
+	settings := cli.New()
+	isAppendTakeOwnership := false
+
+	switch {
+	case release.TakeOwnership != nil && *release.TakeOwnership:
+		isAppendTakeOwnership = true
+	case takeOwnership:
+		isAppendTakeOwnership = true
+	case st.HelmDefaults.TakeOwnership != nil && *st.HelmDefaults.TakeOwnership:
+		isAppendTakeOwnership = true
+	}
+	if isAppendTakeOwnership {
+		diffVersion, err := helmexec.GetPluginVersion("diff", settings.PluginsDirectory)
+		if err != nil {
+			return flags, err
+		}
+		dv, _ := semver.NewVersion("v3.11.0")
+
+		if diffVersion.LessThan(dv) {
+			return flags, fmt.Errorf("take-ownership is not supported by helm-diff plugin version %s, please use at least v3.11.0", diffVersion)
+		}
+		flags = append(flags, "--take-ownership")
+	}
+	return flags, nil
+}
+
 func (st *HelmState) appendChartVersionFlags(flags []string, release *ReleaseSpec) []string {
 	if release.Version != "" {
 		flags = append(flags, "--version", release.Version)
--- a/pkg/state/temp_test.go
+++ b/pkg/state/temp_test.go
@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) {
 	run(testcase{
 		subject: "baseline",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
-		want:    "foo-values-54f5f6cdb5",
+		want:    "foo-values-57ff559cd5",
 	})

 	run(testcase{
 		subject: "different bytes content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    []byte(`{"k":"v"}`),
-		want:    "foo-values-6bc8f7944b",
+		want:    "foo-values-75cd785b8b",
 	})

 	run(testcase{
 		subject: "different map content",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
 		data:    map[string]any{"k": "v"},
-		want:    "foo-values-dcffdcb8",
+		want:    "foo-values-5b44fdc768",
 	})

 	run(testcase{
 		subject: "different chart",
 		release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
-		want:    "foo-values-6d4c6fd548",
+		want:    "foo-values-6ddb5db94d",
 	})

 	run(testcase{
 		subject: "different name",
 		release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
-		want:    "bar-values-76974767c8",
+		want:    "bar-values-79bbb8df74",
 	})

 	run(testcase{
 		subject: "specific ns",
 		release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
-		want:    "myns-foo-values-77bd9cc6fb",
+		want:    "myns-foo-values-c9457ccfb",
 	})

 	for id, n := range ids {
--- a/test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/output.yaml
+++ b/test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/output.yaml
@ -19,7 +19,6 @@ releases:
    name: default-shared-resources
    namespace: default
    service: shared-resources
-  syncReleaseLabels: false
 - chart: aservo/util
  version: 0.0.1
  needs:
@ -31,10 +30,8 @@ releases:
    name: default-release-resources
    namespace: default
    service: release-resources
-  syncReleaseLabels: false
 templates:
  defaults:
    name: default-{{ .Release.Labels.service }}
    namespace: default
-    syncReleaseLabels: false
 renderedvalues: {}