Fix webhook certificates always being issued for the marblerun name…

…space when installing with CLI (#573)

* Issue webhook certificates with the correct DNS names
  * instead of always using `marble-injector.marblerun.svc`, the certificate is now issued for `marble-injector.<namespace>.sv`
* Automatically detect if cert-manager is installed, and if it is, use it to provision webhook certificates 
* Add warning to auto-injection docs when using multiple MarbleRun deployments in one cluster

---------

Signed-off-by: Daniel Weiße <dw@edgeless.systems>

cli/internal/cmd/cmd.go

@@ -18,7 +18,9 @@ import (
 	"k8s.io/client-go/kubernetes"
 )
 
-const webhookName = "marble-injector.marblerun"
+func webhookDNSName(namespace string) string {
+	return "marble-injector." + namespace
+}
 
 type getter interface {
 	Get(ctx context.Context, path string, body io.Reader, queryParameters ...string) ([]byte, error)

cli/internal/cmd/csr.go

@@ -41,15 +41,22 @@ type certificateInterface interface {
 // certificateV1 acts as a handler for generating signed certificates.
 type certificateV1 struct {
 	kubeClient kubernetes.Interface
+	namespace  string
 	privKey    *rsa.PrivateKey
 	csr        *certv1.CertificateSigningRequest
-	timeout    int
+
+	retryAttempts int
+	waitInterval  time.Duration
 }
 
 // newCertificateV1 creates a certificate handler using the certificatesv1 kubernetes api.
-func newCertificateV1(kubeClient kubernetes.Interface) (*certificateV1, error) {
-	crt := &certificateV1{kubeClient: kubeClient}
-	crt.timeout = 10
+func newCertificateV1(kubeClient kubernetes.Interface, namespace string) (*certificateV1, error) {
+	crt := &certificateV1{
+		kubeClient:    kubeClient,
+		namespace:     namespace,
+		retryAttempts: 20,
+		waitInterval:  1 * time.Second,
+	}
 
 	privKey, err := rsa.GenerateKey(rand.Reader, 4096)
 	if err != nil {
@@ -58,15 +65,15 @@ func newCertificateV1(kubeClient kubernetes.Interface) (*certificateV1, error) {
 
 	crt.privKey = privKey
 
-	csrPEM, err := createCSR(privKey)
+	csrPEM, err := createCSR(privKey, crt.namespace)
 	if err != nil {
 		return nil, err
 	}
 
 	// create the k8s certificate request which bundles the x509 csr
 	certificateRequest := &certv1.CertificateSigningRequest{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: webhookName,
+			Name: webhookDNSName(crt.namespace),
 		},
 		Spec: certv1.CertificateSigningRequestSpec{
 			Request:    pem.EncodeToMemory(csrPEM),
@@ -84,7 +91,7 @@ func newCertificateV1(kubeClient kubernetes.Interface) (*certificateV1, error) {
 
 // get returns the certificate signed by the kubernetes api server.
 func (crt *certificateV1) get(ctx context.Context) ([]byte, error) {
-	csr, err := crt.kubeClient.CertificatesV1().CertificateSigningRequests().Get(ctx, webhookName, metav1.GetOptions{})
+	csr, err := crt.kubeClient.CertificatesV1().CertificateSigningRequests().Get(ctx, webhookDNSName(crt.namespace), metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
@@ -137,10 +144,12 @@ func (crt *certificateV1) signRequest(ctx context.Context) error {
 		return err
 	}
 
-	if err := waitForResource(webhookName, crt.kubeClient, crt.timeout, func(name string, client kubernetes.Interface) bool {
-		_, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, name, metav1.GetOptions{})
-		return err == nil
-	}); err != nil {
+	if err := waitForResource(
+		webhookDNSName(crt.namespace), crt.kubeClient, crt.retryAttempts, crt.waitInterval,
+		func(name string, client kubernetes.Interface) bool {
+			_, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, name, metav1.GetOptions{})
+			return err == nil
+		}); err != nil {
 		return err
 	}
 
@@ -154,21 +163,23 @@ func (crt *certificateV1) signRequest(ctx context.Context) error {
 		LastUpdateTime: metav1.Now(),
 	})
 
-	_, err = crt.kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, webhookName, certReturn, metav1.UpdateOptions{})
+	_, err = crt.kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, webhookDNSName(crt.namespace), certReturn, metav1.UpdateOptions{})
 	if err != nil {
 		return err
 	}
 
-	return waitForResource(webhookName, crt.kubeClient, crt.timeout, func(name string, client kubernetes.Interface) bool {
-		csr, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, webhookName, metav1.GetOptions{})
-		if err != nil {
-			return false
-		}
-		if len(csr.Status.Certificate) <= 0 {
-			return false
-		}
-		return true
-	})
+	return waitForResource(
+		webhookDNSName(crt.namespace), crt.kubeClient, crt.retryAttempts, crt.waitInterval,
+		func(name string, client kubernetes.Interface) bool {
+			csr, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, name, metav1.GetOptions{})
+			if err != nil {
+				return false
+			}
+			if len(csr.Status.Certificate) <= 0 {
+				return false
+			}
+			return true
+		})
 }
 
 // getKey returns the private key of the webhook server.
@@ -177,9 +188,9 @@ func (crt *certificateV1) getKey() *rsa.PrivateKey {
 }
 
 // createCSR creates a x509 certificate signing request.
-func createCSR(privKey *rsa.PrivateKey) (*pem.Block, error) {
+func createCSR(privKey *rsa.PrivateKey, namespace string) (*pem.Block, error) {
 	subj := pkix.Name{
-		CommonName:   "system:node:marble-injector.marblerun.svc",
+		CommonName:   fmt.Sprintf("system:node:%s.svc", webhookDNSName(namespace)),
 		Organization: []string{"system:nodes"},
 	}
 
@@ -204,7 +215,7 @@ func createCSR(privKey *rsa.PrivateKey) (*pem.Block, error) {
 		Subject:            subj,
 		SignatureAlgorithm: x509.SHA256WithRSA,
 		Extensions:         []pkix.Extension{extendedUsage, keyUsage},
-		DNSNames:           []string{"marble-injector.marblerun.svc"},
+		DNSNames:           []string{fmt.Sprintf("%s.svc", webhookDNSName(namespace))},
 	}
 
 	csrRaw, err := x509.CreateCertificateRequest(rand.Reader, template, privKey)
@@ -222,12 +233,15 @@ func createCSR(privKey *rsa.PrivateKey) (*pem.Block, error) {
 
 // calls to the CertificateSigningRequests interface are non blocking, we use this function
 // to check if a resource has been created and can be used.
-func waitForResource(name string, kubeClient kubernetes.Interface, timeout int, resourceCheck func(string, kubernetes.Interface) bool) error {
-	for i := 0; i < timeout; i++ {
+func waitForResource(
+	name string, kubeClient kubernetes.Interface, retryAttempts int, waitInterval time.Duration,
+	resourceCheck func(string, kubernetes.Interface) bool,
+) error {
+	for i := 0; i < retryAttempts; i++ {
 		if resourceCheck(name, kubeClient) {
 			return nil
 		}
-		time.Sleep(1 * time.Second)
+		time.Sleep(waitInterval)
 	}
-	return fmt.Errorf("certificate signing request was not updated after %d attempts. Giving up", timeout)
+	return fmt.Errorf("certificate signing request was not updated after %d attempts. Giving up", retryAttempts)
 }

cli/internal/cmd/csr_legay.go

@@ -22,19 +22,20 @@ import (
 
 // certificateLegacy acts as a handler for generating signed certificates.
 type certificateLegacy struct {
+	namespace     string
 	serverPrivKey *rsa.PrivateKey
 	serverCert    *pem.Block
 	caPrivKey     *rsa.PrivateKey
 	caCert        *pem.Block
 }
 
 // newCertificateLegacy creates a certificate handler for kubernetes versions <=18.
-func newCertificateLegacy() (*certificateLegacy, error) {
+func newCertificateLegacy(namespace string) (*certificateLegacy, error) {
 	serial, err := util.GenerateCertificateSerialNumber()
 	if err != nil {
 		return nil, err
 	}
-	crt := &certificateLegacy{}
+	crt := &certificateLegacy{namespace: namespace}
 
 	caCert := &x509.Certificate{
 		SerialNumber: serial,
@@ -100,10 +101,10 @@ func (crt *certificateLegacy) signRequest(_ context.Context) error {
 	serverCert := &x509.Certificate{
 		SerialNumber: serial,
 		Subject: pkix.Name{
-			CommonName:   "system:node:marble-injector.marblerun.svc",
+			CommonName:   fmt.Sprintf("system:node:%s.svc", webhookDNSName(crt.namespace)),
 			Organization: []string{"system:nodes"},
 		},
-		DNSNames:           []string{"marble-injector.marblerun.svc"},
+		DNSNames:           []string{fmt.Sprintf("%s.svc", webhookDNSName(crt.namespace))},
 		SignatureAlgorithm: x509.SHA256WithRSA,
 		KeyUsage:           x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
 		ExtKeyUsage:        []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},

cli/internal/cmd/csr_test.go

@@ -14,7 +14,9 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
+	"github.com/edgelesssys/marblerun/cli/internal/helm"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/client-go/kubernetes/fake"
@@ -41,7 +43,7 @@ func TestCreateCSR(t *testing.T) {
 	testKey, err := rsa.GenerateKey(rand.Reader, 1024)
 	require.NoError(err)
 
-	pem, err := createCSR(testKey)
+	pem, err := createCSR(testKey, helm.Namespace)
 	require.NoError(err)
 	assert.Equal("CERTIFICATE REQUEST", pem.Type)
 }
@@ -53,13 +55,13 @@ func TestCertificateV1(t *testing.T) {
 
 	testClient := fake.NewSimpleClientset()
 
-	testHandler, err := newCertificateV1(testClient)
+	testHandler, err := newCertificateV1(testClient, helm.Namespace)
 	require.NoError(err)
+	testHandler.waitInterval = 10 * time.Millisecond
 
 	testKey := testHandler.getKey()
 	assert.Equal(testKey, testHandler.privKey, "private key of the handler and private key returned by its method were not equal")
 
-	testHandler.timeout = 2
 	// this should error with a timeout since the fakeClient does not keep updated resources, but only returns them once on API call
 	err = testHandler.signRequest(ctx)
 	require.Error(err)
@@ -87,7 +89,7 @@ func TestCertificateLegacy(t *testing.T) {
 	assert := assert.New(t)
 	ctx := context.Background()
 
-	testHandler, err := newCertificateLegacy()
+	testHandler, err := newCertificateLegacy(helm.Namespace)
 	require.NoError(err)
 	assert.True(len(testHandler.caCert.Bytes) > 0, "failed creating caCert")
 

cli/internal/cmd/install.go

@@ -14,6 +14,7 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/cert-manager/cert-manager/pkg/util/cmapichecker"
 	"github.com/edgelesssys/marblerun/cli/internal/helm"
 	"github.com/edgelesssys/marblerun/cli/internal/kube"
 	"github.com/edgelesssys/marblerun/util/k8sutil"
@@ -72,12 +73,16 @@ func runInstall(cmd *cobra.Command, _ []string) error {
 	if err != nil {
 		return err
 	}
+	cmChecker, err := kube.NewCertManagerChecker()
+	if err != nil {
+		return err
+	}
 
-	return cliInstall(cmd, helmClient, kubeClient, namespace)
+	return cliInstall(cmd, helmClient, kubeClient, cmChecker, namespace)
 }
 
 // cliInstall installs MarbleRun on the cluster.
-func cliInstall(cmd *cobra.Command, helmClient *helm.Client, kubeClient kubernetes.Interface, namespace string) error {
+func cliInstall(cmd *cobra.Command, helmClient *helm.Client, kubeClient kubernetes.Interface, cmChecker cmapichecker.Interface, namespace string) error {
 	flags, err := parseInstallFlags(cmd)
 	if err != nil {
 		return fmt.Errorf("parsing install flags: %w", err)
@@ -95,9 +100,14 @@ func cliInstall(cmd *cobra.Command, helmClient *helm.Client, kubeClient kubernet
 		}
 	}
 
+	// verify namespace exists, if not create it
+	if err := verifyNamespace(cmd.Context(), namespace, kubeClient); err != nil {
+		return err
+	}
+
 	var webhookSettings []string
 	if !flags.disableInjection {
-		webhookSettings, err = installWebhook(cmd, kubeClient, namespace)
+		webhookSettings, err = installWebhookCerts(cmd, kubeClient, cmChecker, namespace)
 		if err != nil {
 			return errorAndCleanup(cmd.Context(), fmt.Errorf("installing webhook certs: %w", err), kubeClient, namespace)
 		}
@@ -129,15 +139,20 @@ func cliInstall(cmd *cobra.Command, helmClient *helm.Client, kubeClient kubernet
 	return nil
 }
 
-// installWebhook enables a mutating admission webhook to allow automatic injection of values into pods.
-func installWebhook(cmd *cobra.Command, kubeClient kubernetes.Interface, namespace string) ([]string, error) {
-	// verify 'marblerun' namespace exists, if not create it
-	if err := verifyNamespace(cmd.Context(), namespace, kubeClient); err != nil {
-		return nil, err
+// installWebhookCerts sets up TLS certificates and keys required by MarbleRun's mutating admission webhook.
+// Depending on the cluster, either a certificate issued by cert-manager or a self-created certificate using the Kubernetes API is used.
+func installWebhookCerts(cmd *cobra.Command, kubeClient kubernetes.Interface, cmChecker cmapichecker.Interface, namespace string) ([]string, error) {
+	cmd.Print("Setting up MarbleRun Webhook")
+
+	if err := cmChecker.Check(cmd.Context()); err == nil {
+		cmd.Printf("... Done\n")
+		return []string{
+			fmt.Sprintf("marbleInjector.start=%t", true),
+			fmt.Sprintf("marbleInjector.useCertManager=%t", true),
+		}, nil
 	}
 
-	cmd.Print("Setting up MarbleRun Webhook")
-	certificateHandler, err := getCertificateHandler(cmd.OutOrStdout(), kubeClient)
+	certificateHandler, err := getCertificateHandler(cmd.OutOrStdout(), kubeClient, namespace)
 	if err != nil {
 		return nil, err
 	}
@@ -190,16 +205,16 @@ func createSecret(ctx context.Context, namespace string, privKey *rsa.PrivateKey
 	return err
 }
 
-func getCertificateHandler(out io.Writer, kubeClient kubernetes.Interface) (certificateInterface, error) {
+func getCertificateHandler(out io.Writer, kubeClient kubernetes.Interface, namespace string) (certificateInterface, error) {
 	isLegacy, err := checkLegacyKubernetesVersion(kubeClient)
 	if err != nil {
 		return nil, err
 	}
 	if isLegacy {
 		fmt.Fprintf(out, "\nKubernetes version lower than 1.19 detected, using self-signed certificates as CABundle")
-		return newCertificateLegacy()
+		return newCertificateLegacy(namespace)
 	}
-	return newCertificateV1(kubeClient)
+	return newCertificateV1(kubeClient, namespace)
 }
 
 func verifyNamespace(ctx context.Context, namespace string, kubeClient kubernetes.Interface) error {
@@ -250,7 +265,7 @@ func getSGXResourceKey(ctx context.Context, kubeClient kubernetes.Interface) (st
 // This prevents secrets and CSRs to stay on the cluster after a failed installation attempt.
 func errorAndCleanup(ctx context.Context, err error, kubeClient kubernetes.Interface, namespace string) error {
 	// We dont care about any additional errors here
-	_ = cleanupCSR(ctx, kubeClient)
+	_ = cleanupCSR(ctx, kubeClient, namespace)
 	_ = cleanupSecrets(ctx, kubeClient, namespace)
 	return err
 }