Fix Webhook certificate recreate (#226)

* wip cert webhook

* fix lint

* cleanup and refactor

* fix go.mod

* removed logs

* renamed

* small simplification

* improved logging

* improved logging

* some tests for config data

* fix logs

* moved interface

pkg/controller/cluster/agent/agent.go

@@ -1,28 +1,55 @@
 package agent
 
 import (
+	"context"
+	"fmt"
+
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1alpha1"
 	"github.com/rancher/k3k/pkg/controller"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 const (
 	configName = "agent-config"
 )
 
-type Agent interface {
-	Name() string
-	Config() ctrlruntimeclient.Object
-	Resources() ([]ctrlruntimeclient.Object, error)
+type ResourceEnsurer interface {
+	EnsureResources(context.Context) error
 }
 
-func New(cluster *v1alpha1.Cluster, serviceIP, sharedAgentImage, sharedAgentImagePullPolicy, token string) Agent {
-	if cluster.Spec.Mode == VirtualNodeMode {
-		return NewVirtualAgent(cluster, serviceIP, token)
+type Config struct {
+	cluster *v1alpha1.Cluster
+	client  ctrlruntimeclient.Client
+	scheme  *runtime.Scheme
+}
+
+func NewConfig(cluster *v1alpha1.Cluster, client ctrlruntimeclient.Client, scheme *runtime.Scheme) *Config {
+	return &Config{
+		cluster: cluster,
+		client:  client,
+		scheme:  scheme,
 	}
-	return NewSharedAgent(cluster, serviceIP, sharedAgentImage, sharedAgentImagePullPolicy, token)
 }
 
 func configSecretName(clusterName string) string {
 	return controller.SafeConcatNameWithPrefix(clusterName, configName)
 }
+
+func ensureObject(ctx context.Context, cfg *Config, obj ctrlruntimeclient.Object) error {
+	log := ctrl.LoggerFrom(ctx)
+
+	result, err := controllerutil.CreateOrUpdate(ctx, cfg.client, obj, func() error {
+		return controllerutil.SetControllerReference(cfg.cluster, obj, cfg.scheme)
+	})
+
+	if result != controllerutil.OperationResultNone {
+		key := client.ObjectKeyFromObject(obj)
+		log.Info(fmt.Sprintf("ensuring %T", obj), "key", key, "result, result")
+	}
+
+	return err
+}

pkg/controller/cluster/agent/shared.go

@@ -1,8 +1,10 @@
 package agent
 
 import (
+	"context"
 	"crypto"
 	"crypto/x509"
+	"errors"
 	"fmt"
 	"time"
 
@@ -14,8 +16,10 @@ import (
 	apps "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -26,16 +30,16 @@ const (
 )
 
 type SharedAgent struct {
-	cluster         *v1alpha1.Cluster
+	*Config
 	serviceIP       string
 	image           string
 	imagePullPolicy string
 	token           string
 }
 
-func NewSharedAgent(cluster *v1alpha1.Cluster, serviceIP, image, imagePullPolicy, token string) Agent {
+func NewSharedAgent(config *Config, serviceIP, image, imagePullPolicy, token string) *SharedAgent {
 	return &SharedAgent{
-		cluster:         cluster,
+		Config:          config,
 		serviceIP:       serviceIP,
 		image:           image,
 		imagePullPolicy: imagePullPolicy,
@@ -43,10 +47,35 @@ func NewSharedAgent(cluster *v1alpha1.Cluster, serviceIP, image, imagePullPolicy
 	}
 }
 
-func (s *SharedAgent) Config() ctrlruntimeclient.Object {
+func (s *SharedAgent) Name() string {
+	return controller.SafeConcatNameWithPrefix(s.cluster.Name, SharedNodeAgentName)
+}
+
+func (s *SharedAgent) EnsureResources(ctx context.Context) error {
+	if err := errors.Join(
+		s.config(ctx),
+		s.serviceAccount(ctx),
+		s.role(ctx),
+		s.roleBinding(ctx),
+		s.service(ctx),
+		s.deployment(ctx),
+		s.dnsService(ctx),
+		s.webhookTLS(ctx),
+	); err != nil {
+		return fmt.Errorf("failed to ensure some resources: %w\n", err)
+	}
+
+	return nil
+}
+
+func (s *SharedAgent) ensureObject(ctx context.Context, obj ctrlruntimeclient.Object) error {
+	return ensureObject(ctx, s.Config, obj)
+}
+
+func (s *SharedAgent) config(ctx context.Context) error {
 	config := sharedAgentData(s.cluster, s.Name(), s.token, s.serviceIP)
 
-	return &v1.Secret{
+	configSecret := &v1.Secret{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Secret",
 			APIVersion: "v1",
@@ -59,6 +88,8 @@ func (s *SharedAgent) Config() ctrlruntimeclient.Object {
 			"config.yaml": []byte(config),
 		},
 	}
+
+	return s.ensureObject(ctx, configSecret)
 }
 
 func sharedAgentData(cluster *v1alpha1.Cluster, serviceName, token, ip string) string {
@@ -75,30 +106,14 @@ version: %s`,
 		cluster.Name, cluster.Namespace, ip, serviceName, token, version)
 }
 
-func (s *SharedAgent) Resources() ([]ctrlruntimeclient.Object, error) {
-	// generate certs for webhook
-	certSecret, err := s.webhookTLS()
-	if err != nil {
-		return nil, err
-	}
-	return []ctrlruntimeclient.Object{
-		s.serviceAccount(),
-		s.role(),
-		s.roleBinding(),
-		s.service(),
-		s.deployment(),
-		s.dnsService(),
-		certSecret}, nil
-}
-
-func (s *SharedAgent) deployment() *apps.Deployment {
+func (s *SharedAgent) deployment(ctx context.Context) error {
 	labels := map[string]string{
 		"cluster": s.cluster.Name,
 		"type":    "agent",
 		"mode":    "shared",
 	}
 
-	return &apps.Deployment{
+	deploy := &apps.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Deployment",
 			APIVersion: "apps/v1",
@@ -120,6 +135,8 @@ func (s *SharedAgent) deployment() *apps.Deployment {
 			},
 		},
 	}
+
+	return s.ensureObject(ctx, deploy)
 }
 
 func (s *SharedAgent) podSpec() v1.PodSpec {
@@ -208,11 +225,12 @@ func (s *SharedAgent) podSpec() v1.PodSpec {
 					},
 				},
 			},
-		}}
+		},
+	}
 }
 
-func (s *SharedAgent) service() *v1.Service {
-	return &v1.Service{
+func (s *SharedAgent) service(ctx context.Context) error {
+	svc := &v1.Service{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Service",
 			APIVersion: "v1",
@@ -243,16 +261,20 @@ func (s *SharedAgent) service() *v1.Service {
 			},
 		},
 	}
+
+	return s.ensureObject(ctx, svc)
 }
 
-func (s *SharedAgent) dnsService() *v1.Service {
-	return &v1.Service{
+func (s *SharedAgent) dnsService(ctx context.Context) error {
+	dnsServiceName := controller.SafeConcatNameWithPrefix(s.cluster.Name, "kube-dns")
+
+	svc := &v1.Service{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Service",
 			APIVersion: "v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      s.DNSName(),
+			Name:      dnsServiceName,
 			Namespace: s.cluster.Namespace,
 		},
 		Spec: v1.ServiceSpec{
@@ -283,10 +305,12 @@ func (s *SharedAgent) dnsService() *v1.Service {
 			},
 		},
 	}
+
+	return s.ensureObject(ctx, svc)
 }
 
-func (s *SharedAgent) serviceAccount() *v1.ServiceAccount {
-	return &v1.ServiceAccount{
+func (s *SharedAgent) serviceAccount(ctx context.Context) error {
+	svcAccount := &v1.ServiceAccount{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "ServiceAccount",
 			APIVersion: "v1",
@@ -296,10 +320,12 @@ func (s *SharedAgent) serviceAccount() *v1.ServiceAccount {
 			Namespace: s.cluster.Namespace,
 		},
 	}
+
+	return s.ensureObject(ctx, svcAccount)
 }
 
-func (s *SharedAgent) role() *rbacv1.Role {
-	return &rbacv1.Role{
+func (s *SharedAgent) role(ctx context.Context) error {
+	role := &rbacv1.Role{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Role",
 			APIVersion: "rbac.authorization.k8s.io/v1",
@@ -321,10 +347,12 @@ func (s *SharedAgent) role() *rbacv1.Role {
 			},
 		},
 	}
+
+	return s.ensureObject(ctx, role)
 }
 
-func (s *SharedAgent) roleBinding() *rbacv1.RoleBinding {
-	return &rbacv1.RoleBinding{
+func (s *SharedAgent) roleBinding(ctx context.Context) error {
+	roleBinding := &rbacv1.RoleBinding{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "RoleBinding",
 			APIVersion: "rbac.authorization.k8s.io/v1",
@@ -346,49 +374,12 @@ func (s *SharedAgent) roleBinding() *rbacv1.RoleBinding {
 			},
 		},
 	}
+
+	return s.ensureObject(ctx, roleBinding)
 }
 
-func (s *SharedAgent) Name() string {
-	return controller.SafeConcatNameWithPrefix(s.cluster.Name, SharedNodeAgentName)
-}
-
-func (s *SharedAgent) DNSName() string {
-	return controller.SafeConcatNameWithPrefix(s.cluster.Name, "kube-dns")
-}
-
-func (s *SharedAgent) webhookTLS() (*v1.Secret, error) {
-	// generate CA CERT/KEY
-	caKeyBytes, err := certutil.MakeEllipticPrivateKeyPEM()
-	if err != nil {
-		return nil, err
-	}
-
-	caKey, err := certutil.ParsePrivateKeyPEM(caKeyBytes)
-	if err != nil {
-		return nil, err
-	}
-
-	cfg := certutil.Config{
-		CommonName: fmt.Sprintf("k3k-webhook-ca@%d", time.Now().Unix()),
-	}
-
-	caCert, err := certutil.NewSelfSignedCACert(cfg, caKey.(crypto.Signer))
-	if err != nil {
-		return nil, err
-	}
-
-	caCertBytes := certutil.EncodeCertPEM(caCert)
-	// generate webhook cert bundle
-	altNames := certs.AddSANs([]string{s.Name(), s.cluster.Name})
-	webhookCert, webhookKey, err := certs.CreateClientCertKey(
-		s.Name(), nil,
-		&altNames, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, time.Hour*24*time.Duration(356),
-		string(caCertBytes),
-		string(caKeyBytes))
-	if err != nil {
-		return nil, err
-	}
-	return &v1.Secret{
+func (s *SharedAgent) webhookTLS(ctx context.Context) error {
+	webhookSecret := &v1.Secret{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Secret",
 			APIVersion: "v1",
@@ -397,13 +388,80 @@ func (s *SharedAgent) webhookTLS() (*v1.Secret, error) {
 			Name:      WebhookSecretName(s.cluster.Name),
 			Namespace: s.cluster.Namespace,
 		},
-		Data: map[string][]byte{
+	}
+
+	key := client.ObjectKeyFromObject(webhookSecret)
+	if err := s.client.Get(ctx, key, webhookSecret); err != nil {
+		if !apierrors.IsNotFound(err) {
+			return err
+		}
+
+		caPrivateKeyPEM, caCertPEM, err := newWebhookSelfSignedCACerts()
+		if err != nil {
+			return err
+		}
+
+		altNames := []string{s.Name(), s.cluster.Name}
+		webhookCert, webhookKey, err := newWebhookCerts(s.Name(), altNames, caPrivateKeyPEM, caCertPEM)
+		if err != nil {
+			return err
+		}
+
+		webhookSecret.Data = map[string][]byte{
 			"tls.crt": webhookCert,
 			"tls.key": webhookKey,
-			"ca.crt":  caCertBytes,
-			"ca.key":  caKeyBytes,
-		},
-	}, nil
+			"ca.crt":  caCertPEM,
+			"ca.key":  caPrivateKeyPEM,
+		}
+
+		return s.ensureObject(ctx, webhookSecret)
+	}
+
+	// if the webhook secret is found we can skip
+	// we should check for their validity
+	return nil
+}
+
+func newWebhookSelfSignedCACerts() ([]byte, []byte, error) {
+	// generate CA CERT/KEY
+	caPrivateKeyPEM, err := certutil.MakeEllipticPrivateKeyPEM()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	caPrivateKey, err := certutil.ParsePrivateKeyPEM(caPrivateKeyPEM)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	cfg := certutil.Config{
+		CommonName: fmt.Sprintf("k3k-webhook-ca@%d", time.Now().Unix()),
+	}
+
+	caCert, err := certutil.NewSelfSignedCACert(cfg, caPrivateKey.(crypto.Signer))
+	if err != nil {
+		return nil, nil, err
+	}
+
+	caCertPEM := certutil.EncodeCertPEM(caCert)
+
+	return caPrivateKeyPEM, caCertPEM, nil
+}
+
+func newWebhookCerts(commonName string, subAltNames []string, caPrivateKey, caCert []byte) ([]byte, []byte, error) {
+	// generate webhook cert bundle
+	altNames := certs.AddSANs(subAltNames)
+	oneYearExpiration := time.Until(time.Now().AddDate(1, 0, 0))
+
+	return certs.CreateClientCertKey(
+		commonName,
+		nil,
+		&altNames,
+		[]x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		oneYearExpiration,
+		string(caCert),
+		string(caPrivateKey),
+	)
 }
 
 func WebhookSecretName(clusterName string) string {

pkg/controller/cluster/agent/shared_test.go

@@ -0,0 +1,114 @@
+package agent
+
+import (
+	"testing"
+
+	"github.com/rancher/k3k/pkg/apis/k3k.io/v1alpha1"
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v2"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func Test_sharedAgentData(t *testing.T) {
+	type args struct {
+		cluster     *v1alpha1.Cluster
+		serviceName string
+		ip          string
+		token       string
+	}
+	tests := []struct {
+		name         string
+		args         args
+		expectedData map[string]string
+	}{
+		{
+			name: "simple config",
+			args: args{
+				cluster: &v1alpha1.Cluster{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "mycluster",
+						Namespace: "ns-1",
+					},
+					Spec: v1alpha1.ClusterSpec{
+						Version: "v1.2.3",
+					},
+				},
+				ip:          "10.0.0.21",
+				serviceName: "service-name",
+				token:       "dnjklsdjnksd892389238",
+			},
+			expectedData: map[string]string{
+				"clusterName":      "mycluster",
+				"clusterNamespace": "ns-1",
+				"serverIP":         "10.0.0.21",
+				"serviceName":      "service-name",
+				"token":            "dnjklsdjnksd892389238",
+				"version":          "v1.2.3",
+			},
+		},
+		{
+			name: "version in status",
+			args: args{
+				cluster: &v1alpha1.Cluster{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "mycluster",
+						Namespace: "ns-1",
+					},
+					Spec: v1alpha1.ClusterSpec{
+						Version: "v1.2.3",
+					},
+					Status: v1alpha1.ClusterStatus{
+						HostVersion: "v1.3.3",
+					},
+				},
+				ip:          "10.0.0.21",
+				serviceName: "service-name",
+				token:       "dnjklsdjnksd892389238",
+			},
+			expectedData: map[string]string{
+				"clusterName":      "mycluster",
+				"clusterNamespace": "ns-1",
+				"serverIP":         "10.0.0.21",
+				"serviceName":      "service-name",
+				"token":            "dnjklsdjnksd892389238",
+				"version":          "v1.2.3",
+			},
+		},
+		{
+			name: "missing version in spec",
+			args: args{
+				cluster: &v1alpha1.Cluster{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "mycluster",
+						Namespace: "ns-1",
+					},
+					Status: v1alpha1.ClusterStatus{
+						HostVersion: "v1.3.3",
+					},
+				},
+				ip:          "10.0.0.21",
+				serviceName: "service-name",
+				token:       "dnjklsdjnksd892389238",
+			},
+			expectedData: map[string]string{
+				"clusterName":      "mycluster",
+				"clusterNamespace": "ns-1",
+				"serverIP":         "10.0.0.21",
+				"serviceName":      "service-name",
+				"token":            "dnjklsdjnksd892389238",
+				"version":          "v1.3.3",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			config := sharedAgentData(tt.args.cluster, tt.args.serviceName, tt.args.token, tt.args.ip)
+
+			data := make(map[string]string)
+			err := yaml.Unmarshal([]byte(config), data)
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expectedData, data)
+		})
+	}
+}

pkg/controller/cluster/agent/virtual.go

@@ -1,9 +1,10 @@
 package agent
 
 import (
+	"context"
+	"errors"
 	"fmt"
 
-	"github.com/rancher/k3k/pkg/apis/k3k.io/v1alpha1"
 	"github.com/rancher/k3k/pkg/controller"
 	apps "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
@@ -18,23 +19,42 @@ const (
 )
 
 type VirtualAgent struct {
-	cluster   *v1alpha1.Cluster
+	*Config
 	serviceIP string
 	token     string
 }
 
-func NewVirtualAgent(cluster *v1alpha1.Cluster, serviceIP, token string) Agent {
+func NewVirtualAgent(config *Config, serviceIP, token string) *VirtualAgent {
 	return &VirtualAgent{
-		cluster:   cluster,
+		Config:    config,
 		serviceIP: serviceIP,
 		token:     token,
 	}
 }
 
-func (v *VirtualAgent) Config() ctrlruntimeclient.Object {
+func (v *VirtualAgent) Name() string {
+	return controller.SafeConcatNameWithPrefix(v.cluster.Name, virtualNodeAgentName)
+}
+
+func (v *VirtualAgent) EnsureResources(ctx context.Context) error {
+	if err := errors.Join(
+		v.config(ctx),
+		v.deployment(ctx),
+	); err != nil {
+		return fmt.Errorf("failed to ensure some resources: %w\n", err)
+	}
+
+	return nil
+}
+
+func (v *VirtualAgent) ensureObject(ctx context.Context, obj ctrlruntimeclient.Object) error {
+	return ensureObject(ctx, v.Config, obj)
+}
+
+func (v *VirtualAgent) config(ctx context.Context) error {
 	config := virtualAgentData(v.serviceIP, v.token)
 
-	return &v1.Secret{
+	configSecret := &v1.Secret{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Secret",
 			APIVersion: "v1",
@@ -47,10 +67,8 @@ func (v *VirtualAgent) Config() ctrlruntimeclient.Object {
 			"config.yaml": []byte(config),
 		},
 	}
-}
 
-func (v *VirtualAgent) Resources() ([]ctrlruntimeclient.Object, error) {
-	return []ctrlruntimeclient.Object{v.deployment()}, nil
+	return v.ensureObject(ctx, configSecret)
 }
 
 func virtualAgentData(serviceIP, token string) string {
@@ -59,7 +77,7 @@ token: %s
 with-node-id: true`, serviceIP, token)
 }
 
-func (v *VirtualAgent) deployment() *apps.Deployment {
+func (v *VirtualAgent) deployment(ctx context.Context) error {
 	image := controller.K3SImage(v.cluster)
 
 	const name = "k3k-agent"
@@ -70,7 +88,8 @@ func (v *VirtualAgent) deployment() *apps.Deployment {
 			"mode":    "virtual",
 		},
 	}
-	return &apps.Deployment{
+
+	deployment := &apps.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Deployment",
 			APIVersion: "apps/v1",
@@ -91,6 +110,8 @@ func (v *VirtualAgent) deployment() *apps.Deployment {
 			},
 		},
 	}
+
+	return v.ensureObject(ctx, deployment)
 }
 
 func (v *VirtualAgent) podSpec(image, name string, args []string, affinitySelector *metav1.LabelSelector) v1.PodSpec {
@@ -206,7 +227,3 @@ func (v *VirtualAgent) podSpec(image, name string, args []string, affinitySelect
 
 	return podSpec
 }
-
-func (v *VirtualAgent) Name() string {
-	return controller.SafeConcatNameWithPrefix(v.cluster.Name, virtualNodeAgentName)
-}

pkg/controller/cluster/agent/virtual_test.go

@@ -0,0 +1,44 @@
+package agent
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v2"
+)
+
+func Test_virtualAgentData(t *testing.T) {
+	type args struct {
+		serviceIP string
+		token     string
+	}
+	tests := []struct {
+		name         string
+		args         args
+		expectedData map[string]string
+	}{
+		{
+			name: "simple config",
+			args: args{
+				serviceIP: "10.0.0.21",
+				token:     "dnjklsdjnksd892389238",
+			},
+			expectedData: map[string]string{
+				"server":       "https://10.0.0.21:6443",
+				"token":        "dnjklsdjnksd892389238",
+				"with-node-id": "true",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			config := virtualAgentData(tt.args.serviceIP, tt.args.token)
+
+			data := make(map[string]string)
+			err := yaml.Unmarshal([]byte(config), data)
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expectedData, data)
+		})
+	}
+}