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

Cleanup: Remove "substituted context" task validation #6671

Merged
merged 1 commit into from
May 18, 2023
Merged

Cleanup: Remove "substituted context" task validation #6671

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
14 changes: 0 additions & 14 deletions pkg/apis/config/contexts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,9 @@ import (
"context"
)

// isSubstituted is used for associating the parameter substitution inside the context.Context.
type isSubstituted struct{}

// validatePropagatedVariables is used for deciding whether to validate or skip parameters and workspaces inside the contect.Context.
type validatePropagatedVariables string

// WithinSubstituted is used to note that it is calling within
// the context of a substitute variable operation.
func WithinSubstituted(ctx context.Context) context.Context {
return context.WithValue(ctx, isSubstituted{}, true)
}

// IsSubstituted indicates that the variables have been substituted.
func IsSubstituted(ctx context.Context) bool {
return ctx.Value(isSubstituted{}) != nil
}

// SkipValidationDueToPropagatedParametersAndWorkspaces sets the context to skip validation of parameters when embedded vs referenced to true or false.
func SkipValidationDueToPropagatedParametersAndWorkspaces(ctx context.Context, skip bool) context.Context {
return context.WithValue(ctx, validatePropagatedVariables("ValidatePropagatedParameterVariablesAndWorkspaces"), !skip)
Expand Down
7 changes: 0 additions & 7 deletions pkg/apis/pipeline/v1/task_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,6 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
errs = errs.Also(apis.ErrMissingField("steps"))
}

if config.IsSubstituted(ctx) {
// Validate the task's workspaces only.
errs = errs.Also(validateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate).ViaField("workspaces"))
errs = errs.Also(validateWorkspaceUsages(ctx, ts))

return errs
}
// When propagating parameters, parameters used in the Task spec may not be declared by the Task.
// Only perform this validation after all declared parameters have been propagated.
// TODO(#6647): Remove this flag and call this function in the reconciler instead
Expand Down
84 changes: 0 additions & 84 deletions pkg/apis/pipeline/v1/task_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,90 +1648,6 @@ func TestIncompatibleAPIVersions(t *testing.T) {
}
}

func TestSubstitutedContext(t *testing.T) {
type fields struct {
Params []v1.ParamSpec
Steps []v1.Step
SubstitutionContext bool
}
tests := []struct {
name string
fields fields
expectedError apis.FieldError
}{{
name: "variable not substituted",
fields: fields{
Steps: []v1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: `
#!/usr/bin/env bash
hello "$(params.a)"`,
}},
SubstitutionContext: false,
},
expectedError: apis.FieldError{
Message: `non-existent variable in "\n\t\t\t\t#!/usr/bin/env bash\n\t\t\t\thello \"$(params.a)\""`,
Paths: []string{"steps[0].script"},
},
}, {
name: "variable substituted double quoted",
fields: fields{
Steps: []v1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: `
#!/usr/bin/env bash
hello "$(params.a)"`,
}},
SubstitutionContext: true,
},
}, {
name: "variable substituted not quoted",
fields: fields{
Steps: []v1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: `
#!/usr/bin/env bash
hello $(params.a)`,
}},
SubstitutionContext: true,
},
}, {
name: "variable substituted single quoted",
fields: fields{
Steps: []v1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: "echo `$(params.a)`",
}},
SubstitutionContext: true,
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ts := &v1.TaskSpec{
Params: tt.fields.Params,
Steps: tt.fields.Steps,
}
ctx := context.Background()
ts.SetDefaults(ctx)
ctx = config.SkipValidationDueToPropagatedParametersAndWorkspaces(ctx, false)
if tt.fields.SubstitutionContext {
ctx = config.WithinSubstituted(ctx)
}
err := ts.Validate(ctx)
if err == nil && tt.expectedError.Error() != "" {
t.Fatalf("Expected an error, got nothing for %v", ts)
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d))
}
})
}
}

func TestValidateParamArrayIndex(t *testing.T) {
stepsInvalidReferences := []string{}
for i := 10; i <= 26; i++ {
Expand Down
7 changes: 0 additions & 7 deletions pkg/apis/pipeline/v1beta1/task_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,6 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
errs = errs.Also(apis.ErrMissingField("steps"))
}

if config.IsSubstituted(ctx) {
// Validate the task's workspaces only.
errs = errs.Also(validateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate).ViaField("workspaces"))
errs = errs.Also(validateWorkspaceUsages(ctx, ts))

return errs
}
// When propagating parameters, parameters used in the Task spec may not be declared by the Task.
// Only perform this validation after all declared parameters have been propagated.
// TODO(#6647): Remove this flag and call this function in the reconciler instead
Expand Down
84 changes: 0 additions & 84 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,90 +1660,6 @@ func TestIncompatibleAPIVersions(t *testing.T) {
}
}

func TestSubstitutedContext(t *testing.T) {
type fields struct {
Params []v1beta1.ParamSpec
Steps []v1beta1.Step
SubstitutionContext bool
}
tests := []struct {
name string
fields fields
expectedError apis.FieldError
}{{
name: "variable not substituted",
fields: fields{
Steps: []v1beta1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: `
#!/usr/bin/env bash
hello "$(params.a)"`,
}},
SubstitutionContext: false,
},
expectedError: apis.FieldError{
Message: `non-existent variable in "\n\t\t\t\t#!/usr/bin/env bash\n\t\t\t\thello \"$(params.a)\""`,
Paths: []string{"steps[0].script"},
},
}, {
name: "variable substituted double quoted",
fields: fields{
Steps: []v1beta1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: `
#!/usr/bin/env bash
hello "$(params.a)"`,
}},
SubstitutionContext: true,
},
}, {
name: "variable substituted not quoted",
fields: fields{
Steps: []v1beta1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: `
#!/usr/bin/env bash
hello $(params.a)`,
}},
SubstitutionContext: true,
},
}, {
name: "variable substituted single quoted",
fields: fields{
Steps: []v1beta1.Step{{
Image: "my-image",
Args: []string{"params"},
Script: "echo `$(params.a)`",
}},
SubstitutionContext: true,
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ts := &v1beta1.TaskSpec{
Params: tt.fields.Params,
Steps: tt.fields.Steps,
}
ctx := context.Background()
ts.SetDefaults(ctx)
ctx = config.SkipValidationDueToPropagatedParametersAndWorkspaces(ctx, false)
if tt.fields.SubstitutionContext {
ctx = config.WithinSubstituted(ctx)
}
err := ts.Validate(ctx)
if err == nil && tt.expectedError.Error() != "" {
t.Fatalf("Expected an error, got nothing for %v", ts)
}
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d))
}
})
}
}

func TestValidateParamArrayIndex(t *testing.T) {
stepsInvalidReferences := []string{}
for i := 10; i <= 26; i++ {
Expand Down