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

fix(pipelinerun): block pipelinerun spec updates once the pipelinerun has started #8149

Merged
merged 1 commit into from
Jul 25, 2024
Merged

fix(pipelinerun): block pipelinerun spec updates once the pipelinerun has started #8149

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
29 changes: 29 additions & 0 deletions pkg/apis/pipeline/v1/pipelinerun_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/validate"
"github.com/tektoncd/pipeline/pkg/internal/resultref"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/apis"
"knative.dev/pkg/webhook/resourcesemantics"
Expand Down Expand Up @@ -55,6 +56,9 @@ func (pr *PipelineRun) Validate(ctx context.Context) *apis.FieldError {

// Validate pipelinerun spec
func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
// Validate the spec changes
errs = errs.Also(ps.ValidateUpdate(ctx))

// Must have exactly one of pipelineRef and pipelineSpec.
if ps.PipelineRef == nil && ps.PipelineSpec == nil {
errs = errs.Also(apis.ErrMissingOneOf("pipelineRef", "pipelineSpec"))
Expand Down Expand Up @@ -124,6 +128,31 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError)
return errs
}

// ValidateUpdate validates the update of a PipelineRunSpec
func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldError) {
if !apis.IsInUpdate(ctx) {
return
}
oldObj, ok := apis.GetBaseline(ctx).(*PipelineRun)
if !ok || oldObj == nil {
return
}
old := &oldObj.Spec

// If already in the done state, the spec cannot be modified. Otherwise, only the status field can be modified.
tips := "Once the PipelineRun is complete, no updates are allowed"
if !oldObj.IsDone() {
old = old.DeepCopy()
old.Status = ps.Status
tips = "Once the PipelineRun has started, only status updates are allowed"
}
if !equality.Semantic.DeepEqual(old, ps) {
errs = errs.Also(apis.ErrInvalidValue(tips, ""))
}

return
}

func (ps *PipelineRunSpec) validatePipelineRunParameters(ctx context.Context) (errs *apis.FieldError) {
if len(ps.Params) == 0 {
return errs
Expand Down
179 changes: 179 additions & 0 deletions pkg/apis/pipeline/v1/pipelinerun_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/pipeline/pkg/apis/config"
cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
Expand All @@ -31,6 +32,7 @@ import (
corev1resources "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
)

func TestPipelineRun_Invalid(t *testing.T) {
Expand Down Expand Up @@ -1511,3 +1513,180 @@ func TestPipelineRunSpecBetaFeatures(t *testing.T) {
})
}
}

func TestPipelineRunSpec_ValidateUpdate(t *testing.T) {
tests := []struct {
name string
isCreate bool
isUpdate bool
baselinePipelineRun *v1.PipelineRun
pipelineRun *v1.PipelineRun
expectedError apis.FieldError
}{
{
name: "is create ctx",
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{},
},
isCreate: true,
isUpdate: false,
expectedError: apis.FieldError{},
}, {
name: "is update ctx, no changes",
baselinePipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "",
},
},
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "",
},
},
isCreate: false,
isUpdate: true,
expectedError: apis.FieldError{},
}, {
name: "is update ctx, baseline is nil, skip validation",
baselinePipelineRun: nil,
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Timeouts: &v1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 1},
},
},
},
isCreate: false,
isUpdate: true,
expectedError: apis.FieldError{},
}, {
name: "is update ctx, baseline is unknown, status changes from Empty to Cancelled",
baselinePipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "",
},
Status: v1.PipelineRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
},
},
},
},
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "Cancelled",
},
},
isCreate: false,
isUpdate: true,
expectedError: apis.FieldError{},
}, {
name: "is update ctx, baseline is unknown, timeouts changes",
baselinePipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "",
Timeouts: &v1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 0},
},
},
Status: v1.PipelineRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
},
},
},
},
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Timeouts: &v1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 1},
},
},
},
isCreate: false,
isUpdate: true,
expectedError: apis.FieldError{
Message: `invalid value: Once the PipelineRun has started, only status updates are allowed`,
Paths: []string{""},
},
}, {
name: "is update ctx, baseline is unknown, status changes from PipelineRunPending to Empty, and timeouts changes",
baselinePipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "PipelineRunPending",
Timeouts: &v1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 0},
},
},
Status: v1.PipelineRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{
{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
},
},
},
},
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "",
Timeouts: &v1.TimeoutFields{
Pipeline: &metav1.Duration{Duration: 1},
},
},
},
isCreate: false,
isUpdate: true,
expectedError: apis.FieldError{
Message: `invalid value: Once the PipelineRun has started, only status updates are allowed`,
Paths: []string{""},
},
}, {
name: "is update ctx, baseline is done, status changes",
baselinePipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "PipelineRunPending",
},
Status: v1.PipelineRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{
{Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue},
},
},
},
},
pipelineRun: &v1.PipelineRun{
Spec: v1.PipelineRunSpec{
Status: "TaskRunCancelled",
},
},
isCreate: false,
isUpdate: true,
expectedError: apis.FieldError{
Message: `invalid value: Once the PipelineRun is complete, no updates are allowed`,
Paths: []string{""},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := config.ToContext(context.Background(), &config.Config{
FeatureFlags: &config.FeatureFlags{},
Defaults: &config.Defaults{},
})
if tt.isCreate {
ctx = apis.WithinCreate(ctx)
}
if tt.isUpdate {
ctx = apis.WithinUpdate(ctx, tt.baselinePipelineRun)
}
pr := tt.pipelineRun
err := pr.Spec.ValidateUpdate(ctx)
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
t.Errorf("PipelineRunSpec.ValidateUpdate() errors diff %s", diff.PrintWantGot(d))
}
})
}
}
29 changes: 29 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/validate"
"github.com/tektoncd/pipeline/pkg/internal/resultref"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/utils/strings/slices"
Expand Down Expand Up @@ -60,6 +61,9 @@ func (pr *PipelineRun) Validate(ctx context.Context) *apis.FieldError {

// Validate pipelinerun spec
func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
// Validate the spec changes
errs = errs.Also(ps.ValidateUpdate(ctx))

// Must have exactly one of pipelineRef and pipelineSpec.
if ps.PipelineRef == nil && ps.PipelineSpec == nil {
errs = errs.Also(apis.ErrMissingOneOf("pipelineRef", "pipelineSpec"))
Expand Down Expand Up @@ -145,6 +149,31 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError)
return errs
}

// ValidateUpdate validates the update of a PipelineRunSpec
func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldError) {
if !apis.IsInUpdate(ctx) {
return
}
oldObj, ok := apis.GetBaseline(ctx).(*PipelineRun)
if !ok || oldObj == nil {
return
}
old := &oldObj.Spec

// If already in the done state, the spec cannot be modified. Otherwise, only the status field can be modified.
tips := "Once the PipelineRun is complete, no updates are allowed"
if !oldObj.IsDone() {
old = old.DeepCopy()
old.Status = ps.Status
tips = "Once the PipelineRun has started, only status updates are allowed"
}
if !equality.Semantic.DeepEqual(old, ps) {
errs = errs.Also(apis.ErrInvalidValue(tips, ""))
}

return
}

func (ps *PipelineRunSpec) validatePipelineRunParameters(ctx context.Context) (errs *apis.FieldError) {
if len(ps.Params) == 0 {
return errs
Expand Down
Loading