Repository: flant/shell-operator
Pull request #483 (2023-05-04 09:23:45)

Add healthz handler
<!--
Thank you for sending a pull request! Here some tips for contributors:

1. Fill the description template below.
2. Include appropriate tests (if necessary). Make sure that all CI checks passed.
3. If the Pull Request is a work in progress, make use of GitHub's "Draft PR" feature and mark it as such.
-->

#### Overview

Dedicate /healthz route for admission and conversion controllers

#### What this PR does / why we need it

Without this route we can do only tcpCheck which will fail on the TCP Handshake.
We want to check service liveness gracefully with extra logs

#### Special notes for your reviewer



Patch:
diff --git a/examples/204-validating-webhook/templates/deployment.yaml b/examples/204-validating-webhook/templates/deployment.yaml
index badd5a7..8711e05 100644
--- a/examples/204-validating-webhook/templates/deployment.yaml
+++ b/examples/204-validating-webhook/templates/deployment.yaml
@@ -34,11 +34,17 @@ spec:
           value: {{ .Values.shellOperator.validatingWebhookServiceName | quote }}
         - name: VALIDATING_WEBHOOK_CONFIGURATION_NAME
           value: {{ .Values.shellOperator.validatingWebhookConfigurationName | quote }}
+        livenessProbe:
+          httpGet:
+            port: 9680
+            path: /healthz
+            scheme: HTTPS
         volumeMounts:
         - name: validating-certs
           mountPath: /validating-certs/
           readOnly: true
       serviceAccountName: example-204-acc
+
       volumes:
       - name: validating-certs
         secret:
diff --git a/examples/206-mutating-webhook/templates/deployment.yaml b/examples/206-mutating-webhook/templates/deployment.yaml
index 9f87971..6460dda 100644
--- a/examples/206-mutating-webhook/templates/deployment.yaml
+++ b/examples/206-mutating-webhook/templates/deployment.yaml
@@ -36,6 +36,11 @@ spec:
           value: {{ .Values.shellOperator.webhookServiceName | quote }}
         - name: VALIDATING_WEBHOOK_CONFIGURATION_NAME
           value: {{ .Values.shellOperator.webhookConfigurationName | quote }}
+        livenessProbe:
+          httpGet:
+            port: 9680
+            path: /healthz
+            scheme: HTTPS
         volumeMounts:
         - name: validating-certs
           mountPath: /validating-certs/
diff --git a/examples/210-conversion-webhook/templates/deployment.yaml b/examples/210-conversion-webhook/templates/deployment.yaml
index 50e175f..b11c1d5 100644
--- a/examples/210-conversion-webhook/templates/deployment.yaml
+++ b/examples/210-conversion-webhook/templates/deployment.yaml
@@ -32,6 +32,11 @@ spec:
               fieldPath: metadata.namespace
         - name: CONVERSION_WEBHOOK_SERVICE_NAME
           value: {{ .Values.shellOperator.conversionWebhookServiceName | quote }}
+        livenessProbe:
+          httpGet:
+            port: 9681
+            path: /healthz
+            scheme: HTTPS
         volumeMounts:
         - name: conversion-certs
           mountPath: /conversion-certs/
diff --git a/pkg/webhook/admission/handler.go b/pkg/webhook/admission/handler.go
index 2b27e1c..9cc6597 100644
--- a/pkg/webhook/admission/handler.go
+++ b/pkg/webhook/admission/handler.go
@@ -29,15 +29,24 @@ func NewWebhookHandler() *WebhookHandler {
 	h := &WebhookHandler{
 		Router: rtr,
 	}
-	rtr.Use(structured_logger.NewStructuredLogger(log.StandardLogger(), "admissionWebhook"))
-	rtr.Use(middleware.Recoverer)
-	rtr.Use(middleware.AllowContentType("application/json"))
-	rtr.Post("/*", h.ServeReviewRequest)
+
+	rtr.Group(func(r chi.Router) {
+		r.Get("/healthz", func(writer http.ResponseWriter, _ *http.Request) {
+			writer.WriteHeader(http.StatusOK)
+		})
+	})
+
+	rtr.Group(func(r chi.Router) {
+		r.Use(structured_logger.NewStructuredLogger(log.StandardLogger(), "admissionWebhook"))
+		r.Use(middleware.Recoverer)
+		r.Use(middleware.AllowContentType("application/json"))
+		r.Post("/*", h.serveReviewRequest)
+	})
 
 	return h
 }
 
