From 469ce7af96b3442f29ad4106d056c75d7bb6b312 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Mon, 18 Apr 2022 12:09:06 +0300 Subject: [PATCH] fix(render): manifests keys sort order not preserved after rendering Signed-off-by: Timofey Kirillov --- integration/suites/helm/render/extra_test.go | 53 ++++++++++++-- ...ra_annotations_and_labels_post_renderer.go | 73 ++++++++++++++----- 2 files changed, 101 insertions(+), 25 deletions(-) diff --git a/integration/suites/helm/render/extra_test.go b/integration/suites/helm/render/extra_test.go index 2653758595..4dc6b2543e 100644 --- a/integration/suites/helm/render/extra_test.go +++ b/integration/suites/helm/render/extra_test.go @@ -1,10 +1,11 @@ package render_test import ( - "strings" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/releaseutil" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/yaml" "github.com/werf/werf/test/pkg/utils" ) @@ -21,9 +22,27 @@ var _ = Describe("helm render with extra annotations and labels", func() { "render", "--add-annotation=anno1=value1", "--add-annotation=anno2=value2", ) - Ω(strings.Count(output, `annotations: - anno1: value1 - anno2: value2`)).Should(Equal(3)) + splitManifests := releaseutil.SplitManifests(output) + for _, content := range splitManifests { + var obj unstructured.Unstructured + Expect(yaml.Unmarshal([]byte(content), &obj)).To(Succeed()) + + annotations := obj.GetAnnotations() + labels := obj.GetLabels() + + // Hooks not supported yet by the helm + if _, isHook := annotations["helm.sh/hook"]; isHook { + continue + } + + Expect(annotations["anno1"]).To(Equal("value1")) + Expect(annotations["anno2"]).To(Equal("value2")) + + _, exists := labels["anno1"] + Expect(exists).To(BeFalse()) + _, exists = labels["anno2"] + Expect(exists).To(BeFalse()) + } }) It("should be rendered with extra labels (except hooks)", func() { @@ -33,8 +52,26 @@ var _ = Describe("helm render with extra annotations and labels", func() { "render", "--add-label=label1=value1", "--add-label=label2=value2", ) - Ω(strings.Count(output, `labels: - label1: value1 - label2: value2`)).Should(Equal(3)) + splitManifests := releaseutil.SplitManifests(output) + for _, content := range splitManifests { + var obj unstructured.Unstructured + Expect(yaml.Unmarshal([]byte(content), &obj)).To(Succeed()) + + annotations := obj.GetAnnotations() + labels := obj.GetLabels() + + // Hooks not supported yet by the helm + if _, isHook := annotations["helm.sh/hook"]; isHook { + continue + } + + Expect(labels["label1"]).To(Equal("value1")) + Expect(labels["label2"]).To(Equal("value2")) + + _, exists := annotations["label1"] + Expect(exists).To(BeFalse()) + _, exists = annotations["label2"] + Expect(exists).To(BeFalse()) + } }) }) diff --git a/pkg/deploy/helm/extra_annotations_and_labels_post_renderer.go b/pkg/deploy/helm/extra_annotations_and_labels_post_renderer.go index f0bdd9c13f..32e4bdd6c7 100644 --- a/pkg/deploy/helm/extra_annotations_and_labels_post_renderer.go +++ b/pkg/deploy/helm/extra_annotations_and_labels_post_renderer.go @@ -8,6 +8,7 @@ import ( "sort" "strings" + orig_yaml "gopkg.in/yaml.v2" "helm.sh/helm/v3/pkg/releaseutil" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/yaml" @@ -34,6 +35,39 @@ type ExtraAnnotationsAndLabelsPostRenderer struct { ExtraLabels map[string]string } +func findMapByKey(mapSlice orig_yaml.MapSlice, key string) orig_yaml.MapSlice { + for _, item := range mapSlice { + if itemKey, ok := item.Key.(string); ok { + if itemKey == key { + if itemValue, ok := item.Value.(orig_yaml.MapSlice); ok { + return itemValue + } + } + } + } + + return nil +} + +func setMapValueByKey(mapSlice orig_yaml.MapSlice, key string, value interface{}) (res orig_yaml.MapSlice) { + var found bool + for _, item := range mapSlice { + if itemKey, ok := item.Key.(string); ok { + if itemKey == key { + res = append(res, orig_yaml.MapItem{Key: key, Value: value}) + found = true + continue + } + } + res = append(res, item) + } + + if !found { + res = append(res, orig_yaml.MapItem{Key: key, Value: value}) + } + return +} + func (pr *ExtraAnnotationsAndLabelsPostRenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, error) { extraAnnotations := map[string]string{} for k, v := range WerfRuntimeAnnotations { @@ -73,18 +107,23 @@ func (pr *ExtraAnnotationsAndLabelsPostRenderer) Run(renderedManifests *bytes.Bu } var obj unstructured.Unstructured - if err := yaml.Unmarshal([]byte(manifestContent), &obj); err != nil { logboek.Warn().LogF("Unable to decode yaml manifest as unstructured object: %s: will not add extra annotations and labels to this object:\n%s\n---\n", err, manifestContent) splitModifiedManifests = append(splitModifiedManifests, manifestContent) continue } - if obj.GetKind() == "" { logboek.Debug().LogF("Skipping empty object\n") continue } + var objMapSlice orig_yaml.MapSlice + if err := orig_yaml.Unmarshal([]byte(manifestContent), &objMapSlice); err != nil { + logboek.Warn().LogF("Unable to decode yaml manifest as map slice: %s: will not add extra annotations and labels to this object:\n%s\n---\n", err, manifestContent) + splitModifiedManifests = append(splitModifiedManifests, manifestContent) + continue + } + if os.Getenv("WERF_HELM_V3_EXTRA_ANNOTATIONS_AND_LABELS_DEBUG") == "1" { fmt.Printf("Unpacket obj annotations: %#v\n", obj.GetAnnotations()) } @@ -92,30 +131,30 @@ func (pr *ExtraAnnotationsAndLabelsPostRenderer) Run(renderedManifests *bytes.Bu if obj.IsList() && len(extraAnnotations) > 0 { logboek.Warn().LogF("werf annotations won't be applied to *List resource Kinds, including %s. We advise to replace *List resources with multiple separate resources of the same Kind\n", obj.GetKind()) } else if len(extraAnnotations) > 0 { - annotations := obj.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) + if metadata := findMapByKey(objMapSlice, "metadata"); metadata != nil { + annotations := findMapByKey(metadata, "annotations") + for k, v := range extraAnnotations { + annotations = append(annotations, orig_yaml.MapItem{Key: k, Value: v}) + } + metadata = setMapValueByKey(metadata, "annotations", annotations) + objMapSlice = setMapValueByKey(objMapSlice, "metadata", metadata) } - for k, v := range extraAnnotations { - annotations[k] = v - } - obj.SetAnnotations(annotations) } if obj.IsList() && len(extraLabels) > 0 { logboek.Warn().LogF("werf labels won't be applied to *List resource Kinds, including %s. We advise to replace *List resources with multiple separate resources of the same Kind\n", obj.GetKind()) } else if len(extraLabels) > 0 { - labels := obj.GetLabels() - if labels == nil { - labels = make(map[string]string) - } - for k, v := range extraLabels { - labels[k] = v + if metadata := findMapByKey(objMapSlice, "metadata"); metadata != nil { + labels := findMapByKey(metadata, "labels") + for k, v := range extraLabels { + labels = append(labels, orig_yaml.MapItem{Key: k, Value: v}) + } + metadata = setMapValueByKey(metadata, "labels", labels) + objMapSlice = setMapValueByKey(objMapSlice, "metadata", metadata) } - obj.SetLabels(labels) } - if modifiedManifestContent, err := yaml.Marshal(obj.Object); err != nil { + if modifiedManifestContent, err := orig_yaml.Marshal(objMapSlice); err != nil { return nil, fmt.Errorf("unable to modify manifest: %w\n%s\n---\n", err, manifestContent) } else { splitModifiedManifests = append(splitModifiedManifests, manifestSource+"\n"+string(modifiedManifestContent))