Skip to content

Commit c18675f

Browse files
guillomepguillomep
authored
Add support for --take-ownership parameter (databus23#742)
* Add support for --take-ownership parameter Fix databus23#731 Signed-off-by: Guillaume Perrin <[email protected]> * Fix tests Signed-off-by: Guillaume Perrin <[email protected]> * Add tests Signed-off-by: Guillaume Perrin <[email protected]> * fix lint Signed-off-by: Guillaume Perrin <[email protected]> * Add test for util Signed-off-by: Guillaume Perrin <[email protected]> --------- Signed-off-by: Guillaume Perrin <[email protected]>
1 parent de0391c commit c18675f

File tree

12 files changed

+442
-39
lines changed

12 files changed

+442
-39
lines changed

‎cmd/helm3.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,10 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) {
219219
flags = append(flags, "--skip-schema-validation")
220220
}
221221

222+
if d.takeOwnership {
223+
flags = append(flags, "--take-ownership")
224+
}
225+
222226
var (
223227
subcmd string
224228
filter func([]byte) []byte

‎cmd/upgrade.go

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"bytes"
45
"errors"
56
"fmt"
67
"log"
@@ -12,6 +13,9 @@ import (
1213
"github.com/spf13/cobra"
1314
"helm.sh/helm/v3/pkg/action"
1415
"helm.sh/helm/v3/pkg/cli"
16+
"helm.sh/helm/v3/pkg/kube"
17+
apierrors "k8s.io/apimachinery/pkg/api/errors"
18+
"k8s.io/cli-runtime/pkg/resource"
1519

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

252258
AddDiffOptions(f, &diff.Options)
253259

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

264270
var err error
265271

272+
if d.takeOwnership {
273+
// We need to do a three way merge between the manifests of the new
274+
// release, the manifests of the old release and what is currently deployed
275+
d.threeWayMerge = true
276+
}
277+
266278
if d.clusterAccessAllowed() {
267279
releaseManifest, err = getRelease(d.release, d.namespace)
268280
}
@@ -287,14 +299,18 @@ func (d *diffCmd) runHelm3() error {
287299
return fmt.Errorf("Failed to render chart: %w", err)
288300
}
289301

290-
if d.threeWayMerge {
291-
actionConfig := new(action.Configuration)
302+
var actionConfig *action.Configuration
303+
if d.threeWayMerge || d.takeOwnership {
304+
actionConfig = new(action.Configuration)
292305
if err := actionConfig.Init(envSettings.RESTClientGetter(), envSettings.Namespace(), os.Getenv("HELM_DRIVER"), log.Printf); err != nil {
293306
log.Fatalf("%+v", err)
294307
}
295308
if err := actionConfig.KubeClient.IsReachable(); err != nil {
296309
return err
297310
}
311+
}
312+
313+
if d.threeWayMerge {
298314
releaseManifest, installManifest, err = manifest.Generate(actionConfig, releaseManifest, installManifest)
299315
if err != nil {
300316
return fmt.Errorf("unable to generate manifests: %w", err)
@@ -316,13 +332,27 @@ func (d *diffCmd) runHelm3() error {
316332
currentSpecs = manifest.Parse(string(releaseManifest), d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
317333
}
318334
}
335+
336+
var newOwnedReleases map[string]diff.OwnershipDiff
337+
if d.takeOwnership {
338+
resources, err := actionConfig.KubeClient.Build(bytes.NewBuffer(installManifest), false)
339+
if err != nil {
340+
return err
341+
}
342+
newOwnedReleases, err = checkOwnership(d, resources, currentSpecs)
343+
if err != nil {
344+
return err
345+
}
346+
}
347+
319348
var newSpecs map[string]*manifest.MappingResult
320349
if d.includeTests {
321350
newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests)
322351
} else {
323352
newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
324353
}
325-
seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, &d.Options, os.Stdout)
354+
355+
seenAnyChanges := diff.ManifestsOwnership(currentSpecs, newSpecs, newOwnedReleases, &d.Options, os.Stdout)
326356

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

334364
return nil
335365
}
366+
367+
func checkOwnership(d *diffCmd, resources kube.ResourceList, currentSpecs map[string]*manifest.MappingResult) (map[string]diff.OwnershipDiff, error) {
368+
newOwnedReleases := make(map[string]diff.OwnershipDiff)
369+
err := resources.Visit(func(info *resource.Info, err error) error {
370+
if err != nil {
371+
return err
372+
}
373+
374+
helper := resource.NewHelper(info.Client, info.Mapping)
375+
currentObj, err := helper.Get(info.Namespace, info.Name)
376+
if err != nil {
377+
if !apierrors.IsNotFound(err) {
378+
return err
379+
}
380+
return nil
381+
}
382+
383+
var result *manifest.MappingResult
384+
var oldRelease string
385+
if d.includeTests {
386+
result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace)
387+
} else {
388+
result, oldRelease, err = manifest.ParseObject(currentObj, d.namespace, manifest.Helm3TestHook, manifest.Helm2TestSuccessHook)
389+
}
390+
391+
if err != nil {
392+
return err
393+
}
394+
395+
newRelease := d.namespace + "/" + d.release
396+
if oldRelease == newRelease {
397+
return nil
398+
}
399+
400+
newOwnedReleases[result.Name] = diff.OwnershipDiff{
401+
OldRelease: oldRelease,
402+
NewRelease: newRelease,
403+
}
404+
currentSpecs[result.Name] = result
405+
406+
return nil
407+
})
408+
return newOwnedReleases, err
409+
}

‎diff/diff.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,26 @@ type Options struct {
3030
SuppressedOutputLineRegex []string
3131
}
3232

33+
type OwnershipDiff struct {
34+
OldRelease string
35+
NewRelease string
36+
}
37+
3338
// Manifests diff on manifests
3439
func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, options *Options, to io.Writer) bool {
40+
return ManifestsOwnership(oldIndex, newIndex, nil, options, to)
41+
}
42+
43+
func ManifestsOwnership(oldIndex, newIndex map[string]*manifest.MappingResult, newOwnedReleases map[string]OwnershipDiff, options *Options, to io.Writer) bool {
3544
report := Report{}
3645
report.setupReportFormat(options.OutputFormat)
3746
var possiblyRemoved []string
3847

48+
for name, diff := range newOwnedReleases {
49+
diff := diffStrings(diff.OldRelease, diff.NewRelease, true)
50+
report.addEntry(name, options.SuppressedKinds, "", 0, diff, "OWNERSHIP")
51+
}
52+
3953
for _, key := range sortedKeys(oldIndex) {
4054
oldContent := oldIndex[key]
4155

‎diff/diff_test.go

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ annotations:
492492
}
493493

494494
require.Equal(t, `default, nginx, Deployment (apps) to be changed.
495-
Plan: 0 to add, 1 to change, 0 to destroy.
495+
Plan: 0 to add, 1 to change, 0 to destroy, 0 to change ownership.
496496
`, buf1.String())
497497
})
498498