-func (h *WebhookHandler) ServeReviewRequest(w http.ResponseWriter, r *http.Request) {
+func (h *WebhookHandler) serveReviewRequest(w http.ResponseWriter, r *http.Request) {
 	defer r.Body.Close()
 
 	bodyBytes, err := io.ReadAll(r.Body)
@@ -48,7 +57,7 @@ func (h *WebhookHandler) ServeReviewRequest(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	admissionResponse, err := h.HandleReviewRequest(r.URL.Path, bodyBytes)
+	admissionResponse, err := h.handleReviewRequest(r.URL.Path, bodyBytes)
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
 		_, _ = w.Write([]byte(err.Error()))
@@ -68,8 +77,8 @@ func (h *WebhookHandler) ServeReviewRequest(w http.ResponseWriter, r *http.Reque
 	_, _ = w.Write(respBytes)
 }
 
-func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.AdmissionReview, error) {
-	configurationID, webhookID := DetectConfigurationAndWebhook(path)
+func (h *WebhookHandler) handleReviewRequest(path string, body []byte) (*v1.AdmissionReview, error) {
+	configurationID, webhookID := detectConfigurationAndWebhook(path)
 	log.Infof("Got AdmissionReview request for confId='%s' webhookId='%s'", configurationID, webhookID)
 
 	var review v1.AdmissionReview
@@ -89,7 +98,7 @@ func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.Admi
 	if h.Handler == nil {
 		response.Response.Allowed = false
 		response.Response.Result = &metav1.Status{
-			Code:    500,
+			Code:    http.StatusInternalServerError,
 			Message: "AdmissionReview handler is not defined",
 		}
 		return response, nil
@@ -105,7 +114,7 @@ func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.Admi
 	if err != nil {
 		response.Response.Allowed = false
 		response.Response.Result = &metav1.Status{
-			Code:    500,
+			Code:    http.StatusInternalServerError,
 			Message: err.Error(),
 		}
 		return response, nil
@@ -118,7 +127,7 @@ func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.Admi
 	if !admissionResponse.Allowed {
 		response.Response.Allowed = false
 		response.Response.Result = &metav1.Status{
-			Code:    403,
+			Code:    http.StatusForbidden,
 			Message: admissionResponse.Message,
 		}
 		return response, nil
@@ -140,10 +149,10 @@ func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.Admi
 	return response, nil
 }
 
-// DetectConfigurationAndWebhook extracts configurationID and a webhookID from the url path.
-func DetectConfigurationAndWebhook(path string) (configurationID string, webhookID string) {
+// detectConfigurationAndWebhook extracts configurationID and a webhookID from the url path.
+func detectConfigurationAndWebhook(path string) (configurationID string, webhookID string) {
 	parts := strings.Split(path, "/")
-	webhookParts := []string{}
+	webhookParts := make([]string, 0)
 	for _, p := range parts {
 		if p == "" {
 			continue
diff --git a/pkg/webhook/conversion/handler.go b/pkg/webhook/conversion/handler.go
index ef3145a..be1eaee 100644
--- a/pkg/webhook/conversion/handler.go
+++ b/pkg/webhook/conversion/handler.go
@@ -10,12 +10,12 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 	log "github.com/sirupsen/logrus"
-	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 
-	"github.com/flant/shell-operator/pkg/utils/structured-logger"
+	structured_logger "github.com/flant/shell-operator/pkg/utils/structured-logger"
 )
 
 type WebhookHandler struct {
@@ -29,15 +29,23 @@ func NewWebhookHandler() *WebhookHandler {
 		Router: rtr,
 	}
 
-	rtr.Use(structured_logger.NewStructuredLogger(log.StandardLogger(), "conversionWebhook"))
-	rtr.Use(middleware.Recoverer)
-	rtr.Use(middleware.AllowContentType("application/json"))
-	rtr.Post("/*", h.ServeReviewRequest)
+	rtr.Group(func(r chi.Router) {
+		r.Get("/healthz", func(writer http.ResponseWriter, _ *http.Request) {
+			writer.WriteHeader(http.StatusOK)
+		})
+	})
+
+	rtr.Group(func(r chi.Router) {
+		r.Use(structured_logger.NewStructuredLogger(log.StandardLogger(), "conversionWebhook"))
+		r.Use(middleware.Recoverer)
+		r.Use(middleware.AllowContentType("application/json"))
+		r.Post("/*", h.serveReviewRequest)
+	})
 
 	return h
 }
 
-func (h *WebhookHandler) ServeReviewRequest(w http.ResponseWriter, r *http.Request) {
+func (h *WebhookHandler) serveReviewRequest(w http.ResponseWriter, r *http.Request) {
 	defer r.Body.Close()
 
 	bodyBytes, err := io.ReadAll(r.Body)
@@ -48,7 +56,7 @@ func (h *WebhookHandler) ServeReviewRequest(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	conversionResponse, err := h.HandleReviewRequest(r.URL.Path, bodyBytes)
+	conversionResponse, err := h.handleReviewRequest(r.URL.Path, bodyBytes)
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
 		_, _ = w.Write([]byte(err.Error()))
@@ -70,8 +78,8 @@ func (h *WebhookHandler) ServeReviewRequest(w http.ResponseWriter, r *http.Reque
 
 // See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#write-a-conversion-webhook-server
 // This code always response with v1 ConversionReview: it works for 1.16+.
-func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.ConversionReview, error) {
-	crdName := DetectCrdName(path)
+func (h *WebhookHandler) handleReviewRequest(path string, body []byte) (*v1.ConversionReview, error) {
+	crdName := detectCrdName(path)
 	log.Infof("Got ConversionReview request for crd/%s", crdName)
 
 	var inReview v1.ConversionReview
@@ -96,7 +104,7 @@ func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.Conv
 		return review, nil
 	}
 
-	event, err := PrepareConversionEvent(crdName, &inReview)
+	event, err := prepareConversionEvent(crdName, &inReview)
 	if err != nil {
 		return nil, err
 	}
@@ -141,15 +149,15 @@ func (h *WebhookHandler) HandleReviewRequest(path string, body []byte) (*v1.Conv
 	return review, nil
 }
 
-// DetectCrdName extracts crdName from the url path.
-func DetectCrdName(path string) string {
+// detectCrdName extracts crdName from the url path.
+func detectCrdName(path string) string {
 	return strings.TrimPrefix(path, "/")
 }
 
-func PrepareConversionEvent(crdName string, review *v1.ConversionReview) (event Event, err error) {
+func prepareConversionEvent(crdName string, review *v1.ConversionReview) (event Event, err error) {
 	event.CrdName = crdName
 	event.Review = review
-	event.Objects, err = RawExtensionToUnstructured(review.Request.Objects)
+	event.Objects, err = rawExtensionToUnstructured(review.Request.Objects)
 	return event, err
 }
 
@@ -165,7 +173,7 @@ func ExtractAPIVersions(objs []unstructured.Unstructured) []string {
 	return res
 }
 
-func RawExtensionToUnstructured(objects []runtime.RawExtension) ([]unstructured.Unstructured, error) {
+func rawExtensionToUnstructured(objects []runtime.RawExtension) ([]unstructured.Unstructured, error) {
 	res := make([]unstructured.Unstructured, 0)
 
 	for _, obj := range objects {


Test patch:
diff --git a/pkg/webhook/admission/handler_test.go b/pkg/webhook/admission/handler_test.go
index ca2893b..926e872 100644
--- a/pkg/webhook/admission/handler_test.go
+++ b/pkg/webhook/admission/handler_test.go
@@ -42,7 +42,7 @@ func Test_DetectConfigurationAndWebhook(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c, w := DetectConfigurationAndWebhook(tt.path)
+			c, w := detectConfigurationAndWebhook(tt.path)
 			if c != tt.expect[0] {
 				t.Fatalf("expected configurationId '%s', got '%s'", tt.expect[0], c)
 			}