From 374d790a213253a510b50b48cfa0fd0904b08723 Mon Sep 17 00:00:00 2001
From: Mengqi Yu <mengqiy@google.com>
Date: Mon, 8 Nov 2021 21:35:49 -0800
Subject: [PATCH] Make ResourceList follow k8s api conventions

Make optional fields as pointers.
Add omitempty for optional fields.
---
 kyaml/fn/framework/command/example_test.go |  4 +-
 kyaml/fn/framework/framework.go            |  4 +-
 kyaml/fn/framework/framework_test.go       |  8 ++--
 kyaml/fn/framework/result.go               | 44 +++++++++++++---------
 4 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/kyaml/fn/framework/command/example_test.go b/kyaml/fn/framework/command/example_test.go
index 435b2559f..46d5c2c49 100644
--- a/kyaml/fn/framework/command/example_test.go
+++ b/kyaml/fn/framework/command/example_test.go
@@ -322,11 +322,11 @@ func ExampleBuild_validate() {
 			validationResults = append(validationResults, &framework.Result{
 				Severity: framework.Error,
 				Message:  "field is required",
-				ResourceRef: yaml.ResourceIdentifier{
+				ResourceRef: &yaml.ResourceIdentifier{
 					TypeMeta: meta.TypeMeta,
 					NameMeta: meta.ObjectMeta.NameMeta,
 				},
-				Field: framework.Field{
+				Field: &framework.Field{
 					Path:          "spec.replicas",
 					ProposedValue: "1",
 				},
diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go
index 7e8555e05..b1ddbc331 100644
--- a/kyaml/fn/framework/framework.go
+++ b/kyaml/fn/framework/framework.go
@@ -47,12 +47,12 @@ type ResourceList struct {
 	//      kind: Example
 	//      spec:
 	//        foo: var
-	FunctionConfig *yaml.RNode `yaml:"functionConfig" json:"functionConfig"`
+	FunctionConfig *yaml.RNode `yaml:"functionConfig,omitempty" json:"functionConfig,omitempty"`
 
 	// Results is ResourceList.results output value.
 	// Validating functions can optionally use this field to communicate structured
 	// validation error data to downstream functions.
-	Results Results `yaml:"results" json:"results"`
+	Results Results `yaml:"results,omitempty" json:"results,omitempty"`
 }
 
 // ResourceListProcessor is implemented by configuration functions built with this framework
diff --git a/kyaml/fn/framework/framework_test.go b/kyaml/fn/framework/framework_test.go
index 400f1ba7f..6e61345d5 100644
--- a/kyaml/fn/framework/framework_test.go
+++ b/kyaml/fn/framework/framework_test.go
@@ -20,16 +20,16 @@ func TestExecute_Result(t *testing.T) {
 			{
 				Message:  "bad value for replicas",
 				Severity: framework.Error,
-				ResourceRef: yaml.ResourceIdentifier{
+				ResourceRef: &yaml.ResourceIdentifier{
 					TypeMeta: yaml.TypeMeta{APIVersion: "v1", Kind: "Deployment"},
 					NameMeta: yaml.NameMeta{Name: "tester", Namespace: "default"},
 				},
-				Field: framework.Field{
+				Field: &framework.Field{
 					Path:          ".spec.Replicas",
 					CurrentValue:  "0",
 					ProposedValue: "3",
 				},
-				File: framework.File{
+				File: &framework.File{
 					Path:  "/path/to/deployment.yaml",
 					Index: 0,
 				},
@@ -59,7 +59,7 @@ items:
 	err := framework.Execute(p, source)
 	assert.EqualError(t, err, `[error] v1/Deployment/default/tester .spec.Replicas: bad value for replicas
 
-[error] : some error`)
+[error]: some error`)
 	assert.Equal(t, 1, err.(*framework.Results).ExitCode())
 	assert.Equal(t, `apiVersion: config.kubernetes.io/v1
 kind: ResourceList
diff --git a/kyaml/fn/framework/result.go b/kyaml/fn/framework/result.go
index 90c660641..b0db3717a 100644
--- a/kyaml/fn/framework/result.go
+++ b/kyaml/fn/framework/result.go
@@ -32,13 +32,13 @@ type Result struct {
 
 	// ResourceRef is a reference to a resource.
 	// Required fields: apiVersion, kind, name.
-	ResourceRef yaml.ResourceIdentifier `yaml:"resourceRef,omitempty" json:"resourceRef,omitempty"`
+	ResourceRef *yaml.ResourceIdentifier `yaml:"resourceRef,omitempty" json:"resourceRef,omitempty"`
 
 	// Field is a reference to the field in a resource this result refers to
-	Field Field `yaml:"field,omitempty" json:"field,omitempty"`
+	Field *Field `yaml:"field,omitempty" json:"field,omitempty"`
 
 	// File references a file containing the resource this result refers to
-	File File `yaml:"file,omitempty" json:"file,omitempty"`
+	File *File `yaml:"file,omitempty" json:"file,omitempty"`
 
 	// Tags is an unstructured key value map stored with a result that may be set
 	// by external tools to store and retrieve arbitrary metadata
@@ -49,23 +49,33 @@ type Result struct {
 func (i Result) String() string {
 	identifier := i.ResourceRef
 	var idStringList []string
-	if identifier.APIVersion != "" {
-		idStringList = append(idStringList, identifier.APIVersion)
-	}
-	if identifier.Kind != "" {
-		idStringList = append(idStringList, identifier.Kind)
-	}
-	if identifier.Namespace != "" {
-		idStringList = append(idStringList, identifier.Namespace)
-	}
-	if identifier.Name != "" {
-		idStringList = append(idStringList, identifier.Name)
+	if identifier != nil {
+		if identifier.APIVersion != "" {
+			idStringList = append(idStringList, identifier.APIVersion)
+		}
+		if identifier.Kind != "" {
+			idStringList = append(idStringList, identifier.Kind)
+		}
+		if identifier.Namespace != "" {
+			idStringList = append(idStringList, identifier.Namespace)
+		}
+		if identifier.Name != "" {
+			idStringList = append(idStringList, identifier.Name)
+		}
 	}
+	formatString := "[%s]"
+	list := []interface{}{i.Severity}
 	if len(idStringList) > 0 {
-		idString := strings.Join(idStringList, "/")
-		return fmt.Sprintf("[%s] %s %s: %s", i.Severity, idString, i.Field.Path, i.Message)
+		formatString += " %s"
+		list = append(list, strings.Join(idStringList, "/"))
 	}
-	return fmt.Sprintf("[%s] %s: %s", i.Severity, i.Field.Path, i.Message)
+	if i.Field != nil {
+		formatString += " %s"
+		list = append(list, i.Field.Path)
+	}
+	formatString += ": %s"
+	list = append(list, i.Message)
+	return fmt.Sprintf(formatString, list...)
 }
 
 // File references a file containing a resource