Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extract substitution code out of v1alpha1 ✂ #1611

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Extract substitution code out of v1alpha1 ✂ #1611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions pkg/apis/pipeline/v1alpha1/param_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"context"
"encoding/json"
"fmt"

"github.com/tektoncd/pipeline/pkg/substitution"
)

// ParamSpec defines arbitrary parameters needed beyond typed inputs (such as
Expand Down Expand Up @@ -116,11 +118,11 @@ func (arrayOrString ArrayOrString) MarshalJSON() ([]byte, error) {

func (arrayOrString *ArrayOrString) ApplyReplacements(stringReplacements map[string]string, arrayReplacements map[string][]string) {
if arrayOrString.Type == ParamTypeString {
arrayOrString.StringVal = ApplyReplacements(arrayOrString.StringVal, stringReplacements)
arrayOrString.StringVal = substitution.ApplyReplacements(arrayOrString.StringVal, stringReplacements)
} else {
var newArrayVal []string
for _, v := range arrayOrString.ArrayVal {
newArrayVal = append(newArrayVal, ApplyArrayReplacements(v, stringReplacements, arrayReplacements)...)
newArrayVal = append(newArrayVal, substitution.ApplyArrayReplacements(v, stringReplacements, arrayReplacements)...)
}
arrayOrString.ArrayVal = newArrayVal
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/validate"
"github.com/tektoncd/pipeline/pkg/list"
"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
"github.com/tektoncd/pipeline/pkg/substitution"
"golang.org/x/xerrors"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/validation"
Expand Down Expand Up @@ -233,13 +234,13 @@ func validatePipelineVariables(tasks []PipelineTask, prefix string, paramNames m
}

func validatePipelineVariable(name, value, prefix string, vars map[string]struct{}) *apis.FieldError {
return ValidateVariable(name, value, prefix, "", "task parameter", "pipelinespec.params", vars)
return substitution.ValidateVariable(name, value, prefix, "", "task parameter", "pipelinespec.params", vars)
}

func validatePipelineNoArrayReferenced(name, value, prefix string, vars map[string]struct{}) *apis.FieldError {
return ValidateVariableProhibited(name, value, prefix, "", "task parameter", "pipelinespec.params", vars)
return substitution.ValidateVariableProhibited(name, value, prefix, "", "task parameter", "pipelinespec.params", vars)
}

func validatePipelineArraysIsolated(name, value, prefix string, vars map[string]struct{}) *apis.FieldError {
return ValidateVariableIsolated(name, value, prefix, "", "task parameter", "pipelinespec.params", vars)
return substitution.ValidateVariableIsolated(name, value, prefix, "", "task parameter", "pipelinespec.params", vars)
}
38 changes: 21 additions & 17 deletions pkg/apis/pipeline/v1alpha1/step_replacements.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,53 +16,57 @@

package v1alpha1

import (
"github.com/tektoncd/pipeline/pkg/substitution"
)

func ApplyStepReplacements(step *Step, stringReplacements map[string]string, arrayReplacements map[string][]string) {
step.Name = ApplyReplacements(step.Name, stringReplacements)
step.Image = ApplyReplacements(step.Image, stringReplacements)
step.Script = ApplyReplacements(step.Script, stringReplacements)
step.Name = substitution.ApplyReplacements(step.Name, stringReplacements)
step.Image = substitution.ApplyReplacements(step.Image, stringReplacements)
step.Script = substitution.ApplyReplacements(step.Script, stringReplacements)

// Use ApplyArrayReplacements here, as additional args may be added via an array parameter.
var newArgs []string
for _, a := range step.Args {
newArgs = append(newArgs, ApplyArrayReplacements(a, stringReplacements, arrayReplacements)...)
newArgs = append(newArgs, substitution.ApplyArrayReplacements(a, stringReplacements, arrayReplacements)...)
}
step.Args = newArgs

for ie, e := range step.Env {
step.Env[ie].Value = ApplyReplacements(e.Value, stringReplacements)
step.Env[ie].Value = substitution.ApplyReplacements(e.Value, stringReplacements)
if step.Env[ie].ValueFrom != nil {
if e.ValueFrom.SecretKeyRef != nil {
step.Env[ie].ValueFrom.SecretKeyRef.LocalObjectReference.Name = ApplyReplacements(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, stringReplacements)
step.Env[ie].ValueFrom.SecretKeyRef.Key = ApplyReplacements(e.ValueFrom.SecretKeyRef.Key, stringReplacements)
step.Env[ie].ValueFrom.SecretKeyRef.LocalObjectReference.Name = substitution.ApplyReplacements(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, stringReplacements)
step.Env[ie].ValueFrom.SecretKeyRef.Key = substitution.ApplyReplacements(e.ValueFrom.SecretKeyRef.Key, stringReplacements)
}
if e.ValueFrom.ConfigMapKeyRef != nil {
step.Env[ie].ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name = ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, stringReplacements)
step.Env[ie].ValueFrom.ConfigMapKeyRef.Key = ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.Key, stringReplacements)
step.Env[ie].ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name = substitution.ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, stringReplacements)
step.Env[ie].ValueFrom.ConfigMapKeyRef.Key = substitution.ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.Key, stringReplacements)
}
}
}

