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>

images/kube-webhook-certgen/rootfs/cmd/create.go

@@ -7,17 +7,16 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var (
+var create = &cobra.Command{
-	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}
+	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")

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 (
+var patch = &cobra.Command{
-	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}
+	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 {

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
+}

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

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}}

images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go

@@ -9,42 +9,53 @@ 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 {
+func New(clientset kubernetes.Interface) *k8s {
-	config, err := clientcmd.BuildConfigFromFlags("", kubeconfig)
+	if clientset == nil {
-	if err != nil {
+		log.Fatal("no kubernetes client given")
-		log.WithError(err).Fatal("error building kubernetes config")
 	}
 
-	c, err := kubernetes.NewForConfig(config)
+	return &k8s{
-	if err != nil {
+		clientset: clientset,
-		log.WithError(err).Fatal("error creating kubernetes client")
 	}
-
-	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) {
+	patchMutating bool,
-
+	patchValidating bool,
-	log.Infof("patching webhook configurations '%s' mutating=%t, validating=%t, failurePolicy=%s", configurationNames, patchMutating, patchValidating, *failurePolicy)
+) {
+	log.Infof("patching webhook configurations '%s' mutating=%t, validating=%t, failurePolicy=%s", configurationName, patchMutating, patchValidating, *failurePolicy)
 
 	if patchValidating {
+		k8s.patchValidating(configurationName, ca, failurePolicy)
+	} else {
+		log.Debug("validating hook patching not required")
+	}
+
+	if patchMutating {
+		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(), configurationNames, metav1.GetOptions{})
+		Get(context.TODO(), configurationName, metav1.GetOptions{})
-
 	if err != nil {
 		log.WithField("err", err).Fatal("failed getting validating webhook")
 	}
@@ -63,17 +74,15 @@ func (k8s *k8s) PatchWebhookConfigurations(
 		log.WithField("err", err).Fatal("failed patching validating webhook")
 	}
 	log.Debug("patched validating hook")
-	} else {
+}
-		log.Debug("validating hook patching not required")
-	}
 
-	if patchMutating {
+func (k8s *k8s) patchMutating(configurationName string, ca []byte, failurePolicy *admissionv1.FailurePolicyType) {
 	mutHook, err := k8s.clientset.
 		AdmissionregistrationV1().
 		MutatingWebhookConfigurations().
-			Get(context.TODO(), configurationNames, metav1.GetOptions{})
+		Get(context.TODO(), configurationName, metav1.GetOptions{})
 	if err != nil {
-			log.WithField("err", err).Fatal("failed getting validating webhook")
+		log.WithField("err", err).Fatal("failed getting mutating webhook")
 	}
 
 	for i := range mutHook.Webhooks {
@@ -87,14 +96,9 @@ func (k8s *k8s) PatchWebhookConfigurations(
 	if _, err = k8s.clientset.AdmissionregistrationV1().
 		MutatingWebhookConfigurations().
 		Update(context.TODO(), mutHook, metav1.UpdateOptions{}); err != nil {
-			log.WithField("err", err).Fatal("failed patching validating webhook")
+		log.WithField("err", err).Fatal("failed patching mutating webhook")
 	}
 	log.Debug("patched mutating hook")
-	} else {
-		log.Debug("mutating hook patching not required")
-	}
-
-	log.Info("Patched hook(s)")
 }
 
 // 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{

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,7 +110,8 @@ 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)
 
@@ -117,16 +119,14 @@ func TestPatchWebhookConfigurations(t *testing.T) {
 		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)
 	}
-
 }