From 646368a0b2727f02fbd774515363ccc87aec6544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Fri, 25 Oct 2024 11:26:34 +0200 Subject: [PATCH] refactor: sidecar injector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .github/workflows/tests.yaml | 10 +-- .vscode/launch.json | 1 - .../templates/_helpers/_deployment.tpl | 10 +++ .../sidecar-injector/deployment.yaml | 8 ++ charts/kyverno-envoy-plugin/values.yaml | 10 ++- pkg/commands/inject/command.go | 89 +++++++++++++++++-- pkg/httpd/simpleserver.go | 70 --------------- pkg/server/handlers/admission.go | 36 ++++++++ pkg/signals/context.go | 22 ++++- pkg/webhook/health.go | 10 --- .../{ => sidecar-injector}/chainsaw-test.yaml | 0 .../{ => sidecar-injector}/deployment.yaml | 0 12 files changed, 165 insertions(+), 101 deletions(-) create mode 100644 charts/kyverno-envoy-plugin/templates/_helpers/_deployment.tpl delete mode 100644 pkg/httpd/simpleserver.go create mode 100644 pkg/server/handlers/admission.go delete mode 100644 pkg/webhook/health.go rename tests/e2e-test/{ => sidecar-injector}/chainsaw-test.yaml (100%) rename tests/e2e-test/{ => sidecar-injector}/deployment.yaml (100%) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ea6dde69..8c31f4d0 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -86,16 +86,16 @@ jobs: with: go-version-file: go.mod cache-dependency-path: go.sum - - name: Run tests - run: | - set -e - make kind-create-cluster - make kind-load-taged-image - name: Install Cosign uses: sigstore/cosign-installer@dc72c7d5c4d10cd6bcb8cf6e3fd625a9e5e537da # v3.7.0 - name: Install chainsaw uses: kyverno/action-install-chainsaw@d311eacde764f806c9658574ff64c9c3b21f8397 # v0.2.11 with: verify: true + - name: Run tests + run: | + set -e + make kind-create-cluster + make chart-install - name: Run Chainsaw Tests run: chainsaw test tests/e2e-test diff --git a/.vscode/launch.json b/.vscode/launch.json index 404706c0..5506068c 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -19,7 +19,6 @@ "program": "${workspaceFolder}", "args": [ "sidecar-injector", - "--local" ], } ] diff --git a/charts/kyverno-envoy-plugin/templates/_helpers/_deployment.tpl b/charts/kyverno-envoy-plugin/templates/_helpers/_deployment.tpl new file mode 100644 index 00000000..5898ed08 --- /dev/null +++ b/charts/kyverno-envoy-plugin/templates/_helpers/_deployment.tpl @@ -0,0 +1,10 @@ +{{/* vim: set filetype=mustache: */}} + +{{- define "kyverno.deployment.replicas" -}} + {{- if and (not (kindIs "invalid" .)) (not (kindIs "string" .)) -}} + {{- if eq (int .) 0 -}} + {{- fail "Kyverno does not support running with 0 replicas. Please provide a non-zero integer value." -}} + {{- end -}} + {{- end -}} + {{- . -}} +{{- end -}} diff --git a/charts/kyverno-envoy-plugin/templates/sidecar-injector/deployment.yaml b/charts/kyverno-envoy-plugin/templates/sidecar-injector/deployment.yaml index 30f92053..1ddb5ba2 100644 --- a/charts/kyverno-envoy-plugin/templates/sidecar-injector/deployment.yaml +++ b/charts/kyverno-envoy-plugin/templates/sidecar-injector/deployment.yaml @@ -74,6 +74,10 @@ spec: {{- tpl (toYaml .) $ | nindent 10 }} {{- end }} serviceAccountName: {{ template "kyverno.sidecar-injector.service-account.name" . }} + volumes: + - name: certs + secret: + secretName: {{ template "kyverno.sidecar-injector.name" . }}.{{ template "kyverno.namespace" . }}.svc.kyverno-tls-pair containers: {{- with .Values.sidecarInjector.containers.injector }} - name: injector @@ -107,5 +111,9 @@ spec: args: {{- tpl (toYaml .) $ | nindent 12 }} {{- end }} + volumeMounts: + - name: certs + mountPath: /opt/kubernetes-sidecar-injector/certs + readOnly: true {{- end }} {{- end -}} diff --git a/charts/kyverno-envoy-plugin/values.yaml b/charts/kyverno-envoy-plugin/values.yaml index 192be520..1498e31b 100644 --- a/charts/kyverno-envoy-plugin/values.yaml +++ b/charts/kyverno-envoy-plugin/values.yaml @@ -165,7 +165,7 @@ sidecarInjector: # @default -- See [values.yaml](values.yaml) startupProbe: httpGet: - path: /health/liveness + path: /livez port: 9443 scheme: HTTPS failureThreshold: 20 @@ -178,7 +178,7 @@ sidecarInjector: # @default -- See [values.yaml](values.yaml) livenessProbe: httpGet: - path: /health/liveness + path: /livez port: 9443 scheme: HTTPS initialDelaySeconds: 15 @@ -193,7 +193,7 @@ sidecarInjector: # @default -- See [values.yaml](values.yaml) readinessProbe: httpGet: - path: /health/readiness + path: /readyz port: 9443 scheme: HTTPS initialDelaySeconds: 5 @@ -211,7 +211,9 @@ sidecarInjector: # -- Container args. args: - sidecar-injector - - --port=9443 + - --address=:9443 + - --cert-file=/opt/kubernetes-sidecar-injector/certs/tls.crt + - --key-file=/opt/kubernetes-sidecar-injector/certs/tls.key service: diff --git a/pkg/commands/inject/command.go b/pkg/commands/inject/command.go index c727d9d8..ac32066c 100644 --- a/pkg/commands/inject/command.go +++ b/pkg/commands/inject/command.go @@ -1,26 +1,97 @@ package inject import ( + "context" + "crypto/tls" + "errors" "fmt" + "net/http" + "time" - "github.com/kyverno/kyverno-envoy-plugin/pkg/httpd" + "github.com/kyverno/kyverno-envoy-plugin/pkg/server/handlers" + "github.com/kyverno/kyverno-envoy-plugin/pkg/signals" "github.com/spf13/cobra" + "go.uber.org/multierr" + "k8s.io/apimachinery/pkg/util/wait" ) func Command() *cobra.Command { - var httpdConf httpd.SimpleServer + var address string + var certFile string + var keyFile string command := &cobra.Command{ Use: "sidecar-injector", Short: "Responsible for injecting sidecars into pod containers", RunE: func(cmd *cobra.Command, args []string) error { - fmt.Printf("SimpleServer starting to listen in port %v", httpdConf.Port) - return httpdConf.Start() + return runServer(context.Background(), address, certFile, keyFile) }, } - command.Flags().IntVar(&httpdConf.Port, "port", 443, "server port.") - command.Flags().StringVar(&httpdConf.CertFile, "certFile", "/etc/mutator/certs/tls.crt", "File containing tls certificate") - command.Flags().StringVar(&httpdConf.KeyFile, "keyFile", "/etc/mutator/certs/tls.key", "File containing tls private key") - command.Flags().BoolVar(&httpdConf.Local, "local", false, "Local run mode") - command.Flags().StringVar(&(&httpdConf.Patcher).SidecarDataKey, "sidecarDataKey", "sidecars.yaml", "ConfigMap Sidecar Data Key") + command.Flags().StringVar(&address, "address", ":9443", "Address to listen on") + command.Flags().StringVar(&certFile, "cert-file", "", "File containing tls certificate") + command.Flags().StringVar(&keyFile, "key-file", "", "File containing tls private key") + // command.Flags().BoolVar(&httpdConf.Local, "local", false, "Local run mode") + // command.Flags().StringVar(&(&httpdConf.Patcher).SidecarDataKey, "sidecarDataKey", "sidecars.yaml", "ConfigMap Sidecar Data Key") return command } + +func setupMux() http.Handler { + mux := http.NewServeMux() + mux.Handle("/livez", handlers.Health()) + mux.Handle("/readyz", handlers.Health()) + mux.Handle("/mutate", handlers.AdmissionReview(nil)) + return mux +} + +func setupServer(addr string) *http.Server { + return &http.Server{ + Addr: addr, + Handler: setupMux(), + TLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{ + // AEADs w/ ECDHE + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, + }, + }, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + ReadHeaderTimeout: 30 * time.Second, + IdleTimeout: 5 * time.Minute, + } +} + +func runServer(ctx context.Context, addr, certFile, keyFile string) error { + var group wait.Group + server := setupServer(addr) + err := func() error { + signalsCtx, signalsCancel := signals.Context(ctx) + defer signalsCancel() + var shutdownErr error + group.StartWithContext(signalsCtx, func(ctx context.Context) { + <-ctx.Done() + fmt.Println("Shutting down server...") + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer shutdownCancel() + shutdownErr = server.Shutdown(shutdownCtx) + }) + fmt.Printf("Starting server at %s\n", addr) + var serveErr error + if certFile != "" && keyFile != "" { + serveErr = server.ListenAndServeTLS(certFile, keyFile) + } else { + serveErr = server.ListenAndServe() + } + if errors.Is(serveErr, http.ErrServerClosed) { + serveErr = nil + } + return multierr.Combine(serveErr, shutdownErr) + }() + group.Wait() + fmt.Println("Server stopped") + return err +} diff --git a/pkg/httpd/simpleserver.go b/pkg/httpd/simpleserver.go deleted file mode 100644 index 2c7f7613..00000000 --- a/pkg/httpd/simpleserver.go +++ /dev/null @@ -1,70 +0,0 @@ -package httpd - -import ( - "fmt" - "net/http" - "os" - "path/filepath" - - "github.com/kyverno/kyverno-envoy-plugin/pkg/admission" - "github.com/kyverno/kyverno-envoy-plugin/pkg/webhook" - "github.com/pkg/errors" - log "github.com/sirupsen/logrus" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/clientcmd" -) - -/*SimpleServer is the required config to create httpd server*/ -type SimpleServer struct { - Local bool - Port int - CertFile string - KeyFile string - Patcher webhook.SidecarInjectorPatcher - Debug bool -} - -// Start the simple http server supporting TLS -func (simpleServer *SimpleServer) Start() error { - k8sClient, err := simpleServer.CreateClient() - if err != nil { - return err - } - simpleServer.Patcher.K8sClient = k8sClient - server := &http.Server{ - Addr: fmt.Sprintf(":%d", simpleServer.Port), - } - mux := http.NewServeMux() - server.Handler = mux - admissionHandler := &admission.Handler{ - Handler: &admission.PodAdmissionRequestHandler{ - PodHandler: &simpleServer.Patcher, - }, - } - mux.HandleFunc("/healthz", webhook.HealthCheckHandler) - mux.HandleFunc("/mutate", admissionHandler.HandleAdmission) - if simpleServer.Local { - return server.ListenAndServe() - } - return server.ListenAndServeTLS(simpleServer.CertFile, simpleServer.KeyFile) -} - -// CreateClient Create the server -func (simpleServer *SimpleServer) CreateClient() (*kubernetes.Clientset, error) { - config, err := simpleServer.buildConfig() - if err != nil { - return nil, errors.Wrapf(err, "error setting up cluster config") - } - return kubernetes.NewForConfig(config) -} - -func (simpleServer *SimpleServer) buildConfig() (*rest.Config, error) { - if simpleServer.Local { - log.Debug("Using local kubeconfig.") - kubeconfig := filepath.Join(os.Getenv("HOME"), ".kube", "config") - return clientcmd.BuildConfigFromFlags("", kubeconfig) - } - log.Debug("Using in cluster kubeconfig.") - return rest.InClusterConfig() -} diff --git a/pkg/server/handlers/admission.go b/pkg/server/handlers/admission.go new file mode 100644 index 00000000..b8b25da8 --- /dev/null +++ b/pkg/server/handlers/admission.go @@ -0,0 +1,36 @@ +package handlers + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + + admissionv1 "k8s.io/api/admission/v1" +) + +func AdmissionReview(inner func(*admissionv1.AdmissionRequest) *admissionv1.AdmissionResponse) http.HandlerFunc { + return func(writer http.ResponseWriter, request *http.Request) { + if request.Body == nil { + // HttpError(request.Context(), writer, request, logger, errors.New("empty body"), http.StatusBadRequest) + return + } + defer request.Body.Close() + body, err := io.ReadAll(request.Body) + if err != nil { + // HttpError(request.Context(), writer, request, logger, err, http.StatusBadRequest) + return + } + contentType := request.Header.Get("Content-Type") + if contentType != "application/json" { + // HttpError(request.Context(), writer, request, logger, errors.New("invalid Content-Type"), http.StatusUnsupportedMediaType) + return + } + fmt.Println(string(body)) + var admissionReview admissionv1.AdmissionReview + if err := json.Unmarshal(body, &admissionReview); err != nil { + // HttpError(request.Context(), writer, request, logger, err, http.StatusExpectationFailed) + return + } + } +} diff --git a/pkg/signals/context.go b/pkg/signals/context.go index f3fbccba..8f9be68e 100644 --- a/pkg/signals/context.go +++ b/pkg/signals/context.go @@ -2,11 +2,29 @@ package signals import ( "context" - "os" + "fmt" "os/signal" "syscall" + + "k8s.io/apimachinery/pkg/util/wait" ) func Context(ctx context.Context) (context.Context, context.CancelFunc) { - return signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) + return signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM) +} + +func WithContext(ctx context.Context, funcs ...func(context.Context)) { + var group wait.Group + func() { + fmt.Println("Setting up signal aware context") + ctx, cancel := Context(ctx) + defer cancel() + fmt.Println("Starting group routines") + for _, f := range funcs { + group.StartWithContext(ctx, f) + } + <-ctx.Done() + }() + fmt.Println("Waiting for group routines to terminate") + group.Wait() } diff --git a/pkg/webhook/health.go b/pkg/webhook/health.go deleted file mode 100644 index f015807b..00000000 --- a/pkg/webhook/health.go +++ /dev/null @@ -1,10 +0,0 @@ -package webhook - -import ( - "net/http" -) - -// HealthCheckHandler HttpServer function to handle Health check -func HealthCheckHandler(writer http.ResponseWriter, _ *http.Request) { - writer.WriteHeader(http.StatusOK) -} diff --git a/tests/e2e-test/chainsaw-test.yaml b/tests/e2e-test/sidecar-injector/chainsaw-test.yaml similarity index 100% rename from tests/e2e-test/chainsaw-test.yaml rename to tests/e2e-test/sidecar-injector/chainsaw-test.yaml diff --git a/tests/e2e-test/deployment.yaml b/tests/e2e-test/sidecar-injector/deployment.yaml similarity index 100% rename from tests/e2e-test/deployment.yaml rename to tests/e2e-test/sidecar-injector/deployment.yaml