From 4628705494f15df1c652637839a4d170faa3609d Mon Sep 17 00:00:00 2001
From: Phillip Wittrock <pwittroc@google.com>
Date: Tue, 7 Jan 2020 12:53:23 -0800
Subject: [PATCH] Parse metadata directly instead of struct serialization hack

---
 kyaml/yaml/types.go      | 89 ++++++++++++++++++++++++++++++++++------
 kyaml/yaml/types_test.go | 49 ++++++++++++++++++++++
 2 files changed, 125 insertions(+), 13 deletions(-)
 create mode 100644 kyaml/yaml/types_test.go

diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go
index 99d3eb668..af9f11692 100644
--- a/kyaml/yaml/types.go
+++ b/kyaml/yaml/types.go
@@ -6,7 +6,6 @@ package yaml
 import (
 	"bytes"
 	"fmt"
-	"reflect"
 	"strings"
 
 	"gopkg.in/yaml.v3"
@@ -70,6 +69,14 @@ func IsFieldEmpty(node *MapNode) bool {
 	return false
 }
 
+// GetValue returns underlying yaml.Node Value field
+func GetValue(node *RNode) string {
+	if IsEmpty(node) {
+		return ""
+	}
+	return node.YNode().Value
+}
+
 func IsFieldNull(node *MapNode) bool {
 	return node != nil && node.Value != nil && node.Value.YNode() != nil &&
 		node.Value.YNode().Tag == NullNodeTag
@@ -280,23 +287,79 @@ func (r *ResourceIdentifier) GetKind() string {
 
 var ErrMissingMetadata = fmt.Errorf("missing Resource metadata")
 
-// GetMeta returns the ResourceMeta for a RNode
+// Field names
+const (
+	AnnotationsField = "annotations"
+	APIVersionField  = "apiVersion"
+	KindField        = "kind"
+	MetadataField    = "metadata"
+	NameField        = "name"
+	NamespaceField   = "namespace"
+	LabelsField      = "labels"
+)
+
+// GetMeta returns the ResourceMeta for an RNode
 func (rn *RNode) GetMeta() (ResourceMeta, error) {
+	if IsEmpty(rn) {
+		return ResourceMeta{}, nil
+	}
+	missingMeta := true
+	n := rn
+	if n.YNode().Kind == DocumentNode {
+		// get the content is this is the document node
+		n = NewRNode(n.Content()[0])
+	}
+
+	// don't decode into the struct directly or it will fail on UTF-8 issues
+	// which appear in comments
 	m := ResourceMeta{}
-	b := &bytes.Buffer{}
-	e := NewEncoder(b)
-	if err := e.Encode(rn.YNode()); err != nil {
-		return m, errors.Wrap(err)
+
+	// TODO: consider optimizing this parsing
+	if f := n.Field(APIVersionField); !IsFieldEmpty(f) {
+		m.APIVersion = GetValue(f.Value)
+		missingMeta = false
 	}
-	if err := e.Close(); err != nil {
-		return m, errors.Wrap(err)
+	if f := n.Field(KindField); !IsFieldEmpty(f) {
+		m.Kind = GetValue(f.Value)
+		missingMeta = false
 	}
-	d := yaml.NewDecoder(b)
-	d.KnownFields(false) // only want to parse the metadata
-	if err := d.Decode(&m); err != nil {
-		return m, errors.Wrap(err)
+
+	mf := n.Field(MetadataField)
+	if IsFieldEmpty(mf) {
+		if missingMeta {
+			return m, ErrMissingMetadata
+		}
+		return m, nil
 	}
-	if reflect.DeepEqual(m, ResourceMeta{}) {
+	meta := mf.Value
+
+	if f := meta.Field(NameField); !IsFieldEmpty(f) {
+		m.Name = f.Value.YNode().Value
+		missingMeta = false
+	}
+	if f := meta.Field(NamespaceField); !IsFieldEmpty(f) {
+		m.Namespace = GetValue(f.Value)
+		missingMeta = false
+	}
+
+	if f := meta.Field(LabelsField); !IsFieldEmpty(f) {
+		m.Labels = map[string]string{}
+		_ = f.Value.VisitFields(func(node *MapNode) error {
+			m.Labels[GetValue(node.Key)] = GetValue(node.Value)
+			return nil
+		})
+		missingMeta = false
+	}
+	if f := meta.Field(AnnotationsField); !IsFieldEmpty(f) {
+		m.Annotations = map[string]string{}
+		_ = f.Value.VisitFields(func(node *MapNode) error {
+			m.Annotations[GetValue(node.Key)] = GetValue(node.Value)
+			return nil
+		})
+		missingMeta = false
+	}
+
+	if missingMeta {
 		return m, ErrMissingMetadata
 	}
 	return m, nil
diff --git a/kyaml/yaml/types_test.go b/kyaml/yaml/types_test.go
new file mode 100644
index 000000000..3812f4e37
--- /dev/null
+++ b/kyaml/yaml/types_test.go
@@ -0,0 +1,49 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package yaml
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// Test that non-UTF8 characters in comments don't cause failures
+func TestRNode_GetMeta_UTF16(t *testing.T) {
+	sr, err := Parse(`apiVersion: rbac.istio.io/v1alpha1
+kind: ServiceRole
+metadata:
+  name: wildcard
+  namespace: default
+  # If set to [“*”], it refers to all services in the namespace
+  annotations:
+    foo: bar
+spec:
+  rules:
+    # There is one service in default namespace, should not result in a validation error
+    # If set to [“*”], it refers to all services in the namespace
+    - services: ["*"]
+      methods: ["GET", "HEAD"]
+`)
+	if !assert.NoError(t, err) {
+		t.FailNow()
+	}
+	actual, err := sr.GetMeta()
+	if !assert.NoError(t, err) {
+		t.FailNow()
+	}
+
+	expected := ResourceMeta{
+		APIVersion: "rbac.istio.io/v1alpha1",
+		Kind:       "ServiceRole",
+		ObjectMeta: ObjectMeta{
+			Name:        "wildcard",
+			Namespace:   "default",
+			Annotations: map[string]string{"foo": "bar"},
+		},
+	}
+	if !assert.Equal(t, expected, actual) {
+		t.FailNow()
+	}
+}