@@ -503,7 +503,7 @@ Plan: 0 to add, 1 to change, 0 to destroy.
503503
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`")
504504
}
505505

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

509509
t.Run("OnChangeTemplate", func(t *testing.T) {
@@ -768,3 +768,85 @@ func TestDoSuppress(t *testing.T) {
768768
})
769769
}
770770
}
771+
772+
func TestChangeOwnership(t *testing.T) {
773+
ansi.DisableColors(true)
774+
775+
specOriginal := map[string]*manifest.MappingResult{
776+
"default, foobar, ConfigMap (v1)": {
777+
Name: "default, foobar, ConfigMap (v1)",
778+
Kind: "Secret",
779+
Content: `
780+
apiVersion: v1
781+
kind: ConfigMap
782+
metadata:
783+
name: foobar
784+
data:
785+
key1: value1
786+
`,
787+
}}
788+
789+
t.Run("OnChangeOwnershipWithoutSpecChange", func(t *testing.T) {
790+
var buf1 bytes.Buffer
791+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false
792+
793+
newOwnedReleases := map[string]OwnershipDiff{
794+
"default, foobar, ConfigMap (v1)": {
795+
OldRelease: "default/oldfoobar",
796+
NewRelease: "default/foobar",
797+
},
798+
}
799+
if changesSeen := ManifestsOwnership(specOriginal, specOriginal, newOwnedReleases, &diffOptions, &buf1); !changesSeen {
800+
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`")
801+
}
802+
803+
require.Equal(t, `default, foobar, ConfigMap (v1) changed ownership:
804+
- default/oldfoobar
805+
+ default/foobar
806+
`, buf1.String())
807+
})
808+
809+
t.Run("OnChangeOwnershipWithSpecChange", func(t *testing.T) {
810+
var buf1 bytes.Buffer
811+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false
812+
813+
specNew := map[string]*manifest.MappingResult{
814+
"default, foobar, ConfigMap (v1)": {
815+
Name: "default, foobar, ConfigMap (v1)",
816+
Kind: "Secret",
817+
Content: `
818+
apiVersion: v1
819+
kind: ConfigMap
820+
metadata:
821+
name: foobar
822+
data:
823+
key1: newValue1
824+
`,
825+
}}
826+
827+
newOwnedReleases := map[string]OwnershipDiff{
828+
"default, foobar, ConfigMap (v1)": {
829+
OldRelease: "default/oldfoobar",
830+
NewRelease: "default/foobar",
831+
},
832+
}
833+
if changesSeen := ManifestsOwnership(specOriginal, specNew, newOwnedReleases, &diffOptions, &buf1); !changesSeen {
834+
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`")
835+
}
836+
837+
require.Equal(t, `default, foobar, ConfigMap (v1) changed ownership:
838+
- default/oldfoobar
839+
+ default/foobar
840+
default, foobar, ConfigMap (v1) has changed:
841+
842+
apiVersion: v1
843+
kind: ConfigMap
844+
metadata:
845+
name: foobar
846+
data:
847+
- key1: value1
848+
+ key1: newValue1
849+
850+
`, buf1.String())
851+
})
852+
}

‎diff/report.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ func setupDiffReport(r *Report) {
143143
r.format.changestyles["ADD"] = ChangeStyle{color: "green", message: "has been added:"}
144144
r.format.changestyles["REMOVE"] = ChangeStyle{color: "red", message: "has been removed:"}
145145
r.format.changestyles["MODIFY"] = ChangeStyle{color: "yellow", message: "has changed:"}
146+
r.format.changestyles["OWNERSHIP"] = ChangeStyle{color: "magenta", message: "changed ownership:"}
146147
}
147148

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

165167
// print report for simple output
166168
func printSimpleReport(r *Report, to io.Writer) {
167169
var summary = map[string]int{
168-
"ADD": 0,
169-
"REMOVE": 0,
170-
"MODIFY": 0,
170+
"ADD": 0,
171+
"REMOVE": 0,
172+
"MODIFY": 0,
173+
"OWNERSHIP": 0,
171174
}
172175
for _, entry := range r.entries {
173176
_, _ = 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) {
176179
)
177180
summary[entry.changeType]++
178181
}
179-
_, _ = fmt.Fprintf(to, "Plan: %d to add, %d to change, %d to destroy.\n", summary["ADD"], summary["MODIFY"], summary["REMOVE"])
182+
_, _ = 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"])
180183
}
181184

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

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

237242
// report with template output will only have access to ReportTemplateSpec.

‎manifest/generate.go

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -212,33 +212,3 @@ func existingResourceConflict(resources kube.ResourceList) (kube.ResourceList, e
212212

213213
return requireUpdate, err
214214
}
215-
216-
func deleteStatusAndTidyMetadata(obj []byte) (map[string]interface{}, error) {
217-
var objectMap map[string]interface{}
218-
err := jsoniter.Unmarshal(obj, &objectMap)
219-
if err != nil {
220-
return nil, fmt.Errorf("could not unmarshal byte sequence: %w", err)
221-
}
222-
223-
delete(objectMap, "status")
224-
225-
metadata := objectMap["metadata"].(map[string]interface{})
226-
227-
delete(metadata, "managedFields")
228-
delete(metadata, "generation")
229-
230-
// See the below for the goal of this metadata tidy logic.
231-
// https://github.com/databus23/helm-diff/issues/326#issuecomment-1008253274
232-
if a := metadata["annotations"]; a != nil {
233-
annotations := a.(map[string]interface{})
234-
delete(annotations, "meta.helm.sh/release-name")
235-
delete(annotations, "meta.helm.sh/release-namespace")
236-
delete(annotations, "deployment.kubernetes.io/revision")
237-
238-
if len(annotations) == 0 {
239-
delete(metadata, "annotations")
240-
}
241-
}
242-
243-
return objectMap, nil
244-
}

0 commit comments

Comments
 (0)