Enable webhook tls tertificate management (#2417)

webhook-tls secret is now read on startup looking for
already created certificates. If valids they are used as the
current tls certificates. Otherwise a new set of certificates is
created and installed.

fixes: 245

addons/controllers/certificates_test.go

@@ -30,7 +30,8 @@ var _ = Describe("Webhook", func() {
 
 	Context("server's certificate and key", func() {
 		It("should be generated and written to the webhook server CertDir", func() {
-			secret, err := webhooks.NewTLSSecret(ctx, webhookScrtName, webhookServiceName, certPath, keyPath, addonNamespace)
+			print(certPath, keyPath, webhookScrtName, addonNamespace, webhookServiceName, webhookSelectorString)
+			secret, err := webhooks.InstallNewCertificates(ctx, k8sConfig, certPath, keyPath, webhookScrtName, addonNamespace, webhookServiceName, webhookSelectorString)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(secret).NotTo(BeNil())
 			cert, err := cert2.CertsFromFile(certPath)

addons/controllers/suite_test.go

@@ -79,19 +79,20 @@ const (
 )
 
 var (
-	cfg                *rest.Config
-	k8sClient          client.Client
-	k8sConfig          *rest.Config
-	testEnv            *envtest.Environment
-	ctx                = ctrl.SetupSignalHandler()
-	scheme             = runtime.NewScheme()
-	mgr                manager.Manager
-	dynamicClient      dynamic.Interface
-	cancel             context.CancelFunc
-	certPath           string
-	keyPath            string
-	tmpDir             string
-	webhookCertDetails testutil.WebhookCertificatesDetails
+	cfg                   *rest.Config
+	k8sClient             client.Client
+	k8sConfig             *rest.Config
+	testEnv               *envtest.Environment
+	ctx                   = ctrl.SetupSignalHandler()
+	scheme                = runtime.NewScheme()
+	mgr                   manager.Manager
+	dynamicClient         dynamic.Interface
+	cancel                context.CancelFunc
+	certPath              string
+	keyPath               string
+	tmpDir                string
+	webhookCertDetails    testutil.WebhookCertificatesDetails
+	webhookSelectorString string
 )
 
 func TestAddonController(t *testing.T) {
@@ -325,9 +326,10 @@ var _ = BeforeSuite(func(done Done) {
 
 	labelMatch, err := labels.NewRequirement(constants.AddonWebhookLabelKey, selection.Equals, []string{constants.AddonWebhookLabelValue})
 	Expect(err).ToNot(HaveOccurred())
-	whSelector := labels.NewSelector()
-	whSelector = whSelector.Add(*labelMatch)
-	_, err = webhooks.InstallNewCertificates(ctx, k8sConfig, certPath, keyPath, webhookScrtName, addonNamespace, webhookServiceName, whSelector.String())
+	webhookSelector := labels.NewSelector()
+	webhookSelector = webhookSelector.Add(*labelMatch)
+	webhookSelectorString = webhookSelector.String()
+	_, err = webhooks.InstallNewCertificates(ctx, k8sConfig, certPath, keyPath, webhookScrtName, addonNamespace, webhookServiceName, webhookSelectorString)
 	Expect(err).ToNot(HaveOccurred())
 
 	// Set up the webhooks in the manager
@@ -367,17 +369,13 @@ var _ = BeforeSuite(func(done Done) {
 
 	// set up the certificates and webhook before creating any objects
 	By("Creating and installing new certificates for ClusterBootstrap Admission Webhooks")
-	labelMatch, err = labels.NewRequirement(constants.AddonWebhookLabelKey, selection.Equals, []string{constants.AddonWebhookLabelValue})
-	Expect(err).ToNot(HaveOccurred())
-	whSelector = labels.NewSelector()
-	whSelector = whSelector.Add(*labelMatch)
 	webhookCertDetails = testutil.WebhookCertificatesDetails{
 		CertPath:           certPath,
 		KeyPath:            keyPath,
 		WebhookScrtName:    webhookScrtName,
 		AddonNamespace:     addonNamespace,
 		WebhookServiceName: webhookServiceName,
-		LabelSelector:      whSelector,
+		LabelSelector:      webhookSelector,
 	}
 	err = testutil.SetupWebhookCertificates(ctx, k8sClient, k8sConfig, &webhookCertDetails)
 	Expect(err).ToNot(HaveOccurred())

addons/main.go

@@ -305,8 +305,20 @@ func enableClusterBootstrapAndConfigControllers(ctx context.Context, mgr ctrl.Ma
 func enableWebhooks(ctx context.Context, mgr ctrl.Manager, flags *addonFlags) {
 	certPath := path.Join(constants.WebhookCertDir, "tls.crt")
 	keyPath := path.Join(constants.WebhookCertDir, "tls.key")
-	if _, err := webhooks.InstallNewCertificates(ctx, mgr.GetConfig(), certPath, keyPath, constants.WebhookScrtName, flags.addonNamespace, constants.WebhookServiceName, constants.AddonWebhookLabelKey+"="+constants.AddonWebhookLabelValue); err != nil {
-		setupLog.Error(err, "unable to install certificates for webhooks")
+	webhookTLS := webhooks.WebhookTLS{
+		Ctx:           ctx,
+		K8sConfig:     mgr.GetConfig(),
+		CertPath:      certPath,
+		KeyPath:       keyPath,
+		Name:          constants.WebhookScrtName,
+		ServiceName:   constants.WebhookServiceName,
+		LabelSelector: constants.AddonWebhookLabelKey + "=" + constants.AddonWebhookLabelValue,
+		Logger:        setupLog,
+		Namespace:     flags.addonNamespace,
+		RotationTime:  constants.WebhookCertLifeTime,
+	}
+	if err := webhookTLS.ManageCertificates(constants.WebhookCertManagementFrequency); err != nil {
+		setupLog.Error(err, "Unable to start webhook tls certificate management")
 		os.Exit(1)
 	}
 	// Set up the webhooks in the manager

addons/pkg/constants/constants.go

@@ -180,6 +180,12 @@ const (
 	// WebhookCertDir is the directory where the certificate and key are stored for webhook server TLS handshake
 	WebhookCertDir = "/tmp/k8s-webhook-server/serving-certs"
 
+	// WebhookCertManagementFrequency is how often the the certificates for webhook server TLS are managed
+	WebhookCertManagementFrequency = time.Second * 60
+
+	// WebhookCertLifeTime is how long the webhook server TLS certificates are good for
+	WebhookCertLifeTime = time.Hour * 24 * 7
+
 	// WebhookServiceName is the name of the k8s service that serves the admission requests
 	WebhookServiceName = "tanzu-addons-manager-webhook-service"
 

pkg/v1/webhooks/certificates.go

@@ -63,52 +63,65 @@ func addCertsToWebhookConfigs(ctx context.Context, k8sclient kubernetes.Interfac
 	return nil
 }
 
-func NewTLSSecret(ctx context.Context, secretName, serviceName, certPath, keyPath, namespace string) (*corev1.Secret, error) {
-	secret, err := resources.MakeSecretInternal(ctx, secretName, namespace, serviceName)
-	if err != nil {
-		return nil, err
-	}
-
+// WriteServerTLSToFileSystem writes servers certificate and key in provided secret to the filesystem paths provided.
+func WriteServerTLSToFileSystem(ctx context.Context, certPath, keyPath string, secret *corev1.Secret) error {
 	if err := certutil.WriteCert(certPath, secret.Data[resources.ServerCert]); err != nil {
-		return secret, err
+		return err
 	}
 	if err := keyutil.WriteKey(keyPath, secret.Data[resources.ServerKey]); err != nil {
-		return secret, err
+		return err
 	}
-
-	return secret, nil
+	return nil
 }
 
+// InstallNewCertificates creates a new set of keys and certificates and saves them to the filesystem paths provided.
+// Adds the CA certificate to webhook configurations matching label selector.
+// Returns a secret containing the server key, sever certificate and CA certificate.
 func InstallNewCertificates(ctx context.Context, k8sConfig *rest.Config, certPath, keyPath, secretName, namespace, serviceName, labelSelector string) (*corev1.Secret, error) {
 	if labelSelector == "" {
 		return nil, fmt.Errorf("label selector not provided for webhook configurations udpate")
 	}
-	secret, err := NewTLSSecret(ctx, secretName, serviceName, certPath, keyPath, namespace)
+	secret, err := resources.MakeSecret(ctx, secretName, namespace, serviceName)
 	if err != nil {
 		return nil, err
 	}
 
-	k8sclient, err := client.New(k8sConfig, client.Options{})
+	err = InstallCertificates(ctx, k8sConfig, secret, certPath, keyPath, labelSelector)
 	if err != nil {
 		return nil, err
 	}
+	return secret, nil
+}
+
+// InstallCertificates saves server certificate and key in provided secret to the filesystem paths provided.
+// Adds the CA certificate to webhook configuration matching label selector.
+func InstallCertificates(ctx context.Context, k8sConfig *rest.Config, secret *corev1.Secret, certPath, keyPath, labelSelector string) error {
+	if err := WriteServerTLSToFileSystem(ctx, certPath, keyPath, secret); err != nil {
+		return err
+	}
+	k8sclient, err := client.New(k8sConfig, client.Options{})
+	if err != nil {
+		return err
+	}
 	err = updateOrCreateTLSSecret(ctx, k8sclient, secret)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	clientSet, err := kubernetes.NewForConfig(k8sConfig)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	err = addCertsToWebhookConfigs(ctx, clientSet, labelSelector, secret)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	return secret, nil
+	return nil
 }
 
+// ValidateTLSSecret checks secret has all required keys and certificates.
+// Checks certificate lifetime is valid.
 func ValidateTLSSecret(tlsSecret *corev1.Secret, certGracePeriod time.Duration) error {
 	if tlsSecret == nil {
 		return fmt.Errorf("webhook certificate secret is empty")
@@ -137,7 +150,6 @@ func ValidateTLSSecret(tlsSecret *corev1.Secret, certGracePeriod time.Duration)
 	return nil
 }
 
-// write secret to cluster
 func updateOrCreateTLSSecret(ctx context.Context, k8sclient client.Client, tlsSecret *corev1.Secret) error {
 	if tlsSecret == nil {
 		return nil
@@ -151,6 +163,7 @@ func updateOrCreateTLSSecret(ctx context.Context, k8sclient client.Client, tlsSe
 		if err != nil {
 			return err
 		}
+		return nil
 	}
 	if err != nil {
 		return err

pkg/v1/webhooks/certificates_test.go

@@ -61,7 +61,9 @@ var _ = Describe("Webhook", func() {
 
 	Context("server's certificate and key", func() {
 		It("should be generated and written to the webhook server CertDir", func() {
-			secret, err := NewTLSSecret(ctx, webhookScrtName, webhookServiceName, certPath, keyPath, addonNamespace)
+			secret, err := resources.MakeSecret(ctx, webhookScrtName, addonNamespace, webhookServiceName)
+			Expect(err).ToNot(HaveOccurred())
+			err = WriteServerTLSToFileSystem(ctx, certPath, keyPath, secret)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(secret).NotTo(BeNil())
 			cert, err := cert2.CertsFromFile(certPath)
@@ -76,7 +78,7 @@ var _ = Describe("Webhook", func() {
 			Expect(key).To(Equal(orgKey))
 		})
 		It("should become invalid after one week", func() {
-			secret, err := NewTLSSecret(ctx, webhookScrtName, webhookServiceName, certPath, keyPath, addonNamespace)
+			secret, err := resources.MakeSecret(ctx, webhookScrtName, addonNamespace, webhookServiceName)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(secret).NotTo(BeNil())
 			err = ValidateTLSSecret(secret, time.Hour*24) // valid cert life is one week. One day should not make it invalid

pkg/v1/webhooks/webhooks.go

@@ -56,9 +56,14 @@ func (w *WebhookTLS) UpdateOrCreate() error {
 	err = clusterClient.Get(w.Ctx, client.ObjectKey{
 		Namespace: w.Namespace,
 		Name:      w.Name}, currentSecret)
-	if err == nil {
+	if err == nil { // secret found. Will use if valid
 		w.secret = currentSecret
-	} else if !apierrors.IsNotFound(err) { // secret not found = "Create" case.
+	} else if apierrors.IsNotFound(err) { // secret not found = "Create" case.
+		w.secret, err = resources.MakeSecret(w.Ctx, w.Name, w.Namespace, w.ServiceName)
+		if err != nil {
+			return err
+		}
+	} else {
 		return err
 	}
 
@@ -71,6 +76,10 @@ func (w *WebhookTLS) UpdateOrCreate() error {
 			return err
 		}
 	}
+	err = InstallCertificates(w.Ctx, w.K8sConfig, w.secret, w.CertPath, w.KeyPath, w.LabelSelector)
+	if err != nil {
+		return err
+	}
 	return nil
 }