for ie, e := range step.EnvFrom {
step.EnvFrom[ie].Prefix = ApplyReplacements(e.Prefix, stringReplacements)
step.EnvFrom[ie].Prefix = substitution.ApplyReplacements(e.Prefix, stringReplacements)
if e.ConfigMapRef != nil {
step.EnvFrom[ie].ConfigMapRef.LocalObjectReference.Name = ApplyReplacements(e.ConfigMapRef.LocalObjectReference.Name, stringReplacements)
step.EnvFrom[ie].ConfigMapRef.LocalObjectReference.Name = substitution.ApplyReplacements(e.ConfigMapRef.LocalObjectReference.Name, stringReplacements)
}
if e.SecretRef != nil {
step.EnvFrom[ie].SecretRef.LocalObjectReference.Name = ApplyReplacements(e.SecretRef.LocalObjectReference.Name, stringReplacements)
step.EnvFrom[ie].SecretRef.LocalObjectReference.Name = substitution.ApplyReplacements(e.SecretRef.LocalObjectReference.Name, stringReplacements)
}
}
step.WorkingDir = ApplyReplacements(step.WorkingDir, stringReplacements)
step.WorkingDir = substitution.ApplyReplacements(step.WorkingDir, stringReplacements)

// Use ApplyArrayReplacements here, as additional commands may be added via an array parameter.
var newCommand []string
for _, c := range step.Command {
newCommand = append(newCommand, ApplyArrayReplacements(c, stringReplacements, arrayReplacements)...)
newCommand = append(newCommand, substitution.ApplyArrayReplacements(c, stringReplacements, arrayReplacements)...)
}
step.Command = newCommand

for iv, v := range step.VolumeMounts {
step.VolumeMounts[iv].Name = ApplyReplacements(v.Name, stringReplacements)
step.VolumeMounts[iv].MountPath = ApplyReplacements(v.MountPath, stringReplacements)
step.VolumeMounts[iv].SubPath = ApplyReplacements(v.SubPath, stringReplacements)
step.VolumeMounts[iv].Name = substitution.ApplyReplacements(v.Name, stringReplacements)
step.VolumeMounts[iv].MountPath = substitution.ApplyReplacements(v.MountPath, stringReplacements)
step.VolumeMounts[iv].SubPath = substitution.ApplyReplacements(v.SubPath, stringReplacements)
}
}
7 changes: 4 additions & 3 deletions pkg/apis/pipeline/v1alpha1/task_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strings"

"github.com/tektoncd/pipeline/pkg/apis/validate"
"github.com/tektoncd/pipeline/pkg/substitution"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/validation"
Expand Down Expand Up @@ -301,15 +302,15 @@ func validateVariables(steps []Step, prefix string, vars map[string]struct{}) *a
}

