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.

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

Skip *heavy* validation on deletion 🙃 #3937

Merged
merged 1 commit into from
Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/cluster_task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ func (t *ClusterTask) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(t.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}
if apis.IsInDelete(ctx) {
return nil
}
return t.Spec.Validate(ctx)
}
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/condition_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ func (c Condition) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(c.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}
if apis.IsInDelete(ctx) {
return nil
}
return c.Spec.Validate(ctx).ViaField("Spec")
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(p.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}
if apis.IsInDelete(ctx) {
return nil
}
return p.Spec.Validate(ctx)
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ func (pr *PipelineRun) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(pr.GetObjectMeta()).ViaField("metadata"); err != nil {
return err
}
if apis.IsInDelete(ctx) {
return nil
}
return pr.Spec.Validate(ctx)
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/run_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ func (r *Run) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(r.GetObjectMeta()).ViaField("metadata"); err != nil {
return err
}
if apis.IsInDelete(ctx) {
return nil
}
return r.Spec.Validate(ctx)
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(t.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}
if apis.IsInDelete(ctx) {
return nil
}
return t.Spec.Validate(ctx)
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func (tr *TaskRun) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(tr.GetObjectMeta()).ViaField("metadata"); err != nil {
return err
}
if apis.IsInDelete(ctx) {
return nil
}
return tr.Spec.Validate(ctx)
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/cluster_task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,8 @@ var _ apis.Validatable = (*ClusterTask)(nil)

func (t *ClusterTask) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
if apis.IsInDelete(ctx) {
return nil
}
return errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
}
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ var _ apis.Validatable = (*Pipeline)(nil)
// that any references resources exist, that is done at run time.
func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(p.GetObjectMeta()).ViaField("metadata")
if apis.IsInDelete(ctx) {
return nil
}
return errs.Also(p.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ func TestPipeline_Validate_Success(t *testing.T) {
}},
},
},
}, {
name: "do not validate spec on delete",
p: &Pipeline{
ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
},
wc: apis.WithinDelete,
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ var _ apis.Validatable = (*PipelineRun)(nil)
func (pr *PipelineRun) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(pr.GetObjectMeta()).ViaField("metadata")

if apis.IsInDelete(ctx) {
return nil
}

if pr.IsPending() && pr.HasStarted() {
errs = errs.Also(apis.ErrInvalidValue("PipelineRun cannot be Pending after it is started", "spec.status"))
}
Expand Down
61 changes: 39 additions & 22 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,31 +136,29 @@ func TestPipelineRun_Invalid(t *testing.T) {
},
want: apis.ErrInvalidValue("invalid bundle reference (could not parse reference: not a valid reference)", "spec.pipelineref.bundle"),
wc: enableTektonOCIBundles(t),
},
{
name: "pipelinerun pending while running",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinerunname",
},
Spec: v1beta1.PipelineRunSpec{
Status: v1beta1.PipelineRunSpecStatusPending,
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
},
Status: v1beta1.PipelineRunStatus{
PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
StartTime: &metav1.Time{time.Now()},
},
}, {
name: "pipelinerun pending while running",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinerunname",
},
Spec: v1beta1.PipelineRunSpec{
Status: v1beta1.PipelineRunSpecStatusPending,
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
},
want: &apis.FieldError{
Message: "invalid value: PipelineRun cannot be Pending after it is started",
Paths: []string{"spec.status"},
Status: v1beta1.PipelineRunStatus{
PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
StartTime: &metav1.Time{time.Now()},
},
},
},
}
want: &apis.FieldError{
Message: "invalid value: PipelineRun cannot be Pending after it is started",
Paths: []string{"spec.status"},
},
}}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -180,6 +178,7 @@ func TestPipelineRun_Validate(t *testing.T) {
tests := []struct {
name string
pr v1beta1.PipelineRun
wc func(context.Context) context.Context
}{{
name: "normal case",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -256,11 +255,29 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
}, {
name: "do not validate spec on delete",
pr: v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinelinename",
},
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{
Name: "prname",
},
Timeout: &metav1.Duration{Duration: -48 * time.Hour},
},
},
wc: apis.WithinDelete,
}}

