Skip to content

Commit

Permalink
fix(pipelinerun): block pipelinerun spec updates once the pipelinerun…
Browse files Browse the repository at this point in the history 
… has started

Typically, the spec field of a PipelineRun resource is not allowed to be updated
after creation, such as the `timeouts` configuration.
Only a few fields, such as `status`, are allowed to be updated.
  • Loading branch information
@l-qing @tekton-robot
l-qing authored and tekton-robot committed Jul 25, 2024
1 parent d6a2cdb commit 95fbf31
Show file tree
Hide file tree
Showing 4 changed files with 415 additions and 0 deletions.
29 changes: 29 additions & 0 deletions pkg/apis/pipeline/v1/pipelinerun_validation.go
View file
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
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
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

0 comments on commit 95fbf31

Please sign in to comment.