diff --git a/pkg/apis/pipeline/v1alpha1/resource_types.go b/pkg/apis/pipeline/v1alpha1/resource_types.go index 5d680591204..fc57de68d4d 100644 --- a/pkg/apis/pipeline/v1alpha1/resource_types.go +++ b/pkg/apis/pipeline/v1alpha1/resource_types.go @@ -164,3 +164,36 @@ func ResourceFromType(r *PipelineResource) (PipelineResourceInterface, error) { } return nil, fmt.Errorf("%s is an invalid or unimplemented PipelineResource", r.Spec.Type) } + +// AttributesFromType returns a list of available attributes that can be replaced +// through templating. +func AttributesFromType(prt PipelineResourceType) ([]string, error) { + r := &PipelineResource{} + r.Spec.Type = prt + // Todo : The TaskResource struct lacks data to correctly infer the type of + // a PipelineResourceTypeStorage. While all the currently implemented types + // have the same attributes, this doesn't appear to be an explicit design + // choice. Future types could not fit this constraint. So we cannot safely + // make any assumptions about the attributes. + if prt == PipelineResourceTypeStorage { + r.Spec.Params = []Param{ + { + Name: "type", + Value: string(PipelineResourceTypeGCS), + }, + { + Name: "Location", + Value: "/", + }, + } + } + resource, err := ResourceFromType(r) + if err != nil { + return nil, err + } + keys := []string{} + for k := range resource.Replacements() { + keys = append(keys, k) + } + return keys, nil +} diff --git a/pkg/apis/pipeline/v1alpha1/resource_types_test.go b/pkg/apis/pipeline/v1alpha1/resource_types_test.go new file mode 100644 index 00000000000..a6c746d5546 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/resource_types_test.go @@ -0,0 +1,60 @@ +/* +Copyright 2018 The Knative Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestAttributesFromType(t *testing.T) { + tests := []struct { + name string + resourceType PipelineResourceType + expectedErr error + }{ + { + name: "git resource type", + resourceType: PipelineResourceTypeGit, + expectedErr: nil, + }, + { + name: "storage resource type", + resourceType: PipelineResourceTypeStorage, + expectedErr: nil, + }, + { + name: "image resource type", + resourceType: PipelineResourceTypeImage, + expectedErr: nil, + }, + { + name: "cluster resource type", + resourceType: PipelineResourceTypeCluster, + expectedErr: nil, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := AttributesFromType(tc.resourceType) + if d := cmp.Diff(err, tc.expectedErr); d != "" { + t.Errorf("AttributesFromType error did not match expected error %s", d) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index e40d57fa740..aed0ca2a923 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -132,58 +132,91 @@ func validateInputParameterVariables(steps []corev1.Container, inputs *Inputs) * parameterNames[p.Name] = struct{}{} } } - return validateVariables(steps, "params", parameterNames) + return validateVariables(steps, "params", "inputs.", parameterNames) } func validateResourceVariables(steps []corev1.Container, inputs *Inputs, outputs *Outputs) *apis.FieldError { - resourceNames := map[string]struct{}{} + var err *apis.FieldError + // Keep track of input and output resources separately. + // This ensures we can validate against appropriate variables set. + inputVars := map[string]struct{}{} + outputVars := map[string]struct{}{} if inputs != nil { - for _, r := range inputs.Resources { - resourceNames[r.Name] = struct{}{} + inputVars, err = getResourceVariables(inputs.Resources, "taskspec.inputs.resources.") + if err != nil { + return err } } if outputs != nil { - for _, r := range outputs.Resources { - resourceNames[r.Name] = struct{}{} + outputVars, err = getResourceVariables(outputs.Resources, "taskspec.outputs.resources.") + if err != nil { + return err + } + } + err = validateVariables(steps, "resources", "inputs.", inputVars) + if err != nil { + return err + } + err = validateVariables(steps, "resources", "outputs.", outputVars) + if err != nil { + return err + } + return nil +} + +func getResourceVariables(resources []TaskResource, pathPrefix string) (map[string]struct{}, *apis.FieldError) { + vars := map[string]struct{}{} + for _, r := range resources { + attrs, err := AttributesFromType(r.Type) + if err != nil { + return nil, &apis.FieldError{ + Message: fmt.Sprintf("invalid resource type %s", r.Type), + Paths: []string{pathPrefix + r.Name}, + Details: err.Error(), + } + } + for _, a := range attrs { + rv := r.Name + "." + a + vars[rv] = struct{}{} } } - return validateVariables(steps, "resources", resourceNames) + return vars, nil } -func validateVariables(steps []corev1.Container, prefix string, vars map[string]struct{}) *apis.FieldError { +func validateVariables(steps []corev1.Container, prefix, contextPrefix string, vars map[string]struct{}) *apis.FieldError { for _, step := range steps { - if err := validateTaskVariable("name", step.Name, prefix, vars); err != nil { + if err := validateTaskVariable("name", step.Name, prefix, contextPrefix, vars); err != nil { return err } - if err := validateTaskVariable("image", step.Image, prefix, vars); err != nil { + if err := validateTaskVariable("image", step.Image, prefix, contextPrefix, vars); err != nil { return err } - if err := validateTaskVariable("workingDir", step.WorkingDir, prefix, vars); err != nil { + if err := validateTaskVariable("workingDir", step.WorkingDir, prefix, contextPrefix, vars); err != nil { return err } for i, cmd := range step.Command { - if err := validateTaskVariable(fmt.Sprintf("command[%d]", i), cmd, prefix, vars); err != nil { + if err := validateTaskVariable(fmt.Sprintf("command[%d]", i), cmd, prefix, contextPrefix, vars); err != nil { return err } } for i, arg := range step.Args { - if err := validateTaskVariable(fmt.Sprintf("arg[%d]", i), arg, prefix, vars); err != nil { + if err := validateTaskVariable(fmt.Sprintf("arg[%d]", i), arg, prefix, contextPrefix, vars); err != nil { return err } } for _, env := range step.Env { - if err := validateTaskVariable(fmt.Sprintf("env[%s]", env.Name), env.Value, prefix, vars); err != nil { + if err := validateTaskVariable(fmt.Sprintf("env[%s]", env.Name), env.Value, prefix, contextPrefix, vars); err != nil { return err } } for i, v := range step.VolumeMounts { - if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].Name", i), v.Name, prefix, vars); err != nil { + if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].Name", i), v.Name, prefix, contextPrefix, vars); err != nil { return err } - if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].MountPath", i), v.MountPath, prefix, vars); err != nil { + if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].MountPath", i), v.MountPath, prefix, contextPrefix, vars); err != nil { return err } - if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].SubPath", i), v.SubPath, prefix, vars); err != nil { + if err := validateTaskVariable(fmt.Sprintf("volumeMount[%d].SubPath", i), v.SubPath, prefix, contextPrefix, vars); err != nil { return err } } @@ -191,8 +224,8 @@ func validateVariables(steps []corev1.Container, prefix string, vars map[string] return nil } -func validateTaskVariable(name, value, prefix string, vars map[string]struct{}) *apis.FieldError { - return templating.ValidateVariable(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", vars) +func validateTaskVariable(name, value, prefix, contextPrefix string, vars map[string]struct{}) *apis.FieldError { + return templating.ValidateVariable(name, value, prefix, contextPrefix, "step", "taskspec.steps", vars) } func checkForDuplicates(resources []TaskResource, path string) *apis.FieldError { diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index 93ecd5b544d..5f1b97403cd 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation_test.go @@ -105,7 +105,7 @@ func TestTaskSpec_Validate(t *testing.T) { Name: "mystep", Image: "${inputs.resources.foo.url}", Args: []string{"--flag=${inputs.params.baz} && ${input.params.foo-is-baz}"}, - WorkingDir: "/foo/bar/${outputs.resources.source}", + WorkingDir: "/foo/bar/${outputs.resources.source.name}", }}, }, }} @@ -321,7 +321,46 @@ func TestTaskSpec_ValidateError(t *testing.T) { Message: `non-existent variable in "${inputs.params.foo} && ${inputs.params.inexistent}" for step arg[0]`, Paths: []string{"taskspec.steps.arg[0]"}, }, - }} + }, { + name: "invalid resource variable usage", + fields: fields{ + Inputs: &Inputs{ + Resources: []TaskResource{{ + Name: "foo", + Type: PipelineResourceTypeImage, + }}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "myimage", + Args: []string{"${inputs.resources.foo}"}, + }}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "${inputs.resources.foo}" for step arg[0]`, + Paths: []string{"taskspec.steps.arg[0]"}, + }, + }, + { + name: "inexistent output resource variable", + fields: fields{ + Inputs: &Inputs{ + Resources: []TaskResource{{ + Name: "foo", + Type: PipelineResourceTypeImage, + }}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "myimage", + Args: []string{"${outputs.resources.foo.url}"}, + }}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "${outputs.resources.foo.url}" for step arg[0]`, + Paths: []string{"taskspec.steps.arg[0]"}, + }, + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ts := &TaskSpec{ @@ -339,3 +378,62 @@ func TestTaskSpec_ValidateError(t *testing.T) { }) } } + +func TestGetResourceVariables(t *testing.T) { + tests := []struct { + name string + pathPrefix string + resources []TaskResource + expectedVars map[string]struct{} + }{ + { + name: "empty resources", + pathPrefix: "taskspec.outputs.resources.", + resources: []TaskResource{}, + expectedVars: map[string]struct{}{}, + }, + { + name: "single resource", + pathPrefix: "taskspec.outputs.resources.", + resources: []TaskResource{validResource}, + expectedVars: map[string]struct{}{ + "source.name": struct{}{}, + "source.type": struct{}{}, + "source.url": struct{}{}, + "source.revision": struct{}{}, + }, + }, + { + name: "multiple resources", + pathPrefix: "taskspec.outputs.resources.", + resources: []TaskResource{ + validResource, + TaskResource{ + Name: "foo", + Type: PipelineResourceTypeImage, + }, + }, + expectedVars: map[string]struct{}{ + "source.name": struct{}{}, + "source.type": struct{}{}, + "source.url": struct{}{}, + "source.revision": struct{}{}, + "foo.name": struct{}{}, + "foo.type": struct{}{}, + "foo.url": struct{}{}, + "foo.digest": struct{}{}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + vars, err := getResourceVariables(tt.resources, tt.pathPrefix) + if err != nil { + t.Errorf("getResourceVariables() = %v", err) + } + if d := cmp.Diff(tt.expectedVars, vars); d != "" { + t.Errorf("getResourceVariables results diff -want, +got: %v", d) + } + }) + } +} diff --git a/pkg/templating/templating.go b/pkg/templating/templating.go index c2ccb0c9500..2c1654cb602 100644 --- a/pkg/templating/templating.go +++ b/pkg/templating/templating.go @@ -47,10 +47,7 @@ func extractVariablesFromString(s, prefix string) ([]string, bool) { vars := make([]string, len(matches)) for i, match := range matches { groups := matchGroups(match, re) - // foo -> foo - // foo.bar -> foo - // foo.bar.baz -> foo - vars[i] = strings.SplitN(groups["var"], ".", 2)[0] + vars[i] = groups["var"] } return vars, true } diff --git a/pkg/templating/templating_test.go b/pkg/templating/templating_test.go index 54e8d58a21e..0b44108a0d3 100644 --- a/pkg/templating/templating_test.go +++ b/pkg/templating/templating_test.go @@ -78,6 +78,23 @@ func TestValidateVariables(t *testing.T) { }, expectedError: nil, }, + { + name: "nested variable", + args: args{ + input: "--flag=${inputs.resources.foo.url}", + prefix: "resources", + contextPrefix: "inputs.", + locationName: "step", + path: "taskspec.steps", + vars: map[string]struct{}{ + "foo.name": {}, + "foo.type": {}, + "foo.url": {}, + "foo.digest": {}, + }, + }, + expectedError: nil, + }, { name: "undefined variable", args: args{