diff --git a/apis/contexts.go b/apis/contexts.go index b9835fc00..a3550dceb 100644 --- a/apis/contexts.go +++ b/apis/contexts.go @@ -85,14 +85,21 @@ func IsInUpdate(ctx context.Context) bool { return ctx.Value(inUpdateKey{}) != nil } -// IsInStatusUpdate checks whether the context is an Update. -func IsInStatusUpdate(ctx context.Context) bool { +// GetUpdatedSubresource returns the subresource being updated or "" if there +// is no subresource that's being updated. Examples are "status" for Status +// updates, or "scale" for scaling Deployment. +func GetUpdatedSubresource(ctx context.Context) string { value := ctx.Value(inUpdateKey{}) if value == nil { - return false + return "" } up := value.(*updatePayload) - return up.subresource == "status" + return up.subresource +} + +// IsInStatusUpdate checks whether the context is an Update. +func IsInStatusUpdate(ctx context.Context) bool { + return GetUpdatedSubresource(ctx) == "status" } // GetBaseline returns the baseline of the update, or nil when we diff --git a/testing/resource.go b/testing/resource.go index e26ca317e..97d424b3b 100644 --- a/testing/resource.go +++ b/testing/resource.go @@ -55,6 +55,9 @@ type ResourceSpec struct { FieldForCallbackDefaultingUsername string `json:"fieldForCallbackDefaultingUsername,omitempty"` } +// Reject any udpates to this subresource. +const disallowedSubresource = "badbadsubresource" + // GetGroupVersionKind returns the GroupVersionKind. func (r *Resource) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("Resource") @@ -86,6 +89,7 @@ func (r *Resource) Validate(ctx context.Context) *apis.FieldError { if apis.IsInUpdate(ctx) { original := apis.GetBaseline(ctx).(*Resource) err = err.Also(r.CheckImmutableFields(ctx, original)) + err = err.Also(r.CheckAllowedSubresourceUpdate(ctx, original)) } return err } @@ -146,6 +150,16 @@ func (r *Resource) CheckImmutableFields(ctx context.Context, original *Resource) return nil } +func (r *Resource) CheckAllowedSubresourceUpdate(ctx context.Context, original *Resource) *apis.FieldError { + if apis.GetUpdatedSubresource(ctx) == disallowedSubresource { + return &apis.FieldError{ + Message: "Disallowed subresource update", + Details: fmt.Sprintf("Disallowed subresource update: %s", disallowedSubresource), + } + } + return nil +} + // GetListType implements apis.Listable func (r *Resource) GetListType() runtime.Object { return &ResourceList{} diff --git a/webhook/resourcesemantics/validation/validation_admit.go b/webhook/resourcesemantics/validation/validation_admit.go index d1e7da94d..8ac55e805 100644 --- a/webhook/resourcesemantics/validation/validation_admit.go +++ b/webhook/resourcesemantics/validation/validation_admit.go @@ -141,13 +141,13 @@ func (ac *reconciler) decodeRequestAndPrepareContext( ctx = apis.WithDryRun(ctx) } - if newObj != nil && oldObj != nil && req.SubResource == "" { - ctx = apis.WithinSubResourceUpdate(ctx, oldObj, req.SubResource) - } - switch req.Operation { case admissionv1.Update: - ctx = apis.WithinUpdate(ctx, oldObj) + if req.SubResource != "" { + ctx = apis.WithinSubResourceUpdate(ctx, oldObj, req.SubResource) + } else { + ctx = apis.WithinUpdate(ctx, oldObj) + } case admissionv1.Create: ctx = apis.WithinCreate(ctx) case admissionv1.Delete: diff --git a/webhook/resourcesemantics/validation/validation_admit_test.go b/webhook/resourcesemantics/validation/validation_admit_test.go index 43d049b12..b63e5788d 100644 --- a/webhook/resourcesemantics/validation/validation_admit_test.go +++ b/webhook/resourcesemantics/validation/validation_admit_test.go @@ -468,10 +468,11 @@ func createCreateResource(ctx context.Context, t *testing.T, r *Resource) *admis func TestAdmitUpdates(t *testing.T) { tests := []struct { - name string - setup func(context.Context, *Resource) - mutate func(context.Context, *Resource) - rejection string + name string + setup func(context.Context, *Resource) + mutate func(context.Context, *Resource) + subresource string + rejection string }{{ name: "test simple update (no diff)", setup: func(ctx context.Context, r *Resource) { @@ -499,6 +500,23 @@ func TestAdmitUpdates(t *testing.T) { r.Spec.FieldWithValidation = "not what's expected" }, rejection: "invalid value", + }, { + name: "bad mutation (invalid subresource)", + setup: func(ctx context.Context, r *Resource) { + r.SetDefaults(ctx) + }, + mutate: func(ctx context.Context, r *Resource) { + }, + subresource: "badbadsubresource", + rejection: "validation failed: Disallowed subresource update: \nDisallowed subresource update: badbadsubresource", + }, { + name: "good mutation with valid subresource", + setup: func(ctx context.Context, r *Resource) { + r.SetDefaults(ctx) + }, + mutate: func(ctx context.Context, r *Resource) { + }, + subresource: "goodgoodsubresource", }} for _, tc := range tests { @@ -521,7 +539,7 @@ func TestAdmitUpdates(t *testing.T) { tc.mutate(ctx, new) _, ac := newNonRunningTestResourceAdmissionController(t) - resp := ac.Admit(ctx, createUpdateResource(ctx, t, old, new)) + resp := ac.Admit(ctx, createUpdateResource(ctx, t, old, new, tc.subresource)) if tc.rejection == "" { ExpectAllowed(t, resp) @@ -532,7 +550,7 @@ func TestAdmitUpdates(t *testing.T) { } } -func createUpdateResource(ctx context.Context, t *testing.T, old, new *Resource) *admissionv1.AdmissionRequest { +func createUpdateResource(ctx context.Context, t *testing.T, old, new *Resource, subresource string) *admissionv1.AdmissionRequest { t.Helper() req := &admissionv1.AdmissionRequest{ Operation: admissionv1.Update, @@ -541,7 +559,8 @@ func createUpdateResource(ctx context.Context, t *testing.T, old, new *Resource) Version: "v1alpha1", Kind: "Resource", }, - UserInfo: *apis.GetUserInfo(ctx), + UserInfo: *apis.GetUserInfo(ctx), + SubResource: subresource, } marshaled, err := json.Marshal(new) if err != nil { diff --git a/webhook/resourcesemantics/validation/validation_deprecated_test.go b/webhook/resourcesemantics/validation/validation_deprecated_test.go index 0998c7f65..c973e20f0 100644 --- a/webhook/resourcesemantics/validation/validation_deprecated_test.go +++ b/webhook/resourcesemantics/validation/validation_deprecated_test.go @@ -47,7 +47,7 @@ func TestStrictValidation(t *testing.T) { newUpdateReq := func(old, new []byte) *admissionv1.AdmissionRequest { req := &admissionv1.AdmissionRequest{ - Operation: admissionv1.Create, + Operation: admissionv1.Update, Kind: metav1.GroupVersionKind{ Group: "pkg.knative.dev", Version: "v1alpha1",