Skip to content

Commit

Permalink
Add unit tests for csrsigning controller (#611)
Browse files Browse the repository at this point in the history
Add unit tests for csrsigning controller reconcile loop
  • Loading branch information
HomayoonAlimohammadi committed Aug 26, 2024
1 parent 84551b1 commit 0c90519
Show file tree
Hide file tree
Showing 8 changed files with 842 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/k8s/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/k8s/pkg/k8sd/controllers/csrsigning/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions src/k8s/pkg/k8sd/controllers/csrsigning/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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")
Expand Down
6 changes: 4 additions & 2 deletions src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
163 changes: 163 additions & 0 deletions src/k8s/pkg/k8sd/controllers/csrsigning/reconcile_approve_test.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 0c90519

Please sign in to comment.