diff --git a/pkg/http/handler.go b/pkg/http/handler.go index e19c005..3986f60 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -278,6 +278,15 @@ func (h handler) validatingModelResponseToJSON(ctx context.Context, review model } func (h handler) mutatingModelResponseToJSON(ctx context.Context, review model.AdmissionReview, resp *model.MutatingAdmissionResponse) (data []byte, err error) { + var resultStatus *metav1.Status + if resp.Rejected { + resultStatus = &metav1.Status{ + Message: resp.Message, + Status: metav1.StatusFailure, + Code: http.StatusBadRequest, + } + } + switch review.OriginalAdmissionReview.(type) { case *admissionv1beta1.AdmissionReview: if len(resp.Warnings) > 0 { @@ -290,7 +299,8 @@ func (h handler) mutatingModelResponseToJSON(ctx context.Context, review model.A UID: types.UID(review.ID), PatchType: v1beta1JSONPatchType, Patch: resp.JSONPatchPatch, - Allowed: true, + Allowed: !resp.Rejected, + Result: resultStatus, }, }) return data, err @@ -302,8 +312,9 @@ func (h handler) mutatingModelResponseToJSON(ctx context.Context, review model.A UID: types.UID(review.ID), PatchType: v1JSONPatchType, Patch: resp.JSONPatchPatch, - Allowed: true, + Allowed: !resp.Rejected, Warnings: resp.Warnings, + Result: resultStatus, }, }) diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index ba07c60..124fd41 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -174,13 +174,29 @@ func TestDefaultWebhookFlow(t *testing.T) { expCode: 200, }, + "A correct rejected mutating admission v1beta1 webhook without mutation should not fail.": { + body: getTestAdmissionReviewV1beta1RequestStr("1234567890"), + mock: func(mw *webhookmock.Webhook) { + resp := &model.MutatingAdmissionResponse{ + ID: "1234567890", + Rejected: true, + Message: "this is not valid because reasons", + JSONPatchPatch: []byte(``), + Warnings: []string{"warn1", "warn2"}, // v1beta1 ignores warnings. + } + mw.On("Review", mock.Anything, mock.Anything).Once().Return(resp, nil) + }, + expBody: `{"kind":"AdmissionReview","apiVersion":"admission.k8s.io/v1beta1","response":{"uid":"1234567890","allowed":false,"status":{"metadata":{},"status":"Failure","message":"this is not valid because reasons","code":400},"patchType":"JSONPatch"}}`, + expCode: 200, + }, + "A correct validation admission v1 webhook that allows should not fail.": { body: getTestAdmissionReviewV1RequestStr("1234567890"), mock: func(mw *webhookmock.Webhook) { resp := &model.ValidatingAdmissionResponse{ ID: "1234567890", Allowed: true, - Warnings: []string{"warn1", "warn2"}, // v1beta1 ignores warnings. + Warnings: []string{"warn1", "warn2"}, // v1 includes warnings. } mw.On("Review", mock.Anything, mock.Anything).Once().Return(resp, nil) }, @@ -195,7 +211,7 @@ func TestDefaultWebhookFlow(t *testing.T) { ID: "1234567890", Allowed: false, Message: "this is not valid because reasons", - Warnings: []string{"warn1", "warn2"}, // v1beta1 ignores warnings. + Warnings: []string{"warn1", "warn2"}, // v1 includes warnings. } mw.On("Review", mock.Anything, mock.Anything).Once().Return(resp, nil) }, @@ -209,7 +225,7 @@ func TestDefaultWebhookFlow(t *testing.T) { resp := &model.MutatingAdmissionResponse{ ID: "1234567890", JSONPatchPatch: []byte(`{"something": something}`), - Warnings: []string{"warn1", "warn2"}, // v1beta1 ignores warnings. + Warnings: []string{"warn1", "warn2"}, // v1 includes warnings. } mw.On("Review", mock.Anything, mock.Anything).Once().Return(resp, nil) }, @@ -223,7 +239,7 @@ func TestDefaultWebhookFlow(t *testing.T) { resp := &model.MutatingAdmissionResponse{ ID: "1234567890", JSONPatchPatch: []byte(``), - Warnings: []string{"warn1", "warn2"}, // v1beta1 ignores warnings. + Warnings: []string{"warn1", "warn2"}, // v1 includes warnings. } mw.On("Review", mock.Anything, mock.Anything).Once().Return(resp, nil) }, @@ -231,6 +247,22 @@ func TestDefaultWebhookFlow(t *testing.T) { expCode: 200, }, + "A correct rejected mutating admission v1 webhook without mutation should not fail.": { + body: getTestAdmissionReviewV1RequestStr("1234567890"), + mock: func(mw *webhookmock.Webhook) { + resp := &model.MutatingAdmissionResponse{ + ID: "1234567890", + Rejected: true, + Message: "this is not valid because reasons", + JSONPatchPatch: []byte(``), + Warnings: []string{"warn1", "warn2"}, // v1 includes warnings. + } + mw.On("Review", mock.Anything, mock.Anything).Once().Return(resp, nil) + }, + expBody: `{"kind":"AdmissionReview","apiVersion":"admission.k8s.io/v1","response":{"uid":"1234567890","allowed":false,"status":{"metadata":{},"status":"Failure","message":"this is not valid because reasons","code":400},"patchType":"JSONPatch","warnings":["warn1","warn2"]}}`, + expCode: 200, + }, + "A regular mutating admission v1beta1 call to the webhook handler should execute the webhook and return error if something failed": { body: getTestAdmissionReviewV1beta1RequestStr("1234567890"), mock: func(mw *webhookmock.Webhook) { diff --git a/pkg/model/response.go b/pkg/model/response.go index 951df7f..af7f2b9 100644 --- a/pkg/model/response.go +++ b/pkg/model/response.go @@ -21,6 +21,8 @@ type MutatingAdmissionResponse struct { admissionResponse ID string + Rejected bool + Message string JSONPatchPatch []byte Warnings []string } diff --git a/pkg/webhook/mutating/mutator.go b/pkg/webhook/mutating/mutator.go index 673c773..77363b1 100644 --- a/pkg/webhook/mutating/mutator.go +++ b/pkg/webhook/mutating/mutator.go @@ -14,6 +14,12 @@ import ( type MutatorResult struct { // StopChain will stop the chain of validators in case there is a chain set. StopChain bool + // Reject tells the apiserver that the resource is incorrect and it should reject the request. + // Contrary to ValidatorResult, Reject is used instead of Allowed so that the default value is to allow. + // If Reject is true, this implies StopChain. + Reject bool + // Message will be used by the apiserver to give more information in case the resource is not valid. + Message string // MutatedObject is the object that has been mutated. If is nil, it will be used the one // received by the Mutator. MutatedObject metav1.Object @@ -83,7 +89,7 @@ func (c *Chain) Mutate(ctx context.Context, ar *model.AdmissionReview, obj metav obj = res.MutatedObject } - if res.StopChain { + if res.StopChain || res.Reject { res.Warnings = warnings return res, nil } diff --git a/pkg/webhook/mutating/webhook.go b/pkg/webhook/mutating/webhook.go index 8ee8e3d..95931d8 100644 --- a/pkg/webhook/mutating/webhook.go +++ b/pkg/webhook/mutating/webhook.go @@ -146,6 +146,8 @@ func (w mutatingWebhook) mutatingAdmissionReview(ctx context.Context, ar model.A // Forge response. return &model.MutatingAdmissionResponse{ ID: ar.ID, + Rejected: res.Reject, + Message: res.Message, JSONPatchPatch: marshalledPatch, Warnings: res.Warnings, }, nil