images/kube-webhook-certgen/rootfs: improvements (#7630)

* images/kube-webhook-certgen/rootfs/README.md: remove trailing whitespace Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs: improve code formatting Automatically using gofumpt. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs: remove executable bits from files Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/cmd: remove unreachable code log.Fatal(|f) will alread call os.Exit(1), so this code is never reached. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/pkg/k8s: fix unit tests Right now they fail as everything else migrated from using v1beta1 to v1. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs: create clientset in cmd package So one can easily mock the client, without touching unexported parts of the code and to soften the dependency between CLI code (kubeconfig path). Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/cmd: simplify bool logic Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/pkg/k8s: improve formatting Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/pkg/k8s: improve variable names Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/pkg/k8s: refactor a bit Move patching logic to separate functions. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> * images/kube-webhook-certgen/rootfs/pkg/k8s: fix error log messages In patchMutating() function, log messages were waying still patching validating webhook. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
kubernetes · Sep 16, 2021 · 260910c · 260910c
1 parent b3389a1
commit 260910c
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 93 deletions.
diff --git a/images/kube-webhook-certgen/rootfs/README.md b/images/kube-webhook-certgen/rootfs/README.md
@@ -7,14 +7,14 @@ creators.
 
 ## Overview
 Generates a CA and leaf certificate with a long (100y) expiration, then patches [Kubernetes Admission Webhooks](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/)
-by setting the `caBundle` field with the generated CA. 
+by setting the `caBundle` field with the generated CA.
 Can optionally patch the hooks `failurePolicy` setting - useful in cases where a single Helm chart needs to provision resources
 and hooks at the same time as patching.
 
 The utility works in two parts, optimized to work better with the Helm provisioning process that leverages pre-install and post-install hooks to execute this as a Kubernetes job.
 
 ## Security Considerations
-This tool may not be adequate in all security environments. If a more complete solution is required, you may want to 
+This tool may not be adequate in all security environments. If a more complete solution is required, you may want to
 seek alternatives such as [jetstack/cert-manager](https://github.com/jetstack/cert-manager)
 
 ## Command line options

diff --git a/images/kube-webhook-certgen/rootfs/cmd/create.go b/images/kube-webhook-certgen/rootfs/cmd/create.go
@@ -7,17 +7,16 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var (
-	create = &cobra.Command{
-		Use:    "create",
-		Short:  "Generate a ca and server cert+key and store the results in a secret 'secret-name' in 'namespace'",
-		Long:   "Generate a ca and server cert+key and store the results in a secret 'secret-name' in 'namespace'",
-		PreRun: configureLogging,
-		Run:    createCommand}
-)
+var create = &cobra.Command{
+	Use:    "create",
+	Short:  "Generate a ca and server cert+key and store the results in a secret 'secret-name' in 'namespace'",
+	Long:   "Generate a ca and server cert+key and store the results in a secret 'secret-name' in 'namespace'",
+	PreRun: configureLogging,
+	Run:    createCommand,
+}
 
 func createCommand(cmd *cobra.Command, args []string) {
-	k := k8s.New(cfg.kubeconfig)
+	k := k8s.New(newKubernetesClient(cfg.kubeconfig))
 	ca := k.GetCaFromSecret(cfg.secretName, cfg.namespace)
 	if ca == nil {
 		log.Info("creating new secret")

diff --git a/images/kube-webhook-certgen/rootfs/cmd/patch.go b/images/kube-webhook-certgen/rootfs/cmd/patch.go
@@ -1,28 +1,24 @@
 package cmd
 
 import (
-	"os"
-
 	"github.com/jet/kube-webhook-certgen/pkg/k8s"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
 )
 
-var (
-	patch = &cobra.Command{
-		Use:    "patch",
-		Short:  "Patch a validatingwebhookconfiguration and mutatingwebhookconfiguration 'webhook-name' by using the ca from 'secret-name' in 'namespace'",
-		Long:   "Patch a validatingwebhookconfiguration and mutatingwebhookconfiguration 'webhook-name' by using the ca from 'secret-name' in 'namespace'",
-		PreRun: prePatchCommand,
-		Run:    patchCommand}
-)
+var patch = &cobra.Command{
+	Use:    "patch",
+	Short:  "Patch a validatingwebhookconfiguration and mutatingwebhookconfiguration 'webhook-name' by using the ca from 'secret-name' in 'namespace'",
+	Long:   "Patch a validatingwebhookconfiguration and mutatingwebhookconfiguration 'webhook-name' by using the ca from 'secret-name' in 'namespace'",
+	PreRun: prePatchCommand,
+	Run:    patchCommand,
+}
 
 func prePatchCommand(cmd *cobra.Command, args []string) {
 	configureLogging(cmd, args)
-	if cfg.patchMutating == false && cfg.patchValidating == false {
+	if !cfg.patchMutating && !cfg.patchValidating {
 		log.Fatal("patch-validating=false, patch-mutating=false. You must patch at least one kind of webhook, otherwise this command is a no-op")
-		os.Exit(1)
 	}
 	switch cfg.patchFailurePolicy {
 	case "":
@@ -33,12 +29,11 @@ func prePatchCommand(cmd *cobra.Command, args []string) {
 		break
 	default:
 		log.Fatalf("patch-failure-policy %s is not valid", cfg.patchFailurePolicy)
-		os.Exit(1)
 	}
 }
 
 func patchCommand(_ *cobra.Command, _ []string) {
-	k := k8s.New(cfg.kubeconfig)
+	k := k8s.New(newKubernetesClient(cfg.kubeconfig))
 	ca := k.GetCaFromSecret(cfg.secretName, cfg.namespace)
 
 	if ca == nil {

diff --git a/images/kube-webhook-certgen/rootfs/cmd/root.go b/images/kube-webhook-certgen/rootfs/cmd/root.go
@@ -7,6 +7,8 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/tools/clientcmd"
 )
 
 var (
@@ -81,3 +83,17 @@ func getFormatter(logfmt string) log.Formatter {
 	log.Fatalf("invalid log format '%s'", logfmt)
 	return nil
 }
+
+func newKubernetesClient(kubeconfig string) kubernetes.Interface {
+	config, err := clientcmd.BuildConfigFromFlags("", kubeconfig)
+	if err != nil {
+		log.WithError(err).Fatal("error building kubernetes config")
+	}
+
+	c, err := kubernetes.NewForConfig(config)
+	if err != nil {
+		log.WithError(err).Fatal("error creating kubernetes client")
+	}
+
+	return c
+}
diff --git a/images/kube-webhook-certgen/rootfs/pkg/certs/certs.go b/images/kube-webhook-certgen/rootfs/pkg/certs/certs.go
@@ -7,11 +7,12 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
-	log "github.com/sirupsen/logrus"
 	"math/big"
 	"net"
 	"strings"
 	"time"
+
+	log "github.com/sirupsen/logrus"
 )
 
 // GenerateCerts venerates a ca with a leaf certificate and key and returns the ca, cert and key as PEM encoded slices

diff --git a/images/kube-webhook-certgen/rootfs/pkg/certs/certs_test.go b/images/kube-webhook-certgen/rootfs/pkg/certs/certs_test.go
@@ -16,7 +16,6 @@ func handler(w http.ResponseWriter, r *http.Request) {
 }
 
 func TestCertificateCreation(t *testing.T) {
-
 	ca, cert, key := GenerateCerts("localhost")
 
 	c, err := tls.X509KeyPair(cert, key)
@@ -30,7 +29,9 @@ func TestCertificateCreation(t *testing.T) {
 	tr := &http.Transport{
 		TLSClientConfig: &tls.Config{
 			RootCAs:    caCertPool,
-			ServerName: "localhost"}}
+			ServerName: "localhost",
+		},
+	}
 
 	ts := httptest.NewUnstartedServer(http.HandlerFunc(handler))
 	ts.TLS = &tls.Config{Certificates: []tls.Certificate{c}}

diff --git a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go
@@ -9,92 +9,96 @@ import (
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/tools/clientcmd"
 )
 
 type k8s struct {
 	clientset kubernetes.Interface
 }
 
-func New(kubeconfig string) *k8s {
-	config, err := clientcmd.BuildConfigFromFlags("", kubeconfig)
-	if err != nil {
-		log.WithError(err).Fatal("error building kubernetes config")
+func New(clientset kubernetes.Interface) *k8s {
+	if clientset == nil {
+		log.Fatal("no kubernetes client given")
 	}
 
-	c, err := kubernetes.NewForConfig(config)
-	if err != nil {
-		log.WithError(err).Fatal("error creating kubernetes client")
+	return &k8s{
+		clientset: clientset,
 	}
-
-	return &k8s{clientset: c}
 }
 
 // PatchWebhookConfigurations will patch validatingWebhook and mutatingWebhook clientConfig configurations with
 // the provided ca data. If failurePolicy is provided, patch all webhooks with this value
 func (k8s *k8s) PatchWebhookConfigurations(
-	configurationNames string, ca []byte,
+	configurationName string,
+	ca []byte,
 	failurePolicy *admissionv1.FailurePolicyType,
-	patchMutating bool, patchValidating bool) {
-
-	log.Infof("patching webhook configurations '%s' mutating=%t, validating=%t, failurePolicy=%s", configurationNames, patchMutating, patchValidating, *failurePolicy)
+	patchMutating bool,
+	patchValidating bool,
+) {
+	log.Infof("patching webhook configurations '%s' mutating=%t, validating=%t, failurePolicy=%s", configurationName, patchMutating, patchValidating, *failurePolicy)
 
 	if patchValidating {
-		valHook, err := k8s.clientset.
-			AdmissionregistrationV1().
-			ValidatingWebhookConfigurations().
-			Get(context.TODO(), configurationNames, metav1.GetOptions{})
-
-		if err != nil {
-			log.WithField("err", err).Fatal("failed getting validating webhook")
-		}
-
-		for i := range valHook.Webhooks {
-			h := &valHook.Webhooks[i]
-			h.ClientConfig.CABundle = ca
-			if *failurePolicy != "" {
-				h.FailurePolicy = failurePolicy
-			}
-		}
-
-		if _, err = k8s.clientset.AdmissionregistrationV1().
-			ValidatingWebhookConfigurations().
-			Update(context.TODO(), valHook, metav1.UpdateOptions{}); err != nil {
-			log.WithField("err", err).Fatal("failed patching validating webhook")
-		}
-		log.Debug("patched validating hook")
+		k8s.patchValidating(configurationName, ca, failurePolicy)
 	} else {
 		log.Debug("validating hook patching not required")
 	}
 
 	if patchMutating {
-		mutHook, err := k8s.clientset.
-			AdmissionregistrationV1().
-			MutatingWebhookConfigurations().
-			Get(context.TODO(), configurationNames, metav1.GetOptions{})
-		if err != nil {
-			log.WithField("err", err).Fatal("failed getting validating webhook")
-		}
+		k8s.patchMutating(configurationName, ca, failurePolicy)
+	} else {
+		log.Debug("mutating hook patching not required")
+	}
+
+	log.Info("Patched hook(s)")
+}
+
+func (k8s *k8s) patchValidating(configurationName string, ca []byte, failurePolicy *admissionv1.FailurePolicyType) {
+	valHook, err := k8s.clientset.
+		AdmissionregistrationV1().
+		ValidatingWebhookConfigurations().
+		Get(context.TODO(), configurationName, metav1.GetOptions{})
+	if err != nil {
+		log.WithField("err", err).Fatal("failed getting validating webhook")
+	}
 
-		for i := range mutHook.Webhooks {
-			h := &mutHook.Webhooks[i]
-			h.ClientConfig.CABundle = ca
-			if *failurePolicy != "" {
-				h.FailurePolicy = failurePolicy
-			}
+	for i := range valHook.Webhooks {
+		h := &valHook.Webhooks[i]
+		h.ClientConfig.CABundle = ca
+		if *failurePolicy != "" {
+			h.FailurePolicy = failurePolicy
 		}
+	}
+
+	if _, err = k8s.clientset.AdmissionregistrationV1().
+		ValidatingWebhookConfigurations().
+		Update(context.TODO(), valHook, metav1.UpdateOptions{}); err != nil {
+		log.WithField("err", err).Fatal("failed patching validating webhook")
+	}
+	log.Debug("patched validating hook")
+}
 
-		if _, err = k8s.clientset.AdmissionregistrationV1().
-			MutatingWebhookConfigurations().
-			Update(context.TODO(), mutHook, metav1.UpdateOptions{}); err != nil {
-			log.WithField("err", err).Fatal("failed patching validating webhook")
+func (k8s *k8s) patchMutating(configurationName string, ca []byte, failurePolicy *admissionv1.FailurePolicyType) {
+	mutHook, err := k8s.clientset.
+		AdmissionregistrationV1().
+		MutatingWebhookConfigurations().
+		Get(context.TODO(), configurationName, metav1.GetOptions{})
+	if err != nil {
+		log.WithField("err", err).Fatal("failed getting mutating webhook")
+	}
+
+	for i := range mutHook.Webhooks {
+		h := &mutHook.Webhooks[i]
+		h.ClientConfig.CABundle = ca
+		if *failurePolicy != "" {
+			h.FailurePolicy = failurePolicy
 		}
-		log.Debug("patched mutating hook")
-	} else {
-		log.Debug("mutating hook patching not required")
 	}
 
-	log.Info("Patched hook(s)")
+	if _, err = k8s.clientset.AdmissionregistrationV1().
+		MutatingWebhookConfigurations().
+		Update(context.TODO(), mutHook, metav1.UpdateOptions{}); err != nil {
+		log.WithField("err", err).Fatal("failed patching mutating webhook")
+	}
+	log.Debug("patched mutating hook")
 }
 
 // GetCaFromSecret will check for the presence of a secret. If it exists, will return the content of the
@@ -120,7 +124,6 @@ func (k8s *k8s) GetCaFromSecret(secretName string, namespace string) []byte {
 
 // SaveCertsToSecret saves the provided ca, cert and key into a secret in the specified namespace.
 func (k8s *k8s) SaveCertsToSecret(secretName, namespace, certName, keyName string, ca, cert, key []byte) {
-
 	log.Debugf("saving to secret '%s' in namespace '%s'", secretName, namespace)
 	secret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go
@@ -99,7 +99,8 @@ func TestPatchWebhookConfigurations(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: testWebhookName,
 			},
-			Webhooks: []admissionv1.MutatingWebhook{{Name: "m1"}, {Name: "m2"}}}, metav1.CreateOptions{})
+			Webhooks: []admissionv1.MutatingWebhook{{Name: "m1"}, {Name: "m2"}},
+		}, metav1.CreateOptions{})
 
 	k.clientset.
 		AdmissionregistrationV1().
@@ -109,24 +110,23 @@ func TestPatchWebhookConfigurations(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: testWebhookName,
 			},
-			Webhooks: []admissionv1.ValidatingWebhook{{Name: "v1"}, {Name: "v2"}}}, metav1.CreateOptions{})
+			Webhooks: []admissionv1.ValidatingWebhook{{Name: "v1"}, {Name: "v2"}},
+		}, metav1.CreateOptions{})
 
 	k.PatchWebhookConfigurations(testWebhookName, ca, &fail, true, true)
 
 	whmut, err := k.clientset.
 		AdmissionregistrationV1().
 		MutatingWebhookConfigurations().
 		Get(context.Background(), testWebhookName, metav1.GetOptions{})
-
 	if err != nil {
 		t.Error(err)
 	}
 
 	whval, err := k.clientset.
-		AdmissionregistrationV1beta1().
+		AdmissionregistrationV1().
 		MutatingWebhookConfigurations().
 		Get(context.Background(), testWebhookName, metav1.GetOptions{})
-
 	if err != nil {
 		t.Error(err)
 	}
@@ -155,5 +155,4 @@ func TestPatchWebhookConfigurations(t *testing.T) {
 	if whval.Webhooks[1].FailurePolicy == nil {
 		t.Errorf("Expected second validating webhook failure policy to be set to %s", fail)
 	}
-
 }