Skip to content

Commit

Permalink
TEP-0075: Extract out the validation of object keys
Browse files Browse the repository at this point in the history
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. Also functions are reordered a bit to make the whole
file easier to track and read.
  • Loading branch information
chuangw6 committed May 12, 2022
1 parent 3c0fb3b commit 76d2632
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 47 deletions.
110 changes: 64 additions & 46 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ func ValidateParameterVariables(steps []Step, params []ParamSpec) *apis.FieldErr

errs := validateVariables(steps, "params", parameterNames)
errs = errs.Also(validateArrayUsage(steps, "params", arrayParameterNames))
errs = errs.Also(validateObjectDefault(objectParamSpecs))
return errs.Also(validateObjectUsage(steps, objectParamSpecs))
}

Expand Down Expand Up @@ -361,52 +362,6 @@ func ValidateResourcesVariables(steps []Step, resources *TaskResources) *apis.Fi
return validateVariables(steps, "resources.(?:inputs|outputs)", resourceNames)
}

// TODO (@chuangw6): Make sure an object param is not used as a whole when providing values for strings.
// https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#variable-replacement-with-object-params
// "When providing values for strings, Task and Pipeline authors can access
// individual attributes of an object param; they cannot access the object
// as whole (we could add support for this later)."
func validateObjectUsage(steps []Step, params []ParamSpec) (errs *apis.FieldError) {
objectParameterNames := sets.NewString()
for _, p := range params {
// collect all names of object type params
objectParameterNames.Insert(p.Name)

// collect all keys for this object param
objectKeys := sets.NewString()
for key := range p.Properties {
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(steps, fmt.Sprintf("params\\.%s", p.Name), objectKeys))
}

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)
}

missingObjectKeys := list.DiffLeft(neededObjectKeysInSpec, providedObjectKeysInDefault)
if len(missingObjectKeys) != 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)},
}
}
return nil
}

func validateArrayUsage(steps []Step, prefix string, vars sets.String) (errs *apis.FieldError) {
for idx, step := range steps {
errs = errs.Also(validateStepArrayUsage(step, prefix, vars)).ViaFieldIndex("steps", idx)
Expand Down Expand Up @@ -488,6 +443,69 @@ func validateStepVariables(step Step, prefix string, vars sets.String) *apis.Fie
return errs
}

// 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, p.Name))
}
return errs
}

// validate 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, name string) (errs *apis.FieldError) {
if propertiesProvider == nil || propertiesProvider.ObjectVal == nil {
return nil
}

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 missing in the value provider.", missings, name),
Paths: []string{fmt.Sprintf("%s.properties", name), fmt.Sprintf("%s.default", name)},
}
}

return nil
}

// TODO (@chuangw6): Make sure an object param is not used as a whole when providing values for strings.
// https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#variable-replacement-with-object-params
// "When providing values for strings, Task and Pipeline authors can access
// individual attributes of an object param; they cannot access the object
// as whole (we could add support for this later)."
func validateObjectUsage(steps []Step, params []ParamSpec) (errs *apis.FieldError) {
objectParameterNames := sets.NewString()
for _, p := range params {
// collect all names of object type params
objectParameterNames.Insert(p.Name)

// collect all keys for this object param
objectKeys := sets.NewString()
for key := range p.Properties {
objectKeys.Insert(key)
}

// check if the object's key names are referenced correctly i.e. param.objectParam.key1
errs = errs.Also(validateVariables(steps, fmt.Sprintf("params\\.%s", p.Name), objectKeys))
}

return errs
}

func validateTaskVariable(value, prefix string, vars sets.String) *apis.FieldError {
return substitution.ValidateVariableP(value, prefix, vars)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,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 for the parameter %s are missing in the value provider.", []string{"key2"}, "myobjectParam"),
Paths: []string{"myobjectParam.properties", "myobjectParam.default"},
},
}, {
Expand Down

0 comments on commit 76d2632

Please sign in to comment.