fix(validation): apply SetDefaults to both specs before comparison

pkg/apis/pipeline/v1/pipelinerun_validation.go

@@ -139,6 +139,13 @@ func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.Field
 		return
 	}
 	old := &oldObj.Spec
+	// Apply defaults to both old and new specs before comparison to handle
+	// cases where default field values have changed between API versions.
+	// This prevents upgrade scenarios from being incorrectly flagged as
+	// user modifications when only default values have changed.
+	old.SetDefaults(ctx)
+	psCopy := ps.DeepCopy()
+	psCopy.SetDefaults(ctx)
 
 	// 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"
@@ -147,7 +154,7 @@ func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.Field
 		old.Status = ps.Status
 		tips = "Once the PipelineRun has started, only status updates are allowed"
 	}
-	if !equality.Semantic.DeepEqual(old, ps) {
+	if !equality.Semantic.DeepEqual(old, psCopy) {
 		errs = errs.Also(apis.ErrInvalidValue(tips, ""))
 	}
 

pkg/apis/pipeline/v1/pipelinerun_validation_test.go

@@ -1751,11 +1751,79 @@ func TestPipelineRunSpec_ValidateUpdate(t *testing.T) {
 				Message: `invalid value: Once the PipelineRun is complete, no updates are allowed`,
 				Paths:   []string{""},
 			},
+		}, {
+			name: "update with API version upgrade default changes should not error",
+			// Bug scenario: v1beta1 PipelineRun resources in production after Tekton upgrade
+			// 1. Original v1beta1 PipelineRun had Timeouts: nil (stored in etcd)
+			// 2. User tries to modify annotations via kubectl after upgrade
+			// 3. API server converts v1beta1 -> v1: conversion keeps Timeouts: nil
+			// 4. Mutating webhook applies v1 SetDefaults: Timeouts becomes &TimeoutFields{}
+			// 5. ValidateUpdate compares: old (nil) vs new (&TimeoutFields{}) -> false error
+			// 6. Solution: Apply SetDefaults to both old and new before comparison
+			baselinePipelineRun: &v1.PipelineRun{
+				Spec: v1.PipelineRunSpec{
+					PipelineRef: &v1.PipelineRef{
+						Name: "test",
+					},
+					Timeouts: nil, // Simulates v1beta1 -> v1 converted object (no defaults applied yet)
+				},
+				Status: v1.PipelineRunStatus{
+					Status: duckv1.Status{
+						Conditions: duckv1.Conditions{
+							{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
+						},
+					},
+				},
+			},
+			pipelineRun: &v1.PipelineRun{
+				Spec: v1.PipelineRunSpec{
+					PipelineRef: &v1.PipelineRef{
+						Name: "test",
+					},
+					Timeouts: &v1.TimeoutFields{}, // Simulates object after mutating webhook SetDefaults
+				},
+			},
+			isCreate:      false,
+			isUpdate:      true,
+			expectedError: apis.FieldError{}, // Should NOT error - difference is only due to SetDefaults, not user changes
+		}, {
+			name: "update with actual user timeout change should error",
+			// Test that actual user modifications are still caught correctly
+			baselinePipelineRun: &v1.PipelineRun{
+				Spec: v1.PipelineRunSpec{
+					PipelineRef: &v1.PipelineRef{Name: "test"},
+					Timeouts: &v1.TimeoutFields{
+						Pipeline: &metav1.Duration{Duration: 30 * time.Minute}, // User set this explicitly
+					},
+				},
+				Status: v1.PipelineRunStatus{
+					Status: duckv1.Status{
+						Conditions: duckv1.Conditions{
+							{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
+						},
+					},
+				},
+			},
+			pipelineRun: &v1.PipelineRun{
+				Spec: v1.PipelineRunSpec{
+					PipelineRef: &v1.PipelineRef{Name: "test"},
+					Timeouts: &v1.TimeoutFields{
+						Pipeline: &metav1.Duration{Duration: 45 * time.Minute}, // User changed this
+					},
+				},
+			},
+			isCreate: false,
+			isUpdate: true,
+			expectedError: apis.FieldError{
+				Message: `invalid value: Once the PipelineRun has started, only status updates are allowed`,
+				Paths:   []string{""},
+			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			// Configure context with default resolver to simulate upgrade scenario
 			ctx := config.ToContext(t.Context(), &config.Config{
 				FeatureFlags: &config.FeatureFlags{},
 				Defaults:     &config.Defaults{},
@@ -1769,7 +1837,7 @@ func TestPipelineRunSpec_ValidateUpdate(t *testing.T) {
 			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))
+				t.Errorf("PipelineRunSpec.ValidateUpdate() validation error mismatch (-want +got):\n%s", diff.PrintWantGot(d))
 			}
 		})
 	}

pkg/apis/pipeline/v1/taskrun_validation.go

@@ -131,6 +131,13 @@ func (ts *TaskRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldErro
 		return
 	}
 	old := &oldObj.Spec
+	// Apply defaults to both old and new specs before comparison to handle
+	// cases where default field values have changed between API versions.
+	// This prevents upgrade scenarios from being incorrectly flagged as
+	// user modifications when only default values have changed.
+	old.SetDefaults(ctx)
+	tsCopy := ts.DeepCopy()
+	tsCopy.SetDefaults(ctx)
 
 	// If already in the done state, the spec cannot be modified.
 	// Otherwise, only the status, statusMessage field can be modified.
@@ -142,7 +149,7 @@ func (ts *TaskRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldErro
 		tips = "Once the TaskRun has started, only status and statusMessage updates are allowed"
 	}
 
-	if !equality.Semantic.DeepEqual(old, ts) {
+	if !equality.Semantic.DeepEqual(old, tsCopy) {
 		errs = errs.Also(apis.ErrInvalidValue(tips, ""))
 	}
 

pkg/apis/pipeline/v1/taskrun_validation_test.go

@@ -1107,6 +1107,41 @@ func TestTaskRunSpec_ValidateUpdate(t *testing.T) {
 				Message: `invalid value: Once the TaskRun is complete, no updates are allowed`,
 				Paths:   []string{""},
 			},
+		}, {
+			name: "update with API version upgrade default changes should not error",
+			// Bug scenario: v1beta1 TaskRun resources in production after Tekton upgrade
+			// 1. Original v1beta1 TaskRun had Timeout field or was nil (stored in etcd)
+			// 2. User tries to modify annotations via kubectl after upgrade
+			// 3. API server converts v1beta1 -> v1: timeout field structure may change
+			// 4. Mutating webhook applies v1 SetDefaults: creates default Timeout if nil
+			// 5. ValidateUpdate compares: old (nil Timeout) vs new (default Timeout) -> false error
+			// 6. Solution: Apply SetDefaults to both old and new before comparison
+			baselineTaskRun: &v1.TaskRun{
+				Spec: v1.TaskRunSpec{
+					TaskRef: &v1.TaskRef{
+						Name: "test",
+					},
+					Timeout: nil, // Simulates v1beta1 -> v1 converted object (no defaults applied yet)
+				},
+				Status: v1.TaskRunStatus{
+					Status: duckv1.Status{
+						Conditions: duckv1.Conditions{
+							{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
+						},
+					},
+				},
+			},
+			taskRun: &v1.TaskRun{
+				Spec: v1.TaskRunSpec{
+					TaskRef: &v1.TaskRef{
+						Name: "test",
+					},
+					Timeout: &metav1.Duration{}, // Simulates object after mutating webhook applies SetDefaults
+				},
+			},
+			isCreate:      false,
+			isUpdate:      true,
+			expectedError: apis.FieldError{}, // Should NOT error - difference is only due to SetDefaults, not user changes
 		},
 	}
 

pkg/apis/pipeline/v1beta1/pipelinerun_validation.go

@@ -159,6 +159,13 @@ func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.Field
 		return
 	}
 	old := &oldObj.Spec
