From c2c3272b7448cffea029526a5272fc0a6f25b0d5 Mon Sep 17 00:00:00 2001
From: Chuang Wang <chuangw@google.com>
Date: Thu, 12 May 2022 14:26:20 -0700
Subject: [PATCH] TEP-0075: Extract out the validation of object keys

Prior to this commit, the validation against the required and
provided keys for object type was part of the validateObjectUsage
function. That makes the purpose of validateObjectUsage confusing.

In this commit, a new helper function is created to validate that
separately.
---
 pkg/apis/pipeline/v1beta1/task_validation.go  | 46 +++++++++++++------
 .../pipeline/v1beta1/task_validation_test.go  |  2 +-
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go
index 771aaeed74c..32ab8bbc242 100644
--- a/pkg/apis/pipeline/v1beta1/task_validation.go
+++ b/pkg/apis/pipeline/v1beta1/task_validation.go
@@ -338,6 +338,7 @@ func ValidateParameterVariables(ctx context.Context, steps []Step, params []Para
 
 	errs = errs.Also(validateVariables(ctx, steps, "params", parameterNames))
 	errs = errs.Also(validateArrayUsage(steps, "params", arrayParameterNames))
+	errs = errs.Also(validateObjectDefault(objectParamSpecs))
 	return errs.Also(validateObjectUsage(ctx, steps, objectParamSpecs))
 }
 
@@ -391,10 +392,6 @@ func validateObjectUsage(ctx context.Context, steps []Step, params []ParamSpec)
 			objectKeys.Insert(key)
 		}
 
-		if p.Default != nil && p.Default.ObjectVal != nil {
-			errs = errs.Also(validateObjectKeysInDefault(p.Default.ObjectVal, objectKeys, p.Name))
-		}
-
 		// check if the object's key names are referenced correctly i.e. param.objectParam.key1
 		errs = errs.Also(validateVariables(ctx, steps, fmt.Sprintf("params\\.%s", p.Name), objectKeys))
 	}
@@ -402,21 +399,42 @@ func validateObjectUsage(ctx context.Context, steps []Step, params []ParamSpec)
 	return errs
 }
 
-// validate if object keys defined in properties are all provided in default
-func validateObjectKeysInDefault(defaultObject map[string]string, neededObjectKeys sets.String, paramName string) (errs *apis.FieldError) {
-	neededObjectKeysInSpec := neededObjectKeys.List()
-	providedObjectKeysInDefault := []string{}
-	for k := range defaultObject {
-		providedObjectKeysInDefault = append(providedObjectKeysInDefault, k)
+// validateObjectDefault validates the keys of all the object params within a
+// slice of ParamSpecs are provided in default iff the default section is provided.
+func validateObjectDefault(objectParams []ParamSpec) (errs *apis.FieldError) {
+	for _, p := range objectParams {
+		errs = errs.Also(validateObjectKeys(p.Properties, p.Default).ViaField(p.Name))
+	}
+	return errs
+}
+
+// validateObjectKeys validates if object keys defined in properties are all provided in its value provider iff the provider is not nil.
+func validateObjectKeys(properties map[string]PropertySpec, propertiesProvider *ArrayOrString) (errs *apis.FieldError) {
+	if propertiesProvider == nil || propertiesProvider.ObjectVal == nil {
+		return nil
 	}
 
-	missingObjectKeys := list.DiffLeft(neededObjectKeysInSpec, providedObjectKeysInDefault)
-	if len(missingObjectKeys) != 0 {
+	neededKeys := []string{}
+	providedKeys := []string{}
+
+	// collect all needed keys
+	for key := range properties {
+		neededKeys = append(neededKeys, key)
+	}
+
+	// collect all provided keys
+	for key := range propertiesProvider.ObjectVal {
+		providedKeys = append(providedKeys, key)
+	}
+
+	missings := list.DiffLeft(neededKeys, providedKeys)
+	if len(missings) != 0 {
 		return &apis.FieldError{
-			Message: fmt.Sprintf("Required key(s) %s for the parameter %s are not provided in default.", missingObjectKeys, paramName),
-			Paths:   []string{fmt.Sprintf("%s.properties", paramName), fmt.Sprintf("%s.default", paramName)},
+			Message: fmt.Sprintf("Required key(s) %s are missing in the value provider.", missings),
+			Paths:   []string{fmt.Sprintf("properties"), fmt.Sprintf("default")},
 		}
 	}
+
 	return nil
 }
 
diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go
index c69e2604756..c07bdc9a5e5 100644
--- a/pkg/apis/pipeline/v1beta1/task_validation_test.go
+++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go
@@ -706,7 +706,7 @@ func TestTaskSpecValidateError(t *testing.T) {
 			Steps: validSteps,
 		},
 		expectedError: apis.FieldError{
-			Message: fmt.Sprintf("Required key(s) %s for the parameter %s are not provided in default.", []string{"key2"}, "myobjectParam"),
+			Message: fmt.Sprintf("Required key(s) %s are missing in the value provider.", []string{"key2"}),
 			Paths:   []string{"myobjectParam.properties", "myobjectParam.default"},
 		},
 	}, {