fix(render): manifests keys sort order not preserved after rendering

Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
werf · Apr 26, 2022 · 469ce7a · 469ce7a
1 parent c5c0e9f
commit 469ce7a
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 25 deletions.
diff --git 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
@@ -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,49 +107,54 @@ 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())
 		}
 
 		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))