+	// Apply defaults to both old and new specs before comparison to handle
+	// cases where default field values have changed between API versions.
+	// This prevents upgrade scenarios from being incorrectly flagged as
+	// user modifications when only default values have changed.
+	old.SetDefaults(ctx)
+	psCopy := ps.DeepCopy()
+	psCopy.SetDefaults(ctx)
 
 	// 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"
@@ -167,7 +174,7 @@ func (ps *PipelineRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.Field
 		old.Status = ps.Status
 		tips = "Once the PipelineRun has started, only status updates are allowed"
 	}
-	if !equality.Semantic.DeepEqual(old, ps) {
+	if !equality.Semantic.DeepEqual(old, psCopy) {
 		errs = errs.Also(apis.ErrInvalidValue(tips, ""))
 	}
 

pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go

@@ -1898,6 +1898,42 @@ func TestPipelineRunSpec_ValidateUpdate(t *testing.T) {
 				Message: `invalid value: Once the PipelineRun is complete, no updates are allowed`,
 				Paths:   []string{""},
 			},
+		}, {
+			name: "update with API version upgrade default changes should not error",
+			// Bug scenario: v1beta1 PipelineRun resources in production after Tekton upgrade
+			// 1. Original v1beta1 PipelineRun had Timeout field (deprecated) or nil Timeouts (stored in etcd)
+			// 2. User tries to modify annotations via kubectl after upgrade
+			// 3. API server processes v1beta1 object with potential field migrations
+			// 4. Mutating webhook applies v1beta1 SetDefaults: may convert Timeout to Timeouts structure
+			// 5. ValidateUpdate compares: old (original structure) vs new (migrated structure) -> false error
+			// 6. Solution: Apply SetDefaults to both old and new before comparison
+			baselinePipelineRun: &v1beta1.PipelineRun{
+				Spec: v1beta1.PipelineRunSpec{
+					PipelineRef: &v1beta1.PipelineRef{
+						Name: "test",
+					},
+					Timeout:  nil, // Simulates legacy v1beta1 object with old timeout structure
+					Timeouts: nil, // No new timeout structure yet
+				},
+				Status: v1beta1.PipelineRunStatus{
+					Status: duckv1.Status{
+						Conditions: duckv1.Conditions{
+							{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
+						},
+					},
+				},
+			},
+			pipelineRun: &v1beta1.PipelineRun{
+				Spec: v1beta1.PipelineRunSpec{
+					PipelineRef: &v1beta1.PipelineRef{
+						Name: "test",
+					},
+					Timeout: &metav1.Duration{}, // Simulates object after SetDefaults migration
+				},
+			},
+			isCreate:      false,
+			isUpdate:      true,
+			expectedError: apis.FieldError{}, // Should NOT error - difference is only due to SetDefaults migration, not user changes
 		},
 	}
 

