Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for --take-ownership parameter #742

Merged
merged 5 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/helm3.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) {
flags = append(flags, "--skip-schema-validation")
}

if d.takeOwnership {
flags = append(flags, "--take-ownership")
}

var (
subcmd string
filter func([]byte) []byte
Expand Down
80 changes: 77 additions & 3 deletions cmd/upgrade.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"bytes"
"errors"
"fmt"
"log"
Expand All @@ -12,6 +13,9 @@ import (
"github.com/spf13/cobra"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/cli"
"helm.sh/helm/v3/pkg/kube"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/cli-runtime/pkg/resource"

"github.com/databus23/helm-diff/v3/diff"
"github.com/databus23/helm-diff/v3/manifest"
Expand Down Expand Up @@ -54,6 +58,7 @@ type diffCmd struct {
insecureSkipTLSVerify bool
install bool
normalizeManifests bool
takeOwnership bool
threeWayMerge bool
extraAPIs []string
kubeVersion string
Expand Down Expand Up @@ -248,6 +253,7 @@ func newChartCommand() *cobra.Command {
f.StringArrayVar(&diff.postRendererArgs, "post-renderer-args", []string{}, "an argument to the post-renderer (can specify multiple)")
f.BoolVar(&diff.insecureSkipTLSVerify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart download")
f.BoolVar(&diff.normalizeManifests, "normalize-manifests", false, "normalize manifests before running diff to exclude style differences from the output")
f.BoolVar(&diff.takeOwnership, "take-ownership", false, "if set, upgrade will ignore the check for helm annotations and take ownership of the existing resources")

AddDiffOptions(f, &diff.Options)

Expand All @@ -263,6 +269,12 @@ func (d *diffCmd) runHelm3() error {

var err error

if d.takeOwnership {
// We need to do a three way merge between the manifests of the new
// release, the manifests of the old release and what is currently deployed
d.threeWayMerge = true
}

if d.clusterAccessAllowed() {
releaseManifest, err = getRelease(d.release, d.namespace)
}
Expand All @@ -287,14 +299,18 @@ func (d *diffCmd) runHelm3() error {
return fmt.Errorf("Failed to render chart: %w", err)
}

if d.threeWayMerge {
actionConfig := new(action.Configuration)
var actionConfig *action.Configuration
if d.threeWayMerge || d.takeOwnership {
actionConfig = new(action.Configuration)
if err := actionConfig.Init(envSettings.RESTClientGetter(), envSettings.Namespace(), os.Getenv("HELM_DRIVER"), log.Printf); err != nil {
log.Fatalf("%+v", err)
}
if err := actionConfig.KubeClient.IsReachable(); err != nil {
return err
}
}

if d.threeWayMerge {
releaseManifest, installManifest, err = manifest.Generate(actionConfig, releaseManifest, installManifest)
if err != nil {
return fmt.Errorf("unable to generate manifests: %w", err)
Expand All @@ -316,13 +332,27 @@ func (d *diffCmd) runHelm3() error {
currentSpecs = manifest.Parse(string(releaseManifest), d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
}
}

var newOwnedReleases map[string]diff.OwnershipDiff
if d.takeOwnership {
resources, err := actionConfig.KubeClient.Build(bytes.NewBuffer(installManifest), false)
if err != nil {
return err
}
newOwnedReleases, err = checkOwnership(d, resources, currentSpecs)
if err != nil {
return err
}
}

var newSpecs map[string]*manifest.MappingResult
if d.includeTests {
newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests)
} else {
newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
}
seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, &d.Options, os.Stdout)

seenAnyChanges := diff.ManifestsOwnership(currentSpecs, newSpecs, newOwnedReleases, &d.Options, os.Stdout)

if d.detailedExitCode && seenAnyChanges {
return Error{
Expand All @@ -333,3 +363,47 @@ func (d *diffCmd) runHelm3() error {

return nil
}

func checkOwnership(d *diffCmd, resources kube.ResourceList, currentSpecs map[string]*manifest.MappingResult) (map[string]diff.OwnershipDiff, error) {
newOwnedReleases := make(map[string]diff.OwnershipDiff)
err := resources.Visit(func(info *resource.Info, err error) error {
if err != nil {
return err
}

helper := resource.NewHelper(info.Client, info.Mapping)
currentObj, err := helper.Get(info.Namespace, info.Name)
if err != nil {
if !apierrors.IsNotFound(err) {
return err
}
return nil
}

var result *manifest.MappingResult
var oldRelease string
if d.includeTests {
result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace)
} else {
result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
}

if err != nil {
return err
}

newRelease := d.namespace + "/" + d.release
if oldRelease == newRelease {
return nil
}

newOwnedReleases[result.Name] = diff.OwnershipDiff{
OldRelease: oldRelease,
NewRelease: newRelease,
}
currentSpecs[result.Name] = result

return nil
})
return newOwnedReleases, err
}
14 changes: 14 additions & 0 deletions diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,26 @@ type Options struct {
SuppressedOutputLineRegex []string
}

type OwnershipDiff struct {
OldRelease string
NewRelease string
}

// Manifests diff on manifests
func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, options *Options, to io.Writer) bool {
return ManifestsOwnership(oldIndex, newIndex, nil, options, to)
}

func ManifestsOwnership(oldIndex, newIndex map[string]*manifest.MappingResult, newOwnedReleases map[string]OwnershipDiff, options *Options, to io.Writer) bool {
report := Report{}
report.setupReportFormat(options.OutputFormat)
var possiblyRemoved []string

for name, diff := range newOwnedReleases {
diff := diffStrings(diff.OldRelease, diff.NewRelease, true)
report.addEntry(name, options.SuppressedKinds, "", 0, diff, "OWNERSHIP")
}

for _, key := range sortedKeys(oldIndex) {
oldContent := oldIndex[key]

Expand Down
86 changes: 84 additions & 2 deletions diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ annotations:
}

require.Equal(t, `default, nginx, Deployment (apps) to be changed.
Plan: 0 to add, 1 to change, 0 to destroy.
Plan: 0 to add, 1 to change, 0 to destroy, 0 to change ownership.
`, buf1.String())
})

Expand All @@ -503,7 +503,7 @@ Plan: 0 to add, 1 to change, 0 to destroy.
t.Error("Unexpected return value from Manifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`")
}

require.Equal(t, "Plan: 0 to add, 0 to change, 0 to destroy.\n", buf2.String())
require.Equal(t, "Plan: 0 to add, 0 to change, 0 to destroy, 0 to change ownership.\n", buf2.String())
})

t.Run("OnChangeTemplate", func(t *testing.T) {
Expand Down Expand Up @@ -768,3 +768,85 @@ func TestDoSuppress(t *testing.T) {
})
}
}

func TestChangeOwnership(t *testing.T) {
ansi.DisableColors(true)

specOriginal := map[string]*manifest.MappingResult{
"default, foobar, ConfigMap (v1)": {
Name: "default, foobar, ConfigMap (v1)",
Kind: "Secret",
Content: `
apiVersion: v1
kind: ConfigMap
metadata:
name: foobar
data:
key1: value1
`,
}}

t.Run("OnChangeOwnershipWithoutSpecChange", func(t *testing.T) {
var buf1 bytes.Buffer
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false

newOwnedReleases := map[string]OwnershipDiff{
"default, foobar, ConfigMap (v1)": {
OldRelease: "default/oldfoobar",
NewRelease: "default/foobar",
},
}
if changesSeen := ManifestsOwnership(specOriginal, specOriginal, newOwnedReleases, &diffOptions, &buf1); !changesSeen {
t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`")
}

require.Equal(t, `default, foobar, ConfigMap (v1) changed ownership:
- default/oldfoobar
+ default/foobar
`, buf1.String())
})

t.Run("OnChangeOwnershipWithSpecChange", func(t *testing.T) {
var buf1 bytes.Buffer
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false

specNew := map[string]*manifest.MappingResult{
"default, foobar, ConfigMap (v1)": {
Name: "default, foobar, ConfigMap (v1)",
Kind: "Secret",
Content: `
apiVersion: v1
kind: ConfigMap
metadata:
name: foobar
data:
key1: newValue1
`,
}}

newOwnedReleases := map[string]OwnershipDiff{
"default, foobar, ConfigMap (v1)": {
OldRelease: "default/oldfoobar",
NewRelease: "default/foobar",
},
}
if changesSeen := ManifestsOwnership(specOriginal, specNew, newOwnedReleases, &diffOptions, &buf1); !changesSeen {
t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`")
}

require.Equal(t, `default, foobar, ConfigMap (v1) changed ownership:
- default/oldfoobar
+ default/foobar
default, foobar, ConfigMap (v1) has changed:

apiVersion: v1
kind: ConfigMap
metadata:
name: foobar
data:
- key1: value1
+ key1: newValue1

`, buf1.String())
})
}
13 changes: 9 additions & 4 deletions diff/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ func setupDiffReport(r *Report) {
r.format.changestyles["ADD"] = ChangeStyle{color: "green", message: "has been added:"}
r.format.changestyles["REMOVE"] = ChangeStyle{color: "red", message: "has been removed:"}
r.format.changestyles["MODIFY"] = ChangeStyle{color: "yellow", message: "has changed:"}
r.format.changestyles["OWNERSHIP"] = ChangeStyle{color: "magenta", message: "changed ownership:"}
}

// print report for default output: diff
Expand All @@ -160,14 +161,16 @@ func setupSimpleReport(r *Report) {
r.format.changestyles["ADD"] = ChangeStyle{color: "green", message: "to be added."}
r.format.changestyles["REMOVE"] = ChangeStyle{color: "red", message: "to be removed."}
r.format.changestyles["MODIFY"] = ChangeStyle{color: "yellow", message: "to be changed."}
r.format.changestyles["OWNERSHIP"] = ChangeStyle{color: "magenta", message: "to change ownership."}
}

// print report for simple output
func printSimpleReport(r *Report, to io.Writer) {
var summary = map[string]int{
"ADD": 0,
"REMOVE": 0,
"MODIFY": 0,
"ADD": 0,
"REMOVE": 0,
"MODIFY": 0,
"OWNERSHIP": 0,
}
for _, entry := range r.entries {
_, _ = fmt.Fprintf(to, ansi.Color("%s %s", r.format.changestyles[entry.changeType].color)+"\n",
Expand All @@ -176,7 +179,7 @@ func printSimpleReport(r *Report, to io.Writer) {
)
summary[entry.changeType]++
}
_, _ = fmt.Fprintf(to, "Plan: %d to add, %d to change, %d to destroy.\n", summary["ADD"], summary["MODIFY"], summary["REMOVE"])
_, _ = fmt.Fprintf(to, "Plan: %d to add, %d to change, %d to destroy, %d to change ownership.\n", summary["ADD"], summary["MODIFY"], summary["REMOVE"], summary["OWNERSHIP"])
}

func newTemplate(name string) *template.Template {
Expand All @@ -202,6 +205,7 @@ func setupJSONReport(r *Report) {
r.format.changestyles["ADD"] = ChangeStyle{color: "green", message: ""}
r.format.changestyles["REMOVE"] = ChangeStyle{color: "red", message: ""}
r.format.changestyles["MODIFY"] = ChangeStyle{color: "yellow", message: ""}
r.format.changestyles["OWNERSHIP"] = ChangeStyle{color: "magenta", message: ""}
}

// setup report for template output
Expand Down Expand Up @@ -232,6 +236,7 @@ func setupTemplateReport(r *Report) {
r.format.changestyles["ADD"] = ChangeStyle{color: "green", message: ""}
r.format.changestyles["REMOVE"] = ChangeStyle{color: "red", message: ""}
r.format.changestyles["MODIFY"] = ChangeStyle{color: "yellow", message: ""}
r.format.changestyles["OWNERSHIP"] = ChangeStyle{color: "magenta", message: ""}
}

// report with template output will only have access to ReportTemplateSpec.
Expand Down
30 changes: 0 additions & 30 deletions manifest/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,33 +212,3 @@ func existingResourceConflict(resources kube.ResourceList) (kube.ResourceList, e

return requireUpdate, err
}

func deleteStatusAndTidyMetadata(obj []byte) (map[string]interface{}, error) {
var objectMap map[string]interface{}
err := jsoniter.Unmarshal(obj, &objectMap)
if err != nil {
return nil, fmt.Errorf("could not unmarshal byte sequence: %w", err)
}

delete(objectMap, "status")

metadata := objectMap["metadata"].(map[string]interface{})

delete(metadata, "managedFields")
delete(metadata, "generation")

// See the below for the goal of this metadata tidy logic.
// https://github.com/databus23/helm-diff/issues/326#issuecomment-1008253274
if a := metadata["annotations"]; a != nil {
annotations := a.(map[string]interface{})
delete(annotations, "meta.helm.sh/release-name")
delete(annotations, "meta.helm.sh/release-namespace")
delete(annotations, "deployment.kubernetes.io/revision")

if len(annotations) == 0 {
delete(metadata, "annotations")
}
}

return objectMap, nil
}
Loading
Loading