for _, ts := range tests {
t.Run(ts.name, func(t *testing.T) {
if err := ts.pr.Validate(context.Background()); err != nil {
ctx := context.Background()
if ts.wc != nil {
ctx = ts.wc(ctx)
}
if err := ts.pr.Validate(ctx); err != nil {
t.Error(err)
}
})
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ var _ apis.Validatable = (*Task)(nil)

func (t *Task) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
if apis.IsInDelete(ctx) {
return nil
}
return errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
}

Expand Down
26 changes: 26 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,32 @@ var invalidSteps = []v1beta1.Step{{Container: corev1.Container{
Image: "myimage",
}}}

func TestTaskValidate(t *testing.T) {
tests := []struct {
name string
t *v1beta1.Task
wc func(context.Context) context.Context
}{{
name: "do not validate spec on delete",
t: &v1beta1.Task{
ObjectMeta: metav1.ObjectMeta{Name: "task"},
},
wc: apis.WithinDelete,
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
if tt.wc != nil {
ctx = tt.wc(ctx)
}
err := tt.t.Validate(ctx)
if err != nil {
t.Errorf("Task.Validate() returned error for valid Task: %v", err)
}
})
}
}

func TestTaskSpecValidate(t *testing.T) {
type fields struct {
Params []v1beta1.ParamSpec
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ var _ apis.Validatable = (*TaskRun)(nil)
// Validate taskrun
func (tr *TaskRun) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(tr.GetObjectMeta()).ViaField("metadata")
if apis.IsInDelete(ctx) {
return nil
}
return errs.Also(tr.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
}

Expand Down
49 changes: 35 additions & 14 deletions pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ import (

func TestTaskRun_Invalidate(t *testing.T) {
tests := []struct {
name string
task *v1beta1.TaskRun
want *apis.FieldError
name string
taskRun *v1beta1.TaskRun
want *apis.FieldError
}{{
name: "invalid taskspec",
task: &v1beta1.TaskRun{},
name: "invalid taskspec",
taskRun: &v1beta1.TaskRun{},
want: apis.ErrMissingField("spec.taskref.name", "spec.taskspec").Also(
apis.ErrGeneric(`invalid resource name "": must be a valid DNS label`, "metadata.name")),
}}
for _, ts := range tests {
t.Run(ts.name, func(t *testing.T) {
err := ts.task.Validate(context.Background())
err := ts.taskRun.Validate(context.Background())
if d := cmp.Diff(err.Error(), ts.want.Error()); d != "" {
t.Error(diff.PrintWantGot(d))
}
Expand All @@ -52,16 +52,37 @@ func TestTaskRun_Invalidate(t *testing.T) {
}

func TestTaskRun_Validate(t *testing.T) {
tr := &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "taskrname",
tests := []struct {
name string
taskRun *v1beta1.TaskRun
wc func(context.Context) context.Context
}{{
name: "simple taskref",
taskRun: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "taskrname",
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{Name: "taskrefname"},
},
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{Name: "taskrefname"},
}, {
name: "do not validate spec on delete",
taskRun: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{Name: "taskrname"},
},
}
if err := tr.Validate(context.Background()); err != nil {
t.Errorf("TaskRun.Validate() error = %v", err)
wc: apis.WithinDelete,
}}
for _, ts := range tests {
t.Run(ts.name, func(t *testing.T) {
ctx := context.Background()
if ts.wc != nil {
ctx = ts.wc(ctx)
}
if err := ts.taskRun.Validate(ctx); err != nil {
t.Errorf("TaskRun.Validate() error = %v", err)
}
})
}
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/apis/resource/v1alpha1/pipelineresource_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ func (r *PipelineResource) Validate(ctx context.Context) *apis.FieldError {
if err := validate.ObjectMetadata(r.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}

if apis.IsInDelete(ctx) {
return nil
}
return r.Spec.Validate(ctx)
}

Expand Down