diff --git a/Dockerfile b/Dockerfile index 8844e70b..c8876fb8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.19 as builder +FROM golang:1.20 as builder WORKDIR /workspace # Copy the Go Modules manifests diff --git a/Makefile b/Makefile index 981d1d39..5bb25e27 100644 --- a/Makefile +++ b/Makefile @@ -108,6 +108,7 @@ setup-envtest: $(SETUP_ENVTEST) # Build setup-envtest unit-tests: manifests generate fmt vet setup-envtest ## Run unit tests. KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test ./internal/... -test.v -coverprofile cover.out KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test ./pkg/... -test.v -coverprofile cover.out + KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test . -test.v -coverprofile cover.out .PHONY: setup-envtest integration-tests integration-tests: manifests generate fmt vet setup-envtest ## Run integration tests. @@ -124,7 +125,7 @@ build: generate fmt vet ## Build manager binary. go build -o bin/manager . run: manifests generate fmt vet ## Run a controller from your host. - go run . -deployments-namespace kubewarden --default-policy-server default + go run . -deployments-namespace kubewarden --default-policy-server default -zap-log-level debug docker-build: test ## Build docker image with the manager. docker build -t ${IMG} . diff --git a/controllers/admissionpolicy_controller_test.go b/controllers/admissionpolicy_controller_test.go index 80df5471..19782861 100644 --- a/controllers/admissionpolicy_controller_test.go +++ b/controllers/admissionpolicy_controller_test.go @@ -60,8 +60,8 @@ var _ = Describe("Given an AdmissionPolicy", func() { Context("and it has a non-empty policy server set on its spec", func() { var ( policyNamespace = someNamespace.Name - policyName = "scheduled-policy" policyServerName = "some-policy-server" + policyName = "scheduled-policy" ) BeforeEach(func() { Expect( @@ -76,9 +76,7 @@ var _ = Describe("Given an AdmissionPolicy", func() { func(admissionPolicy *policiesv1.AdmissionPolicy) policiesv1.PolicyStatusEnum { return admissionPolicy.Status.PolicyStatus }, - Equal(policiesv1.PolicyStatusScheduled), - ), - ) + Equal(policiesv1.PolicyStatusScheduled))) }) Context("and the targeted policy server is created", func() { BeforeEach(func() { @@ -86,6 +84,11 @@ var _ = Describe("Given an AdmissionPolicy", func() { k8sClient.Create(ctx, policyServer(policyServerName)), ).To(HaveSucceededOrAlreadyExisted()) }) + AfterEach(func() { + Expect( + k8sClient.Delete(ctx, policyServer(policyServerName)), + ).To(Succeed()) + }) It(fmt.Sprintf("should set its policy status to %q", policiesv1.PolicyStatusPending), func() { Eventually(func(g Gomega) (*policiesv1.AdmissionPolicy, error) { return getFreshAdmissionPolicy(policyNamespace, policyName) diff --git a/controllers/clusteradmissionpolicy_controller.go b/controllers/clusteradmissionpolicy_controller.go index 0b98a9e1..adcb6ed5 100644 --- a/controllers/clusteradmissionpolicy_controller.go +++ b/controllers/clusteradmissionpolicy_controller.go @@ -67,7 +67,7 @@ func (r *ClusterAdmissionPolicyReconciler) Reconcile(ctx context.Context, req ct if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } - return ctrl.Result{}, fmt.Errorf("cannot retrieve admission policy: %w", err) + return ctrl.Result{}, fmt.Errorf("cannot retrieve cluster admission policy: %w", err) } return startReconciling(ctx, r.Reconciler.Client, r.Reconciler, &clusterAdmissionPolicy) diff --git a/controllers/clusteradmissionpolicy_controller_test.go b/controllers/clusteradmissionpolicy_controller_test.go index f66bbdaf..481f9ce5 100644 --- a/controllers/clusteradmissionpolicy_controller_test.go +++ b/controllers/clusteradmissionpolicy_controller_test.go @@ -20,9 +20,12 @@ import ( "fmt" "time" + // "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + // admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + // "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("Given a ClusterAdmissionPolicy", func() { @@ -54,11 +57,11 @@ var _ = Describe("Given a ClusterAdmissionPolicy", func() { var ( policyName = "scheduled-policy" policyServerName = "other-policy-server" + policy *policiesv1.ClusterAdmissionPolicy ) BeforeEach(func() { - Expect( - k8sClient.Create(ctx, clusterAdmissionPolicyWithPolicyServerName(policyName, policyServerName)), - ).To(HaveSucceededOrAlreadyExisted()) + policy = clusterAdmissionPolicyWithPolicyServerName(policyName, policyServerName) + Expect(k8sClient.Create(ctx, policy)).To(HaveSucceededOrAlreadyExisted()) }) It(fmt.Sprintf("should set its policy status to %q", policiesv1.PolicyStatusScheduled), func() { Eventually(func(g Gomega) (*policiesv1.ClusterAdmissionPolicy, error) { diff --git a/controllers/policy_utils.go b/controllers/policy_utils.go index 6403ae40..d8714b6d 100644 --- a/controllers/policy_utils.go +++ b/controllers/policy_utils.go @@ -76,6 +76,12 @@ func startReconciling(ctx context.Context, client client.Client, reconciler admi _ = setPolicyStatus(ctx, reconciler.DeploymentsNamespace, reconciler.APIReader, policy) if err := client.Status().Update(ctx, policy); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{ + Requeue: true, + RequeueAfter: time.Second * 5, + }, nil + } return ctrl.Result{}, fmt.Errorf("update admission policy status error: %w", err) } @@ -144,8 +150,8 @@ func reconcilePolicy(ctx context.Context, client client.Client, reconciler admis ) secret := corev1.Secret{} - if err := client.Get(ctx, types.NamespacedName{Namespace: reconciler.DeploymentsNamespace, Name: constants.PolicyServerCARootSecretName}, &secret); err != nil { - return ctrl.Result{}, errors.Wrap(err, "cannot find policy server secret") + if err := client.Get(ctx, types.NamespacedName{Namespace: reconciler.DeploymentsNamespace, Name: constants.KubewardenCARootSecretName}, &secret); err != nil { + return ctrl.Result{}, errors.Wrap(err, "cannot find root CA secret") } if policy.IsMutating() { diff --git a/controllers/policyserver_controller.go b/controllers/policyserver_controller.go index 047848a2..f96eed55 100644 --- a/controllers/policyserver_controller.go +++ b/controllers/policyserver_controller.go @@ -26,6 +26,7 @@ import ( "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -80,6 +81,12 @@ func (r *PolicyServerReconciler) Reconcile(ctx context.Context, req ctrl.Request reconcileResult, reconcileErr := r.reconcile(ctx, &policyServer, policies) if err := r.Client.Status().Update(ctx, &policyServer); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{ + Requeue: true, + RequeueAfter: time.Second * 5, + }, nil + } return ctrl.Result{}, fmt.Errorf("update policy server status error: %w", err) } @@ -191,6 +198,27 @@ func (r *PolicyServerReconciler) SetupWithManager(mgr ctrl.Manager) error { }, } })). + Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(func(object client.Object) []reconcile.Request { + // watches for secret to detect when a policy server + // secret change. Therefore, the certificate can be + // recreated and the webhooks updated + secret, ok := object.(*corev1.Secret) + if !ok { + r.Log.Info("object is not type of secret: %+v", "secret", secret) + return []ctrl.Request{} + } + + if policyServerName, isPolicyServerSecret := secret.Labels[constants.PolicyServerLabelKey]; isPolicyServerSecret { + return []ctrl.Request{ + { + NamespacedName: client.ObjectKey{ + Name: policyServerName, + }, + }, + } + } + return []ctrl.Request{} + })). Complete(r) return errors.Wrap(err, "failed enrolling controller with manager") diff --git a/controllers/policyserver_controller_test.go b/controllers/policyserver_controller_test.go index f77cfef4..9ea4263a 100644 --- a/controllers/policyserver_controller_test.go +++ b/controllers/policyserver_controller_test.go @@ -23,20 +23,110 @@ import ( policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" ) var _ = Describe("Given a PolicyServer", func() { var ( - policyServerName = "policy-server" + policyServerName = "policy-server" + policyServerNameWithPrefix = policyServer(policyServerName).NameWithPrefix() ) BeforeEach(func() { Expect( k8sClient.Create(ctx, policyServer(policyServerName)), ).To(HaveSucceededOrAlreadyExisted()) }) + Context("policy server certificate", func() { + It("a secret for the policy server certificate should be created", func() { + Eventually(func(g Gomega) ([]string, error) { + secret := &corev1.Secret{} + err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret) + if err != nil { + return []string{}, fmt.Errorf("failed to get policy server certificate secret: %s", err.Error()) + } + dataKeys := []string{} + for key := range secret.Data { + dataKeys = append(dataKeys, key) + } + return dataKeys, nil + }, 30*time.Second, 250*time.Millisecond).Should(Equal([]string{constants.PolicyServerTLSCert, constants.PolicyServerTLSKey})) + }) + It("policy server should have a label with the latest certificate secret resource version", func() { + Eventually(func(g Gomega) bool { + secret := &corev1.Secret{} + err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret) + Expect(err).ToNot(HaveOccurred()) + + policyServerDeploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, policyServerDeploy) + Expect(err).ToNot(HaveOccurred()) + + return secret.GetResourceVersion() == policyServerDeploy.Spec.Template.Labels[constants.PolicyServerCertificateSecret] + }, 30*time.Second, 250*time.Millisecond).Should(Equal(true)) + }) + }) + When("policy server secret is deleted", func() { + BeforeEach(func() { + Expect( + k8sClient.Delete(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: policyServerNameWithPrefix, + Namespace: DeploymentsNamespace, + }, + })).To(Succeed()) + Eventually(func(g Gomega) bool { + err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, &corev1.Secret{}) + return apierrors.IsNotFound(err) + }, 30*time.Second, 250*time.Millisecond).Should(BeTrue()) + }) + It("it should be recreated", func() { + Eventually(func(g Gomega) ([]string, error) { + secret := &corev1.Secret{} + err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret) + if err != nil { + return []string{}, fmt.Errorf("failed to get policy server certificate secret: %s", err.Error()) + } + dataKeys := []string{} + for key := range secret.Data { + dataKeys = append(dataKeys, key) + } + return dataKeys, nil + }, 30*time.Second, 250*time.Millisecond).Should(Equal([]string{constants.PolicyServerTLSCert, constants.PolicyServerTLSKey})) + }) + It("policy server should have a label with the latest certificate secret resource version", func() { + Eventually(func(g Gomega) (bool, error) { + secret := &corev1.Secret{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, secret); err != nil { + return false, fmt.Errorf("failed to get policy server certificate secret: $%s", err.Error()) + } + + policyServerDeploy := &appsv1.Deployment{} + if err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, policyServerDeploy); err != nil { + return false, fmt.Errorf("failed to get policy server deployment: %s", err.Error()) + } + + return secret.GetResourceVersion() == policyServerDeploy.Spec.Template.Labels[constants.PolicyServerCertificateSecret], nil + }, 30*time.Second, 250*time.Millisecond).Should(Equal(true)) + }) + }) + When("policy server is deleted", func() { + It("its secret should be deleted as well", func() { + Expect( + k8sClient.Delete(ctx, policyServer(policyServerName)), + ).To(Succeed()) + + Eventually(func(g Gomega) bool { + err := k8sClient.Get(ctx, client.ObjectKey{Name: policyServerNameWithPrefix, Namespace: DeploymentsNamespace}, &corev1.Secret{}) + return apierrors.IsNotFound(err) + }, 30*time.Second, 250*time.Millisecond).Should(BeTrue()) + }) + }) When("it has no assigned policies", func() { Context("and it is deleted", func() { BeforeEach(func() { @@ -64,6 +154,7 @@ var _ = Describe("Given a PolicyServer", func() { k8sClient.Create(ctx, clusterAdmissionPolicyWithPolicyServerName(policyName, policyServerName)), ).To(HaveSucceededOrAlreadyExisted()) }) + Context("and it is deleted", func() { BeforeEach(func() { Expect( diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 78d540e7..f1268618 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "crypto/x509" "log" "path/filepath" "testing" @@ -36,6 +37,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/kubewarden/kubewarden-controller/internal/pkg/admission" + "github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration" + "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" //+kubebuilder:scaffold:imports ) @@ -48,6 +51,7 @@ var testEnv *envtest.Environment var ctx context.Context var cancel context.CancelFunc var reconciler admission.Reconciler +var caRootSecret *corev1.Secret const ( DeploymentsNamespace = "kubewarden-integration-tests" @@ -125,6 +129,34 @@ var _ = BeforeSuite(func() { log.Fatalf("could not create namespace %q needed for the integration tests", DeploymentsNamespace) } + // Create the root CA generated in the main of the controller + caRoot, err := admissionregistration.GenerateCA() + if err != nil { + log.Fatalf("cannot generate policy-server secret CA") + } + caPEMEncoded, err := admissionregistration.PemEncodeCertificate(caRoot.CaCert) + if err != nil { + log.Fatalf("cannot encode policy-server secret CA") + } + caPrivateKeyBytes := x509.MarshalPKCS1PrivateKey(caRoot.CaPrivateKey) + caRootSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.KubewardenCARootSecretName, + Namespace: DeploymentsNamespace, + }, + Data: map[string][]byte{ + constants.CARootCACert: caRoot.CaCert, + constants.CARootCACertPem: caPEMEncoded, + constants.CARootPrivateKeyCertName: caPrivateKeyBytes, + }, + Type: corev1.SecretTypeOpaque, + } + + // Create the integration tests deployments namespace + if err := k8sClient.Create(ctx, caRootSecret); err != nil { + log.Fatalf("could not create root CA secret: ") + } + go func() { defer GinkgoRecover() err = k8sManager.Start(ctx) diff --git a/controllers/utils_test.go b/controllers/utils_test.go index efee947f..bb4b15bf 100644 --- a/controllers/utils_test.go +++ b/controllers/utils_test.go @@ -63,7 +63,7 @@ var ( } ) -func AlreadyExists() types.GomegaMatcher { //nolint:ireturn +func AlreadyExists() types.GomegaMatcher { //nolint:ireturn,nolintlint return WithTransform( func(err error) bool { return err != nil && apierrors.IsAlreadyExists(err) diff --git a/go.mod b/go.mod index 8a5eb24f..a6216350 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/kubewarden/kubewarden-controller go 1.20 require ( - github.com/ereslibre/kube-webhook-wrapper v0.0.2 github.com/go-logr/logr v1.2.4 github.com/google/go-cmp v0.5.9 github.com/onsi/ginkgo/v2 v2.9.2 @@ -46,6 +45,7 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/kr/pretty v0.2.1 // indirect + github.com/kubewarden/kube-webhook-wrapper v0.0.0-20230830124812-3fdf2b58aa57 github.com/mailru/easyjson v0.7.7 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect diff --git a/go.sum b/go.sum index 408647bb..0b55f45a 100644 --- a/go.sum +++ b/go.sum @@ -123,8 +123,6 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210217033140-668b12f5399d/go.m github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ= github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= -github.com/ereslibre/kube-webhook-wrapper v0.0.2 h1:GaSN5jfSPJV7KNuVzSFflS5Rr5wQNKVsZbL6Gq1ZHYw= -github.com/ereslibre/kube-webhook-wrapper v0.0.2/go.mod h1:vDSrlA2/6bFRlDdVCC0Woe21U5J9s9a7FfSgaxuTPv4= github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= github.com/evanphx/json-patch v4.11.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U= @@ -332,6 +330,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/kubewarden/kube-webhook-wrapper v0.0.0-20230830124812-3fdf2b58aa57 h1:vfvTjvPzMzc4Y+icuoAW7uITZvcMJpPQMgY0KfjY93s= +github.com/kubewarden/kube-webhook-wrapper v0.0.0-20230830124812-3fdf2b58aa57/go.mod h1:2VL5IDaVqv0klHS7TwpKGzFXs+1h9RvjF0qEvxN+TH4= github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= diff --git a/internal/pkg/admission/mutating-webhook.go b/internal/pkg/admission/mutating-webhook.go index 10d3ebc5..27e898dc 100644 --- a/internal/pkg/admission/mutating-webhook.go +++ b/internal/pkg/admission/mutating-webhook.go @@ -90,7 +90,7 @@ func (r *Reconciler) mutatingWebhookConfiguration( Name: fmt.Sprintf("%s.kubewarden.admission", policy.GetUniqueName()), ClientConfig: admissionregistrationv1.WebhookClientConfig{ Service: &service, - CABundle: admissionSecret.Data[constants.PolicyServerCARootPemName], + CABundle: admissionSecret.Data[constants.CARootCACertPem], }, Rules: policy.GetRules(), FailurePolicy: policy.GetFailurePolicy(), diff --git a/internal/pkg/admission/policy-server-ca-secret.go b/internal/pkg/admission/policy-server-ca-secret.go index be52e273..e8130f14 100644 --- a/internal/pkg/admission/policy-server-ca-secret.go +++ b/internal/pkg/admission/policy-server-ca-secret.go @@ -17,7 +17,7 @@ import ( type generateCAFunc = func() (*admissionregistration.CA, error) type pemEncodeCertificateFunc = func(certificate []byte) ([]byte, error) -type generateCertFunc = func(ca []byte, commonName string, extraSANs []string, CAPrivateKey *rsa.PrivateKey) ([]byte, []byte, error) +type generateCertFunc = func(ca []byte, extraSANs []string, CAPrivateKey *rsa.PrivateKey) ([]byte, []byte, error) func (r *Reconciler) reconcileCASecret(ctx context.Context, secret *corev1.Secret) error { err := r.Client.Create(ctx, secret) @@ -28,64 +28,48 @@ func (r *Reconciler) reconcileCASecret(ctx context.Context, secret *corev1.Secre return fmt.Errorf("error reconciling policy-server CA Secret: %w", err) } -func (r *Reconciler) fetchOrInitializePolicyServerCASecret(ctx context.Context, policyServerName string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) { - policyServerSecret := corev1.Secret{} - err := r.Client.Get( - ctx, - client.ObjectKey{ - Namespace: r.DeploymentsNamespace, - Name: policyServerName}, - &policyServerSecret) - if err != nil && apierrors.IsNotFound(err) { - secret, err := r.buildPolicyServerCASecret(policyServerName, caSecret, generateCert) - if err != nil { - return secret, fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err) +func ReconcileSecret(ctx context.Context, clusterClient client.Client, secret *corev1.Secret) error { + err := clusterClient.Create(ctx, secret) + if err != nil && apierrors.IsAlreadyExists(err) { + if err := clusterClient.Update(ctx, secret); err != nil { + return fmt.Errorf("failed to update secret \"%s\": %s", secret.Name, err.Error()) } - return secret, nil + return nil } - if err != nil { - return &corev1.Secret{}, - fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err) + if err == nil { + return nil } - - policyServerSecret.ResourceVersion = "" - - return &policyServerSecret, nil + return fmt.Errorf("error reconciling policy-server CA Secret: %w", err) } -func (r *Reconciler) buildPolicyServerCASecret(policyServerName string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) { - admissionregCA, err := extractCaFromSecret(caSecret) - if err != nil { - return nil, err +func (r *Reconciler) fetchOrInitializePolicyServerCASecret(ctx context.Context, policyServerName string, policyServerServiceName string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) { + secret, initialized, err := FetchOrInitializeCertificate(ctx, r.Client, policyServerServiceName, r.DeploymentsNamespace, policyServerServiceName, caSecret, generateCert) + if initialized { + // label used to detect when the policy server certificate + // change and triggering the webhook's caBundle updates + secret.Labels[constants.PolicyServerLabelKey] = policyServerName } - servingCert, servingKey, err := generateCert( - admissionregCA.CaCert, - fmt.Sprintf("%s.%s.svc", policyServerName, r.DeploymentsNamespace), - []string{fmt.Sprintf("%s.%s.svc", policyServerName, r.DeploymentsNamespace)}, - admissionregCA.CaPrivateKey) - if err != nil { - return nil, fmt.Errorf("cannot generate policy-server %s certificate: %w", policyServerName, err) + return secret, err +} + +func ExtractCertificateData(secret *corev1.Secret) ([]byte, []byte, error) { + certificate, ok := secret.Data[constants.CARootCACertPem] + if !ok { + return []byte{}, []byte{}, fmt.Errorf("failed to initialize root CA certificate") } - secretContents := map[string]string{ - constants.PolicyServerTLSCert: string(servingCert), - constants.PolicyServerTLSKey: string(servingKey), + privateKey, ok := secret.Data[constants.CARootPrivateKeyCertName] + if !ok { + return []byte{}, []byte{}, fmt.Errorf("failed to initialize root CA private key") } - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: policyServerName, - Namespace: r.DeploymentsNamespace, - }, - StringData: secretContents, - Type: corev1.SecretTypeOpaque, - }, nil + return certificate, privateKey, nil } func extractCaFromSecret(caSecret *corev1.Secret) (*admissionregistration.CA, error) { - caCert, ok := caSecret.Data[constants.PolicyServerCARootCACert] + caCert, ok := caSecret.Data[constants.CARootCACert] if !ok { return nil, fmt.Errorf("CA could not be extracted from secret %s", caSecret.Kind) } - caPrivateKeyBytes, ok := caSecret.Data[constants.PolicyServerCARootPrivateKeyCertName] + caPrivateKeyBytes, ok := caSecret.Data[constants.CARootPrivateKeyCertName] if !ok { return nil, fmt.Errorf("CA private key bytes could not be extracted from secret %s", caSecret.Kind) } @@ -97,27 +81,7 @@ func extractCaFromSecret(caSecret *corev1.Secret) (*admissionregistration.CA, er return &admissionregistration.CA{CaCert: caCert, CaPrivateKey: caPrivateKey}, nil } -func (r *Reconciler) fetchOrInitializePolicyServerCARootSecret(ctx context.Context, generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, error) { - policyServerSecret := corev1.Secret{} - err := r.Client.Get( - ctx, - client.ObjectKey{ - Namespace: r.DeploymentsNamespace, - Name: constants.PolicyServerCARootSecretName}, - &policyServerSecret) - if err != nil && apierrors.IsNotFound(err) { - return r.buildPolicyServerCARootSecret(generateCA, pemEncodeCertificate) - } - policyServerSecret.ResourceVersion = "" - if err != nil { - return &corev1.Secret{}, - fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err) - } - - return &policyServerSecret, nil -} - -func (r *Reconciler) buildPolicyServerCARootSecret(generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, error) { +func buildCARootSecret(namespace string, generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, error) { caRoot, err := generateCA() if err != nil { return nil, fmt.Errorf("cannot generate policy-server secret CA: %w", err) @@ -128,16 +92,106 @@ func (r *Reconciler) buildPolicyServerCARootSecret(generateCA generateCAFunc, pe } caPrivateKeyBytes := x509.MarshalPKCS1PrivateKey(caRoot.CaPrivateKey) secretContents := map[string][]byte{ - constants.PolicyServerCARootCACert: caRoot.CaCert, - constants.PolicyServerCARootPemName: caPEMEncoded, - constants.PolicyServerCARootPrivateKeyCertName: caPrivateKeyBytes, + constants.CARootCACert: caRoot.CaCert, + constants.CARootCACertPem: caPEMEncoded, + constants.CARootPrivateKeyCertName: caPrivateKeyBytes, } return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: constants.PolicyServerCARootSecretName, - Namespace: r.DeploymentsNamespace, + Name: constants.KubewardenCARootSecretName, + Namespace: namespace, }, Data: secretContents, Type: corev1.SecretTypeOpaque, }, nil } + +func FetchOrInitializeCARootSecret(ctx context.Context, clusterClient client.Client, namespace string, generateCA generateCAFunc, pemEncodeCertificate pemEncodeCertificateFunc) (*corev1.Secret, bool, error) { + secret, err := fetchKubewardenCARootSecret(ctx, clusterClient, namespace) + if err != nil && apierrors.IsNotFound(err) || isMissingSecretDataFields(secret, constants.CARootCACert, constants.CARootCACertPem, constants.CARootPrivateKeyCertName) { + secret, err = buildCARootSecret(namespace, generateCA, pemEncodeCertificate) + if err != nil { + return &corev1.Secret{}, false, fmt.Errorf("cannot getch or initialize root CA secret: %w", err) + } + return secret, true, err + } + secret.ResourceVersion = "" + if err != nil { + return &corev1.Secret{}, false, + fmt.Errorf("cannot fetch or initialize Policy Server CA secret: %w", err) + } + + return secret, false, nil +} + +func fetchKubewardenCARootSecret(ctx context.Context, clusterClient client.Client, namespace string) (*corev1.Secret, error) { + return fetchSecret(ctx, clusterClient, namespace, constants.KubewardenCARootSecretName) +} + +func fetchSecret(ctx context.Context, clusterClient client.Client, namespace string, name string) (*corev1.Secret, error) { + secret := &corev1.Secret{} + if err := clusterClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, secret); err != nil { + return &corev1.Secret{}, fmt.Errorf("failed to fetch secret \"%s\": [%w]", name, err) + } + return secret, nil +} + +// Fetch or init a certificate to be used with service +func FetchOrInitializeCertificate(ctx context.Context, clusterClient client.Client, serviceName string, namespace string, secretName string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, bool, error) { + secret, err := fetchSecret(ctx, clusterClient, namespace, secretName) + if err != nil && apierrors.IsNotFound(err) || isMissingSecretDataFields(secret, constants.PolicyServerTLSCert, constants.PolicyServerTLSKey) { + sans := []string{fmt.Sprintf("%s.%s.svc", serviceName, namespace)} + secret, err = buildCertificateSecret(sans, secretName, namespace, caSecret, generateCert) + if err != nil { + return &corev1.Secret{}, false, fmt.Errorf("cannot fetch or initialize certificate secret: %w", err) + } + return secret, true, nil + } + if err != nil { + return &corev1.Secret{}, false, + fmt.Errorf("cannot fetch or initialize certificate secret: %w", err) + } + secret.ResourceVersion = "" + + return secret, false, nil +} + +// isMissingSecretDataFields check if the given fields exists in the Data field +// of the given secret. It does not validates the content of the data fields +func isMissingSecretDataFields(secret *corev1.Secret, fields ...string) bool { + for _, field := range fields { + // It's not necessary to check the `StringData` field because + // it is write-only and it will be merged with the `Data` field + // by the API. https://pkg.go.dev/k8s.io/api/core/v1#Secret + if _, ok := secret.Data[field]; !ok { + return true + } + } + return false +} + +func buildCertificateSecret(sans []string, secretName string, namespace string, caSecret *corev1.Secret, generateCert generateCertFunc) (*corev1.Secret, error) { + admissionregCA, err := extractCaFromSecret(caSecret) + if err != nil { + return nil, err + } + servingCert, servingKey, err := generateCert( + admissionregCA.CaCert, sans, + admissionregCA.CaPrivateKey) + if err != nil { + return nil, fmt.Errorf("cannot generate \"%s\" certificate: %w", sans, err) + } + secretContents := map[string]string{ + constants.PolicyServerTLSCert: string(servingCert), + constants.PolicyServerTLSKey: string(servingKey), + } + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + Labels: map[string]string{}, + }, + StringData: secretContents, + Type: corev1.SecretTypeOpaque, + }, nil +} diff --git a/internal/pkg/admission/policy-server-ca-secret_test.go b/internal/pkg/admission/policy-server-ca-secret_test.go index bb9609ad..80cdbabf 100644 --- a/internal/pkg/admission/policy-server-ca-secret_test.go +++ b/internal/pkg/admission/policy-server-ca-secret_test.go @@ -13,11 +13,67 @@ import ( "github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration" "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func TestFetchOrInitializePolicyServerCARootSecret(t *testing.T) { +func TestFetchOrInitializePolicyServerSecret(t *testing.T) { + generateCertCalled := false + servingCert := []byte{1} + servingKey := []byte{2} + admissionregCA, _ := admissionregistration.GenerateCA() + caSecret := &corev1.Secret{Data: map[string][]byte{constants.CARootCACert: admissionregCA.CaCert, constants.CARootPrivateKeyCertName: x509.MarshalPKCS1PrivateKey(admissionregCA.CaPrivateKey)}} + + //nolint:unparam + generateCertFunc := func(ca []byte, extraSANs []string, CAPrivateKey *rsa.PrivateKey) ([]byte, []byte, error) { + generateCertCalled = true + return servingCert, servingKey, nil + } + + caSecretContents := map[string]string{ + constants.PolicyServerTLSCert: string(servingCert), + constants.PolicyServerTLSKey: string(servingKey), + } + + var tests = []struct { + name string + r Reconciler + err error + secretContents map[string]string + generateCertCalled bool + }{ + {"Existing cert", createReconcilerWithExistingCert(), nil, mockSecretCert, false}, + {"cert does not exist", createReconcilerWithEmptyClient(), nil, caSecretContents, true}, + } + + for _, test := range tests { + ttest := test // ensure tt is correctly scoped when used in function literal + t.Run(ttest.name, func(t *testing.T) { + secret, err := ttest.r.fetchOrInitializePolicyServerCASecret(context.Background(), "policyServer", "policyServer", caSecret, generateCertFunc) + if diff := cmp.Diff(secret.StringData, ttest.secretContents); diff != "" { + t.Errorf("got an unexpected secret, diff %s", diff) + } + + if !errors.Is(err, ttest.err) { + t.Errorf("got %s, want %s", err, ttest.err) + } + + if generateCertCalled != ttest.generateCertCalled { + t.Errorf("got %t, want %t", generateCertCalled, ttest.generateCertCalled) + } + if generateCertCalled { + if policyServerName, hasLabel := secret.Labels[constants.PolicyServerLabelKey]; !hasLabel || policyServerName != "policyServer" { + t.Errorf("invalid %s label: got %t, want %t", constants.PolicyServerLabelKey, generateCertCalled, ttest.generateCertCalled) + } + } + generateCertCalled = false + }) + } +} + +func TestFetchOrInitializeCARootSecret(t *testing.T) { caPemBytes := []byte{} admissionregCA, err := admissionregistration.GenerateCA() generateCACalled := false @@ -36,51 +92,62 @@ func TestFetchOrInitializePolicyServerCARootSecret(t *testing.T) { } caSecretContents := map[string][]byte{ - constants.PolicyServerCARootCACert: admissionregCA.CaCert, - constants.PolicyServerCARootPemName: caPemBytes, - constants.PolicyServerCARootPrivateKeyCertName: x509.MarshalPKCS1PrivateKey(admissionregCA.CaPrivateKey), + constants.CARootCACert: admissionregCA.CaCert, + constants.CARootCACertPem: caPemBytes, + constants.CARootPrivateKeyCertName: x509.MarshalPKCS1PrivateKey(admissionregCA.CaPrivateKey), } var tests = []struct { name string - r Reconciler + client client.Client err error secretContents map[string][]byte generateCACalled bool }{ - {"Existing CA", createReconcilerWithExistingCA(), nil, mockSecretContents, false}, - {"CA does not exist", createReconcilerWithEmptyClient(), nil, caSecretContents, true}, + {"Existing CA", fake.NewClientBuilder().WithObjects(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.KubewardenCARootSecretName, + Namespace: namespace, + }, + Data: caSecretContents, + Type: corev1.SecretTypeOpaque, + }).Build(), nil, caSecretContents, false}, + {"CA does not exist", fake.NewClientBuilder().WithObjects().Build(), nil, caSecretContents, true}, } for _, test := range tests { ttest := test // ensure tt is correctly scoped when used in function literal t.Run(ttest.name, func(t *testing.T) { - secret, err := ttest.r.fetchOrInitializePolicyServerCARootSecret(context.Background(), generateCAFunc, pemEncodeCertificateFunc) - if diff := cmp.Diff(secret.Data, ttest.secretContents); diff != "" { - t.Errorf("got an unexpected secret, diff %s", diff) - } + generateCACalled = false + secret, initialized, err := FetchOrInitializeCARootSecret(context.Background(), ttest.client, namespace, generateCAFunc, pemEncodeCertificateFunc) + + if generateCACalled != ttest.generateCACalled { + t.Fatalf("got %t, want %t", generateCACalled, ttest.generateCACalled) + } + if initialized != ttest.generateCACalled { + t.Fatalf("root CA should be initialized") + } if !errors.Is(err, ttest.err) { - t.Errorf("got %s, want %s", err, ttest.err) + t.Fatalf("Unexpected error: %s. Expected %s", err, ttest.err) } - if generateCACalled != ttest.generateCACalled { - t.Errorf("got %t, want %t", generateCACalled, ttest.generateCACalled) + if diff := cmp.Diff(secret.Data, ttest.secretContents); diff != "" { + t.Errorf("got an unexpected secret, diff %s", diff) } - generateCACalled = false }) } } -func TestFetchOrInitializePolicyServerSecret(t *testing.T) { +func TestFetchOrInitializeCertificate(t *testing.T) { generateCertCalled := false servingCert := []byte{1} servingKey := []byte{2} admissionregCA, _ := admissionregistration.GenerateCA() - caSecret := &corev1.Secret{Data: map[string][]byte{constants.PolicyServerCARootCACert: admissionregCA.CaCert, constants.PolicyServerCARootPrivateKeyCertName: x509.MarshalPKCS1PrivateKey(admissionregCA.CaPrivateKey)}} + caSecret := &corev1.Secret{Data: map[string][]byte{constants.CARootCACert: admissionregCA.CaCert, constants.CARootPrivateKeyCertName: x509.MarshalPKCS1PrivateKey(admissionregCA.CaPrivateKey)}} //nolint:unparam - generateCertFunc := func(ca []byte, commonName string, extraSANs []string, CAPrivateKey *rsa.PrivateKey) ([]byte, []byte, error) { + generateCertFunc := func(ca []byte, extraSANs []string, CAPrivateKey *rsa.PrivateKey) ([]byte, []byte, error) { generateCertCalled = true return servingCert, servingKey, nil } @@ -89,61 +156,169 @@ func TestFetchOrInitializePolicyServerSecret(t *testing.T) { constants.PolicyServerTLSCert: string(servingCert), constants.PolicyServerTLSKey: string(servingKey), } + caSecretContentsData := map[string][]byte{ + constants.PolicyServerTLSCert: servingCert, + constants.PolicyServerTLSKey: servingKey, + } var tests = []struct { name string - r Reconciler + client client.Client err error secretContents map[string]string generateCertCalled bool + serviceName string + secretName string }{ - {"Existing cert", createReconcilerWithExistingCert(), nil, mockSecretCert, false}, - {"cert does not exist", createReconcilerWithEmptyClient(), nil, caSecretContents, true}, + {"Existing cert", fake.NewClientBuilder().WithObjects(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testingSecretName", + Namespace: namespace, + }, + StringData: mockSecretCert, + Data: caSecretContentsData, + Type: corev1.SecretTypeOpaque, + }).Build(), nil, mockSecretCert, false, "policyServer", "testingSecretName"}, + {"cert does not exist", fake.NewClientBuilder().WithObjects().Build(), nil, caSecretContents, true, "policyServer", "testingSecretName"}, } for _, test := range tests { ttest := test // ensure tt is correctly scoped when used in function literal t.Run(ttest.name, func(t *testing.T) { - secret, err := ttest.r.fetchOrInitializePolicyServerCASecret(context.Background(), "policyServer", caSecret, generateCertFunc) - if diff := cmp.Diff(secret.StringData, ttest.secretContents); diff != "" { - t.Errorf("got an unexpected secret, diff %s", diff) + generateCertCalled = false + + secret, initialized, err := FetchOrInitializeCertificate(context.Background(), ttest.client, ttest.serviceName, namespace, ttest.secretName, caSecret, generateCertFunc) + + if generateCertCalled != ttest.generateCertCalled { + t.Fatalf("got %t, want %t", generateCertCalled, ttest.generateCertCalled) + } + + if initialized != ttest.generateCertCalled { + t.Fatalf("initialized flag invalid value") } if !errors.Is(err, ttest.err) { - t.Errorf("got %s, want %s", err, ttest.err) + t.Fatalf("got %s, want %s", err, ttest.err) } - if generateCertCalled != ttest.generateCertCalled { - t.Errorf("got %t, want %t", generateCertCalled, ttest.generateCertCalled) + if secret.Name != ttest.secretName { + t.Errorf("invalid secret name. Got %s, want %s", secret.Name, ttest.secretName) + } + + if diff := cmp.Diff(secret.StringData, ttest.secretContents); diff != "" { + t.Errorf("got an unexpected secret, diff %s", diff) } - generateCertCalled = false }) } } -const namespace = "namespace" - -var mockSecretContents = map[string][]byte{"ca": []byte("secretContents")} - -func createReconcilerWithExistingCA() Reconciler { - mockSecret := &corev1.Secret{ +func TestFetchCARootSecret(t *testing.T) { + secret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: constants.PolicyServerCARootSecretName, + Name: constants.KubewardenCARootSecretName, Namespace: namespace, }, Data: mockSecretContents, Type: corev1.SecretTypeOpaque, } + var tests = []struct { + name string + client client.Client + expectedSecret *corev1.Secret + validateError func(error) bool + }{ + {"Existing CA", fake.NewClientBuilder().WithObjects(&secret).Build(), &secret, func(err error) bool { return err == nil }}, + {"CA does not exist", fake.NewClientBuilder().WithObjects().Build(), &corev1.Secret{}, apierrors.IsNotFound}, + } - // Create a fake client to mock API calls. It will return the mock secret - cl := fake.NewClientBuilder().WithObjects(mockSecret).Build() - return Reconciler{ - Client: cl, - DeploymentsNamespace: namespace, + for _, test := range tests { + ttest := test // ensure tt is correctly scoped when used in function literal + t.Run(ttest.name, func(t *testing.T) { + secret, err := fetchKubewardenCARootSecret(context.Background(), ttest.client, namespace) + + if !ttest.validateError(err) { + t.Fatalf("Unexpected error: %s", err) + } + + if diff := cmp.Diff(ttest.expectedSecret.Data, secret.Data); diff != "" { + t.Errorf("got an unexpected secret, diff %s", diff) + } + }) } } -var mockSecretCert = map[string]string{"cert": "certString"} +func TestIsMissingSecretDataFields(t *testing.T) { + var tests = []struct { + name string + secret *corev1.Secret + missingFields []string + secretIsMissingRequiredData bool + }{ + {"Missing all required data", &corev1.Secret{}, []string{constants.CARootCACert, constants.CARootCACertPem, constants.CARootPrivateKeyCertName}, true}, + + {fmt.Sprintf("Missing %s field", constants.CARootCACert), &corev1.Secret{ + Data: map[string][]byte{ + constants.CARootCACertPem: []byte(constants.CARootCACertPem), + constants.CARootPrivateKeyCertName: []byte(constants.CARootPrivateKeyCertName), + }, + }, []string{constants.CARootCACert}, true}, + + {fmt.Sprintf("Missing %s field", constants.CARootCACertPem), &corev1.Secret{ + Data: map[string][]byte{ + constants.CARootCACert: []byte(constants.CARootCACert), + constants.CARootPrivateKeyCertName: []byte(constants.CARootPrivateKeyCertName), + }}, []string{constants.CARootCACertPem}, true}, + + {fmt.Sprintf("Missing %s field", constants.CARootPrivateKeyCertName), &corev1.Secret{ + Data: map[string][]byte{ + constants.CARootCACert: []byte(constants.CARootCACert), + constants.CARootCACertPem: []byte(constants.CARootCACertPem), + }}, []string{constants.CARootPrivateKeyCertName}, true}, + + {fmt.Sprintf("Missing %s field", constants.PolicyServerTLSCert), &corev1.Secret{ + Data: map[string][]byte{ + constants.PolicyServerTLSKey: []byte(constants.PolicyServerTLSKey), + }, + }, []string{constants.PolicyServerTLSCert}, true}, + + {fmt.Sprintf("Missing %s field", constants.PolicyServerTLSKey), &corev1.Secret{ + Data: map[string][]byte{ + constants.PolicyServerTLSCert: []byte(constants.PolicyServerTLSCert), + }, + }, []string{constants.PolicyServerTLSKey}, true}, + + {"All required data define", &corev1.Secret{ + Data: map[string][]byte{ + constants.CARootCACert: []byte(constants.CARootCACert), + constants.CARootCACertPem: []byte(constants.CARootCACertPem), + constants.CARootPrivateKeyCertName: []byte(constants.CARootPrivateKeyCertName), + }, + }, []string{constants.CARootCACert, constants.CARootCACertPem, constants.CARootPrivateKeyCertName}, false}, + {"Only a subset required", &corev1.Secret{ + Data: map[string][]byte{ + constants.CARootCACert: []byte(constants.CARootCACert), + constants.CARootCACertPem: []byte(constants.CARootCACertPem), + constants.CARootPrivateKeyCertName: []byte(constants.CARootPrivateKeyCertName), + }, + }, []string{constants.CARootPrivateKeyCertName, constants.CARootCACertPem}, false}, + } + for _, test := range tests { + ttest := test // ensure tt is correctly scoped when used in function literal + t.Run(ttest.name, func(t *testing.T) { + isMissingCertData := isMissingSecretDataFields(ttest.secret, ttest.missingFields...) + if ttest.secretIsMissingRequiredData != isMissingCertData { + t.Errorf("secret is missing some field. Required fields %s, fields defined %s", ttest.missingFields, ttest.secret.Data) + } + }) + } +} + +const namespace = "namespace" + +var mockSecretContents = map[string][]byte{"ca": []byte("secretContents")} + +var mockSecretCert = map[string]string{constants.PolicyServerTLSCert: "certString", constants.PolicyServerTLSKey: "key"} +var mockSecretCertData = map[string][]byte{constants.PolicyServerTLSCert: []byte("certString"), constants.PolicyServerTLSKey: []byte("key")} func createReconcilerWithExistingCert() Reconciler { mockSecret := &corev1.Secret{ @@ -152,6 +327,7 @@ func createReconcilerWithExistingCert() Reconciler { Namespace: namespace, }, StringData: mockSecretCert, + Data: mockSecretCertData, Type: corev1.SecretTypeOpaque, } diff --git a/internal/pkg/admission/policy-server-deployment.go b/internal/pkg/admission/policy-server-deployment.go index 9f6e51a1..0069e9d1 100644 --- a/internal/pkg/admission/policy-server-deployment.go +++ b/internal/pkg/admission/policy-server-deployment.go @@ -51,8 +51,12 @@ func (r *Reconciler) reconcilePolicyServerDeployment(ctx context.Context, policy return err } } + certificateSecret, err := fetchSecret(ctx, r.Client, r.DeploymentsNamespace, policyServer.NameWithPrefix()) + if err != nil { + return &PolicyServerNotReadyError{fmt.Sprintf("cannot get certificate secret for policy server: %s", err.Error())} + } - deployment := r.deployment(configMapVersion, policyServer) + deployment := r.deployment(configMapVersion, policyServer, certificateSecret.GetResourceVersion()) err = r.Client.Create(ctx, deployment) if err == nil { return nil @@ -153,6 +157,7 @@ func shouldUpdatePolicyServerDeployment(policyServer *policiesv1.PolicyServer, o containerImageChanged || originalDeployment.Spec.Template.Spec.ServiceAccountName != newDeployment.Spec.Template.Spec.ServiceAccountName || originalDeployment.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] != newDeployment.Annotations[constants.PolicyServerDeploymentConfigVersionAnnotation] || + originalDeployment.Spec.Template.Labels[constants.PolicyServerCertificateSecret] != newDeployment.Spec.Template.Labels[constants.PolicyServerCertificateSecret] || !reflect.DeepEqual(originalDeployment.Spec.Template.Spec.Containers[0].Env, newDeployment.Spec.Template.Spec.Containers[0].Env) || !haveEqualAnnotationsWithoutRestart(originalDeployment, newDeployment), nil } @@ -237,7 +242,7 @@ func envVarsContainVariable(envVars []corev1.EnvVar, envVarName string) int { return -1 } -func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv1.PolicyServer) *appsv1.Deployment { +func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv1.PolicyServer, certificateSecretVersion string) *appsv1.Deployment { admissionContainer := corev1.Container{ Name: policyServer.NameWithPrefix(), Image: policyServer.Spec.Image, @@ -399,6 +404,7 @@ func (r *Reconciler) deployment(configMapVersion string, policyServer *policiesv constants.AppLabelKey: policyServer.AppLabel(), constants.PolicyServerDeploymentPodSpecConfigVersionLabel: configMapVersion, constants.PolicyServerLabelKey: policyServer.Name, + constants.PolicyServerCertificateSecret: certificateSecretVersion, }, Annotations: templateAnnotations, }, diff --git a/internal/pkg/admission/policy-server-deployment_test.go b/internal/pkg/admission/policy-server-deployment_test.go index d327de95..0a79b899 100644 --- a/internal/pkg/admission/policy-server-deployment_test.go +++ b/internal/pkg/admission/policy-server-deployment_test.go @@ -239,7 +239,7 @@ func TestPolicyServerWithContainerSecurityContext(t *testing.T) { }, }, } - deployment := reconciler.deployment("v1", policyServer) + deployment := reconciler.deployment("v1", policyServer, "1") if deployment.Spec.Template.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem == nil || *deployment.Spec.Template.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem != readOnlFileSystem { @@ -285,7 +285,7 @@ func TestPolicyServerWithPodSecurityContext(t *testing.T) { }, }, } - deployment := reconciler.deployment("v1", policyServer) + deployment := reconciler.deployment("v1", policyServer, "1") if deployment.Spec.Template.Spec.SecurityContext == nil { t.Error("Pod securityContext should be defined ") @@ -312,7 +312,7 @@ func TestPolicyServerWithoutSecurityContext(t *testing.T) { SecurityContexts: policiesv1.PolicyServerSecurity{}, }, } - deployment := reconciler.deployment("v1", policyServer) + deployment := reconciler.deployment("v1", policyServer, "1") containerDefaultSecurityContext := defaultContainerSecurityContext() if *deployment.Spec.Template.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem != *containerDefaultSecurityContext.ReadOnlyRootFilesystem { @@ -367,7 +367,7 @@ func TestPolicyServerWithPodAndContainerSecurityContext(t *testing.T) { }, }, } - deployment := reconciler.deployment("v1", policyServer) + deployment := reconciler.deployment("v1", policyServer, "1") if *deployment.Spec.Template.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem != readOnlFileSystem { t.Error("Policy server container ReadOnlyRootFilesystem diverge from the expected value") @@ -462,7 +462,7 @@ func TestPolicyServerDeploymentMetricConfigurationWithValueDefinedByUser(t *test Env: []corev1.EnvVar{{Name: constants.PolicyServerEnableMetricsEnvVar, Value: "0"}, {Name: constants.PolicyServerLogFmtEnvVar, Value: "invalid"}}, }, } - deployment := reconciler.deployment("v1", policyServer) + deployment := reconciler.deployment("v1", policyServer, "1") hasMetricEnvvar := false hasLogFmtEnvvar := false for _, envvar := range deployment.Spec.Template.Spec.Containers[0].Env { @@ -507,7 +507,7 @@ func TestPolicyServerDeploymentMetricConfigurationWithNoValueDefinedByUSer(t *te Env: []corev1.EnvVar{}, }, } - deployment := reconciler.deployment("v1", policyServer) + deployment := reconciler.deployment("v1", policyServer, "1") hasMetricEnvvar := false hasLogFmtEnvvar := false for _, envvar := range deployment.Spec.Template.Spec.Containers[0].Env { diff --git a/internal/pkg/admission/policy-server_integration_test.go b/internal/pkg/admission/policy-server_integration_test.go index ee94975e..8ea7b1b4 100644 --- a/internal/pkg/admission/policy-server_integration_test.go +++ b/internal/pkg/admission/policy-server_integration_test.go @@ -18,25 +18,24 @@ const port = "8181" func TestCAAndCertificateCreationInAHttpsServer(t *testing.T) { const domain = "localhost" const maxRetries = 10 - r := createReconcilerWithEmptyClient() // create CA - caSecret, err := r.buildPolicyServerCARootSecret(admissionregistration.GenerateCA, admissionregistration.PemEncodeCertificate) + caSecret, err := buildCARootSecret("namespace", admissionregistration.GenerateCA, admissionregistration.PemEncodeCertificate) if err != nil { t.Errorf("CA secret could not be created: %s", err.Error()) } - admissionregCA, err := extractCaFromSecret(caSecret) - if err != nil { - t.Errorf("CA could not be extracted from secret: %s", err.Error()) - } - // create cert using CA previously created - servingCert, servingKey, err := admissionregistration.GenerateCert( - admissionregCA.CaCert, - domain, - []string{domain}, - admissionregCA.CaPrivateKey) + // create serving certificate + servingCertSecret, err := buildCertificateSecret([]string{domain}, "secretName", "namespace", caSecret, admissionregistration.GenerateCert) if err != nil { t.Errorf("failed generating cert: %s", err.Error()) } + servingCert, ok := servingCertSecret.StringData[constants.PolicyServerTLSCert] + if !ok { + t.Fatalf("missing cert data") + } + servingKey, ok := servingCertSecret.StringData[constants.PolicyServerTLSKey] + if !ok { + t.Fatalf("missing key data") + } var server http.Server var waitGroup sync.WaitGroup @@ -44,7 +43,7 @@ func TestCAAndCertificateCreationInAHttpsServer(t *testing.T) { // create https server with the certificates created go func() { - cert, err := tls.X509KeyPair(servingCert, servingKey) + cert, err := tls.X509KeyPair([]byte(servingCert), []byte(servingKey)) if err != nil { t.Errorf("could not load cert: %s", err.Error()) } @@ -64,7 +63,7 @@ func TestCAAndCertificateCreationInAHttpsServer(t *testing.T) { // wait for https server to be ready to avoid race conditions waitGroup.Wait() rootCAs := x509.NewCertPool() - rootCAs.AppendCertsFromPEM(caSecret.Data[constants.PolicyServerCARootPemName]) + rootCAs.AppendCertsFromPEM(caSecret.Data[constants.CARootCACertPem]) retries := 0 var conn *tls.Conn for retries < maxRetries { diff --git a/internal/pkg/admission/reconciler.go b/internal/pkg/admission/reconciler.go index daccbad2..851543fd 100644 --- a/internal/pkg/admission/reconciler.go +++ b/internal/pkg/admission/reconciler.go @@ -154,7 +154,7 @@ func (r *Reconciler) Reconcile( policyServer *policiesv1.PolicyServer, policies []policiesv1.Policy, ) error { - policyServerCARootSecret, err := r.fetchOrInitializePolicyServerCARootSecret(ctx, admissionregistration.GenerateCA, admissionregistration.PemEncodeCertificate) + rootCASecret, err := fetchKubewardenCARootSecret(ctx, r.Client, r.DeploymentsNamespace) if err != nil { setFalseConditionType( &policyServer.Status.Conditions, @@ -164,21 +164,12 @@ func (r *Reconciler) Reconcile( return err } - if err := r.reconcileCASecret(ctx, policyServerCARootSecret); err != nil { - setFalseConditionType( - &policyServer.Status.Conditions, - string(policiesv1.PolicyServerCARootSecretReconciled), - fmt.Sprintf("error reconciling secret: %v", err), - ) - return err - } - setTrueConditionType( &policyServer.Status.Conditions, string(policiesv1.PolicyServerCARootSecretReconciled), ) - policyServerCASecret, err := r.fetchOrInitializePolicyServerCASecret(ctx, policyServer.NameWithPrefix(), policyServerCARootSecret, admissionregistration.GenerateCert) + policyServerCASecret, err := r.fetchOrInitializePolicyServerCASecret(ctx, policyServer.Name, policyServer.NameWithPrefix(), rootCASecret, admissionregistration.GenerateCert) if err != nil { setFalseConditionType( &policyServer.Status.Conditions, diff --git a/internal/pkg/admission/validating-webhook.go b/internal/pkg/admission/validating-webhook.go index 86462c0d..9f906d8f 100644 --- a/internal/pkg/admission/validating-webhook.go +++ b/internal/pkg/admission/validating-webhook.go @@ -90,7 +90,7 @@ func (r *Reconciler) validatingWebhookConfiguration( Name: fmt.Sprintf("%s.kubewarden.admission", policy.GetUniqueName()), ClientConfig: admissionregistrationv1.WebhookClientConfig{ Service: &service, - CABundle: admissionSecret.Data[constants.PolicyServerCARootPemName], + CABundle: admissionSecret.Data[constants.CARootCACertPem], }, Rules: policy.GetRules(), FailurePolicy: policy.GetFailurePolicy(), diff --git a/internal/pkg/admissionregistration/ca.go b/internal/pkg/admissionregistration/ca.go index 58c78377..6f45c3d5 100644 --- a/internal/pkg/admissionregistration/ca.go +++ b/internal/pkg/admissionregistration/ca.go @@ -35,7 +35,7 @@ func GenerateCA() (*CA, error) { PostalCode: []string{""}, }, NotBefore: time.Now(), - NotAfter: time.Now().AddDate(10, 0, 0), + NotAfter: time.Now().AddDate(10, 0, 0), // 10 years expiration IsCA: true, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, diff --git a/internal/pkg/admissionregistration/cert.go b/internal/pkg/admissionregistration/cert.go index c82d1994..dacf7a87 100644 --- a/internal/pkg/admissionregistration/cert.go +++ b/internal/pkg/admissionregistration/cert.go @@ -14,7 +14,6 @@ import ( ) func GenerateCert(ca []byte, - commonName string, extraSANs []string, CAPrivateKey *rsa.PrivateKey, ) ([]byte, []byte, error) { @@ -50,7 +49,7 @@ func GenerateCert(ca []byte, newCertificate := x509.Certificate{ SerialNumber: serialNumber, Subject: pkix.Name{ - CommonName: commonName, + CommonName: "", // CommonName is deprecated. Use SANS instead. Organization: []string{""}, Country: []string{""}, Province: []string{""}, diff --git a/internal/pkg/constants/constants.go b/internal/pkg/constants/constants.go index 5eda8433..327e7292 100644 --- a/internal/pkg/constants/constants.go +++ b/internal/pkg/constants/constants.go @@ -9,12 +9,16 @@ var ( const ( // PolicyServer CA Secret - PolicyServerTLSCert = "policy-server-cert" - PolicyServerTLSKey = "policy-server-key" - PolicyServerCARootSecretName = "policy-server-root-ca" - PolicyServerCARootPemName = "policy-server-root-ca-pem" - PolicyServerCARootCACert = "policy-server-root-ca-cert" - PolicyServerCARootPrivateKeyCertName = "policy-server-root-ca-privatekey-cert" + PolicyServerTLSCert = "tls.crt" + PolicyServerTLSKey = "tls.key" + PolicyServerCARootSecretName = "policy-server-root-ca" + PolicyServerCARootPemName = "policy-server-root-ca-pem" + // Root CA secret + CARootCACert = "tls.crt" + CARootCACertPem = "cert-pem" + CARootPrivateKeyCertName = "tls.key" + KubewardenCARootSecretName = "kubewarden-root-ca" + ControllerCertificateSecretName = "webhook-server-cert" //nolint:gosec // PolicyServer Deployment PolicyServerEnableMetricsEnvVar = "KUBEWARDEN_ENABLE_METRICS" @@ -37,8 +41,9 @@ const ( PolicyServerVerificationConfigContainerPath = "/verification" // Label - AppLabelKey = "app" - PolicyServerLabelKey = "kubewarden/policy-server" + AppLabelKey = "app" + PolicyServerLabelKey = "kubewarden/policy-server" + PolicyServerCertificateSecret = "kubewarden/policy-server-certificate-secret-version" //nolint:gosec // Index PolicyServerIndexKey = ".spec.policyServer" @@ -51,4 +56,8 @@ const ( // OPTEL OptelInjectAnnotation = "sidecar.opentelemetry.io/inject" + + ControllerReturnCodeSuccess = 0 + ControllerReturnCodeError = 1 + ControllerReturnCodeCAInitialized = 2 ) diff --git a/main.go b/main.go index c27a329b..b785e33e 100644 --- a/main.go +++ b/main.go @@ -1,4 +1,5 @@ /* + Copyright 2021. Licensed under the Apache License, Version 2.0 (the "License"); @@ -21,13 +22,14 @@ import ( "flag" "fmt" "os" + "strings" "time" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" - "github.com/ereslibre/kube-webhook-wrapper/webhookwrapper" + "github.com/kubewarden/kube-webhook-wrapper/webhookwrapper" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -42,8 +44,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + // "sigs.k8s.io/controller-runtime/pkg/manager" + controllers "github.com/kubewarden/kubewarden-controller/controllers" "github.com/kubewarden/kubewarden-controller/internal/pkg/admission" + "github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration" "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" "github.com/kubewarden/kubewarden-controller/internal/pkg/metrics" policiesv1 "github.com/kubewarden/kubewarden-controller/pkg/apis/policies/v1" @@ -63,8 +68,8 @@ func init() { //+kubebuilder:scaffold:scheme } -func main() { - retcode := 0 +func main() { //nolint:cyclop + retcode := constants.ControllerReturnCodeSuccess defer func() { os.Exit(retcode) }() var metricsAddr string @@ -74,6 +79,9 @@ func main() { var probeAddr string var enableMetrics bool var openTelemetryEndpoint string + var validatingWebhooks string + var mutatingWebhooks string + var controllerWebhookService string flag.StringVar(&metricsAddr, "metrics-bind-address", ":8088", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, @@ -90,6 +98,18 @@ func main() { "deployments-namespace", "", "The namespace where the kubewarden resources will be created.") + flag.StringVar(&validatingWebhooks, + "validating-webhooks", + "kubewarden-controller-validating-webhook-configuration", + "Comma separated ValidatingWebhookConfiguration names which should be updated adding the root CA bundle") + flag.StringVar(&mutatingWebhooks, + "mutating-webhooks", + "kubewarden-controller-mutating-webhook-configuration", + "Comma separated MutatingWebhookConfiguration names which should be updated adding the root CA bundle") + flag.StringVar(&controllerWebhookService, + "controller-webhook-service", + "kubewarden-controller-webhook-service", + "Controller service name used for controller webhooks") flag.BoolVar(&alwaysAcceptAdmissionReviewsOnDeploymentsNamespace, "always-accept-admission-reviews-on-deployments-namespace", false, @@ -104,11 +124,29 @@ func main() { deploymentsNamespace = environment.deploymentsNamespace } + managerCtx := ctrl.SetupSignalHandler() + rootCACertPem, rootCAPrivateKey, shouldRestart, err := setupClusterClientAndSetupCA(managerCtx, controllerWebhookService, deploymentsNamespace, validatingWebhooks, mutatingWebhooks, environment.developmentMode) + if err != nil { + retcode = constants.ControllerReturnCodeError + return + } else if shouldRestart { + // When the controller is run for the first time, there + // is no root CA. Therefore, we need to restart the pod + // to ensure that secret data is available for the + // controller before start running the manager. Here we + // just exit the controller. Thus, the control plane + // will launch another container with access to the + // root CA data initialized in the first run. + setupLog.Info("Root CA or controller certificate initialized. Restarting. ") + retcode = constants.ControllerReturnCodeCAInitialized + return + } + if enableMetrics { shutdown, err := metrics.New(openTelemetryEndpoint) if err != nil { setupLog.Error(err, "unable to initialize metrics provider") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } setupLog.Info("Metrics initialized") @@ -121,7 +159,7 @@ func main() { if err := shutdown(ctx); err != nil { setupLog.Error(err, "Unable to shutdown telemetry") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } }() @@ -152,7 +190,7 @@ func main() { // this privilege. // For example, when we access a secret inside the `kubewarden` // namespace, the cache will create a Watch against Secrets, that will require - // privileged to acccess ALL the secrets of the cluster. + // privileged to access ALL the secrets of the cluster. // // To be able to have stricter RBAC rules, we need to instruct the cache to // only watch objects inside of the namespace where the controller is running. @@ -172,7 +210,7 @@ func main() { }, }), // These types of resources should never be cached because we need fresh - // data coming from the cliet. This is required to perform the rollout + // data coming from the client. This is required to perform the rollout // of the PolicyServer Deployment whenever a policy is added/changed/removed. // Because of that, there's not need to scope these resources inside // of the cache, like we did for Pods, Services,... right above. @@ -182,10 +220,12 @@ func main() { environment.developmentMode, environment.webhookHostAdvertise, webhooks(), + rootCACertPem, + rootCAPrivateKey, ) if err != nil { setupLog.Error(err, "unable to start manager") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } @@ -205,7 +245,7 @@ func main() { Reconciler: reconciler, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "PolicyServer") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } @@ -216,7 +256,7 @@ func main() { Reconciler: reconciler, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "AdmissionPolicy") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } @@ -227,7 +267,7 @@ func main() { Reconciler: reconciler, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterAdmissionPolicy") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } @@ -235,19 +275,18 @@ func main() { if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") - retcode = 1 return } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up ready check") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } setupLog.Info("starting manager") - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(managerCtx); err != nil { setupLog.Error(err, "problem running manager") - retcode = 1 + retcode = constants.ControllerReturnCodeError return } } @@ -350,3 +389,137 @@ func webhooks() []webhookwrapper.WebhookRegistrator { }, } } + +func setupClusterClientAndSetupCA(ctx context.Context, controllerWebhookService string, deploymentsNamespace string, validatingWebhooks string, mutatingWebhooks string, developmentMode bool) ([]byte, []byte, bool, error) { + // This client is not created inside the setupCA function just to + // facilitate the testing + clusterClient, err := client.New(ctrl.GetConfigOrDie(), client.Options{}) + if err != nil { + setupLog.Error(err, "unable to setup client") + return []byte{}, []byte{}, false, fmt.Errorf("failed to initialize the cluster client: %s", err.Error()) + } + return setupCA(ctx, clusterClient, controllerWebhookService, deploymentsNamespace, strings.Split(strings.TrimSpace(validatingWebhooks), ","), strings.Split(strings.TrimSpace(mutatingWebhooks), ","), developmentMode) +} + +// setupCA is a function used to initialize the root CA, the certificate used +// by the controller to serve the webhook server. +// +// ctx is the context used with the clusterClient to get and patch cluster +// information resources. +// +// controllerWebhookService is the service name used to access the webhook server +// available by the controller. This is necessary to properly generate the +// certificate +// +// deploymentsNamespace is the namespace where the controller is deployed +// +// validatingWebhooks are the ValidatingWebhookConfiguration names which should +// be updated when the root CA is initialized +// +// mutatingWebhooks are the MutatingWebhookConfiguration names which should +// be updated when the root CA is initialized +// +// developmentMode is a flag to tell if the controller is running on +// development mode or not +func setupCA(ctx context.Context, clusterClient client.Client, controllerWebhookService string, deploymentsNamespace string, validatingWebhooks []string, mutatingWebhooks []string, developmentMode bool) ([]byte, []byte, bool, error) { + caSecret, caInitialized, err := admission.FetchOrInitializeCARootSecret(ctx, clusterClient, deploymentsNamespace, admissionregistration.GenerateCA, admissionregistration.PemEncodeCertificate) + if err != nil { + return []byte{}, []byte{}, false, fmt.Errorf("failed to fetch or initialize root CA certificate secret: %s", err.Error()) + } + if caInitialized { + if err = admission.ReconcileSecret(ctx, clusterClient, caSecret); err != nil { + return []byte{}, []byte{}, false, fmt.Errorf("failed to reconcile root CA certificate secret: %s", err.Error()) + } + } + + certificate, privateKey, err := admission.ExtractCertificateData(caSecret) + if err != nil { + return []byte{}, []byte{}, false, fmt.Errorf("failed to initialize root CA certificate") + } + + controllerSecret, initialized, err := admission.FetchOrInitializeCertificate(ctx, clusterClient, controllerWebhookService, deploymentsNamespace, constants.ControllerCertificateSecretName, caSecret, admissionregistration.GenerateCert) + if err != nil { + return []byte{}, []byte{}, false, fmt.Errorf("failed to fetch or init the controller certificate: %s ", err.Error()) + } + if initialized { + if err = admission.ReconcileSecret(ctx, clusterClient, controllerSecret); err != nil { + return []byte{}, []byte{}, false, fmt.Errorf("cannot reconcile secret %s: %s", controllerSecret.Name, err.Error()) + } + } + + if !developmentMode { + if err := setupCAWebhooksInDevelopmentMode(ctx, clusterClient, certificate, validatingWebhooks, mutatingWebhooks); err != nil { + return []byte{}, []byte{}, false, fmt.Errorf("failed to configure webhooks in development mode: %s", err.Error()) + } + } + return certificate, privateKey, (caInitialized || initialized), nil +} + +func setupCAWebhooksInDevelopmentMode(ctx context.Context, clusterClient client.Client, certificate []byte, validatingWebhooks []string, mutatingWebhooks []string) error { + // If the controller is NOT running in development mode, the + // webhooks are created by Helm chart and missing the + // `caBundle` field with the root CA certificate. Therefore, + // the controller needs to patch these resources + if err := configureControllerValidationWebhooksToUseRootCA(ctx, clusterClient, certificate, validatingWebhooks); err != nil { + return err + } + return configureControllerMutatingWebhooksToUseRootCA(ctx, clusterClient, certificate, mutatingWebhooks) +} + +// configureControllerMutatingWebhooksToUseRootCA sets the `caBundle` field for +// the admissionregistrationv1.MutatingWebhookConfiguration webhook targeting +// the controller filling up the field with the root CA certificate. +// +// ctx is the context used with the clusterClient to get and patch cluster +// information resources. +// +// caCertificate is the root CA certificate to be added in the `caBundle` +// field. +// +// mutatingWebhookNames is the MutatingWebhookConfiguration names where the +// webhook should be patched with the root CA certificate +func configureControllerMutatingWebhooksToUseRootCA(ctx context.Context, clusterClient client.Client, caCertificate []byte, mutatingWebhookNames []string) error { + for _, mutatingWebhookName := range mutatingWebhookNames { + webhookConfig := admissionregistrationv1.MutatingWebhookConfiguration{} + if err := clusterClient.Get(ctx, client.ObjectKey{Name: mutatingWebhookName}, &webhookConfig); err != nil { + return fmt.Errorf("cannot get MutatingWebhookConfiguration %s: %s", mutatingWebhookName, err.Error()) + } + patch := webhookConfig.DeepCopy() + for i := range patch.Webhooks { + patch.Webhooks[i].ClientConfig.CABundle = caCertificate + } + if err := clusterClient.Patch(ctx, patch, client.MergeFrom(&webhookConfig)); err != nil { + return fmt.Errorf("cannot patch MutatingWebhookConfiguration %s: %s", mutatingWebhookName, err.Error()) + } + } + return nil +} + +// configureControllerValidationWebhooksToUseRootCA sets the `caBundle` field for +// the admissionregistrationv1.ValidatingWebhookConfiguration webhook targeting +// the controller filling up the field with the root CA certificate. +// +// ctx is the context used with the clusterClient to get and patch cluster +// information resources. +// +// caCertificate is the root CA certificate to be added in the `caBundle` +// field. +// +// validatingWebhookNames is the ValidatingWebhookConfiguration names where the +// webhook should be patched with the root CA certificate +func configureControllerValidationWebhooksToUseRootCA(ctx context.Context, clusterClient client.Client, caCertificate []byte, validatingWebhookNames []string) error { + for _, validatingWebhookName := range validatingWebhookNames { + webhookConfig := admissionregistrationv1.ValidatingWebhookConfiguration{} + if err := clusterClient.Get(ctx, client.ObjectKey{Name: validatingWebhookName}, &webhookConfig); err != nil { + return fmt.Errorf("cannot get ValidatingWebhookConfiguration %s: %s", validatingWebhookName, err.Error()) + } + patch := webhookConfig.DeepCopy() + for i := range patch.Webhooks { + patch.Webhooks[i].ClientConfig.CABundle = caCertificate + } + if err := clusterClient.Patch(ctx, patch, client.MergeFrom(&webhookConfig)); err != nil { + return fmt.Errorf("cannot patch ValidatingWebhookConfiguration %s: %s", validatingWebhookName, err.Error()) + } + } + return nil +} diff --git a/main_test.go b/main_test.go new file mode 100644 index 00000000..1ff46f9a --- /dev/null +++ b/main_test.go @@ -0,0 +1,316 @@ +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "bytes" + "context" + "crypto/x509" + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/kubewarden/kubewarden-controller/internal/pkg/admissionregistration" + "github.com/kubewarden/kubewarden-controller/internal/pkg/constants" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +var admissionregCA *admissionregistration.CA +var otherCABundle []byte +var validatingWebhook *admissionregistrationv1.ValidatingWebhookConfiguration +var mutatingWebhook *admissionregistrationv1.MutatingWebhookConfiguration +var otherValidatingWebhook *admissionregistrationv1.ValidatingWebhookConfiguration +var otherMutatingWebhook *admissionregistrationv1.MutatingWebhookConfiguration +var caSecret *corev1.Secret +var controllerSecret *corev1.Secret +var namespace string + +func setUPTest(t *testing.T) { + t.Helper() + CA, err := admissionregistration.GenerateCA() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + admissionregCA = CA + namespace = "testingns" + otherCABundle = []byte("otherCABundle") + validatingWebhook = &admissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "validatingWebhookConfig", + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "validatingWebhook", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: otherCABundle, + }, + }, + }, + } + mutatingWebhook = &admissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mutatingWebhookConfig", + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "mutatingWebhook", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: otherCABundle, + }, + }, + }, + } + otherValidatingWebhook = &admissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "otherValidatingWebhookConfig", + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "validatingWebhook", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: otherCABundle, + }, + }, + }, + } + otherMutatingWebhook = &admissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "otherMutatingWebhookConfig", + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "mutatingWebhook", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + CABundle: otherCABundle, + }, + }, + }, + } + caSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.KubewardenCARootSecretName, + Namespace: namespace, + }, + Data: map[string][]byte{ + constants.CARootCACert: admissionregCA.CaCert, + constants.CARootCACertPem: admissionregCA.CaCert, + constants.CARootPrivateKeyCertName: x509.MarshalPKCS1PrivateKey(admissionregCA.CaPrivateKey), + }, + } + controllerSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.ControllerCertificateSecretName, + Namespace: namespace, + }, + StringData: map[string]string{ + constants.PolicyServerTLSCert: "cert", + constants.PolicyServerTLSKey: "key", + }, + Data: map[string][]byte{ + constants.PolicyServerTLSCert: []byte("cert"), + constants.PolicyServerTLSKey: []byte("key"), + }, + } +} + +func TestSetupCA(t *testing.T) { //nolint:cyclop + setUPTest(t) + + var tests = []struct { + name string + client client.Client + expectedRootCA *corev1.Secret + expectedControllerCert *corev1.Secret + validatingWebhookNames []string + mutatingWebhookNames []string + expectedWebhooksCABundle map[string][]byte + shouldInitializeCertificates bool + }{ + {"fresh installation", + fake.NewClientBuilder(). + WithObjects(validatingWebhook, mutatingWebhook, otherValidatingWebhook, otherMutatingWebhook).Build(), + nil, nil, + []string{validatingWebhook.Name}, + []string{mutatingWebhook.Name}, + map[string][]byte{validatingWebhook.Name: []byte(""), mutatingWebhook.Name: []byte(""), "otherValidatingWebhookConfig": otherCABundle, "otherMutatingWebhookConfig": otherCABundle}, + true, + }, + {"missing controller cert", + fake.NewClientBuilder(). + WithObjects(caSecret, validatingWebhook, mutatingWebhook, otherValidatingWebhook, otherMutatingWebhook).Build(), + caSecret, nil, + []string{validatingWebhook.Name}, + []string{mutatingWebhook.Name}, + map[string][]byte{validatingWebhook.Name: []byte(""), mutatingWebhook.Name: []byte(""), "otherValidatingWebhookConfig": otherCABundle, "otherMutatingWebhookConfig": otherCABundle}, + true, + }, + {"all certs set", + fake.NewClientBuilder(). + WithObjects(caSecret, controllerSecret, validatingWebhook, mutatingWebhook, otherValidatingWebhook, otherMutatingWebhook).Build(), + caSecret, controllerSecret, + []string{validatingWebhook.Name}, + []string{mutatingWebhook.Name}, + map[string][]byte{validatingWebhook.Name: []byte(""), mutatingWebhook.Name: []byte(""), "otherValidatingWebhookConfig": otherCABundle, "otherMutatingWebhookConfig": otherCABundle}, + false, + }, + } + + for _, test := range tests { + ttest := test // ensure tt is correctly scoped when used in function literal + t.Run(ttest.name, func(t *testing.T) { + rootCACertPem, rootCAPrivateKey, initialized, err := setupCA(context.TODO(), ttest.client, "controllerWebhookServerName", namespace, ttest.validatingWebhookNames, ttest.mutatingWebhookNames, false) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if ttest.expectedRootCA != nil { + if caCertPem, ok := ttest.expectedRootCA.Data[constants.CARootCACertPem]; ok { + if !bytes.Equal(caCertPem, rootCACertPem) { + diff := cmp.Diff(caCertPem, rootCACertPem) + t.Fatalf("invalid root CA cert returned: %s", diff) + } + } + if caPrivateKeyPem, ok := ttest.expectedRootCA.Data[constants.CARootPrivateKeyCertName]; ok { + if !bytes.Equal(caPrivateKeyPem, rootCAPrivateKey) { + diff := cmp.Diff(caPrivateKeyPem, rootCAPrivateKey) + t.Fatalf("invalid root CA private key returned: %s", diff) + } + } + } + secret := corev1.Secret{} + if err := ttest.client.Get(context.Background(), client.ObjectKey{Namespace: namespace, Name: constants.KubewardenCARootSecretName}, &secret); err != nil { + t.Fatalf("unexpected error: %s", err) + } + if ttest.expectedRootCA == nil && len(secret.Data) != 3 { + t.Errorf("invalid secret data: %s", secret.Data) + } + if ttest.expectedRootCA != nil && !cmp.Equal(secret.Data, ttest.expectedRootCA.Data) { + diff := cmp.Diff(secret.Data, ttest.expectedRootCA.Data) + t.Errorf("got an unexpected secret, diff %s", diff) + } + secret = corev1.Secret{} + if err := test.client.Get(context.Background(), client.ObjectKey{Namespace: namespace, Name: constants.ControllerCertificateSecretName}, &secret); err != nil { + t.Fatalf("unexpected error: %s", err) + } + if ttest.expectedControllerCert == nil && len(secret.StringData) != 2 { + t.Errorf("invalid secret data: %s", secret.StringData) + } + if ttest.expectedControllerCert != nil && !cmp.Equal(secret.StringData, ttest.expectedControllerCert.StringData) { + diff := cmp.Diff(secret.StringData, ttest.expectedControllerCert.StringData) + t.Errorf("got an unexpected secret, diff %s", diff) + } + for webhookConfigName, expectedCABundle := range ttest.expectedWebhooksCABundle { + if len(expectedCABundle) == 0 { + expectedCABundle = rootCACertPem + } + if err := validateValidatingWebhookConfiguration(ttest.client, webhookConfigName, expectedCABundle); err == nil { + continue + } + if err := validateMutatingWebhookConfiguration(ttest.client, webhookConfigName, expectedCABundle); err == nil { + continue + } + t.Errorf("missing validation or mutation webhook: %s", webhookConfigName) + } + if ttest.shouldInitializeCertificates != initialized { + t.Errorf("CA and/or certificate should not be initialized") + } + }) + } +} + +func TestConfigureControllerWebhookToUseRootCA(t *testing.T) { + setUPTest(t) + + var tests = []struct { + name string + client client.Client + caCertificate []byte + validatingWebhookNames []string + mutatingWebhookNames []string + expectedWebhooksCABundle map[string][]byte + }{ + {"All webhooks are created", + fake.NewClientBuilder().WithObjects(validatingWebhook, mutatingWebhook).Build(), + admissionregCA.CaCert, + []string{validatingWebhook.Name}, + []string{mutatingWebhook.Name}, + map[string][]byte{"validatingWebhookConfig": admissionregCA.CaCert, "mutatingWebhookConfig": admissionregCA.CaCert}, + }, + {"with others webhooks", + fake.NewClientBuilder().WithObjects(validatingWebhook, mutatingWebhook, otherValidatingWebhook, otherMutatingWebhook).Build(), + admissionregCA.CaCert, + []string{validatingWebhook.Name}, + []string{mutatingWebhook.Name}, + map[string][]byte{"validatingWebhookConfig": admissionregCA.CaCert, "mutatingWebhookConfig": admissionregCA.CaCert, "otherValidatingWebhookConfig": otherCABundle, "otherMutatingWebhookConfig": otherCABundle}, + }, + } + + for _, test := range tests { + ttest := test // ensure tt is correctly scoped when used in function literal + t.Run(ttest.name, func(t *testing.T) { + if err := configureControllerValidationWebhooksToUseRootCA(context.TODO(), ttest.client, ttest.caCertificate, ttest.validatingWebhookNames); err != nil { + t.Fatalf("unexpected error: %s", err) + } + if err := configureControllerMutatingWebhooksToUseRootCA(context.TODO(), ttest.client, ttest.caCertificate, ttest.mutatingWebhookNames); err != nil { + t.Fatalf("unexpected error: %s", err) + } + + for webhookConfigName, expectedCABundle := range ttest.expectedWebhooksCABundle { + if err := validateValidatingWebhookConfiguration(ttest.client, webhookConfigName, expectedCABundle); err == nil { + continue + } + if err := validateMutatingWebhookConfiguration(ttest.client, webhookConfigName, expectedCABundle); err == nil { + continue + } + t.Errorf("missing validation or mutation webhook: %s", webhookConfigName) + } + }) + } +} + +func validateMutatingWebhookConfiguration(clusterClient client.Client, webhookConfigName string, expectedCABundle []byte) error { + webhookConfig := admissionregistrationv1.MutatingWebhookConfiguration{} + if err := clusterClient.Get(context.Background(), client.ObjectKey{Name: webhookConfigName}, &webhookConfig); err == nil { + for _, webhook := range webhookConfig.Webhooks { + if !bytes.Equal(webhook.ClientConfig.CABundle, expectedCABundle) { + diff := cmp.Diff(webhook.ClientConfig.CABundle, expectedCABundle) + return fmt.Errorf("invalid webhook ca bundle: %s", diff) + } + } + } else { + return fmt.Errorf("cannot get MutatingWebhookConfiguration: %s", err.Error()) + } + return nil +} + +func validateValidatingWebhookConfiguration(clusterClient client.Client, webhookConfigName string, expectedCABundle []byte) error { + webhookConfig := admissionregistrationv1.ValidatingWebhookConfiguration{} + if err := clusterClient.Get(context.Background(), client.ObjectKey{Name: webhookConfigName}, &webhookConfig); err == nil { + for _, webhook := range webhookConfig.Webhooks { + if !bytes.Equal(webhook.ClientConfig.CABundle, expectedCABundle) { + diff := cmp.Diff(webhook.ClientConfig.CABundle, expectedCABundle) + return fmt.Errorf("invalid webhook ca bundle: %s", diff) + } + } + } else { + return fmt.Errorf("cannot get ValidatingWebhookConfiguration: %s", err.Error()) + } + return nil +}