diff --git a/webhook/admission.go b/webhook/admission.go index 857987ad3c..d7e241343a 100644 --- a/webhook/admission.go +++ b/webhook/admission.go @@ -17,9 +17,11 @@ limitations under the License. package webhook import ( + "bytes" "context" "encoding/json" "fmt" + "io" "net/http" "strings" "time" @@ -91,10 +93,12 @@ func admissionHandler(rootLogger *zap.SugaredLogger, stats StatsReporter, c Admi logger.Infof("Webhook ServeHTTP request=%#v", r) var review admissionv1.AdmissionReview - if err := json.NewDecoder(r.Body).Decode(&review); err != nil { + bodyBuffer := bytes.Buffer{} + if err := json.NewDecoder(io.TeeReader(r.Body, &bodyBuffer)).Decode(&review); err != nil { http.Error(w, fmt.Sprint("could not decode body:", err), http.StatusBadRequest) return } + r.Body = io.NopCloser(&bodyBuffer) logger = logger.With( logkey.Kind, review.Request.Kind.String(), diff --git a/webhook/admission_integration_test.go b/webhook/admission_integration_test.go index 8e143c9183..caf1760970 100644 --- a/webhook/admission_integration_test.go +++ b/webhook/admission_integration_test.go @@ -60,6 +60,32 @@ func (fac *fixedAdmissionController) Admit(ctx context.Context, req *admissionv1 return fac.response } +type readBodyTwiceAdmissionController struct { + path string + response *admissionv1.AdmissionResponse +} + +var _ AdmissionController = (*readBodyTwiceAdmissionController)(nil) + +func (rbtac *readBodyTwiceAdmissionController) Path() string { + return rbtac.path +} + +func (rbtac *readBodyTwiceAdmissionController) Admit(ctx context.Context, req *admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse { + r := apis.GetHTTPRequest(ctx) + if r == nil { + panic("nil request!") + } else if r.URL.Path != rbtac.path { + panic("wrong path!") + } + + var review admissionv1.AdmissionReview + if err := json.NewDecoder(r.Body).Decode(&review); err != nil { + panic("body closed!") + } + return rbtac.response +} + func TestAdmissionEmptyRequestBody(t *testing.T) { c := &fixedAdmissionController{ path: "/bazinga", @@ -547,3 +573,128 @@ func TestAdmissionWarningResponseForResource(t *testing.T) { } } } + +func TestAdmissionValidResponseForRequestBody(t *testing.T) { + ac := &readBodyTwiceAdmissionController{ + path: "/bazinga", + response: &admissionv1.AdmissionResponse{}, + } + wh, serverURL, ctx, cancel, err := testSetupNoTLS(t, ac) + if err != nil { + t.Fatal("testSetup() =", err) + } + + eg, _ := errgroup.WithContext(ctx) + eg.Go(func() error { return wh.Run(ctx.Done()) }) + defer func() { + cancel() + if err := eg.Wait(); err != nil { + t.Error("Unable to run controller:", err) + } + }() + + pollErr := waitForServerAvailable(t, serverURL, testTimeout) + if pollErr != nil { + t.Fatal("waitForServerAvailable() =", err) + } + client := createNonTLSClient() + + admissionreq := &admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Kind: metav1.GroupVersionKind{ + Group: "pkg.knative.dev", + Version: "v1alpha1", + Kind: "Resource", + }, + } + testRev := createResource("testrev") + marshaled, err := json.Marshal(testRev) + if err != nil { + t.Fatal("Failed to marshal resource:", err) + } + + admissionreq.Resource.Group = "pkg.knative.dev" + admissionreq.Object.Raw = marshaled + rev := &admissionv1.AdmissionReview{ + Request: admissionreq, + } + + reqBuf := new(bytes.Buffer) + err = json.NewEncoder(reqBuf).Encode(&rev) + if err != nil { + t.Fatal("Failed to marshal admission review:", err) + } + + u, err := url.Parse("http://" + serverURL) + if err != nil { + t.Fatal("bad url", err) + } + + u.Path = path.Join(u.Path, ac.Path()) + + req, err := http.NewRequest("GET", u.String(), reqBuf) + if err != nil { + t.Fatal("http.NewRequest() =", err) + } + req.Header.Add("Content-Type", "application/json") + + doneCh := make(chan struct{}) + launchedCh := make(chan struct{}) + go func() { + defer close(doneCh) + + close(launchedCh) + response, err := client.Do(req) + if err != nil { + t.Error("Failed to get response", err) + return + } + + if got, want := response.StatusCode, http.StatusOK; got != want { + t.Errorf("Response status code = %v, wanted %v", got, want) + return + } + + defer response.Body.Close() + responseBody, err := ioutil.ReadAll(response.Body) + if err != nil { + t.Error("Failed to read response body", err) + return + } + + reviewResponse := admissionv1.AdmissionReview{} + + err = json.NewDecoder(bytes.NewReader(responseBody)).Decode(&reviewResponse) + if err != nil { + t.Error("Failed to decode response:", err) + return + } + + if diff := cmp.Diff(rev.TypeMeta, reviewResponse.TypeMeta); diff != "" { + t.Errorf("expected the response typeMeta to be the same as the request (-want, +got)\n%s", diff) + return + } + }() + + // Wait for the goroutine to launch. + <-launchedCh + + // Check that Admit calls block when they are initiated before informers sync. + select { + case <-time.After(100 * time.Millisecond): + case <-doneCh: + t.Fatal("Admit was called before informers had synced.") + } + + // Signal the webhook that informers have synced. + wh.InformersHaveSynced() + + // Check that after informers have synced that things start completing immediately (including outstanding requests). + select { + case <-doneCh: + case <-time.After(5 * time.Second): + t.Error("Timed out waiting on Admit to complete after informers synced.") + } + + metricstest.CheckStatsReported(t, requestCountName, requestLatenciesName) +}