pkg/apis/pipeline/v1beta1/taskrun_validation.go

@@ -131,6 +131,13 @@ func (ts *TaskRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldErro
 		return
 	}
 	old := &oldObj.Spec
+	// Apply defaults to both old and new specs before comparison to handle
+	// cases where default field values have changed between API versions.
+	// This prevents upgrade scenarios from being incorrectly flagged as
+	// user modifications when only default values have changed.
+	old.SetDefaults(ctx)
+	tsCopy := ts.DeepCopy()
+	tsCopy.SetDefaults(ctx)
 
 	// If already in the done state, the spec cannot be modified.
 	// Otherwise, only the status, statusMessage field can be modified.
@@ -142,7 +149,7 @@ func (ts *TaskRunSpec) ValidateUpdate(ctx context.Context) (errs *apis.FieldErro
 		tips = "Once the TaskRun has started, only status and statusMessage updates are allowed"
 	}
 
-	if !equality.Semantic.DeepEqual(old, ts) {
+	if !equality.Semantic.DeepEqual(old, tsCopy) {
 		errs = errs.Also(apis.ErrInvalidValue(tips, ""))
 	}
 

pkg/apis/pipeline/v1beta1/taskrun_validation_test.go

@@ -1078,6 +1078,41 @@ func TestTaskRunSpec_ValidateUpdate(t *testing.T) {
 				Message: `invalid value: Once the TaskRun is complete, no updates are allowed`,
 				Paths:   []string{""},
 			},
+		}, {
+			name: "update with API version upgrade default changes should not error",
+			// Bug scenario: v1beta1 TaskRun resources in production after Tekton upgrade
+			// 1. Original v1beta1 TaskRun had Timeout field or was nil (stored in etcd)
+			// 2. User tries to modify annotations via kubectl after upgrade
+			// 3. API server processes v1beta1 object with potential timeout field changes
+			// 4. Mutating webhook applies v1beta1 SetDefaults: creates default Timeout if nil
+			// 5. ValidateUpdate compares: old (nil Timeout) vs new (default Timeout) -> false error
+			// 6. Solution: Apply SetDefaults to both old and new before comparison
+			baselineTaskRun: &v1beta1.TaskRun{
+				Spec: v1beta1.TaskRunSpec{
+					TaskRef: &v1beta1.TaskRef{
+						Name: "test",
+					},
+					Timeout: nil, // Simulates legacy v1beta1 object with no timeout set
+				},
+				Status: v1beta1.TaskRunStatus{
+					Status: duckv1.Status{
+						Conditions: duckv1.Conditions{
+							{Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown},
+						},
+					},
+				},
+			},
+			taskRun: &v1beta1.TaskRun{
+				Spec: v1beta1.TaskRunSpec{
+					TaskRef: &v1beta1.TaskRef{
+						Name: "test",
+					},
+					Timeout: &metav1.Duration{}, // Simulates object after mutating webhook applies SetDefaults
+				},
+			},
+			isCreate:      false,
+			isUpdate:      true,
+			expectedError: apis.FieldError{}, // Should NOT error - difference is only due to SetDefaults, not user changes
 		},
 	}