Enhance v1beta1 validation code for pipeline 🍸

This make a better use of knative.dev/pkg apis errors package in order to make the code simpler, *and* the report better. - This allows reporting multiple validation error at once — so, one failure message (webhook error) would be more useful to the user. - This reduce `if err != nil` path, simplifying the code. This commit tackles `Pipeline` 🙃 Signed-off-by: Vincent Demeester <[email protected]>
tektoncd · Sep 23, 2020 · 49d4b5a · 49d4b5a
1 parent a9e7b54
commit 49d4b5a
Show file tree

Hide file tree

Showing 4 changed files with 649 additions and 401 deletions.
diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go
@@ -145,39 +145,25 @@ func NewArrayOrString(value string, values ...string) *ArrayOrString {
 	}
 }
 
-func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix string, paramNames sets.String, arrayParamNames sets.String) *apis.FieldError {
+func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) {
 	for _, param := range params {
 		if param.Value.Type == ParamTypeString {
-			if err := validateStringVariableInTaskParameters(fmt.Sprintf("[%s]", param.Name), param.Value.StringVal, prefix, paramNames, arrayParamNames); err != nil {
-				return err
-			}
+			errs = errs.Also(validateStringVariableInTaskParameters(param.Value.StringVal, prefix, paramNames, arrayParamNames).ViaFieldKey("params", param.Name))
 		} else {
-			for _, arrayElement := range param.Value.ArrayVal {
-				if err := validateArrayVariableInTaskParameters(fmt.Sprintf("[%s]", param.Name), arrayElement, prefix, paramNames, arrayParamNames); err != nil {
-					return err
-				}
+			for idx, arrayElement := range param.Value.ArrayVal {
+				errs = errs.Also(validateArrayVariableInTaskParameters(arrayElement, prefix, paramNames, arrayParamNames).ViaFieldIndex("value", idx).ViaFieldKey("params", param.Name))
 			}
 		}
 	}
-	return nil
+	return errs
 }
 
-func validateStringVariableInTaskParameters(name, value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
-	if err := substitution.ValidateVariable(name, value, prefix, "task parameter", "pipelinespec.params", stringVars); err != nil {
-		return err
-	}
-	if err := substitution.ValidateVariableProhibited(name, value, prefix, "task parameter", "pipelinespec.params", arrayVars); err != nil {
-		return err
-	}
-	return nil
+func validateStringVariableInTaskParameters(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
+	errs := substitution.ValidateVariableP(value, prefix, stringVars)
+	return errs.Also(substitution.ValidateVariableProhibitedP(value, prefix, arrayVars))
 }
 
-func validateArrayVariableInTaskParameters(name, value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
-	if err := substitution.ValidateVariable(name, value, prefix, "task parameter", "pipelinespec.params", stringVars); err != nil {
-		return err
-	}
-	if err := substitution.ValidateVariableIsolated(name, value, prefix, "task parameter", "pipelinespec.params", arrayVars); err != nil {
-		return err
-	}
-	return nil
+func validateArrayVariableInTaskParameters(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
+	errs := substitution.ValidateVariableP(value, prefix, stringVars)
+	return errs.Also(substitution.ValidateVariableProhibitedP(value, prefix, arrayVars))
 }