From 0c90519bc516e4e4cebe13bb94c6b7bc05a843c8 Mon Sep 17 00:00:00 2001 From: Homayoon Alimohammadi Date: Mon, 26 Aug 2024 14:55:27 +0400 Subject: [PATCH] Add unit tests for csrsigning controller (#611) Add unit tests for csrsigning controller reconcile loop --- src/k8s/go.mod | 2 +- .../k8sd/controllers/csrsigning/controller.go | 10 +- .../k8sd/controllers/csrsigning/reconcile.go | 4 +- .../csrsigning/reconcile_approve.go | 6 +- .../csrsigning/reconcile_approve_test.go | 163 ++++++ .../controllers/csrsigning/reconcile_test.go | 548 ++++++++++++++++++ .../pkg/k8sd/controllers/csrsigning/setup.go | 20 +- .../controllers/csrsigning/test/k8s_mock.go | 106 ++++ 8 files changed, 842 insertions(+), 17 deletions(-) create mode 100644 src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve_test.go create mode 100644 src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_test.go create mode 100644 src/k8s/pkg/k8sd/controllers/csrsigning/test/k8s_mock.go diff --git a/src/k8s/go.mod b/src/k8s/go.mod index daec34ff7..bbe43f3cc 100644 --- a/src/k8s/go.mod +++ b/src/k8s/go.mod @@ -24,6 +24,7 @@ require ( k8s.io/cli-runtime v0.30.0 k8s.io/client-go v0.30.1 k8s.io/klog/v2 v2.120.1 + k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 sigs.k8s.io/controller-runtime v0.18.4 ) @@ -173,7 +174,6 @@ require ( k8s.io/component-base v0.30.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect k8s.io/kubectl v0.30.0 // indirect - k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect oras.land/oras-go v1.2.5 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3 // indirect diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/controller.go b/src/k8s/pkg/k8sd/controllers/csrsigning/controller.go index 172104d6f..3ecdb7877 100644 --- a/src/k8s/pkg/k8sd/controllers/csrsigning/controller.go +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/controller.go @@ -88,11 +88,13 @@ func (c *Controller) Run(ctx context.Context, getClusterConfig func(context.Cont } if err := (&csrSigningReconciler{ - Manager: mgr, - Logger: mgr.GetLogger(), - Client: mgr.GetClient(), + Manager: mgr, + Logger: mgr.GetLogger(), + Client: mgr.GetClient(), + managedSignerNames: managedSignerNames, - getClusterConfig: getClusterConfig, + getClusterConfig: getClusterConfig, + reconcileAutoApprove: reconcileAutoApprove, }).SetupWithManager(mgr); err != nil { return fmt.Errorf("failed to setup csrsigning controller: %w", err) } diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile.go b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile.go index be3c4c78e..95ccbbb95 100644 --- a/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile.go +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile.go @@ -37,7 +37,7 @@ func (r *csrSigningReconciler) Reconcile(ctx context.Context, req ctrl.Request) } // skip CSRs with an unknown signerName. - if _, ok := managedSignerNames[obj.Spec.SignerName]; !ok { + if _, ok := r.managedSignerNames[obj.Spec.SignerName]; !ok { return ctrl.Result{}, nil } @@ -76,7 +76,7 @@ func (r *csrSigningReconciler) Reconcile(ctx context.Context, req ctrl.Request) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to load cluster RSA key: %w", err) } - return r.reconcileAutoApprove(ctx, log, obj, priv) + return r.reconcileAutoApprove(ctx, log, obj, priv, r.Client) } log.Info("Requeue while waiting for CSR to be approved") diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve.go b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve.go index dd80fc0a2..c767ce9ed 100644 --- a/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve.go +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve.go @@ -9,9 +9,11 @@ import ( certv1 "k8s.io/api/certificates/v1" v1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) -func (r *csrSigningReconciler) reconcileAutoApprove(ctx context.Context, log log.Logger, csr *certv1.CertificateSigningRequest, priv *rsa.PrivateKey) (ctrl.Result, error) { +func reconcileAutoApprove(ctx context.Context, log log.Logger, csr *certv1.CertificateSigningRequest, + priv *rsa.PrivateKey, client client.Client) (ctrl.Result, error) { var result certv1.RequestConditionType if err := validateCSR(csr, priv); err != nil { @@ -39,7 +41,7 @@ func (r *csrSigningReconciler) reconcileAutoApprove(ctx context.Context, log log } log = log.WithValues("result", result) - if err := r.Client.SubResource("approval").Update(ctx, csr); err != nil { + if err := client.SubResource("approval").Update(ctx, csr); err != nil { log.Error(err, "Failed to update CSR approval status") return ctrl.Result{}, err } diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve_test.go b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve_test.go new file mode 100644 index 000000000..78cbb819e --- /dev/null +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve_test.go @@ -0,0 +1,163 @@ +package csrsigning + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509/pkix" + "errors" + "testing" + + . "github.com/onsi/gomega" + certv1 "k8s.io/api/certificates/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + + k8smock "github.com/canonical/k8s/pkg/k8sd/controllers/csrsigning/test" + "github.com/canonical/k8s/pkg/log" + pkiutil "github.com/canonical/k8s/pkg/utils/pki" +) + +func TestAutoApprove(t *testing.T) { + g := NewWithT(t) + + key, err := rsa.GenerateKey(rand.Reader, 2048) + g.Expect(err).NotTo(HaveOccurred()) + + csrPEM, _, err := pkiutil.GenerateCSR( + pkix.Name{ + CommonName: "system:node:valid-node", + Organization: []string{"system:nodes"}, + }, + 2048, + nil, + nil, + ) + g.Expect(err).NotTo(HaveOccurred()) + + for _, tc := range []struct { + name string + csr certv1.CertificateSigningRequest + updateErr error + + expectResult ctrl.Result + expectErr error + expectCondition certv1.CertificateSigningRequestCondition + }{ + { + name: "InvalidCSR/UpdateSuccessful", + csr: certv1.CertificateSigningRequest{}, // invalid csr + + expectResult: ctrl.Result{}, + expectCondition: certv1.CertificateSigningRequestCondition{ + Type: certv1.CertificateDenied, + Status: v1.ConditionTrue, + Reason: "K8sdDeny", + }, + }, + { + name: "InvalidCSR/UpdateFailed", + csr: certv1.CertificateSigningRequest{}, // invalid csr + + updateErr: errors.New("failed to update"), + expectResult: ctrl.Result{}, + expectErr: errors.New("failed to update"), + expectCondition: certv1.CertificateSigningRequestCondition{ + Type: certv1.CertificateDenied, + Status: v1.ConditionTrue, + Reason: "K8sdDeny", + }, + }, + { + name: "ValidCSR/UpdateSuccessful", + csr: certv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "k8sd.io/signature": mustCreateEncryptedSignature(g, &key.PublicKey, csrPEM), + "k8sd.io/node": "valid-node", + }, + }, + Spec: certv1.CertificateSigningRequestSpec{ + Request: []byte(csrPEM), + Username: "system:node:valid-node", + Groups: []string{"system:nodes"}, + SignerName: "k8sd.io/kubelet-serving", + Usages: []certv1.KeyUsage{certv1.UsageServerAuth, certv1.UsageDigitalSignature, certv1.UsageKeyEncipherment}, + }, + }, + + expectResult: ctrl.Result{}, + expectCondition: certv1.CertificateSigningRequestCondition{ + Type: certv1.CertificateApproved, + Status: v1.ConditionTrue, + Reason: "K8sdApprove", + }, + }, + { + name: "ValidCSR/UpdateFailed", + csr: certv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "k8sd.io/signature": mustCreateEncryptedSignature(g, &key.PublicKey, csrPEM), + "k8sd.io/node": "valid-node", + }, + }, + Spec: certv1.CertificateSigningRequestSpec{ + Request: []byte(csrPEM), + Username: "system:node:valid-node", + Groups: []string{"system:nodes"}, + SignerName: "k8sd.io/kubelet-serving", + Usages: []certv1.KeyUsage{certv1.UsageServerAuth, certv1.UsageDigitalSignature, certv1.UsageKeyEncipherment}, + }, + }, + + updateErr: errors.New("failed to update"), + expectResult: ctrl.Result{}, + expectErr: errors.New("failed to update"), + expectCondition: certv1.CertificateSigningRequestCondition{ + Type: certv1.CertificateApproved, + Status: v1.ConditionTrue, + Reason: "K8sdApprove", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(tc.updateErr), + tc.csr, + nil, // we don't call get in reconcileAutoApprove + ) + + result, err := reconcileAutoApprove( + context.Background(), + log.L(), + &tc.csr, + key, + k8sM, + ) + + g := NewWithT(t) + k8sM.AssertUpdateCalled(t) + g.Expect(result).To(Equal(tc.expectResult)) + if tc.expectErr == nil { + g.Expect(err).ToNot(HaveOccurred()) + } else { + g.Expect(err).To(MatchError(tc.expectErr)) + } + g.Expect(containsCondition(tc.csr.Status.Conditions, tc.expectCondition)).To(BeTrue(), "expected condition not found") + }) + } +} + +func containsCondition(cc []certv1.CertificateSigningRequestCondition, c certv1.CertificateSigningRequestCondition) bool { + for _, cond := range cc { + if cond.Type == c.Type && + cond.Status == c.Status && + cond.Reason == c.Reason { + return true + } + } + return false +} diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_test.go b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_test.go new file mode 100644 index 000000000..335ac1d4f --- /dev/null +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_test.go @@ -0,0 +1,548 @@ +package csrsigning + +import ( + "context" + "crypto/rsa" + "crypto/x509/pkix" + "errors" + "testing" + + . "github.com/onsi/gomega" + certv1 "k8s.io/api/certificates/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + k8smock "github.com/canonical/k8s/pkg/k8sd/controllers/csrsigning/test" + "github.com/canonical/k8s/pkg/k8sd/types" + "github.com/canonical/k8s/pkg/log" + pkiutil "github.com/canonical/k8s/pkg/utils/pki" +) + +func TestCSRNotFound(t *testing.T) { + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{}, + &apierrors.StatusError{ + ErrStatus: v1.Status{ + Reason: v1.StatusReasonNotFound, + }, + }, + ) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestFailedToGetCSR(t *testing.T) { + getErr := &apierrors.StatusError{ + ErrStatus: v1.Status{ + Reason: v1.StatusReasonInternalError, + }, + } + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{}, + getErr, + ) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).To(MatchError(getErr)) +} + +func TestHasSignedCertificate(t *testing.T) { + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Status: certv1.CertificateSigningRequestStatus{ + Certificate: []byte("cert"), + }, + }, + nil, + ) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestSkipUnmanagedSignerName(t *testing.T) { + unmanagedSignerName := "unknown" + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: unmanagedSignerName, + }, + }, + nil, + ) + + managedSigners := map[string]struct{}{ + "signer1": {}, + "signer2": {}, + } + + g := NewWithT(t) + // just to make sure the test is correct + g.Expect(managedSigners).ToNot(HaveKey(unmanagedSignerName)) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: managedSigners, + } + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestCertificateDenied(t *testing.T) { + managedSigner := "managed-signer" + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateDenied, + }, + }, + }, + }, + nil, + ) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestCertificateFailed(t *testing.T) { + managedSigner := "managed-signer" + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: "k8sd.io/kubelet-serving", + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateFailed, + }, + }, + }, + }, + nil, + ) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestFailedToGetClusterConfig(t *testing.T) { + managedSigner := "managed-signer" + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateApproved, + }, + }, + }, + }, + nil, + ) + + getCCErr := errors.New("failed to get cluster config") + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{}, getCCErr + }, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).To(MatchError(getCCErr)) +} + +func TestNotApprovedCSR(t *testing.T) { + t.Run("NoAutoApprove", func(t *testing.T) { + managedSigner := "managed-signer" + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + }, + }, + nil, + ) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{}, nil + }, + } + + g := NewWithT(t) + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: requeueAfterWaitingForApproved})) + g.Expect(err).ToNot(HaveOccurred()) + }) + + t.Run("WithAutoApprove", func(t *testing.T) { + managedSigner := "managed-signer" + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + }, + }, + nil, + ) + + priv, _, err := pkiutil.GenerateRSAKey(2048) + + g := NewWithT(t) + g.Expect(err).ToNot(HaveOccurred()) + + var called bool + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{ + Annotations: map[string]string{ + "k8sd/v1alpha1/csrsigning/auto-approve": "true", + }, + Certificates: types.Certificates{ + K8sdPrivateKey: ptr.To(priv), + }, + }, nil + }, + reconcileAutoApprove: func(ctx context.Context, l log.Logger, csr *certv1.CertificateSigningRequest, pk *rsa.PrivateKey, c client.Client) (ctrl.Result, error) { + called = true + return ctrl.Result{}, nil + }, + } + + _, _ = reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(called).To(BeTrue()) + }) +} + +func TestInvalidCSR(t *testing.T) { + managedSigner := "managed-signer" + csr := certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + Request: []byte("invalid-csr"), + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateApproved, + }, + }, + }, + } + + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + csr, + nil, + ) + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{ + Certificates: types.Certificates{}, + }, nil + }, + } + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g := NewWithT(t) + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).To(HaveOccurred()) +} + +func TestInvalidCACertificate(t *testing.T) { + csrPEM, _, err := pkiutil.GenerateCSR( + pkix.Name{ + CommonName: "system:node:valid-node", + Organization: []string{"system:nodes"}, + }, + 2048, + nil, + nil, + ) + + g := NewWithT(t) + g.Expect(err).NotTo(HaveOccurred()) + + managedSigner := "k8sd.io/kubelet-serving" + csr := certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + Request: []byte(csrPEM), + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateApproved, + }, + }, + }, + } + + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + csr, + nil, + ) + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{ + Certificates: types.Certificates{ + CACert: ptr.To("invalid"), + CAKey: ptr.To("invalid"), + }, + }, nil + }, + } + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).To(HaveOccurred()) +} + +func TestUpdateCSRFailed(t *testing.T) { + csrPEM, _, err := pkiutil.GenerateCSR( + pkix.Name{ + CommonName: "system:node:valid-node", + Organization: []string{"system:nodes"}, + }, + 2048, + nil, + nil, + ) + + g := NewWithT(t) + g.Expect(err).NotTo(HaveOccurred()) + + managedSigner := "k8sd.io/kubelet-serving" + csr := certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + Request: []byte(csrPEM), + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateApproved, + }, + }, + }, + } + updateErr := errors.New("failed to update") + + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock( + updateErr, + ), + csr, + nil, + ) + + caCert, caKey, err := pkiutil.GenerateSelfSignedCA(pkix.Name{CommonName: "kubernetes-ca"}, 10, 2048) + g.Expect(err).ToNot(HaveOccurred()) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{ + Certificates: types.Certificates{ + CACert: ptr.To(caCert), + CAKey: ptr.To(caKey), + }, + }, nil + }, + } + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).To(MatchError(updateErr)) + k8sM.AssertUpdateCalled(t) +} + +func TestUpdateCSRSucceed(t *testing.T) { + csrPEM, _, err := pkiutil.GenerateCSR( + pkix.Name{ + CommonName: "system:node:valid-node", + Organization: []string{"system:nodes"}, + }, + 2048, + nil, + nil, + ) + + g := NewWithT(t) + g.Expect(err).NotTo(HaveOccurred()) + + managedSigner := "k8sd.io/kubelet-serving" + csr := certv1.CertificateSigningRequest{ + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: managedSigner, + Request: []byte(csrPEM), + }, + Status: certv1.CertificateSigningRequestStatus{ + Conditions: []certv1.CertificateSigningRequestCondition{ + { + Type: certv1.CertificateApproved, + }, + }, + }, + } + + k8sM := k8smock.New( + t, + k8smock.NewSubResourceClientMock(nil), + csr, + nil, + ) + + caCert, caKey, err := pkiutil.GenerateSelfSignedCA(pkix.Name{CommonName: "kubernetes-ca"}, 10, 2048) + g.Expect(err).ToNot(HaveOccurred()) + + reconciler := &csrSigningReconciler{ + Client: k8sM, + managedSignerNames: map[string]struct{}{ + managedSigner: {}, + }, + getClusterConfig: func(context.Context) (types.ClusterConfig, error) { + return types.ClusterConfig{ + Certificates: types.Certificates{ + CACert: ptr.To(caCert), + CAKey: ptr.To(caKey), + }, + }, nil + }, + } + + result, err := reconciler.Reconcile(context.Background(), getDefaultRequest()) + + g.Expect(result).To(Equal(ctrl.Result{})) + g.Expect(err).ToNot(HaveOccurred()) + k8sM.AssertUpdateCalled(t) +} + +func getDefaultRequest() ctrl.Request { + return ctrl.Request{ + NamespacedName: k8stypes.NamespacedName{ + Name: "csr-1", + Namespace: "default", + }, + } +} diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/setup.go b/src/k8s/pkg/k8sd/controllers/csrsigning/setup.go index 7eb4751c0..165df665b 100644 --- a/src/k8s/pkg/k8sd/controllers/csrsigning/setup.go +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/setup.go @@ -2,8 +2,10 @@ package csrsigning import ( "context" + "crypto/rsa" "github.com/canonical/k8s/pkg/k8sd/types" + "github.com/canonical/k8s/pkg/log" "github.com/go-logr/logr" certv1 "k8s.io/api/certificates/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -13,17 +15,19 @@ import ( ) type csrSigningReconciler struct { - Manager manager.Manager - Logger logr.Logger - Client client.Client + Manager manager.Manager + Logger logr.Logger + Client client.Client + managedSignerNames map[string]struct{} - getClusterConfig func(context.Context) (types.ClusterConfig, error) + getClusterConfig func(context.Context) (types.ClusterConfig, error) + reconcileAutoApprove func(context.Context, log.Logger, *certv1.CertificateSigningRequest, *rsa.PrivateKey, client.Client) (ctrl.Result, error) } var managedSignerNames = map[string]struct{}{ - "k8sd.io/kubelet-serving": struct{}{}, - "k8sd.io/kubelet-client": struct{}{}, - "k8sd.io/kube-proxy-client": struct{}{}, + "k8sd.io/kubelet-serving": {}, + "k8sd.io/kubelet-client": {}, + "k8sd.io/kube-proxy-client": {}, } func (r *csrSigningReconciler) SetupWithManager(mgr ctrl.Manager) error { @@ -32,7 +36,7 @@ func (r *csrSigningReconciler) SetupWithManager(mgr ctrl.Manager) error { WithEventFilter(predicate.NewPredicateFuncs(func(object client.Object) bool { if csr, ok := object.(*certv1.CertificateSigningRequest); !ok { return false - } else if _, ok := managedSignerNames[csr.Spec.SignerName]; !ok { + } else if _, ok := r.managedSignerNames[csr.Spec.SignerName]; !ok { return false } return true diff --git a/src/k8s/pkg/k8sd/controllers/csrsigning/test/k8s_mock.go b/src/k8s/pkg/k8sd/controllers/csrsigning/test/k8s_mock.go new file mode 100644 index 000000000..9c4ca1c60 --- /dev/null +++ b/src/k8s/pkg/k8sd/controllers/csrsigning/test/k8s_mock.go @@ -0,0 +1,106 @@ +package test + +import ( + "context" + "testing" + + certv1 "k8s.io/api/certificates/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type getArgs struct { + name types.NamespacedName + obj client.Object + opts []client.GetOption +} + +type getRet struct { + csr certv1.CertificateSigningRequest + err error +} + +type K8sMock struct { + client.Client + + t *testing.T + srcm *subResourceClientMock + + getCalledWith getArgs + getReturns getRet +} + +func New( + t *testing.T, + srcm *subResourceClientMock, + getCSR certv1.CertificateSigningRequest, + getErr error, +) *K8sMock { + return &K8sMock{ + t: t, + srcm: srcm, + getReturns: getRet{ + csr: getCSR, + err: getErr, + }, + } +} + +func (m *K8sMock) Get(ctx context.Context, name types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + m.getCalledWith = getArgs{name, obj, opts} + csr, ok := obj.(*certv1.CertificateSigningRequest) + if !ok { + m.t.Fatalf("unexpected object type: %T", obj) + } + *csr = m.getReturns.csr + return m.getReturns.err +} + +func (m *K8sMock) Status() client.SubResourceWriter { + return m.srcm +} + +func (m *K8sMock) SubResource(subResource string) client.SubResourceClient { + switch subResource { + case "approval": + return m.srcm + default: + m.t.Fatalf("unexpected subResource: %s", subResource) + } + return nil +} + +func (m *K8sMock) AssertUpdateCalled(t *testing.T) { + m.srcm.assertUpdateCalled(t) +} + +type updateArgs struct { + obj client.Object + opts []client.SubResourceUpdateOption +} + +type subResourceClientMock struct { + client.SubResourceClient + + updateCalledWith []updateArgs + updateErr error +} + +func NewSubResourceClientMock( + updateErr error, +) *subResourceClientMock { + return &subResourceClientMock{ + updateErr: updateErr, + } +} + +func (src *subResourceClientMock) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + src.updateCalledWith = append(src.updateCalledWith, updateArgs{obj, opts}) + return src.updateErr +} + +func (src *subResourceClientMock) assertUpdateCalled(t *testing.T) { + if len(src.updateCalledWith) == 0 { + t.Error("expected update to have been called") + } +}