Fix error handling in wehbook server [TOB-LNKD-9] (#7882)

When the webhook server decodes a request's JSON payload, it may try to
use a nil value when handling the error. Furthermore, if the JSON
payload has a `nil` `Request` value, it may attempt to dereference the
value.

This change improves the webhook server's error handling to return a
`400 Bad Request` status if either of these cases are encountered.

Signed-off-by: Oliver Gould <ver@buoyant.io>

Author: olix0r

controller/webhook/server.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/tls"
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"sync/atomic"
@@ -135,7 +136,12 @@ func (s *Server) serve(res http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	response := s.processReq(req.Context(), data)
+	response, err := s.processReq(req.Context(), data)
+	if err != nil {
+		http.Error(res, err.Error(), http.StatusBadRequest)
+		return
+	}
+
 	responseJSON, err := json.Marshal(response)
 	if err != nil {
 		http.Error(res, err.Error(), http.StatusInternalServerError)
@@ -148,20 +154,15 @@ func (s *Server) serve(res http.ResponseWriter, req *http.Request) {
 	}
 }
 
-func (s *Server) processReq(ctx context.Context, data []byte) *admissionv1beta1.AdmissionReview {
+func (s *Server) processReq(ctx context.Context, data []byte) (*admissionv1beta1.AdmissionReview, error) {
 	admissionReview, err := decode(data)
 	if err != nil {
-		log.Errorf("failed to decode data. Reason: %s", err)
-		admissionReview.Response = &admissionv1beta1.AdmissionResponse{
-			UID:     admissionReview.Request.UID,
-			Allowed: false,
-			Result: &metav1.Status{
-				Message: err.Error(),
-			},
-		}
-		return admissionReview
+		return nil, fmt.Errorf("failed to decode admission review request: %w", err)
+	}
+	if admissionReview.Request == nil || admissionReview.Request.UID == "" {
+		return nil, fmt.Errorf("invalid admission review request")
 	}
-	log.Infof("received admission review request %s", admissionReview.Request.UID)
+	log.Infof("received admission review request %q", admissionReview.Request.UID)
 	log.Debugf("admission request: %+v", admissionReview.Request)
 
 	admissionResponse, err := s.handler(ctx, s.api, admissionReview.Request, s.recorder)
@@ -174,11 +175,11 @@ func (s *Server) processReq(ctx context.Context, data []byte) *admissionv1beta1.
 				Message: err.Error(),
 			},
 		}
-		return admissionReview
+		return admissionReview, nil
 	}
 	admissionReview.Response = admissionResponse
 
-	return admissionReview
+	return admissionReview, nil
 }
 
 // Shutdown initiates a graceful shutdown of the underlying HTTP server.