func validateTaskVariable(name, value, prefix string, vars map[string]struct{}) *apis.FieldError {
return ValidateVariable(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", vars)
return substitution.ValidateVariable(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", vars)
}

func validateTaskNoArrayReferenced(name, value, prefix string, arrayNames map[string]struct{}) *apis.FieldError {
return ValidateVariableProhibited(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", arrayNames)
return substitution.ValidateVariableProhibited(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", arrayNames)
}

func validateTaskArraysIsolated(name, value, prefix string, arrayNames map[string]struct{}) *apis.FieldError {
return ValidateVariableIsolated(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", arrayNames)
return substitution.ValidateVariableIsolated(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", arrayNames)
}

func checkForDuplicates(resources []TaskResource, path string) *apis.FieldError {
Expand Down
9 changes: 5 additions & 4 deletions pkg/reconciler/taskrun/resources/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/substitution"
)

// ApplyParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec
Expand Down Expand Up @@ -95,15 +96,15 @@ func ApplyReplacements(spec *v1alpha1.TaskSpec, stringReplacements map[string]st

// Apply variable expansion to the build's volumes
for i, v := range spec.Volumes {
spec.Volumes[i].Name = v1alpha1.ApplyReplacements(v.Name, stringReplacements)
spec.Volumes[i].Name = substitution.ApplyReplacements(v.Name, stringReplacements)
if v.VolumeSource.ConfigMap != nil {
spec.Volumes[i].ConfigMap.Name = v1alpha1.ApplyReplacements(v.ConfigMap.Name, stringReplacements)
spec.Volumes[i].ConfigMap.Name = substitution.ApplyReplacements(v.ConfigMap.Name, stringReplacements)
}
if v.VolumeSource.Secret != nil {
spec.Volumes[i].Secret.SecretName = v1alpha1.ApplyReplacements(v.Secret.SecretName, stringReplacements)
spec.Volumes[i].Secret.SecretName = substitution.ApplyReplacements(v.Secret.SecretName, stringReplacements)
}
if v.PersistentVolumeClaim != nil {
spec.Volumes[i].PersistentVolumeClaim.ClaimName = v1alpha1.ApplyReplacements(v.PersistentVolumeClaim.ClaimName, stringReplacements)
spec.Volumes[i].PersistentVolumeClaim.ClaimName = substitution.ApplyReplacements(v.PersistentVolumeClaim.ClaimName, stringReplacements)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1alpha1/substitution.go → pkg/substitution/substitution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1
package substitution

import (
"fmt"
Expand Down
10 changes: 5 additions & 5 deletions ...is/pipeline/v1alpha1/substitution_test.go → pkg/substitution/substitution_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1_test
package substitution_test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

total nitpick but i think we might be able to get away with just substitution now? afaik v1alpha_test had to be used b/c we were having some kinda like conflicts or circular dependency problems or something (might be thinking of a different issue tho - i know this happens with the builder libs a lot! - but probably wont once you're done!!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think v1alpha_test might have been for that reason indeed. I tend to like using _test package as it force the test to only use what the user can use and how he will use it 😛

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah kk! makes sense to me. ill have to noodle on it a bit and maybe ill switch to the same thing!


import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/substitution"
"knative.dev/pkg/apis"
)

Expand Down Expand Up @@ -111,7 +111,7 @@ func TestValidateVariables(t *testing.T) {
},
}} {
t.Run(tc.name, func(t *testing.T) {
got := v1alpha1.ValidateVariable("somefield", tc.args.input, tc.args.prefix, tc.args.contextPrefix, tc.args.locationName, tc.args.path, tc.args.vars)
got := substitution.ValidateVariable("somefield", tc.args.input, tc.args.prefix, tc.args.contextPrefix, tc.args.locationName, tc.args.path, tc.args.vars)

if d := cmp.Diff(got, tc.expectedError, cmp.AllowUnexported(apis.FieldError{})); d != "" {
t.Errorf("ValidateVariable() error did not match expected error %s", d)
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestApplyReplacements(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
actualOutput := v1alpha1.ApplyReplacements(tt.args.input, tt.args.replacements)
actualOutput := substitution.ApplyReplacements(tt.args.input, tt.args.replacements)
if d := cmp.Diff(actualOutput, tt.expectedOutput); d != "" {
t.Errorf("ApplyReplacements() output did not match expected value %s", d)
}
Expand Down Expand Up @@ -225,7 +225,7 @@ func TestApplyArrayReplacements(t *testing.T) {
expectedOutput: []string{"1", "2"},
}} {
t.Run(tc.name, func(t *testing.T) {
actualOutput := v1alpha1.ApplyArrayReplacements(tc.args.input, tc.args.stringReplacements, tc.args.arrayReplacements)
actualOutput := substitution.ApplyArrayReplacements(tc.args.input, tc.args.stringReplacements, tc.args.arrayReplacements)
if d := cmp.Diff(actualOutput, tc.expectedOutput); d != "" {
t.Errorf("ApplyArrayReplacements() output did not match expected value %s", d)
}
Expand Down