diff --git a/pkg/apis/autoscaling/v1alpha1/pa_types.go b/pkg/apis/autoscaling/v1alpha1/pa_types.go index 00217b896494..ea94b23e7635 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_types.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_types.go @@ -50,7 +50,7 @@ type PodAutoscaler struct { // Verify that PodAutoscaler adheres to the appropriate interfaces. var ( - // Check that PodAutoscaler can be validated, can be defaulted, and has immutable fields. + // Check that PodAutoscaler can be validated and can be defaulted. _ apis.Validatable = (*PodAutoscaler)(nil) _ apis.Defaultable = (*PodAutoscaler)(nil) diff --git a/pkg/apis/autoscaling/v1alpha1/pa_validation.go b/pkg/apis/autoscaling/v1alpha1/pa_validation.go index 3ef3262809aa..97d09aa49524 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_validation.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_validation.go @@ -31,37 +31,7 @@ import ( func (pa *PodAutoscaler) Validate(ctx context.Context) *apis.FieldError { errs := serving.ValidateObjectMetadata(pa.GetObjectMeta()).ViaField("metadata") errs = errs.Also(pa.validateMetric()) - errs = errs.Also(pa.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec")) - if apis.IsInUpdate(ctx) { - original := apis.GetBaseline(ctx).(*PodAutoscaler) - errs = errs.Also(pa.checkImmutableFields(ctx, original)) - } - return errs -} - -func (current *PodAutoscaler) checkImmutableFields(ctx context.Context, original *PodAutoscaler) *apis.FieldError { - if diff, err := compareSpec(original, current); err != nil { - return &apis.FieldError{ - Message: "Failed to diff PodAutoscaler", - Paths: []string{"spec"}, - Details: err.Error(), - } - } else if diff != "" { - return &apis.FieldError{ - Message: "Immutable fields changed (-old +new)", - Paths: []string{"spec"}, - Details: diff, - } - } - // Verify the PA class does not change. - // For backward compatibility, we allow a new class where there was none before. - if oldClass, newClass, annotationChanged := classAnnotationChanged(original, current); annotationChanged { - return &apis.FieldError{ - Message: fmt.Sprintf("Immutable class annotation changed (-%q +%q)", oldClass, newClass), - Paths: []string{"annotations[autoscaling.knative.dev/class]"}, - } - } - return nil + return errs.Also(pa.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec")) } // Validate validates PodAutoscaler Spec. diff --git a/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go b/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go index 7b6e18041944..5711ef0ec31b 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go @@ -238,192 +238,3 @@ func TestPodAutoscalerValidation(t *testing.T) { }) } } - -func TestImmutableFields(t *testing.T) { - tests := []struct { - name string - new *PodAutoscaler - old *PodAutoscaler - want *apis.FieldError - }{{ - name: "good (no change)", - new: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - old: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - want: nil, - }, { - name: "good (protocol added)", - new: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - ProtocolType: net.ProtocolHTTP1, - }, - }, - old: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - want: nil, - }, { - name: "bad (concurrency model change)", - new: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - old: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ContainerConcurrency: 1, - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - want: &apis.FieldError{ - Message: "Immutable fields changed (-old +new)", - Paths: []string{"spec"}, - Details: `{v1alpha1.PodAutoscalerSpec}.ContainerConcurrency: - -: "1" - +: "0" -`, - }, - }, { - name: "bad (container concurrency change)", - new: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ContainerConcurrency: 0, - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - old: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ContainerConcurrency: 1, - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - want: &apis.FieldError{ - Message: "Immutable fields changed (-old +new)", - Paths: []string{"spec"}, - Details: `{v1alpha1.PodAutoscalerSpec}.ContainerConcurrency: - -: "1" - +: "0" -`, - }, - }, { - name: "bad (multiple changes)", - new: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - DeprecatedServiceName: "foo", - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "bar", - }, - }, - }, - old: &PodAutoscaler{ - ObjectMeta: v1.ObjectMeta{ - Name: "valid", - }, - Spec: PodAutoscalerSpec{ - ContainerConcurrency: 1, - DeprecatedServiceName: "food", - ScaleTargetRef: corev1.ObjectReference{ - APIVersion: "apps/v1", - Kind: "Deployment", - Name: "baz", - }, - }, - }, - want: &apis.FieldError{ - Message: "Immutable fields changed (-old +new)", - Paths: []string{"spec"}, - Details: `{v1alpha1.PodAutoscalerSpec}.ContainerConcurrency: - -: "1" - +: "0" -{v1alpha1.PodAutoscalerSpec}.ScaleTargetRef.Name: - -: "baz" - +: "bar" -{v1alpha1.PodAutoscalerSpec}.DeprecatedServiceName: - -: "food" - +: "foo" -`, - }, - }} - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - ctx := context.Background() - ctx = apis.WithinUpdate(ctx, test.old) - got := test.new.Validate(ctx) - if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { - t.Errorf("Validate (-want, +got) = %v", diff) - } - }) - } -}