From 5461f5c2903155ebd65504a8d39151e1c2e7f6ce Mon Sep 17 00:00:00 2001 From: Guillaume Perrin Date: Wed, 19 Feb 2025 15:09:04 +0100 Subject: [PATCH 1/5] Add support for --take-ownership parameter Fix #731 Signed-off-by: Guillaume Perrin --- cmd/helm3.go | 4 +++ cmd/upgrade.go | 79 +++++++++++++++++++++++++++++++++++++++++++- diff/diff.go | 14 ++++++++ diff/report.go | 13 +++++--- manifest/generate.go | 30 ----------------- manifest/parse.go | 39 ++++++++++++++++++++++ manifest/util.go | 40 ++++++++++++++++++++++ 7 files changed, 184 insertions(+), 35 deletions(-) create mode 100644 manifest/util.go diff --git a/cmd/helm3.go b/cmd/helm3.go index 4ff147c5..3990b60e 100644 --- a/cmd/helm3.go +++ b/cmd/helm3.go @@ -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 diff --git a/cmd/upgrade.go b/cmd/upgrade.go index d7b26eb2..d1a535ec 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -1,6 +1,7 @@ package cmd import ( + "bytes" "errors" "fmt" "log" @@ -12,6 +13,8 @@ import ( "github.com/spf13/cobra" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/cli" + 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" @@ -54,6 +57,7 @@ type diffCmd struct { insecureSkipTLSVerify bool install bool normalizeManifests bool + takeOwnership bool threeWayMerge bool extraAPIs []string kubeVersion string @@ -248,6 +252,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) @@ -263,6 +268,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) } @@ -316,13 +327,23 @@ 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 { + newOwnedReleases, err = checkOwnership(d, installManifest, 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{ @@ -333,3 +354,59 @@ func (d *diffCmd) runHelm3() error { return nil } + +func checkOwnership(d *diffCmd, installManifest []byte, currentSpecs map[string]*manifest.MappingResult) (map[string]diff.OwnershipDiff, error) { + 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 nil, err + } + resources, err := actionConfig.KubeClient.Build(bytes.NewBuffer(installManifest), false) + if err != nil { + return nil, err + } + + 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, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook) + } 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 +} diff --git a/diff/diff.go b/diff/diff.go index aa2cd9b9..fcb16752 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -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] diff --git a/diff/report.go b/diff/report.go index 1638af46..256a5c7d 100644 --- a/diff/report.go +++ b/diff/report.go @@ -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 @@ -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", @@ -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 { @@ -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 @@ -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. diff --git a/manifest/generate.go b/manifest/generate.go index 2b1152e0..f59bf10a 100644 --- a/manifest/generate.go +++ b/manifest/generate.go @@ -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 -} diff --git a/manifest/parse.go b/manifest/parse.go index 1a404998..3c981a24 100644 --- a/manifest/parse.go +++ b/manifest/parse.go @@ -7,7 +7,9 @@ import ( "log" "strings" + jsoniter "github.com/json-iterator/go" "gopkg.in/yaml.v2" + "k8s.io/apimachinery/pkg/runtime" ) const ( @@ -103,6 +105,43 @@ func Parse(manifest string, defaultNamespace string, normalizeManifests bool, ex return result } +func ParseObject(object runtime.Object, defaultNamespace string, excludedHooks ...string) (*MappingResult, string, error) { + json, _ := jsoniter.ConfigCompatibleWithStandardLibrary.Marshal(object) + var objectMap map[string]interface{} + err := jsoniter.Unmarshal(json, &objectMap) + if err != nil { + return nil, "", fmt.Errorf("could not unmarshal byte sequence: %w", err) + } + + metadata := objectMap["metadata"].(map[string]interface{}) + var oldRelease string + if a := metadata["annotations"]; a != nil { + annotations := a.(map[string]interface{}) + oldRelease = annotations["meta.helm.sh/release-namespace"].(string) + "/" + annotations["meta.helm.sh/release-name"].(string) + } + + // Clean namespace metadata as it exists in Kubernetes but not in Helm manifest + purgedObj, _ := deleteStatusAndTidyMetadata(json) + + content, err := yaml.Marshal(purgedObj) + if err != nil { + return nil, "", err + } + + result, err := parseContent(string(content), defaultNamespace, true, excludedHooks...) + if err != nil { + return nil, "", err + } + + if len(result) != 1 { + return nil, "", fmt.Errorf("Failed to parse content of Kubernetes resource %s", metadata["name"]) + } + + result[0].Content = strings.TrimSuffix(result[0].Content, "\n") + + return result[0], oldRelease, nil +} + func parseContent(content string, defaultNamespace string, normalizeManifests bool, excludedHooks ...string) ([]*MappingResult, error) { var parsedMetadata metadata if err := yaml.Unmarshal([]byte(content), &parsedMetadata); err != nil { diff --git a/manifest/util.go b/manifest/util.go new file mode 100644 index 00000000..88b818af --- /dev/null +++ b/manifest/util.go @@ -0,0 +1,40 @@ +package manifest + +import ( + "fmt" + + jsoniter "github.com/json-iterator/go" +) + +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") + delete(metadata, "creationTimestamp") + delete(metadata, "resourceVersion") + delete(metadata, "uid") + + // 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 +} From af4e5e176b4260439637ad5b4cea193ee0157394 Mon Sep 17 00:00:00 2001 From: Guillaume Perrin Date: Thu, 20 Feb 2025 09:32:44 +0100 Subject: [PATCH 2/5] Fix tests Signed-off-by: Guillaume Perrin --- diff/diff_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diff/diff_test.go b/diff/diff_test.go index d65d5cbc..4a0d6859 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -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()) }) @@ -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) { From acbb2449fbc43d75e3b5b34349fca7b01e4890fc Mon Sep 17 00:00:00 2001 From: Guillaume Perrin Date: Mon, 24 Feb 2025 16:39:57 +0100 Subject: [PATCH 3/5] Add tests Signed-off-by: Guillaume Perrin --- cmd/upgrade.go | 33 ++++---- diff/diff_test.go | 82 +++++++++++++++++++ manifest/parse.go | 9 +- manifest/parse_test.go | 42 ++++++++++ .../testdata/pod_no_release_annotations.yaml | 15 ++++ .../testdata/pod_release_annotations.yaml | 16 ++++ 6 files changed, 177 insertions(+), 20 deletions(-) create mode 100644 manifest/testdata/pod_no_release_annotations.yaml create mode 100644 manifest/testdata/pod_release_annotations.yaml diff --git a/cmd/upgrade.go b/cmd/upgrade.go index d1a535ec..2e6f4491 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "helm.sh/helm/v3/pkg/kube" "log" "os" "slices" @@ -298,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) @@ -330,7 +335,11 @@ func (d *diffCmd) runHelm3() error { var newOwnedReleases map[string]diff.OwnershipDiff if d.takeOwnership { - newOwnedReleases, err = checkOwnership(d, installManifest, currentSpecs) + 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 } @@ -355,21 +364,9 @@ func (d *diffCmd) runHelm3() error { return nil } -func checkOwnership(d *diffCmd, installManifest []byte, currentSpecs map[string]*manifest.MappingResult) (map[string]diff.OwnershipDiff, error) { - 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 nil, err - } - resources, err := actionConfig.KubeClient.Build(bytes.NewBuffer(installManifest), false) - if err != nil { - return nil, err - } - +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 { + err := resources.Visit(func(info *resource.Info, err error) error { if err != nil { return err } @@ -386,7 +383,7 @@ func checkOwnership(d *diffCmd, installManifest []byte, currentSpecs map[string] var result *manifest.MappingResult var oldRelease string if d.includeTests { - result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook) + result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace) } else { result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook) } diff --git a/diff/diff_test.go b/diff/diff_test.go index 4a0d6859..469a4753 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -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()) + }) +} diff --git a/manifest/parse.go b/manifest/parse.go index 3c981a24..aae6c71d 100644 --- a/manifest/parse.go +++ b/manifest/parse.go @@ -117,7 +117,12 @@ func ParseObject(object runtime.Object, defaultNamespace string, excludedHooks . var oldRelease string if a := metadata["annotations"]; a != nil { annotations := a.(map[string]interface{}) - oldRelease = annotations["meta.helm.sh/release-namespace"].(string) + "/" + annotations["meta.helm.sh/release-name"].(string) + if releaseNs, ok := annotations["meta.helm.sh/release-namespace"].(string); ok { + oldRelease += releaseNs + "/" + } + if releaseName, ok := annotations["meta.helm.sh/release-name"].(string); ok { + oldRelease += releaseName + } } // Clean namespace metadata as it exists in Kubernetes but not in Helm manifest @@ -134,7 +139,7 @@ func ParseObject(object runtime.Object, defaultNamespace string, excludedHooks . } if len(result) != 1 { - return nil, "", fmt.Errorf("Failed to parse content of Kubernetes resource %s", metadata["name"]) + return nil, "", fmt.Errorf("failed to parse content of Kubernetes resource %s", metadata["name"]) } result[0].Content = strings.TrimSuffix(result[0].Content, "\n") diff --git a/manifest/parse_test.go b/manifest/parse_test.go index 9b0be8b3..8e118616 100644 --- a/manifest/parse_test.go +++ b/manifest/parse_test.go @@ -1,6 +1,8 @@ package manifest_test import ( + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/serializer/yaml" "os" "sort" "testing" @@ -137,3 +139,43 @@ func TestBaseNameAnnotation(t *testing.T) { foundObjects(Parse(string(spec), "default", false)), ) } + +func TestParseObject(t *testing.T) { + for _, tt := range []struct { + name string + filename string + releaseName string + kind string + oldRelease string + }{ + { + name: "no release info", + filename: "testdata/pod_no_release_annotations.yaml", + releaseName: "testNS, nginx, Pod (v1)", + kind: "Pod", + oldRelease: "", + }, + { + name: "get old release info", + filename: "testdata/pod_release_annotations.yaml", + releaseName: "testNS, nginx, Pod (v1)", + kind: "Pod", + oldRelease: "oldNS/oldReleaseName", + }, + } { + t.Run(tt.name, func(t *testing.T) { + spec, err := os.ReadFile(tt.filename) + require.NoError(t, err) + + obj, _, err := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme).Decode(spec, nil, nil) + require.NoError(t, err) + + release, oldRelease, err := ParseObject(obj, "testNS") + require.NoError(t, err) + + require.Equal(t, tt.releaseName, release.Name) + require.Equal(t, tt.kind, release.Kind) + require.Equal(t, tt.oldRelease, oldRelease) + }) + } +} diff --git a/manifest/testdata/pod_no_release_annotations.yaml b/manifest/testdata/pod_no_release_annotations.yaml new file mode 100644 index 00000000..a2fc1082 --- /dev/null +++ b/manifest/testdata/pod_no_release_annotations.yaml @@ -0,0 +1,15 @@ + +--- +# Source: nginx/pod.yaml +apiVersion: v1 +kind: Pod +metadata: + name: nginx + annotations: + some: "annotation" +spec: + containers: + - name: nginx + image: nginx:1.7.9 + ports: + - containerPort: 80 diff --git a/manifest/testdata/pod_release_annotations.yaml b/manifest/testdata/pod_release_annotations.yaml new file mode 100644 index 00000000..3354a97c --- /dev/null +++ b/manifest/testdata/pod_release_annotations.yaml @@ -0,0 +1,16 @@ + +--- +# Source: nginx/pod.yaml +apiVersion: v1 +kind: Pod +metadata: + name: nginx + annotations: + meta.helm.sh/release-namespace: "oldNS" + meta.helm.sh/release-name: "oldReleaseName" +spec: + containers: + - name: nginx + image: nginx:1.7.9 + ports: + - containerPort: 80 From cef5924df53c0d990b4d58c8373cde08e0209528 Mon Sep 17 00:00:00 2001 From: Guillaume Perrin Date: Tue, 25 Feb 2025 08:49:09 +0100 Subject: [PATCH 4/5] fix lint Signed-off-by: Guillaume Perrin --- cmd/upgrade.go | 2 +- manifest/parse_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 2e6f4491..74533679 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "helm.sh/helm/v3/pkg/kube" "log" "os" "slices" @@ -14,6 +13,7 @@ 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" diff --git a/manifest/parse_test.go b/manifest/parse_test.go index 8e118616..eef3ea5f 100644 --- a/manifest/parse_test.go +++ b/manifest/parse_test.go @@ -1,13 +1,13 @@ package manifest_test import ( - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/serializer/yaml" "os" "sort" "testing" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/serializer/yaml" . "github.com/databus23/helm-diff/v3/manifest" ) From a83a4286d557e07bc0d4cdd0d416ae45b03e6edc Mon Sep 17 00:00:00 2001 From: Guillaume Perrin Date: Mon, 3 Mar 2025 11:29:37 +0100 Subject: [PATCH 5/5] Add test for util Signed-off-by: Guillaume Perrin --- manifest/util_test.go | 97 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 manifest/util_test.go diff --git a/manifest/util_test.go b/manifest/util_test.go new file mode 100644 index 00000000..d2db82c9 --- /dev/null +++ b/manifest/util_test.go @@ -0,0 +1,97 @@ +package manifest + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_deleteStatusAndTidyMetadata(t *testing.T) { + tests := []struct { + name string + obj []byte + want map[string]interface{} + wantErr bool + }{ + { + name: "not valid json", + obj: []byte("notvalid"), + want: nil, + wantErr: true, + }, + { + name: "valid json", + obj: []byte(` +{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "annotations": { + "deployment.kubernetes.io/revision": "1", + "meta.helm.sh/release-name": "test-release", + "meta.helm.sh/release-namespace": "test-ns", + "other-annot": "value" + }, + "creationTimestamp": "2025-03-03T10:07:50Z", + "generation": 1, + "name": "nginx-deployment", + "namespace": "test-ns", + "resourceVersion": "33648", + "uid": "7a8d3b74-6452-46f4-a31f-4fdacbe828ac" + }, + "spec": { + "template": { + "spec": { + "containers": [ + { + "image": "nginx:1.14.2", + "imagePullPolicy": "IfNotPresent", + "name": "nginx" + } + ] + } + } + }, + "status": { + "availableReplicas": 2 + } +} +`), + want: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "other-annot": "value", + }, + "name": "nginx-deployment", + "namespace": "test-ns", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx:1.14.2", + "imagePullPolicy": "IfNotPresent", + "name": "nginx", + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := deleteStatusAndTidyMetadata(tt.obj) + if (err != nil) != tt.wantErr { + t.Errorf("deleteStatusAndTidyMetadata() error = %v, wantErr %v", err, tt.wantErr) + return + } + require.EqualValuesf(t, tt.want, got, "deleteStatusAndTidyMetadata() = %v, want %v", got, tt.want) + }) + } +}