diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 1215a766..61d09c58 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -56,5 +56,5 @@ jobs: CASTAI_API_TOKEN: ${{ secrets.CASTAI_API_TOKEN }} GCP_CREDENTIALS: ${{ secrets.GCP_CREDENTIALS }} run: | - go test -timeout 30m ./e2e + go test -timeout 30m -parallel=10 ./e2e diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 7c0a4729..67cdf18f 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -54,7 +54,7 @@ jobs: CGO_ENABLED: 0 - name: Test - run: go test -short -race ./... + run: go test -short -race -timeout 15m ./... - name: Set up QEMU uses: docker/setup-qemu-action@v2 diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 567a57cd..7dffcf3b 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -52,7 +52,7 @@ jobs: CGO_ENABLED: 0 - name: Test - run: go test -short -race ./... + run: go test -short -race -timeout 15m ./... - name: Set up QEMU uses: docker/setup-qemu-action@v2 diff --git a/Makefile b/Makefile index 6c05cfb0..c65ad4dc 100644 --- a/Makefile +++ b/Makefile @@ -41,7 +41,7 @@ fix: $(GOLANGCI_LINT) .PHONY: fix test: - go test ./... -race + go test ./... -race -parallel=20 .PHONY: test generate-e2e-client: @@ -49,4 +49,4 @@ generate-e2e-client: .PHONY: generate-e2e-client deploy-loadtest: release - IMAGE_REPOSITORY=$(DOCKER_REPOSITORY) IMAGE_TAG=$(VERSION) ./hack/loadtest/deploy.sh \ No newline at end of file + IMAGE_REPOSITORY=$(DOCKER_REPOSITORY) IMAGE_TAG=$(VERSION) ./hack/loadtest/deploy.sh diff --git a/internal/actions/csr/approve_csr_handler_test.go b/internal/actions/csr/approve_csr_handler_test.go index cc753e30..38db9fb1 100644 --- a/internal/actions/csr/approve_csr_handler_test.go +++ b/internal/actions/csr/approve_csr_handler_test.go @@ -2,9 +2,10 @@ package csr import ( "context" + "crypto/x509" + "crypto/x509/pkix" "errors" "testing" - "time" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -17,6 +18,9 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/fake" ktest "k8s.io/client-go/testing" + + "github.com/castai/cluster-controller/internal/actions/csr/test" + "github.com/castai/cluster-controller/internal/actions/csr/wrapper" ) func TestApproveCSRHandler(t *testing.T) { @@ -27,7 +31,7 @@ func TestApproveCSRHandler(t *testing.T) { r := require.New(t) csrRes := getCSR() - client := fake.NewSimpleClientset(csrRes) + client := fake.NewClientset(csrRes) am := NewApprovalManager(log, client) csrRes.Status.Conditions = []certv1.CertificateSigningRequestCondition{ { @@ -36,7 +40,9 @@ func TestApproveCSRHandler(t *testing.T) { } ctx := context.Background() - err := am.handle(ctx, log, &Certificate{v1: csrRes}) + csr, err := wrapper.NewCSR(client, csrRes) + r.NoError(err) + err = am.handle(ctx, log, csr) r.NoError(err) }) @@ -53,12 +59,14 @@ func TestApproveCSRHandler(t *testing.T) { out := certv1.CertificateSigningRequestList{Items: []certv1.CertificateSigningRequest{*csrRes}} return true, &out, err }) - client := fake.NewSimpleClientset(csrRes) + client := fake.NewClientset(csrRes) client.PrependReactor("list", "certificatesigningrequests", fn) am := NewApprovalManager(log, client) - err := am.handle(context.Background(), log, &Certificate{v1: csrRes}) + csr, err := wrapper.NewCSR(client, csrRes) + r.NoError(err) + err = am.handle(context.Background(), log, csr) r.NoError(err) }) @@ -67,21 +75,27 @@ func TestApproveCSRHandler(t *testing.T) { signer := certv1beta1.KubeAPIServerClientKubeletSignerName csrRes := &certv1beta1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{Name: "node-csr-123"}, + TypeMeta: metav1.TypeMeta{ + APIVersion: certv1beta1.SchemeGroupVersion.String(), + Kind: "CertificateSigningRequest", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "node-csr-123", + CreationTimestamp: metav1.Now(), + }, Spec: certv1beta1.CertificateSigningRequestSpec{ - Request: []byte(`-----BEGIN CERTIFICATE REQUEST----- - MIIBADCBqAIBADBGMRUwEwYDVQQKEwxzeXN0ZW06bm9kZXMxLTArBgNVBAMTJHN5 - c3RlbTpub2RlOmdrZS1hbS1nY3AtY2FzdC01ZGM0ZjRlYzBZMBMGByqGSM49AgEG - CCqGSM49AwEHA0IABF/9p5y4t09Y6yAlhF0OthexpL0CEyNHVnVmmbB4jridyJzW - vrcLKbFat0qvJftODQhEA/lqByJepB4YGqQGhregADAKBggqhkjOPQQDAgNHADBE - AiAHVYZXHxxspoV0hcfn2Pdsl89fIPCOFy/K1PqSUR6QNAIgYdt51ZbQt9rgM2BD - 39zKjbxU1t82BlrW9/NrmaadNHQ= - -----END CERTIFICATE REQUEST-----`), - Username: "kubelet", + Request: test.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "system:node:gke-am-gcp-cast-pool-5dc4f4ec", + Organization: []string{"system:nodes"}, + }, + }), + Username: "kubelet-bootstrap", + Usages: []certv1beta1.KeyUsage{certv1beta1.UsageClientAuth}, SignerName: &signer, }, } - client := fake.NewSimpleClientset(csrRes) + client := fake.NewClientset(csrRes) // Return NotFound for all v1 resources. client.PrependReactor("*", "*", func(action ktest.Action) (handled bool, ret runtime.Object, err error) { if action.GetResource().Version == "v1" { @@ -95,7 +109,7 @@ func TestApproveCSRHandler(t *testing.T) { approved.Status.Conditions = []certv1beta1.CertificateSigningRequestCondition{ { Type: certv1beta1.CertificateApproved, - Reason: ReasonApproved, + Reason: "ReasonApproved", Message: "approved", LastUpdateTime: metav1.Now(), Status: v1.ConditionTrue, @@ -105,8 +119,11 @@ func TestApproveCSRHandler(t *testing.T) { }) am := NewApprovalManager(log, client) - err := am.handle(context.Background(), log, &Certificate{v1Beta1: csrRes}) + csr, err := wrapper.NewCSR(client, csrRes) + r.NoError(err) + err = am.handle(context.Background(), log, csr) r.NoError(err) + r.True(csr.Approved()) }) t.Run("approve v1beta1 csr failed", func(t *testing.T) { @@ -114,21 +131,27 @@ func TestApproveCSRHandler(t *testing.T) { signer := certv1beta1.KubeAPIServerClientKubeletSignerName csrRes := &certv1beta1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{Name: "node-csr-123"}, + TypeMeta: metav1.TypeMeta{ + APIVersion: certv1beta1.SchemeGroupVersion.String(), + Kind: "CertificateSigningRequest", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "node-csr-123", + CreationTimestamp: metav1.Now(), + }, Spec: certv1beta1.CertificateSigningRequestSpec{ - Request: []byte(`-----BEGIN CERTIFICATE REQUEST----- - MIIBADCBqAIBADBGMRUwEwYDVQQKEwxzeXN0ZW06bm9kZXMxLTArBgNVBAMTJHN5 - c3RlbTpub2RlOmdrZS1hbS1nY3AtY2FzdC01ZGM0ZjRlYzBZMBMGByqGSM49AgEG - CCqGSM49AwEHA0IABF/9p5y4t09Y6yAlhF0OthexpL0CEyNHVnVmmbB4jridyJzW - vrcLKbFat0qvJftODQhEA/lqByJepB4YGqQGhregADAKBggqhkjOPQQDAgNHADBE - AiAHVYZXHxxspoV0hcfn2Pdsl89fIPCOFy/K1PqSUR6QNAIgYdt51ZbQt9rgM2BD - 39zKjbxU1t82BlrW9/NrmaadNHQ= - -----END CERTIFICATE REQUEST-----`), - Username: "kubelet", + Request: test.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "system:node:gke-am-gcp-cast-pool-5dc4f4ec", + Organization: []string{"system:nodes"}, + }, + }), + Username: "kubelet-bootstrap", + Usages: []certv1beta1.KeyUsage{certv1beta1.UsageClientAuth}, SignerName: &signer, }, } - client := fake.NewSimpleClientset(csrRes) + client := fake.NewClientset(csrRes) // Return NotFound for all v1 resources. client.PrependReactor("*", "*", func(action ktest.Action) (handled bool, ret runtime.Object, err error) { if action.GetResource().Version == "v1" { @@ -151,24 +174,20 @@ func TestApproveCSRHandler(t *testing.T) { }) am := NewApprovalManager(log, client) - err := am.handle(context.Background(), log, &Certificate{v1Beta1: csrRes}) + csr, err := wrapper.NewCSR(client, csrRes) + r.NoError(err) + err = am.handle(context.Background(), log, csr) r.Error(err) + r.False(csr.Approved()) }) } -func TestApproveCSRExponentialBackoff(t *testing.T) { - r := require.New(t) - b := newApproveCSRExponentialBackoff() - var sum time.Duration - for i := 0; i < 10; i++ { - tmp := b.Step() - sum += tmp - } - r.Truef(100 < sum.Seconds(), "actual elapsed seconds %v", sum.Seconds()) -} - func getCSR() *certv1.CertificateSigningRequest { return &certv1.CertificateSigningRequest{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certv1.SchemeGroupVersion.String(), + Kind: "CertificateSigningRequest", + }, ObjectMeta: metav1.ObjectMeta{Name: "node-csr-123"}, Spec: certv1.CertificateSigningRequestSpec{ Request: []byte(`-----BEGIN CERTIFICATE REQUEST----- @@ -180,7 +199,7 @@ AiAHVYZXHxxspoV0hcfn2Pdsl89fIPCOFy/K1PqSUR6QNAIgYdt51ZbQt9rgM2BD 39zKjbxU1t82BlrW9/NrmaadNHQ= -----END CERTIFICATE REQUEST-----`), SignerName: certv1.KubeAPIServerClientKubeletSignerName, - Usages: []certv1.KeyUsage{"kubelet"}, + Usages: []certv1.KeyUsage{certv1.UsageClientAuth}, Username: "kubelet-bootstrap", }, // Status: certv1.CertificateSigningRequestStatus{},. diff --git a/internal/actions/csr/csr.go b/internal/actions/csr/csr.go deleted file mode 100644 index 65b1247c..00000000 --- a/internal/actions/csr/csr.go +++ /dev/null @@ -1,487 +0,0 @@ -package csr - -import ( - "context" - "crypto/x509" - "encoding/pem" - "errors" - "fmt" - "strings" - "time" - - "github.com/sirupsen/logrus" - certv1 "k8s.io/api/certificates/v1" - certv1beta1 "k8s.io/api/certificates/v1beta1" - v1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/client-go/informers" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/cache" -) - -const ( - ReasonApproved = "AutoApproved" - approvedMessage = "This CSR was approved by CAST AI" - csrTTL = time.Hour - - // We should approve CSRs, when they are created, so resync can be high. - // Resync plays back all events (create, update, delete), which are in informer cache. - // This does not involve talking to API server, it is not relist. - csrInformerResyncPeriod = 12 * time.Hour -) - -var ErrNodeCertificateNotFound = errors.New("node certificate not found") - -// Certificate wraps v1 and v1beta1 csr. -type Certificate struct { - v1 *certv1.CertificateSigningRequest - v1Beta1 *certv1beta1.CertificateSigningRequest - Name string -} - -var ( - errCSRNotFound = errors.New("v1 or v1beta csr should be set") - errInvalidCSR = errors.New("invalid CSR") -) - -func (c *Certificate) Validate() error { - if c.v1 == nil && c.v1Beta1 == nil { - return errCSRNotFound - } - return nil -} - -func (c *Certificate) Approved() bool { - if c.v1Beta1 != nil { - for _, condition := range c.v1Beta1.Status.Conditions { - if condition.Reason == ReasonApproved { - return true - } - } - return false - } - - for _, condition := range c.v1.Status.Conditions { - if condition.Reason == ReasonApproved && condition.Status == v1.ConditionTrue { - return true - } - } - return false -} - -// Outdated returns, whether the certificate request is old and should not be processed by cluster-controller. -// It has nothing to do with certificate expiration. -func (c *Certificate) Outdated() bool { - if c.v1Beta1 != nil { - return c.v1Beta1.CreationTimestamp.Add(csrTTL).Before(time.Now()) - } - return c.v1.CreationTimestamp.Add(csrTTL).Before(time.Now()) -} - -func (c *Certificate) ForCASTAINode() bool { - if c.Name == "" { - return false - } - - if strings.HasPrefix(c.Name, "system:node") && strings.Contains(c.Name, "cast-pool") { - return true - } - - return false -} - -func (c *Certificate) isRequestedByNodeBootstrap() bool { - // Since we only have one handler per CSR/certificate name, - // which is the node name, we can process the controller's certificates and kubelet-bootstrap`s. - // This covers the case when the controller restarts but the bootstrap certificate was deleted without our own certificate being approved. - return c.RequestingUser() == "kubelet-bootstrap" || c.RequestingUser() == "system:serviceaccount:castai-agent:castai-cluster-controller" -} - -func (c *Certificate) isRequestedBySystemNode() bool { - // To avoid waiting for the certificate to be approved by control plane. - // We can approve the certificate if it was requested by the system node. - return strings.HasPrefix(c.RequestingUser(), "system:node:") -} - -func isAlreadyApproved(err error) bool { - if err == nil { - return false - } - return strings.Contains(err.Error(), "Duplicate value: \"Approved\"") -} - -// ApproveCSRCertificate approves csr. -func (c *Certificate) ApproveCSRCertificate(ctx context.Context, client kubernetes.Interface) (*Certificate, error) { - if err := c.Validate(); err != nil { - return nil, err - } - - if c.v1Beta1 != nil { - c.v1Beta1.Status.Conditions = append(c.v1Beta1.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{ - Type: certv1beta1.CertificateApproved, - Reason: ReasonApproved, - Message: approvedMessage, - LastUpdateTime: metav1.Now(), - }) - resp, err := client.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, c.v1Beta1, metav1.UpdateOptions{}) - if err != nil && !isAlreadyApproved(err) { - return nil, fmt.Errorf("v1beta csr approve: %w", err) - } - return &Certificate{v1Beta1: resp}, nil - } - - c.v1.Status.Conditions = append(c.v1.Status.Conditions, certv1.CertificateSigningRequestCondition{ - Type: certv1.CertificateApproved, - Reason: ReasonApproved, - Message: approvedMessage, - Status: v1.ConditionTrue, - LastUpdateTime: metav1.Now(), - }) - resp, err := client.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, c.v1.Name, c.v1, metav1.UpdateOptions{}) - if err != nil && !isAlreadyApproved(err) { - return nil, fmt.Errorf("v1 csr approve: %w", err) - } - return &Certificate{v1: resp}, nil -} - -// DeleteCSR deletes csr. -func (c *Certificate) DeleteCSR(ctx context.Context, client kubernetes.Interface) error { - if err := c.Validate(); err != nil { - return err - } - - if c.v1Beta1 != nil { - return client.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, c.v1Beta1.Name, metav1.DeleteOptions{}) - } - return client.CertificatesV1().CertificateSigningRequests().Delete(ctx, c.v1.Name, metav1.DeleteOptions{}) -} - -// NewCSR creates new csr. -func (c *Certificate) NewCSR(ctx context.Context, client kubernetes.Interface) (*Certificate, error) { - if err := c.Validate(); err != nil { - return nil, err - } - - if c.v1Beta1 != nil { - resp, err := createV1beta1(ctx, client, c.v1Beta1) - if err != nil { - if apierrors.IsAlreadyExists(err) { - return get(ctx, client, c) - } - return nil, fmt.Errorf("v1beta csr create: %w", err) - } - return &Certificate{v1Beta1: resp}, nil - } - - resp, err := createV1(ctx, client, c.v1) - if err != nil { - if apierrors.IsAlreadyExists(err) { - return get(ctx, client, c) - } - return nil, fmt.Errorf("v1 csr create: %w", err) - } - - return &Certificate{v1: resp}, nil -} - -func startInformers(ctx context.Context, log logrus.FieldLogger, factories ...informers.SharedInformerFactory) { - stopCh := make(chan struct{}) - defer close(stopCh) - - for _, factory := range factories { - factory.Start(stopCh) - } - - log.Info("watching for new node CSRs") - - <-ctx.Done() - log.WithField("context", ctx.Err()).Info("finished watching for new node CSRs") -} - -func get(ctx context.Context, client kubernetes.Interface, cert *Certificate) (*Certificate, error) { - if cert.v1Beta1 != nil { - v1beta1req, err := client.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, cert.v1Beta1.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - return &Certificate{v1Beta1: v1beta1req}, nil - } - - v1req, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, cert.v1.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - return &Certificate{v1: v1req}, nil -} - -func createV1(ctx context.Context, client kubernetes.Interface, csr *certv1.CertificateSigningRequest) (*certv1.CertificateSigningRequest, error) { - csrv1 := &certv1.CertificateSigningRequest{ - // Username, UID, Groups will be injected by API server. - TypeMeta: metav1.TypeMeta{Kind: "CertificateSigningRequest"}, - ObjectMeta: metav1.ObjectMeta{ - Name: csr.Name, - }, - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: csr.Spec.SignerName, - Request: csr.Spec.Request, - Usages: csr.Spec.Usages, - ExpirationSeconds: csr.Spec.ExpirationSeconds, - }, - } - req, err := client.CertificatesV1().CertificateSigningRequests().Create(ctx, csrv1, metav1.CreateOptions{}) - if err != nil { - return nil, err - } - return req, nil -} - -func createV1beta1(ctx context.Context, client kubernetes.Interface, csr *certv1beta1.CertificateSigningRequest) (*certv1beta1.CertificateSigningRequest, error) { - v1beta1csr := &certv1beta1.CertificateSigningRequest{ - TypeMeta: metav1.TypeMeta{Kind: "CertificateSigningRequest"}, - ObjectMeta: metav1.ObjectMeta{ - Name: csr.Name, - }, - Spec: certv1beta1.CertificateSigningRequestSpec{ - SignerName: csr.Spec.SignerName, - Request: csr.Spec.Request, - Usages: csr.Spec.Usages, - }, - } - - req, err := client.CertificatesV1beta1().CertificateSigningRequests().Create(ctx, v1beta1csr, metav1.CreateOptions{}) - if err != nil { - return nil, err - } - return req, nil -} - -func createInformer(ctx context.Context, client kubernetes.Interface, fieldSelectorV1, fieldSelectorV1beta1 string) (informers.SharedInformerFactory, cache.SharedIndexInformer, error) { - var ( - errv1 error - errv1beta1 error - ) - - if _, errv1 = client.CertificatesV1().CertificateSigningRequests().List(ctx, metav1.ListOptions{}); errv1 == nil { - v1Factory := informers.NewSharedInformerFactoryWithOptions(client, csrInformerResyncPeriod, - informers.WithTweakListOptions(func(opts *metav1.ListOptions) { - opts.FieldSelector = fieldSelectorV1 - })) - v1Informer := v1Factory.Certificates().V1().CertificateSigningRequests().Informer() - return v1Factory, v1Informer, nil - } - - if _, errv1beta1 = client.CertificatesV1beta1().CertificateSigningRequests().List(ctx, metav1.ListOptions{}); errv1beta1 == nil { - v1Factory := informers.NewSharedInformerFactoryWithOptions(client, csrInformerResyncPeriod, - informers.WithTweakListOptions(func(opts *metav1.ListOptions) { - opts.FieldSelector = fieldSelectorV1beta1 - })) - v1Informer := v1Factory.Certificates().V1beta1().CertificateSigningRequests().Informer() - return v1Factory, v1Informer, nil - } - - return nil, nil, fmt.Errorf("failed to create informer: v1: %w, v1beta1: %w", errv1, errv1beta1) -} - -var errUnexpectedObjectType = errors.New("unexpected object type") - -func processCSREvent(ctx context.Context, c chan<- *Certificate, csrObj interface{}) error { - cert, err := toCertificate(csrObj) - if err != nil { - return fmt.Errorf("toCertificate: %w", err) - } - - if cert == nil { - return nil - } - - if cert.Approved() || - !cert.ForCASTAINode() || - // approve only node bootstrap and kubelet CSR from node. - (!cert.isRequestedBySystemNode() && !cert.isRequestedByNodeBootstrap()) || - cert.Outdated() { - return nil - } - - sendCertificate(ctx, c, cert) - return nil -} - -func toCertificate(obj interface{}) (cert *Certificate, err error) { - var request []byte - originalCSRName := "" - - switch e := obj.(type) { - case *certv1.CertificateSigningRequest: - request = e.Spec.Request - originalCSRName = e.Name - cert = &Certificate{ - v1: e, - } - case *certv1beta1.CertificateSigningRequest: - request = e.Spec.Request - originalCSRName = e.Name - cert = &Certificate{ - v1Beta1: e, - } - default: - return nil, errUnexpectedObjectType - } - - cn, err := cert.getSubjectCommonName(request) - if err != nil { - return nil, fmt.Errorf("getSubjectCommonName: Name: %v RequestingUser: %v request: %v %w", originalCSRName, cert.RequestingUser(), string(request), err) - } - - cert.Name = cn - - return cert, nil -} - -func sendCertificate(ctx context.Context, c chan<- *Certificate, cert *Certificate) { - select { - case c <- cert: - case <-ctx.Done(): - return - } -} - -func (c *Certificate) OriginalCSRName() string { - if c.v1 != nil { - return c.v1.Name - } - if c.v1Beta1 != nil { - return c.v1Beta1.Name - } - - return "" -} - -func (c *Certificate) RequestingUser() string { - if c.v1 != nil { - return c.v1.Spec.Username - } - if c.v1Beta1 != nil { - return c.v1Beta1.Spec.Username - } - - return "" -} - -func (c *Certificate) SignerName() string { - if c.v1 != nil { - return c.v1.Spec.SignerName - } - if c.v1Beta1 != nil { - if c.v1Beta1.Spec.SignerName != nil { - return *c.v1Beta1.Spec.SignerName - } - } - - return "" -} - -func (c *Certificate) Usages() []string { - if c.v1 != nil { - return toKeyUsage(c.v1.Spec.Usages) - } - if c.v1Beta1 != nil { - return toKeyUsage(c.v1Beta1.Spec.Usages) - } - - return nil -} - -func (c *Certificate) getSubjectCommonName(csrRequest []byte) (string, error) { - // node-csr prefix for bootstrap kubelet csr. - // csr- prefix for kubelet csr. - originalCSRName := c.OriginalCSRName() - if !strings.HasPrefix(originalCSRName, "node-csr") && !strings.HasPrefix(originalCSRName, "csr-") { - return "", fmt.Errorf("invalid CSR name: %s %w", originalCSRName, errInvalidCSR) - } - - certReq, err := c.parseCSR(csrRequest) - if err != nil { - return "", fmt.Errorf("parse CSR: %w", err) - } - return certReq.Subject.CommonName, nil -} - -// parseCSR is mostly needed to extract node name from cert subject common name. -func (c *Certificate) parseCSR(pemData []byte) (*x509.CertificateRequest, error) { - block, _ := pem.Decode(pemData) - if block == nil || block.Type != "CERTIFICATE REQUEST" { - return nil, fmt.Errorf("PEM block type must be CERTIFICATE REQUEST") - } - csr, err := x509.ParseCertificateRequest(block.Bytes) - if err != nil { - return nil, fmt.Errorf("parse certificate request: %w", err) - } - if err := c.validateCSR(csr); err != nil { - return nil, fmt.Errorf("validate CSR: %w", err) - } - return csr, nil -} - -func (c *Certificate) validateCSR(csr *x509.CertificateRequest) error { - if csr == nil { - return fmt.Errorf("%w: nil CSR", errInvalidCSR) - } - if c.SignerName() == certv1.KubeAPIServerClientKubeletSignerName { - // no validation - return nil - } - if c.SignerName() != certv1.KubeletServingSignerName { - return fmt.Errorf("%w: unknown signer name %s", errInvalidCSR, c.SignerName()) - } - - if len(csr.Subject.CommonName) == 0 { - return fmt.Errorf("%w: CSR subject common name", errInvalidCSR) - } - if len(csr.URIs) > 0 { - return fmt.Errorf("%w: CSR subject URIs must be empty: %v", errInvalidCSR, csr.URIs) - } - if len(csr.EmailAddresses) > 0 { - return fmt.Errorf("%w: CSR subject email addresses must be empty: %v", errInvalidCSR, csr.EmailAddresses) - } - - if len(c.Usages()) == 0 { - return fmt.Errorf("%w: CSR Usages is empty", errInvalidCSR) - } - usageServerAuthExisted := false - for _, u := range c.Usages() { - if u != fmt.Sprintf("%v", certv1.UsageServerAuth) && - u != fmt.Sprintf("%v", certv1.UsageDigitalSignature) && - u != fmt.Sprintf("%v", certv1.UsageKeyEncipherment) { - return fmt.Errorf("%v: CSR usages %w", c.Usages(), errInvalidCSR) - } - if u == fmt.Sprintf("%v", certv1.UsageServerAuth) { - usageServerAuthExisted = true - } - } - if !usageServerAuthExisted { - return fmt.Errorf("%w: CSR usages must be for server usage %v", errInvalidCSR, c.Usages()) - } - // TODO add validation of IP and DNS - // https://kubernetes.io/docs/reference/access-authn-authz/kubelet-tls-bootstrapping/#certificate-rotation - - return nil -} - -//nolint:unparam -func getOptions(signer string) metav1.ListOptions { - return metav1.ListOptions{ - FieldSelector: fields.SelectorFromSet(fields.Set{ - "spec.signerName": signer, - }).String(), - } -} - -func toKeyUsage[T certv1.KeyUsage | certv1beta1.KeyUsage](usages []T) []string { - u := make([]string, 0, len(usages)) - for _, usage := range usages { - u = append(u, string(usage)) - } - return u -} diff --git a/internal/actions/csr/csr_test.go b/internal/actions/csr/csr_test.go deleted file mode 100644 index da3a4be6..00000000 --- a/internal/actions/csr/csr_test.go +++ /dev/null @@ -1,450 +0,0 @@ -package csr - -import ( - "crypto/x509" - "crypto/x509/pkix" - "net/url" - "testing" - "time" - - "github.com/stretchr/testify/require" - certv1 "k8s.io/api/certificates/v1" - certv1beta1 "k8s.io/api/certificates/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" -) - -func Test_isCastAINodeCsr(t *testing.T) { - type args struct { - subjectCommonName string - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "empty node name", - want: false, - }, - { - name: "not cast in subjectComma ", - args: args{ - subjectCommonName: "system:node:node1", - }, - want: false, - }, - { - name: "not CastAI node", - args: args{ - subjectCommonName: "node1", - }, - want: false, - }, - { - name: "CAST AI node", - args: args{ - subjectCommonName: "system:node:node1-cast-pool-123", - }, - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - cert := &Certificate{ - Name: tt.args.subjectCommonName, - } - - require.Equal(t, tt.want, cert.ForCASTAINode()) - }) - } -} - -func Test_outdatedCertificate(t *testing.T) { - tt := map[string]struct { - createTimestamp time.Time - want bool - }{ - "Outdated": { - createTimestamp: time.Now().Add(-csrTTL).Add(-time.Second), - want: true, - }, - "Not outdated": { - createTimestamp: time.Now(), - want: false, - }, - "Outdated, right before": { - createTimestamp: time.Now().Add(-csrTTL).Add(2 * time.Second), - want: false, - }, - } - - for name, tc := range tt { - t.Run(name, func(t *testing.T) { - cert := &Certificate{ - v1: &certv1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.NewTime(tc.createTimestamp), - }, - }, - } - require.Equal(t, tc.want, cert.Outdated()) - - certBeta := &Certificate{ - v1Beta1: &certv1beta1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.NewTime(tc.createTimestamp), - }, - }, - } - require.Equal(t, tc.want, certBeta.Outdated()) - }) - } -} - -func Test_nodeBootstrap(t *testing.T) { - tt := map[string]struct { - reqUser string - want bool - }{ - "other one": { - reqUser: "dummy-user", - want: false, - }, - "kubelet-bootstrap": { - reqUser: "kubelet-bootstrap", - want: true, - }, - "castai-cluster-controller": { - reqUser: "system:serviceaccount:castai-agent:castai-cluster-controller", - want: true, - }, - } - - for name, tc := range tt { - t.Run(name, func(t *testing.T) { - cert := &Certificate{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - Username: tc.reqUser, - }, - }, - } - require.Equal(t, tc.want, cert.isRequestedByNodeBootstrap()) - }) - } -} - -func Test_toCertificate(t *testing.T) { - kBootstrapCSRv1 := getCSRv1("node-csr", "kubelet-bootstrap") - kBootstrapCtCSRv1beta1 := getCSRv1betav1("node-csr", "kubelet-bootstrap") - kServingCSRv1 := getCSRv1("csr-s7v44", "system:node:gke-va") - kServingCSRv1beta1 := getCSRv1betav1("csr-s7v44", "system:node:gke-va") - type args struct { - obj interface{} - } - tests := []struct { - name string - args args - checkFunc func(t *testing.T, cert *Certificate) - wantErr bool - }{ - { - name: "empty event", - args: args{ - obj: nil, - }, - wantErr: true, - }, - { - name: "outdated event", - args: args{ - obj: &certv1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.Time{Time: time.Now().Add(-csrTTL)}, - Name: "node-csr-gke-dev-master-cast-pool-cb53177b", - }, - Spec: kBootstrapCSRv1.Spec, - }, - }, - checkFunc: func(t *testing.T, cert *Certificate) { - require.True(t, cert.Outdated()) - }, - wantErr: false, - }, - { - name: "bad owner", - args: args{ - obj: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - Username: "test", - }, - ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: metav1.Time{Time: time.Now().Add(csrTTL)}, - }, - }, - }, - wantErr: true, - }, - { - name: "kubelet-bootstrap: ok v1", - args: args{ - obj: kBootstrapCSRv1, - }, - checkFunc: func(t *testing.T, cert *Certificate) { - require.Equal(t, "system:node:gke-dev-master-cast-pool-cb53177b", cert.Name) - require.Equal(t, "kubelet-bootstrap", cert.RequestingUser()) - require.Equal(t, kBootstrapCSRv1, cert.v1) - }, - wantErr: false, - }, - { - name: "kubelet-bootstrap: ok v1beta1", - args: args{ - obj: kBootstrapCtCSRv1beta1, - }, - wantErr: false, - checkFunc: func(t *testing.T, cert *Certificate) { - require.Equal(t, "system:node:gke-dev-master-cast-pool-cb53177b", cert.Name) - require.Equal(t, "kubelet-bootstrap", cert.RequestingUser()) - require.Equal(t, kBootstrapCtCSRv1beta1, cert.v1Beta1) - }, - }, - { - name: "kubelet-serving: ok v1", - args: args{ - obj: kServingCSRv1, - }, - checkFunc: func(t *testing.T, cert *Certificate) { - require.Equal(t, "system:node:gke-dev-master-cast-pool-cb53177b", cert.Name) - require.Equal(t, "system:node:gke-va", cert.RequestingUser()) - require.Equal(t, kServingCSRv1, cert.v1) - }, - wantErr: false, - }, - { - name: "kubelet-serving: ok v1beta1", - args: args{ - obj: kServingCSRv1beta1, - }, - wantErr: false, - checkFunc: func(t *testing.T, cert *Certificate) { - require.Equal(t, "system:node:gke-dev-master-cast-pool-cb53177b", cert.Name) - require.Equal(t, "system:node:gke-va", cert.RequestingUser()) - require.Equal(t, kServingCSRv1beta1, cert.v1Beta1) - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotCert, err := toCertificate(tt.args.obj) - if (err != nil) != tt.wantErr { - t.Errorf("toCertificate() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if tt.checkFunc != nil { - tt.checkFunc(t, gotCert) - } - }) - } -} - -func TestCertificate_validateCSR(t *testing.T) { - kubeletServingSignerName := certv1beta1.KubeletServingSignerName - type fields struct { - v1 *certv1.CertificateSigningRequest - v1Beta1 *certv1beta1.CertificateSigningRequest - Name string - } - type args struct { - csr *x509.CertificateRequest - } - tests := []struct { - name string - fields fields - args args - wantErr bool - }{ - { - name: "empty csr", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{}, - }, - }, - wantErr: true, - }, - { - name: "empty signer", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{}, - }, - }, - args: args{ - csr: &x509.CertificateRequest{}, - }, - wantErr: true, - }, - { - name: "no validation", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeAPIServerClientKubeletSignerName, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{}, - }, - }, - { - name: "empty sn for serving CSR", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeletServingSignerName, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{}, - }, - wantErr: true, - }, - { - name: "not empty URI for serving CSR", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeletServingSignerName, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "system:node:node1", - }, - URIs: []*url.URL{ - {}, {}, - }, - }, - }, - wantErr: true, - }, - { - name: "not empty Emails for serving CSR", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeletServingSignerName, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "system:node:node1", - }, - EmailAddresses: []string{ - "test", - }, - }, - }, - wantErr: true, - }, - { - name: "empty Usages for serving CSR", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeletServingSignerName, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "system:node:node1", - }, - EmailAddresses: []string{}, - }, - }, - wantErr: true, - }, - { - name: "wrong usages for serving CSR", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeletServingSignerName, - Usages: []certv1.KeyUsage{certv1.UsageServerAuth, "wrong"}, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "system:node:node1", - }, - EmailAddresses: []string{}, - }, - }, - wantErr: true, - }, - { - name: "wrong usages: no server auth for serving CSR", - fields: fields{ - v1Beta1: &certv1beta1.CertificateSigningRequest{ - Spec: certv1beta1.CertificateSigningRequestSpec{ - Usages: []certv1beta1.KeyUsage{certv1beta1.UsageDigitalSignature}, - SignerName: &kubeletServingSignerName, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "system:node:node1", - }, - EmailAddresses: []string{}, - }, - }, - wantErr: true, - }, - { - name: "ok for serving CSR", - fields: fields{ - v1: &certv1.CertificateSigningRequest{ - Spec: certv1.CertificateSigningRequestSpec{ - SignerName: certv1.KubeletServingSignerName, - Usages: []certv1.KeyUsage{certv1.UsageServerAuth}, - }, - }, - }, - args: args{ - csr: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: "system:node:node1", - }, - EmailAddresses: []string{}, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - c := &Certificate{ - v1: tt.fields.v1, - v1Beta1: tt.fields.v1Beta1, - Name: tt.fields.Name, - } - if err := c.validateCSR(tt.args.csr); (err != nil) != tt.wantErr { - t.Errorf("validateCSR() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} diff --git a/internal/actions/csr/informer.go b/internal/actions/csr/informer.go new file mode 100644 index 00000000..5a7d1801 --- /dev/null +++ b/internal/actions/csr/informer.go @@ -0,0 +1,71 @@ +package csr + +import ( + "context" + "fmt" + "time" + + "github.com/sirupsen/logrus" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" +) + +const ( + // We should approve CSRs, when they are created, so resync can be high. + // Resync plays back all events (create, update, delete), which are in informer cache. + // This does not involve talking to API server, it is not relist. + csrInformerResyncPeriod = 12 * time.Hour +) + +func startInformers(ctx context.Context, log logrus.FieldLogger, factories ...informers.SharedInformerFactory) { + stopCh := make(chan struct{}) + defer close(stopCh) + + for _, factory := range factories { + factory.Start(stopCh) + } + + log.Info("watching for new node CSRs") + + <-ctx.Done() + log.WithField("context", ctx.Err()).Info("finished watching for new node CSRs") +} + +func createInformer(ctx context.Context, client kubernetes.Interface, fieldSelectorV1, fieldSelectorV1beta1 string) (informers.SharedInformerFactory, cache.SharedIndexInformer, error) { + var ( + errv1 error + errv1beta1 error + ) + + if _, errv1 = client.CertificatesV1().CertificateSigningRequests().List(ctx, metav1.ListOptions{}); errv1 == nil { + v1Factory := informers.NewSharedInformerFactoryWithOptions(client, csrInformerResyncPeriod, + informers.WithTweakListOptions(func(opts *metav1.ListOptions) { + opts.FieldSelector = fieldSelectorV1 + })) + v1Informer := v1Factory.Certificates().V1().CertificateSigningRequests().Informer() + return v1Factory, v1Informer, nil + } + + if _, errv1beta1 = client.CertificatesV1beta1().CertificateSigningRequests().List(ctx, metav1.ListOptions{}); errv1beta1 == nil { + v1Factory := informers.NewSharedInformerFactoryWithOptions(client, csrInformerResyncPeriod, + informers.WithTweakListOptions(func(opts *metav1.ListOptions) { + opts.FieldSelector = fieldSelectorV1beta1 + })) + v1Informer := v1Factory.Certificates().V1beta1().CertificateSigningRequests().Informer() + return v1Factory, v1Informer, nil + } + + return nil, nil, fmt.Errorf("failed to create informer: v1: %w, v1beta1: %w", errv1, errv1beta1) +} + +//nolint:unparam +func listOptionsWithSigner(signer string) metav1.ListOptions { + return metav1.ListOptions{ + FieldSelector: fields.SelectorFromSet(fields.Set{ + "spec.signerName": signer, + }).String(), + } +} diff --git a/internal/actions/csr/integration_test.go b/internal/actions/csr/integration_test.go new file mode 100644 index 00000000..21e36179 --- /dev/null +++ b/internal/actions/csr/integration_test.go @@ -0,0 +1,576 @@ +package csr_test + +import ( + "context" + "crypto/x509" + "crypto/x509/pkix" + "io" + "net" + "net/url" + "testing" + "time" + + "github.com/samber/lo" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" + certv1 "k8s.io/api/certificates/v1" + certv1beta1 "k8s.io/api/certificates/v1beta1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes/fake" + clienttesting "k8s.io/client-go/testing" + + "github.com/castai/cluster-controller/internal/actions/csr" + csrtest "github.com/castai/cluster-controller/internal/actions/csr/test" +) + +func TestIntegration(t *testing.T) { + t.Parallel() + testIntegration(t, certv1.SchemeGroupVersion) + testIntegration(t, certv1beta1.SchemeGroupVersion) +} + +func testIntegration(t *testing.T, csrVersion schema.GroupVersion) { + ctx := context.TODO() + r := require.New(t) + clientset := setupManagerAndClientset(t, csrVersion) + for _, testcase := range []struct { + creationTimestamp metav1.Time + description string + emails []string + groups []string + nodeCreatedWithStatus *corev1.NodeStatus + nodeName string + notApproved bool + signer string + uris []*url.URL + usages []string + username string + ips []net.IP + dns []string + }{ + { + description: "[client-kubelet] outdated", + nodeName: "node-csr-cast-pool-0", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "kubelet-bootstrap", + creationTimestamp: metav1.NewTime(time.Now().Add(-time.Hour - 1*time.Minute)), + notApproved: true, + }, + { + description: "[client-kubelet] with prefix node-csr", + nodeName: "node-csr-cast-pool-1", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "kubelet-bootstrap", + }, + { + description: "[client-kubelet] with prefix csr", + nodeName: "csr-cast-pool-2", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "kubelet-bootstrap", + }, + { + description: "[client-kubelet] unknown prefix", + nodeName: "unknown-cast-pool-3", + notApproved: true, + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "kubelet-bootstrap", + }, + { + description: "[client-kubelet] without cast-pool in subject CN", + nodeName: "node-csr-some-text", + notApproved: true, + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "kubelet-bootstrap", + }, + { + description: "[client-kubelet] with username kubelet-bootstrap", + nodeName: "csr-cast-pool-4", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "kubelet-bootstrap", + }, + { + description: "[client-kubelet] with username serviceaccount", + nodeName: "csr-cast-pool-5", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "system:serviceaccount:castai-agent:castai-cluster-controller", + }, + { + description: "[client-kubelet] with username prefix sytem:node", + nodeName: "csr-cast-pool-6", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "system:node:some-text", + }, + { + description: "[client-kubelet] with unknown username", + nodeName: "csr-cast-pool-7", + notApproved: true, + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "unknown-username-text", + }, + { + description: "[client-kubelet] with all allowed key usages", + nodeName: "csr-cast-pool-8", + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth), string(certv1.UsageDigitalSignature), string(certv1.UsageKeyEncipherment)}, + username: "kubelet-bootstrap", + }, + { + description: "[client-kubelet] with not allowed key usages", + nodeName: "csr-cast-pool-9", + notApproved: true, + signer: certv1.KubeAPIServerClientKubeletSignerName, + usages: []string{string(certv1.UsageClientAuth), string(certv1.UsageServerAuth)}, + username: "kubelet-bootstrap", + }, + { + description: "[kubelet-serving] with prefix node-csr", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "node-csr-cast-pool-10", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10", + }, + { + description: "[kubelet-serving] without matching node", + groups: []string{"system:nodes"}, + nodeName: "node-csr-cast-pool-10a", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10a", + }, + { + description: "[kubelet-serving] with matching Internal IP", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "10.0.0.123", + }, + }, + }, + nodeName: "node-csr-cast-pool-10b", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10b", + ips: []net.IP{ + net.IPv4(10, 0, 0, 123), + }, + }, + { + description: "[kubelet-serving] with mismatching Internal IP", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "10.0.0.1", + }, + }, + }, + nodeName: "node-csr-cast-pool-10c", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10c", + ips: []net.IP{ + net.IPv4(10, 0, 0, 123), + }, + }, + { + description: "[kubelet-serving] with matching External IP", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeExternalIP, + Address: "10.0.0.123", + }, + }, + }, + nodeName: "node-csr-cast-pool-10d", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10d", + ips: []net.IP{ + net.IPv4(10, 0, 0, 123), + }, + }, + { + description: "[kubelet-serving] with mismatching External IP", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeExternalIP, + Address: "10.0.0.1", + }, + }, + }, + nodeName: "node-csr-cast-pool-10e", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10e", + ips: []net.IP{ + net.IPv4(10, 0, 0, 123), + }, + }, + { + description: "[kubelet-serving] with matching Internal DNS", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalDNS, + Address: "foo.bar", + }, + }, + }, + nodeName: "node-csr-cast-pool-10f", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10f", + dns: []string{"foo.bar"}, + }, + { + description: "[kubelet-serving] with mismatching Internal DNS", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalDNS, + Address: "some.text", + }, + }, + }, + nodeName: "node-csr-cast-pool-10g", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10g", + dns: []string{"foo.bar"}, + }, + { + description: "[kubelet-serving] with matching External DNS", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeExternalDNS, + Address: "foo.bar", + }, + }, + }, + nodeName: "node-csr-cast-pool-10h", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10h", + dns: []string{"foo.bar"}, + }, + { + description: "[kubelet-serving] with mismatching External DNS", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeExternalDNS, + Address: "some.text", + }, + }, + }, + nodeName: "node-csr-cast-pool-10i", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10i", + dns: []string{"node-csr-cast-pool-10i"}, + }, + { + description: "[kubelet-serving] with matching Hostname", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeHostName, + Address: "node-csr-cast-pool-10j", + }, + }, + }, + nodeName: "node-csr-cast-pool-10j", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10j", + dns: []string{"node-csr-cast-pool-10j"}, + }, + { + description: "[kubelet-serving] with mismatching Hostname", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{ + Addresses: []corev1.NodeAddress{ + { + Type: corev1.NodeHostName, + Address: "node-csr-cast-pool-10k", + }, + }, + }, + nodeName: "node-csr-cast-pool-10k", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:node-csr-cast-pool-10k", + dns: []string{"foo.bar"}, + }, + { + description: "[kubelet-serving] with prefix csr", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-11", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:csr-cast-pool-11", + }, + { + description: "[kubelet-serving] unknown prefix", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "unknown-cast-pool-12", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:unknown-cast-pool-12", + }, + { + description: "[kubelet-serving] with unknown username", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-13", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "unknown-username-text", + }, + { + description: "[kubelet-serving] with groups system:nodes", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-13a", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:csr-cast-pool-13a", + }, + { + description: "[kubelet-serving] without required groups system:nodes", + groups: []string{"unknown-group"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-13b", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:csr-cast-pool-13b", + }, + { + description: "[kubelet-serving] with all allowed key usages", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-14", + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth), string(certv1.UsageDigitalSignature), string(certv1.UsageKeyEncipherment)}, + username: "system:node:csr-cast-pool-14", + }, + { + description: "[kubelet-serving] without required server auth key usage", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-15", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageClientAuth)}, + username: "system:node:csr-cast-pool-15", + }, + { + description: "[kubelet-serving] with not allowed key usages", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-16", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageClientAuth), string(certv1.UsageServerAuth)}, + username: "system:node:csr-cast-pool-16", + }, + { + description: "[kubelet-serving] with emails", + emails: []string{"csr-cast-pool-18@some.org"}, + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-18", + notApproved: true, + signer: certv1.KubeletServingSignerName, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:csr-cast-pool-18", + }, + { + description: "[kubelet-serving] with uris", + groups: []string{"system:nodes"}, + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-20", + notApproved: true, + signer: certv1.KubeletServingSignerName, + uris: []*url.URL{{Path: "https://example.com"}}, + usages: []string{string(certv1.UsageServerAuth)}, + username: "system:node:csr-cast-pool-20", + }, + { + description: "with unmanaged signer", + nodeCreatedWithStatus: &corev1.NodeStatus{}, + nodeName: "csr-cast-pool-21", + notApproved: true, + signer: certv1.KubeAPIServerClientKubeletSignerName, + username: "system:nodes:csr-cast-pool-21", + }, + } { + t.Run(csrVersion.Version+" "+testcase.description, func(t *testing.T) { + t.Parallel() + if testcase.creationTimestamp.IsZero() { + testcase.creationTimestamp = metav1.Now() + } + if testcase.nodeCreatedWithStatus != nil { + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: testcase.nodeName, + CreationTimestamp: testcase.creationTimestamp, + }, + Status: *testcase.nodeCreatedWithStatus, + } + _, err := clientset.CoreV1().Nodes().Create(ctx, node, metav1.CreateOptions{}) + r.NoError(err, "failed to create node") + } + if csrVersion == certv1.SchemeGroupVersion { + csr := &certv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: testcase.nodeName, + CreationTimestamp: testcase.creationTimestamp, + }, + Spec: certv1.CertificateSigningRequestSpec{ + Groups: testcase.groups, + Request: csrtest.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + EmailAddresses: testcase.emails, + IPAddresses: testcase.ips, + DNSNames: testcase.dns, + Subject: pkix.Name{ + CommonName: "system:node:" + testcase.nodeName, + }, + URIs: testcase.uris, + }), + SignerName: testcase.signer, + Usages: lo.Map(testcase.usages, func(u string, _ int) certv1.KeyUsage { + return certv1.KeyUsage(u) + }), + Username: testcase.username, + }, + } + _, err := clientset.CertificatesV1().CertificateSigningRequests().Create(ctx, csr, metav1.CreateOptions{}) + r.NoError(err, "failed to create CSR") + time.Sleep(10 * time.Millisecond) + csr, err = clientset.CertificatesV1().CertificateSigningRequests().Get(ctx, csr.Name, metav1.GetOptions{}) + r.NoError(err, "failed to get CSR") + approved := approvedCSRV1(csr) + if testcase.notApproved { + r.Falsef(approved, "%s - must not be approved", testcase.description) + } else { + r.Truef(approved, "%s - must be approved", testcase.description) + } + } else { + csr := &certv1beta1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: testcase.nodeName, + CreationTimestamp: testcase.creationTimestamp, + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + Groups: testcase.groups, + Request: csrtest.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + EmailAddresses: testcase.emails, + IPAddresses: testcase.ips, + DNSNames: testcase.dns, + Subject: pkix.Name{ + CommonName: "system:node:" + testcase.nodeName, + }, + URIs: testcase.uris, + }), + SignerName: lo.ToPtr(testcase.signer), + Usages: lo.Map(testcase.usages, func(u string, _ int) certv1beta1.KeyUsage { + return certv1beta1.KeyUsage(u) + }), + Username: testcase.username, + }, + } + _, err := clientset.CertificatesV1beta1().CertificateSigningRequests().Create(ctx, csr, metav1.CreateOptions{}) + r.NoError(err, "failed to create CSR") + time.Sleep(10 * time.Millisecond) + csr, err = clientset.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csr.Name, metav1.GetOptions{}) + r.NoError(err, "failed to get CSR") + approved := approvedCSRV1beta1(csr) + if testcase.notApproved { + r.Falsef(approved, "%s - must not be approved", testcase.description) + } else { + r.Truef(approved, "%s - must be approved", testcase.description) + } + } + }) + } +} + +func approvedCSRV1(csr *certv1.CertificateSigningRequest) bool { + for _, condition := range csr.Status.Conditions { + if condition.Type == certv1.CertificateApproved && condition.Status == corev1.ConditionTrue { + return true + } + } + return false +} + +func approvedCSRV1beta1(csr *certv1beta1.CertificateSigningRequest) bool { + for _, condition := range csr.Status.Conditions { + if condition.Type == certv1beta1.CertificateApproved && condition.Status == corev1.ConditionTrue { + return true + } + } + return false +} + +func setupManagerAndClientset(t *testing.T, csrVersion schema.GroupVersion) *fake.Clientset { + t.Helper() + + // Coppied and adapter https://github.com/kubernetes/client-go/blob/master/examples/fake-client + // Create the fake client. + client := fake.NewClientset() + client.PrependReactor("*", "*", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + if action.GetResource().Resource == "certificatesigningrequests" && action.GetResource().GroupVersion() != csrVersion { + err = apierrors.NewNotFound(schema.GroupResource{}, action.GetResource().String()) + return true, nil, err + } + return + }) + logger := logrus.New() + logger.SetOutput(io.Discard) + manager := csr.NewApprovalManager(logger, client) + err := manager.Start(context.TODO()) + require.NoError(t, err, "failed to start approval manager") + + return client +} diff --git a/internal/actions/csr/svc.go b/internal/actions/csr/svc.go index ab0b16cd..eee1f651 100644 --- a/internal/actions/csr/svc.go +++ b/internal/actions/csr/svc.go @@ -4,22 +4,32 @@ import ( "context" "errors" "fmt" + "strings" "sync" "time" + "github.com/samber/lo" "github.com/sirupsen/logrus" certv1 "k8s.io/api/certificates/v1" certv1beta1 "k8s.io/api/certificates/v1beta1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" + "github.com/castai/cluster-controller/internal/actions/csr/wrapper" "github.com/castai/cluster-controller/internal/waitext" ) const ( - approveCSRTimeout = 4 * time.Minute + approveCSRTimeout = 4 * time.Minute + groupSystemNodesName = "system:nodes" + kubeletBootstrapRequestingUser = "kubelet-bootstrap" + clusterControllerSAName = "system:serviceaccount:castai-agent:castai-cluster-controller" + approvedMessage = "This CSR was approved by CAST AI" + csrOutdatedAfter = time.Hour ) func NewApprovalManager(log logrus.FieldLogger, clientset kubernetes.Interface) *ApprovalManager { @@ -35,34 +45,41 @@ type ApprovalManager struct { cancelAutoApprove context.CancelFunc inProgress map[string]struct{} // one handler per csr/certificate Name. - m sync.Mutex // Used to make sure there is just one watcher running. + mu sync.Mutex // Used to make sure there is just one watcher running. } -func (h *ApprovalManager) Start(ctx context.Context) error { +func (m *ApprovalManager) Start(ctx context.Context) error { informerKubeletSignerFactory, csrInformerKubeletSigner, err := createInformer( ctx, - h.clientset, - getOptions(certv1.KubeAPIServerClientKubeletSignerName).FieldSelector, - getOptions(certv1beta1.KubeAPIServerClientKubeletSignerName).FieldSelector) + m.clientset, + listOptionsWithSigner(certv1.KubeAPIServerClientKubeletSignerName).FieldSelector, + listOptionsWithSigner(certv1beta1.KubeAPIServerClientKubeletSignerName).FieldSelector) if err != nil { return fmt.Errorf("while creating informer for %v: %w", certv1.KubeAPIServerClientKubeletSignerName, err) } informerKubeletServingFactory, csrInformerKubeletServing, err := createInformer( ctx, - h.clientset, - getOptions(certv1.KubeletServingSignerName).FieldSelector, - getOptions(certv1beta1.KubeletServingSignerName).FieldSelector) + m.clientset, + listOptionsWithSigner(certv1.KubeletServingSignerName).FieldSelector, + listOptionsWithSigner(certv1beta1.KubeletServingSignerName).FieldSelector) if err != nil { return fmt.Errorf("while creating informer for %v: %w", certv1.KubeletServingSignerName, err) } - c := make(chan *Certificate, 1) + c := make(chan *wrapper.CSR, 1) handlerFuncs := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - if err := processCSREvent(ctx, c, obj); err != nil { - h.log.WithError(err).Warn("failed to process csr add event") + csr, err := wrapper.NewCSR(m.clientset, obj) + if err != nil { + m.log.WithError(err).Warn("creating csr wrapper") + return + } + select { + case c <- csr: + case <-ctx.Done(): + return } }, } @@ -76,21 +93,76 @@ func (h *ApprovalManager) Start(ctx context.Context) error { } ctx, cancel := context.WithCancel(ctx) - if !h.startAutoApprove(cancel) { + if !m.startAutoApprove(cancel) { return nil } - go startInformers(ctx, h.log, informerKubeletSignerFactory, informerKubeletServingFactory) - go h.runAutoApproveForCastAINodes(ctx, c) + go startInformers(ctx, m.log, informerKubeletSignerFactory, informerKubeletServingFactory) + go m.runAutoApproveForCastAINodes(ctx, c) + + if !cache.WaitForNamedCacheSync("cluster-controller/approval-manager", ctx.Done(), csrInformerKubeletSigner.HasSynced, csrInformerKubeletServing.HasSynced) { + m.log.WithField("context", ctx.Err()).Info("stopping auto approve csr") + return nil + } return nil } -func (h *ApprovalManager) Stop() { - h.stopAutoApproveForCastAINodes() +func (m *ApprovalManager) Stop() { + m.stopAutoApproveForCastAINodes() +} + +func (m *ApprovalManager) startAutoApprove(cancelFunc context.CancelFunc) bool { + m.mu.Lock() + defer m.mu.Unlock() + if m.cancelAutoApprove != nil { + return false + } + + m.log.Info("starting auto approve CSRs for managed by CAST AI nodes") + m.cancelAutoApprove = cancelFunc + + return true +} + +func (m *ApprovalManager) runAutoApproveForCastAINodes(ctx context.Context, c <-chan *wrapper.CSR) { + defer m.stopAutoApproveForCastAINodes() + + log := m.log.WithField("RunAutoApprove", "auto-approve-csr") + + for { + select { + case <-ctx.Done(): + log.WithError(ctx.Err()).Errorf("auto approve csr finished") + return + case csr := <-c: + // prevent starting goroutine for the same node certificate + if !m.addInProgress(csr.ParsedCertificateRequest().Subject.CommonName, csr.SignerName()) { + continue + } + go func(csr *wrapper.CSR) { + defer m.removeInProgress(csr.ParsedCertificateRequest().Subject.CommonName, csr.SignerName()) + + log := log.WithFields(logrus.Fields{ + "CN": csr.ParsedCertificateRequest().Subject.CommonName, + "signer": csr.SignerName(), + "csr": csr.Name(), + }) + if shouldSkip(log, csr) { + log.Debug("skipping csr") + return + } + log.Info("auto approving csr") + err := m.handleWithRetry(ctx, log, csr) + if err != nil { + log.WithError(err).Errorf("failed to approve csr: %+v", csr) + } + }(csr) + } + } } -func (h *ApprovalManager) handleWithRetry(ctx context.Context, log *logrus.Entry, cert *Certificate) error { +func (m *ApprovalManager) handleWithRetry(ctx context.Context, log *logrus.Entry, csr *wrapper.CSR) error { ctx, cancel := context.WithTimeout(ctx, approveCSRTimeout) defer cancel() @@ -100,7 +172,7 @@ func (h *ApprovalManager) handleWithRetry(ctx context.Context, log *logrus.Entry b, waitext.Forever, func(ctx context.Context) (bool, error) { - return true, h.handle(ctx, log, cert) + return true, m.handle(ctx, log, csr) }, func(err error) { log.Warnf("csr approval failed, will retry: %v", err) @@ -108,115 +180,200 @@ func (h *ApprovalManager) handleWithRetry(ctx context.Context, log *logrus.Entry ) } -var errCSRNotApproved = errors.New("certificate signing request was not approved") +func newApproveCSRExponentialBackoff() wait.Backoff { + b := waitext.DefaultExponentialBackoff() + b.Factor = 2 + return b +} -func (h *ApprovalManager) handle(ctx context.Context, log logrus.FieldLogger, cert *Certificate) (reterr error) { - if cert.Approved() { - return nil +func (m *ApprovalManager) handle(ctx context.Context, log logrus.FieldLogger, csr *wrapper.CSR) (reterr error) { + if err := m.validateCSRRequirements(ctx, csr); err != nil { + return fmt.Errorf("validating csr: %w", err) } - log = log.WithField("csr_name", cert.Name) // Create a new CSR with the same request data as the original one, // since the old csr maybe denied. - log.Info("requesting new csr") - newCert, err := cert.NewCSR(ctx, h.clientset) + log.Info("requesting new csr if doesn't exist") + err := csr.CreateOrRefresh(ctx) if err != nil { - return fmt.Errorf("requesting new csr: %w", err) + return fmt.Errorf("requesting new csr if doesn't exist: %w", err) } - // Approve new csr. - log.Info("approving new csr") - resp, err := newCert.ApproveCSRCertificate(ctx, h.clientset) + // Approve csr. + log.Info("approving csr") + err = csr.Approve(ctx, approvedMessage) if err != nil { return fmt.Errorf("approving csr: %w", err) } - if resp.Approved() { + if csr.Approved() { return nil } // clean original csr. should be the last step for having the possibility. // continue approving csr: old deleted-> restart-> node never join. log.Info("deleting old csr") - if err := cert.DeleteCSR(ctx, h.clientset); err != nil { + if err := csr.Delete(ctx); err != nil { if !apierrors.IsNotFound(err) { return fmt.Errorf("deleting csr: %w", err) } } - return errCSRNotApproved + return errors.New("certificate signing request was not approved") } -func (h *ApprovalManager) runAutoApproveForCastAINodes(ctx context.Context, c <-chan *Certificate) { - defer h.stopAutoApproveForCastAINodes() +func shouldSkip(log logrus.FieldLogger, csr *wrapper.CSR) bool { + if csr.Approved() { + log.Debug("csr already approved") + return true + } + if time.Since(csr.CreatedAt()) > csrOutdatedAfter { + log.Debug("csr is outdated") + return true + } + if !managedSigner(csr.SignerName()) { + log.Debug("csr unknown signer") + return true + } + if !managedCSRNamePrefix(csr.Name()) { + log.Debug("csr name not managed by CAST AI: ", csr.Name()) + return true + } + if !managedCSRRequestingUser(csr.RequestingUser()) { + log.Debug("csr requesting user is not managed by CAST AI") + return true + } + if !managerSubjectCommonName(csr.ParsedCertificateRequest().Subject.CommonName) { + log.Debug("csr common name is not managed by CAST AI") + return true + } + return false +} - log := h.log.WithField("RunAutoApprove", "auto-approve-csr") +func managedSigner(signerName string) bool { + return signerName == certv1.KubeAPIServerClientKubeletSignerName || + signerName == certv1.KubeletServingSignerName +} - for { - select { - case <-ctx.Done(): - log.WithError(ctx.Err()).Errorf("auto approve csr finished") - return - case cert := <-c: - if cert == nil { - continue - } - // prevent starting goroutine for the same node certificate - if !h.addInProgress(cert.Name, cert.SignerName()) { - continue - } - go func(cert *Certificate) { - defer h.removeInProgress(cert.Name, cert.SignerName()) +func managedCSRNamePrefix(n string) bool { + return strings.HasPrefix(n, "node-csr-") || strings.HasPrefix(n, "csr-") +} - log := log.WithFields(logrus.Fields{ - "csr_name": cert.Name, - "signer": cert.SignerName(), - "original_csr_name": cert.OriginalCSRName(), - }) - log.Info("auto approving csr") - err := h.handleWithRetry(ctx, log, cert) - if err != nil { - log.WithError(err).Errorf("failed to approve csr: %+v", cert) - } - }(cert) - } +func managedCSRRequestingUser(s string) bool { + return s == kubeletBootstrapRequestingUser || s == clusterControllerSAName || strings.HasPrefix(s, "system:node:") +} + +func managerSubjectCommonName(commonName string) bool { + return strings.HasPrefix(commonName, "system:node:") && strings.Contains(commonName, "cast-pool") +} + +func (m *ApprovalManager) validateCSRRequirements(ctx context.Context, csr *wrapper.CSR) error { + switch csr.SignerName() { + case certv1.KubeAPIServerClientKubeletSignerName: + return m.validateKubeletClientCSR(csr) + case certv1.KubeletServingSignerName: + return m.validateKubeletServingCSR(ctx, csr) + default: + // Unless logic changes this never returns because unknown signer csr's are skipped. + return fmt.Errorf("unsupported signer name: %s", csr.SignerName()) } } -func (h *ApprovalManager) startAutoApprove(cancelFunc context.CancelFunc) bool { - h.m.Lock() - defer h.m.Unlock() - if h.cancelAutoApprove != nil { - return false +func (m *ApprovalManager) validateKubeletClientCSR(csr *wrapper.CSR) error { + permitted := []string{ + string(certv1.UsageClientAuth), + string(certv1.UsageKeyEncipherment), + string(certv1.UsageDigitalSignature), + } + if notPermitted := lo.Without(csr.Usages(), permitted...); len(notPermitted) > 0 { + return fmt.Errorf("CSR contains not permitted usages: %s", strings.Join(notPermitted, ", ")) } + return nil +} - h.log.Info("starting auto approve CSRs for managed by CAST AI nodes") - h.cancelAutoApprove = cancelFunc +func (m *ApprovalManager) validateKubeletServingCSR(ctx context.Context, csr *wrapper.CSR) error { + // Implement validation suggested from https://kubernetes.io/docs/reference/access-authn-authz/kubelet-tls-bootstrapping/#certificate-rotation - return true + // Check for required group. + if !lo.Contains(csr.Groups(), groupSystemNodesName) { + return fmt.Errorf("CSR does not contain group %s", groupSystemNodesName) + } + // Check for required key usage. + if !lo.Contains(csr.Usages(), string(certv1.UsageServerAuth)) { + return fmt.Errorf("CSR does not contain usage %s", certv1.UsageServerAuth) + } + // Check for not permitted key usages. + permitted := []string{string(certv1.UsageKeyEncipherment), string(certv1.UsageDigitalSignature), string(certv1.UsageServerAuth)} + if notPermitted := lo.Without(csr.Usages(), permitted...); len(notPermitted) > 0 { + return fmt.Errorf("CSR contains not permitted usages: %s", strings.Join(notPermitted, ", ")) + } + x509CSR := csr.ParsedCertificateRequest() + // Check no email addresses. + if len(x509CSR.EmailAddresses) > 0 { + return fmt.Errorf("CSR contains email addresses: %s", strings.Join(x509CSR.EmailAddresses, ", ")) + } + // Check no URIs. + if len(x509CSR.URIs) > 0 { + slice := make([]string, len(x509CSR.URIs)) + for i, u := range x509CSR.URIs { + slice[i] = u.String() + } + return fmt.Errorf("CSR contains URIs: %s", strings.Join(slice, ", ")) + } + + // Check with actual node the last to avoid unnecessary API calls. + nodeName := strings.TrimPrefix(x509CSR.Subject.CommonName, "system:node:") + node, err := m.clientset.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("getting node %s: %w", nodeName, err) + } + if len(x509CSR.IPAddresses) > 0 { + for _, ip := range x509CSR.IPAddresses { + if !lo.ContainsBy(node.Status.Addresses, func(addr corev1.NodeAddress) bool { + return (addr.Type == corev1.NodeInternalIP || addr.Type == corev1.NodeExternalIP) && addr.Address == ip.String() + }) { + return fmt.Errorf("CSR contains IP address %s not in node %s", ip.String(), nodeName) + } + } + } + if !kubeletServingDNSMatchNode(x509CSR.DNSNames, node.Name, node.Status.Addresses) { + return fmt.Errorf("CSR contains DNS %s not specified in the node %s", x509CSR.DNSNames, nodeName) + } + return nil +} + +func kubeletServingDNSMatchNode(dnsNames []string, nodeName string, nodeAddresses []corev1.NodeAddress) bool { + if len(dnsNames) == 0 { + return true + } + for _, dns := range dnsNames { + for _, addr := range nodeAddresses { + if (addr.Type == corev1.NodeInternalDNS || addr.Type == corev1.NodeExternalDNS) && addr.Address == dns { + return true + } + if addr.Type == corev1.NodeHostName && dns == nodeName && dns == addr.Address { + return true + } + } + } + return false } -func (h *ApprovalManager) stopAutoApproveForCastAINodes() { - h.m.Lock() - defer h.m.Unlock() +func (m *ApprovalManager) stopAutoApproveForCastAINodes() { + m.mu.Lock() + defer m.mu.Unlock() - if h.cancelAutoApprove == nil { + if m.cancelAutoApprove == nil { return } - h.log.Info("stopping auto approve CSRs for managed by CAST AI nodes") - h.cancelAutoApprove() - h.cancelAutoApprove = nil -} - -func newApproveCSRExponentialBackoff() wait.Backoff { - b := waitext.DefaultExponentialBackoff() - b.Factor = 2 - return b + m.log.Info("stopping auto approve CSRs for managed by CAST AI nodes") + m.cancelAutoApprove() + m.cancelAutoApprove = nil } func (h *ApprovalManager) addInProgress(certName, signerName string) bool { - h.m.Lock() - defer h.m.Unlock() + h.mu.Lock() + defer h.mu.Unlock() if h.inProgress == nil { h.inProgress = make(map[string]struct{}) } @@ -230,8 +387,8 @@ func (h *ApprovalManager) addInProgress(certName, signerName string) bool { } func (h *ApprovalManager) removeInProgress(certName, signerName string) { - h.m.Lock() - defer h.m.Unlock() + h.mu.Lock() + defer h.mu.Unlock() delete(h.inProgress, createKey(certName, signerName)) } diff --git a/internal/actions/csr/svc_test.go b/internal/actions/csr/svc_test.go index 4f592acc..c380e751 100644 --- a/internal/actions/csr/svc_test.go +++ b/internal/actions/csr/svc_test.go @@ -6,11 +6,9 @@ import ( "testing" "time" - "github.com/samber/lo" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" certv1 "k8s.io/api/certificates/v1" - certv1beta1 "k8s.io/api/certificates/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/fake" @@ -19,6 +17,10 @@ import ( func getCSRv1(name, username string) *certv1.CertificateSigningRequest { return &certv1.CertificateSigningRequest{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certv1.SchemeGroupVersion.String(), + Kind: "CertificateSigningRequest", + }, ObjectMeta: metav1.ObjectMeta{ Name: name, CreationTimestamp: metav1.Now(), @@ -34,31 +36,7 @@ S59zc2bEaJ3y4aSMXLY3gmri14jZvvnFrxaPDT2PAiEA7C3hvZwrCJsoO61JWKqc 1ElMb/fzAVBcP34rfsE7qmQ= -----END CERTIFICATE REQUEST-----`), SignerName: certv1.KubeAPIServerClientKubeletSignerName, - Usages: []certv1.KeyUsage{"kubelet"}, - Username: username, - }, - // Status: certv1.CertificateSigningRequestStatus{},. - } -} - -func getCSRv1betav1(name, username string) *certv1beta1.CertificateSigningRequest { - return &certv1beta1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - CreationTimestamp: metav1.Now(), - }, - Spec: certv1beta1.CertificateSigningRequestSpec{ - Request: []byte(`-----BEGIN CERTIFICATE REQUEST----- -MIIBLTCB0wIBADBPMRUwEwYDVQQKEwxzeXN0ZW06bm9kZXMxNjA0BgNVBAMTLXN5 -c3RlbTpub2RlOmdrZS1kZXYtbWFzdGVyLWNhc3QtcG9vbC1jYjUzMTc3YjBZMBMG -ByqGSM49AgEGCCqGSM49AwEHA0IABMZKNQROiVpxfH4nHaPnE6NaY9Mr8/HBnxCl -mPe4mrvNGRnlJV+LvYCUAVlfinzLcMJSmRjJADgzN0Pn+i+4ra6gIjAgBgkqhkiG -9w0BCQ4xEzARMA8GA1UdEQQIMAaHBAoKADIwCgYIKoZIzj0EAwIDSQAwRgIhAOKQ -S59zc2bEaJ3y4aSMXLY3gmri14jZvvnFrxaPDT2PAiEA7C3hvZwrCJsoO61JWKqc -1ElMb/fzAVBcP34rfsE7qmQ= ------END CERTIFICATE REQUEST-----`), - SignerName: lo.ToPtr(certv1beta1.KubeAPIServerClientKubeletSignerName), - Usages: []certv1beta1.KeyUsage{"kubelet"}, + Usages: []certv1.KeyUsage{certv1.UsageKeyEncipherment, certv1.UsageClientAuth}, Username: username, }, // Status: certv1.CertificateSigningRequestStatus{},. @@ -138,3 +116,14 @@ func TestCSRApprove(t *testing.T) { r.Len(csrResult.Status.Conditions, 0) }) } + +func TestApproveCSRExponentialBackoff(t *testing.T) { + r := require.New(t) + b := newApproveCSRExponentialBackoff() + var sum time.Duration + for i := 0; i < 10; i++ { + tmp := b.Step() + sum += tmp + } + r.Truef(100 < sum.Seconds(), "actual elapsed seconds %v", sum.Seconds()) +} diff --git a/internal/actions/csr/test/test.go b/internal/actions/csr/test/test.go new file mode 100644 index 00000000..28484124 --- /dev/null +++ b/internal/actions/csr/test/test.go @@ -0,0 +1,29 @@ +package test + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + "log" + "testing" +) + +func NewEncodedCertificateRequest(t *testing.T, csr *x509.CertificateRequest) []byte { + t.Helper() + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generate private key: %v", err) + } + + csrDER, err := x509.CreateCertificateRequest(rand.Reader, csr, privateKey) + if err != nil { + log.Fatalf("CreateCertificateRequest: %v", err) + } + + return pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: csrDER, + }) +} diff --git a/internal/actions/csr/wrapper/csr.go b/internal/actions/csr/wrapper/csr.go new file mode 100644 index 00000000..a7aff8a1 --- /dev/null +++ b/internal/actions/csr/wrapper/csr.go @@ -0,0 +1,335 @@ +package wrapper + +import ( + "context" + "crypto/x509" + "encoding/pem" + "fmt" + "strings" + "time" + + certv1 "k8s.io/api/certificates/v1" + certv1beta1 "k8s.io/api/certificates/v1beta1" + v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + certificatesv1 "k8s.io/client-go/kubernetes/typed/certificates/v1" + certificatesv1beta1 "k8s.io/client-go/kubernetes/typed/certificates/v1beta1" +) + +// CSR wraps v1 and v1beta1 for convenient read/write. +// The one and only reason for this type is because there are +// 2 versions of CertificateSigningRequest that need to be supported and always checking version +// is not convenient. +// Note for future: no business logic should be added to this wrapper. +type CSR struct { + v1 *certv1.CertificateSigningRequest + v1beta1 *certv1beta1.CertificateSigningRequest + + certificatesV1 certificatesv1.CertificatesV1Interface + certificatesV1beta1 certificatesv1beta1.CertificatesV1beta1Interface + + parsed *x509.CertificateRequest +} + +// NewCSR validates and creates new certificateRequestFacade. +func NewCSR(clientset kubernetes.Interface, csrObj interface{}) (*CSR, error) { + var ( + v1 *certv1.CertificateSigningRequest + v1b1 *certv1beta1.CertificateSigningRequest + ) + if csrObj == nil { + return nil, fmt.Errorf("either v1 or v1beta1 CertificateSigningRequests expected but got none") + } + switch csr := csrObj.(type) { + case *certv1.CertificateSigningRequest: + v1 = csr + case *certv1beta1.CertificateSigningRequest: + v1b1 = csr + default: + return nil, fmt.Errorf("either v1 or v1beta1 CertificateSigningRequests expected but got %T", csrObj) + } + var result CSR + var err error + if v1 != nil { + err = validateV1(v1) + if err != nil { + return nil, fmt.Errorf("v1 csr invalid: %w", err) + } + result.certificatesV1 = clientset.CertificatesV1() + result.v1 = v1 + result.parsed, err = parseCertificateRequest(v1.Spec.Request) + if err != nil { + return nil, err + } + } + if v1b1 != nil { + err = validateV1Beta1(v1b1) + if err != nil { + return nil, fmt.Errorf("v1beta1 csr invalid: %w", err) + } + result.certificatesV1beta1 = clientset.CertificatesV1beta1() + result.v1beta1 = v1b1 + result.parsed, err = parseCertificateRequest(v1b1.Spec.Request) + if err != nil { + return nil, fmt.Errorf("v1beta1 csr invalid: %w", err) + } + } + return &result, nil +} + +func parseCertificateRequest(raw []byte) (*x509.CertificateRequest, error) { + block, _ := pem.Decode(raw) + if block == nil { + return nil, fmt.Errorf("decode CSR PEM block") + } + csr, err := x509.ParseCertificateRequest(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parse CSR: %w", err) + } + return csr, nil +} + +func validateV1(v1 *certv1.CertificateSigningRequest) error { + if v1.Name == "" { + return fmt.Errorf("v1 CertificateSigningRequest meta.Name is empty") + } + if v1.Spec.Request == nil { + return fmt.Errorf("v1 CertificateSigningRequest spec.Request is nil") + } + if v1.Spec.SignerName == "" { + return fmt.Errorf("v1 CertificateSigningRequest spec.SignerName is empty") + } + if v1.Spec.Username == "" { + return fmt.Errorf("v1 CertificateSigningRequest spec.Username is empty") + } + if len(v1.Spec.Usages) == 0 { + return fmt.Errorf("v1 CertificateSigningRequest spec.Usages is empty") + } + return nil +} + +func validateV1Beta1(v1b1 *certv1beta1.CertificateSigningRequest) error { + if v1b1.Name == "" { + return fmt.Errorf("v1beta1 CertificateSigningRequest meta.Name is empty") + } + if v1b1.Spec.Request == nil { + return fmt.Errorf("v1beta1 CertificateSigningRequest spec.Request is nil") + } + if v1b1.Spec.SignerName == nil { + return fmt.Errorf("v1beta1 CertificateSigningRequest spec.SignerName is nil") + } + if *v1b1.Spec.SignerName == "" { + return fmt.Errorf("v1beta1 CertificateSigningRequest spec.SignerName is empty") + } + if v1b1.Spec.Username == "" { + return fmt.Errorf("v1beta1 CertificateSigningRequest spec.Username is empty") + } + if len(v1b1.Spec.Usages) == 0 { + return fmt.Errorf("v1beta1 CertificateSigningRequest spec.Usages is empty") + } + return nil +} + +// Approved returns whether the CertificateRequest is approved. +func (f *CSR) Approved() bool { + if f.v1 != nil { + for _, condition := range f.v1.Status.Conditions { + if condition.Type == certv1.CertificateApproved { + return condition.Status == v1.ConditionTrue + } + } + } + if f.v1beta1 != nil { + for _, condition := range f.v1beta1.Status.Conditions { + if condition.Type == certv1beta1.CertificateApproved { + return condition.Status == v1.ConditionTrue + } + } + } + return false +} + +// CreatedAt reads and returns the creation timestamp of the CertificateRequest from v1 or v1beta1. +func (f *CSR) CreatedAt() time.Time { + if f.v1 != nil { + return f.v1.CreationTimestamp.Time + } + return f.v1beta1.CreationTimestamp.Time +} + +// Name returns the name of the CertificateRequest. +func (f *CSR) Name() string { + if f.v1 != nil { + return f.v1.Name + } + return f.v1beta1.Name +} + +// RequestingUser reads and returns the user that requested the CertificateRequest from v1 or v1beta1. +func (f *CSR) RequestingUser() string { + if f.v1 != nil { + return f.v1.Spec.Username + } + return f.v1beta1.Spec.Username +} + +// SignerName reads and returns the signer name from v1 or v1beta1. +func (f *CSR) SignerName() string { + if f.v1 != nil { + return f.v1.Spec.SignerName + } + return *f.v1beta1.Spec.SignerName +} + +// Usages reads and returns the usages from v1 or v1beta1. +func (f *CSR) Usages() []string { + var result []string + if f.v1 != nil { + for _, usage := range f.v1.Spec.Usages { + result = append(result, string(usage)) + } + } + if f.v1beta1 != nil { + for _, usage := range f.v1beta1.Spec.Usages { + result = append(result, string(usage)) + } + } + return result +} + +func (f CSR) Groups() []string { + if f.v1 != nil { + return f.v1.Spec.Groups + } + return f.v1beta1.Spec.Groups +} + +// ParsedCertificateRequest returns the CertificateRequest parsed from v1 or v1beta1. +func (f *CSR) ParsedCertificateRequest() *x509.CertificateRequest { + return f.parsed +} + +// Approve add approved condition to the CertificateRequest if it is not already approved. +func (f *CSR) Approve(ctx context.Context, message string) error { + if f.v1 != nil { + return f.approveV1(ctx, message) + } + return f.approveV1Beta1(ctx, message) +} + +func isAlreadyApprovedError(err error) bool { + if err == nil { + return false + } + return strings.Contains(err.Error(), fmt.Sprintf("Duplicate value: \"%s\"", certv1.CertificateApproved)) +} + +func (f *CSR) approveV1(ctx context.Context, message string) error { + csr := f.v1.DeepCopy() + csr.Status.Conditions = append(csr.Status.Conditions, certv1.CertificateSigningRequestCondition{ + LastUpdateTime: metav1.Now(), + Message: message, + Reason: "AutoApproved", + Status: v1.ConditionTrue, + Type: certv1.CertificateApproved, + }) + csr, err := f.certificatesV1.CertificateSigningRequests().UpdateApproval(ctx, csr.Name, csr, metav1.UpdateOptions{}) + if isAlreadyApprovedError(err) { + return nil + } + f.v1 = csr + return err +} + +func (f *CSR) approveV1Beta1(ctx context.Context, message string) error { + csr := f.v1beta1.DeepCopy() + csr.Status.Conditions = append(csr.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{ + LastUpdateTime: metav1.Now(), + Message: message, + Reason: "AutoApproved", + Status: v1.ConditionTrue, + Type: certv1beta1.CertificateApproved, + }) + csr, err := f.certificatesV1beta1.CertificateSigningRequests().UpdateApproval(ctx, csr, metav1.UpdateOptions{}) + if isAlreadyApprovedError(err) { + return nil + } + f.v1beta1 = csr + return err +} + +func (c *CSR) Delete(ctx context.Context) error { + if c.v1 != nil { + return c.certificatesV1.CertificateSigningRequests().Delete(ctx, c.v1.Name, metav1.DeleteOptions{}) + } + return c.certificatesV1beta1.CertificateSigningRequests().Delete(ctx, c.v1beta1.Name, metav1.DeleteOptions{}) +} + +// CreateOrRefresh creates the CertificateSigningRequest if it does not exist. +// If it does exist, it refreshes internally stored CSR object. +func (c *CSR) CreateOrRefresh(ctx context.Context) error { + if c.v1 != nil { + return c.createOrRefreshV1(ctx) + } + return c.createOrRefreshV1beta1(ctx) +} + +func (c *CSR) createOrRefreshV1(ctx context.Context) error { + csr := &certv1.CertificateSigningRequest{ + TypeMeta: metav1.TypeMeta{Kind: "CertificateSigningRequest"}, + ObjectMeta: metav1.ObjectMeta{ + Name: c.v1.Name, + }, + Spec: certv1.CertificateSigningRequestSpec{ + SignerName: c.v1.Spec.SignerName, + Request: c.v1.Spec.Request, + Usages: c.v1.Spec.Usages, + ExpirationSeconds: c.v1.Spec.ExpirationSeconds, + }, + } + csr, err := c.certificatesV1.CertificateSigningRequests().Create(ctx, csr, metav1.CreateOptions{}) + if err != nil { + if k8serrors.IsAlreadyExists(err) { + csr, err = c.certificatesV1.CertificateSigningRequests().Get(ctx, c.v1.Name, metav1.GetOptions{}) + if err != nil { + return err + } + c.v1 = csr + return nil + } + return err + } + c.v1 = csr + return nil +} + +func (c *CSR) createOrRefreshV1beta1(ctx context.Context) error { + csr := &certv1beta1.CertificateSigningRequest{ + TypeMeta: metav1.TypeMeta{Kind: "CertificateSigningRequest"}, + ObjectMeta: metav1.ObjectMeta{ + Name: c.v1beta1.Name, + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + SignerName: c.v1beta1.Spec.SignerName, + Request: c.v1beta1.Spec.Request, + Usages: c.v1beta1.Spec.Usages, + ExpirationSeconds: c.v1beta1.Spec.ExpirationSeconds, + }, + } + csr, err := c.certificatesV1beta1.CertificateSigningRequests().Create(ctx, csr, metav1.CreateOptions{}) + if err != nil { + if k8serrors.IsAlreadyExists(err) { + csr, err := c.certificatesV1beta1.CertificateSigningRequests().Get(ctx, c.v1beta1.Name, metav1.GetOptions{}) + if err != nil { + return err + } + c.v1beta1 = csr + return nil + } + return err + } + c.v1beta1 = csr + return nil +} diff --git a/internal/actions/csr/wrapper/csr_test.go b/internal/actions/csr/wrapper/csr_test.go new file mode 100644 index 00000000..e42b461f --- /dev/null +++ b/internal/actions/csr/wrapper/csr_test.go @@ -0,0 +1,668 @@ +package wrapper_test + +import ( + "context" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "testing" + "time" + + "github.com/samber/lo" + "github.com/stretchr/testify/require" + certv1 "k8s.io/api/certificates/v1" + certv1beta1 "k8s.io/api/certificates/v1beta1" + v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + + csrtest "github.com/castai/cluster-controller/internal/actions/csr/test" + "github.com/castai/cluster-controller/internal/actions/csr/wrapper" +) + +func TestNewCSR(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj interface{} + notOK bool + }{ + { + name: "newCSRFacade() nil arguments", + notOK: true, + }, + { + name: "newCSRFacade() valid V1", + csrObj: modifyValidV1(t, nil), + }, + { + name: "newCSRFacade() valid V1Beta1", + csrObj: modifyValidV1Beta1(t, nil), + }, + { + name: "newCSRFacade() V1 meta.Name=\"\"", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Name = "" + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1 spec.Request=nil", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Request = nil + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1 invalid spec.Request PEM encoding", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Request = []byte("invalid certificate request") + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1 invalid spec.Request x509 encoding", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Request = pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: []byte("invalid certificate request"), + }) + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1 spec.Usages=nil", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Usages = nil + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1 spec.SignerName=\"\"", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.SignerName = "" + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1 spec.Username=\"\"", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Username = "" + return v1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 meta.Name=\"\"", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Name = "" + return v1beta1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 spec.Request=nil", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Request = nil + return v1beta1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 invalid spec.Request", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Request = []byte("invalid certificate request") + return v1beta1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 spec.Usages=nil", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Usages = nil + return v1beta1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 spec.SignerName=nil", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.SignerName = nil + return v1beta1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 spec.SignerName=\"\"", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.SignerName = lo.ToPtr("") + return v1beta1 + }), + notOK: true, + }, + { + name: "newCSRFacade() V1Beta1 spec.Username=\"\"", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Username = "" + return v1beta1 + }), + notOK: true, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + t.Parallel() + _, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + if testcase.notOK { + require.Error(t, err, "expected an error but got none") + } else { + require.NoError(t, err, "unexpected error") + } + }) + } +} + +func TestCSR_Approved(t *testing.T) { + t.Parallel() + clientset := fake.NewClientset() + for _, testcase := range []struct { + name string + obj *wrapper.CSR + result bool + }{ + { + name: "approved() V1 true", + obj: withConditionsV1(t, clientset, []certv1.CertificateSigningRequestCondition{{ + Type: certv1.CertificateApproved, + Status: v1.ConditionTrue, + }}), + result: true, + }, + { + name: "approved() V1 with denied", + obj: withConditionsV1(t, clientset, []certv1.CertificateSigningRequestCondition{{ + Type: certv1.CertificateDenied, + Status: v1.ConditionTrue, + }}), + }, + { + name: "approved() V1 no condition", + obj: withConditionsV1(t, clientset, nil), + }, + { + name: "approved() V1Beta1 true", + obj: withConditionsV1Beta1(t, clientset, []certv1beta1.CertificateSigningRequestCondition{{ + Type: certv1beta1.CertificateApproved, + Status: v1.ConditionTrue, + }}), + result: true, + }, + { + name: "approved() V1Beta1 with denied", + obj: withConditionsV1(t, clientset, []certv1.CertificateSigningRequestCondition{{ + Type: certv1.CertificateDenied, + Status: v1.ConditionTrue, + }}), + }, + { + name: "approved() V1Beta1 false", + obj: withConditionsV1(t, clientset, nil), + }, + } { + t.Run(testcase.name, func(t *testing.T) { + v := testcase.obj.Approved() + require.Equal(t, testcase.result, v, "approved() mismatch") + }) + } +} + +func TestCSR_CreatedAt(t *testing.T) { + t.Parallel() + clientset := fake.NewClientset() + testTime := time.Now().Add(-time.Hour) + for _, testcase := range []struct { + name string + obj *wrapper.CSR + result time.Time + }{ + { + name: "CreatedAt() V1", + obj: v1WithCreationTimestamp(t, clientset, testTime), + result: testTime, + }, + { + name: "CreatedAt() V1Beta1", + obj: v1beta1WithCreationTimestamp(t, clientset, testTime), + result: testTime, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + v := testcase.obj.CreatedAt() + require.Equal(t, testcase.result, v, "CreatedAt() mismatch") + }) + } +} + +func TestCSR_Name(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + result string + }{ + { + name: "name() V1", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Name = "test valid v1 name" + return v1 + }), + result: "test valid v1 name", + }, + { + name: "name() V1Beta1", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Name = "test valid v1beta1 name" + return v1beta1 + }), + result: "test valid v1beta1 name", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + csr, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + require.Equal(t, testcase.result, csr.Name(), "Name() mismatch") + }) + } +} + +func TestCSR_RequestingUser(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + result string + }{ + { + name: "requestingUser() V1", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Username = "test valid v1 username" + return v1 + }), + result: "test valid v1 username", + }, + { + name: "requestingUser() V1Beta1", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Username = "test valid v1beta1 username" + return v1beta1 + }), + result: "test valid v1beta1 username", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + csr, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + require.Equal(t, testcase.result, csr.RequestingUser(), "RequestingUser() mismatch") + }) + } +} + +func TestCSR_SignerName(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + result string + }{ + { + name: "signerName() V1", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.SignerName = "test valid v1 signer name" + return v1 + }), + result: "test valid v1 signer name", + }, + { + name: "signerName() V1Beta1", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.SignerName = lo.ToPtr("test valid v1beta1 signer name") + return v1beta1 + }), + result: "test valid v1beta1 signer name", + }, + } { + t.Run(testcase.name, func(t *testing.T) { + csr, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + require.Equal(t, testcase.result, csr.SignerName(), "SignerName() mismatch") + }) + } +} + +func TestCSR_Usages(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + result []string + }{ + { + name: "usages() V1", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Usages = []certv1.KeyUsage{certv1.UsageClientAuth} + return v1 + }), + result: []string{"client auth"}, + }, + { + name: "usages() V1Beta1", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Usages = []certv1beta1.KeyUsage{certv1beta1.UsageClientAuth} + return v1beta1 + }), + result: []string{"client auth"}, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + csr, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + require.ElementsMatch(t, testcase.result, csr.Usages(), "Usages() mismatch") + }) + } +} + +func TestCSR_Groups(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + result []string + }{ + { + name: "groups() V1", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Groups = []string{"test-group"} + return v1 + }), + result: []string{"test-group"}, + }, + { + name: "groups() V1Beta1", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Groups = []string{"test-group"} + return v1beta1 + }), + result: []string{"test-group"}, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + csr, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + require.ElementsMatch(t, testcase.result, csr.Groups(), "Groups() mismatch") + }) + } +} + +func TestCSR_ParsedCertificateRequest(t *testing.T) { + t.Parallel() + wantEncoded := csrtest.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "test-subject-common-name", + }, + }) + for _, testcase := range []struct { + name string + csrObj runtime.Object + }{ + { + name: "parsedCertificateRequest() V1", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Spec.Request = wantEncoded + return v1 + }), + }, + { + name: "parsedCertificateRequest() V1Beta1", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Spec.Request = wantEncoded + return v1beta1 + }), + }, + } { + t.Run(testcase.name, func(t *testing.T) { + csr, err := wrapper.NewCSR(fake.NewClientset(), testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + got := csr.ParsedCertificateRequest() + require.NotNil(t, got, "ParsedCertificateRequest() is nil") + gotEncoded := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: got.Raw, + }) + require.Equal(t, string(wantEncoded), string(gotEncoded), "ParsedCertificateRequest() mismatch") + }) + } +} + +func TestCSR_Approve(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + }{ + { + name: "Approve() V1 OK", + csrObj: modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Status.Conditions = nil + return v1 + }), + }, + { + name: "Approve() V1Beta1 OK", + csrObj: modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Status.Conditions = nil + return v1beta1 + }), + }, + } { + t.Run(testcase.name, func(t *testing.T) { + clientset := fake.NewClientset(testcase.csrObj) + csr, err := wrapper.NewCSR(clientset, testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + err = csr.Approve(context.Background(), "test message") + require.NoError(t, err, "unexpected error in Approve()") + require.True(t, csr.Approved(), "Approved() should return true") + }) + } +} + +func TestCSR_Delete(t *testing.T) { + t.Parallel() + for _, testcase := range []struct { + name string + csrObj runtime.Object + }{ + { + name: "delete() V1", + csrObj: modifyValidV1(t, nil), + }, + { + name: "delete() V1Beta1", + csrObj: modifyValidV1Beta1(t, nil), + }, + } { + t.Run(testcase.name, func(t *testing.T) { + clientset := fake.NewClientset(testcase.csrObj) + csr, err := wrapper.NewCSR(clientset, testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + err = csr.Delete(context.Background()) + require.NoError(t, err, "failed to delete CSR") + switch testcase.csrObj.GetObjectKind().GroupVersionKind() { + case certv1.SchemeGroupVersion.WithKind("CertificateSigningRequest"): + _, err := clientset.CertificatesV1().CertificateSigningRequests().Get(context.Background(), csr.Name(), metav1.GetOptions{}) + require.True(t, k8serrors.IsNotFound(err), "expected CSR to be deleted") + case certv1beta1.SchemeGroupVersion.WithKind("CertificateSigningRequest"): + _, err := clientset.CertificatesV1beta1().CertificateSigningRequests().Get(context.Background(), csr.Name(), metav1.GetOptions{}) + require.True(t, k8serrors.IsNotFound(err), "expected CSR to be deleted") + } + }) + } +} + +func TestCSR_CreateOrRefresh(t *testing.T) { + t.Parallel() + + for _, testcase := range []struct { + absent bool + name string + csrObj runtime.Object + }{ + { + name: "createOrRefresh() V1 when exists", + csrObj: modifyValidV1(t, nil), + }, + { + absent: true, + name: "createOrRefresh() V1 when absent", + csrObj: modifyValidV1(t, nil), + }, + { + name: "createOrRefresh() V1Beta1 when exists", + csrObj: modifyValidV1Beta1(t, nil), + }, + { + absent: true, + name: "createOrRefresh() V1Beta1 when absent", + csrObj: modifyValidV1Beta1(t, nil), + }, + } { + t.Run(testcase.name, func(t *testing.T) { + clientset := fake.NewClientset() + if !testcase.absent { + switch testcase.csrObj.GetObjectKind().GroupVersionKind() { + case certv1.SchemeGroupVersion.WithKind("CertificateSigningRequest"): + _, err := clientset.CertificatesV1().CertificateSigningRequests().Create(context.Background(), testcase.csrObj.(*certv1.CertificateSigningRequest), metav1.CreateOptions{}) + require.NoError(t, err, "failed to create CSR") + case certv1beta1.SchemeGroupVersion.WithKind("CertificateSigningRequest"): + _, err := clientset.CertificatesV1beta1().CertificateSigningRequests().Create(context.Background(), testcase.csrObj.(*certv1beta1.CertificateSigningRequest), metav1.CreateOptions{}) + require.NoError(t, err, "failed to create CSR") + } + } + csr, err := wrapper.NewCSR(clientset, testcase.csrObj) + require.NoError(t, err, "failed to create CSR") + err = csr.CreateOrRefresh(context.Background()) + require.NoError(t, err, "failed to createOrRefresh CSR") + switch testcase.csrObj.GetObjectKind().GroupVersionKind() { + case certv1.SchemeGroupVersion.WithKind("CertificateSigningRequest"): + _, err := clientset.CertificatesV1().CertificateSigningRequests().Get(context.Background(), csr.Name(), metav1.GetOptions{}) + require.NoError(t, err, "failed to get CSR") + case certv1beta1.SchemeGroupVersion.WithKind("CertificateSigningRequest"): + _, err := clientset.CertificatesV1beta1().CertificateSigningRequests().Get(context.Background(), csr.Name(), metav1.GetOptions{}) + require.NoError(t, err, "failed to get CSR") + } + }) + } +} + +func modifyValidV1(t *testing.T, modify func(*certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + t.Helper() + + result := &certv1.CertificateSigningRequest{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certv1.SchemeGroupVersion.String(), + Kind: "CertificateSigningRequest", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-csr", + CreationTimestamp: metav1.Now(), + }, + Spec: certv1.CertificateSigningRequestSpec{ + Request: csrtest.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "test-common-name", + }, + }), + SignerName: certv1.KubeAPIServerClientKubeletSignerName, + Username: "kubelet-bootstrap", + Usages: []certv1.KeyUsage{certv1.UsageClientAuth}, + }, + } + if modify != nil { + result = modify(result) + } + return result +} + +func modifyValidV1Beta1(t *testing.T, modify func(*certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + t.Helper() + result := &certv1beta1.CertificateSigningRequest{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certv1beta1.SchemeGroupVersion.String(), + Kind: "CertificateSigningRequest", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-csr", + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + Request: csrtest.NewEncodedCertificateRequest(t, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "test-common-name", + }, + }), + SignerName: lo.ToPtr(certv1.KubeAPIServerClientKubeletSignerName), + Username: "kubelet-bootstrap", + Usages: []certv1beta1.KeyUsage{certv1beta1.UsageClientAuth}, + }, + } + if modify != nil { + result = modify(result) + } + return result +} + +func withConditionsV1(t *testing.T, clientset kubernetes.Interface, conditions []certv1.CertificateSigningRequestCondition) *wrapper.CSR { + t.Helper() + result, err := wrapper.NewCSR(clientset, modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.Status.Conditions = conditions + return v1 + })) + require.NoError(t, err, "failed to create CSR") + return result +} + +func withConditionsV1Beta1(t *testing.T, clientset kubernetes.Interface, conditions []certv1beta1.CertificateSigningRequestCondition) *wrapper.CSR { + t.Helper() + result, err := wrapper.NewCSR(clientset, modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.Status.Conditions = conditions + return v1beta1 + })) + require.NoError(t, err, "failed to create CSR") + return result +} + +func v1WithCreationTimestamp(t *testing.T, clientset kubernetes.Interface, creationTime time.Time) *wrapper.CSR { + t.Helper() + result, err := wrapper.NewCSR(clientset, modifyValidV1(t, func(v1 *certv1.CertificateSigningRequest) *certv1.CertificateSigningRequest { + v1.ObjectMeta.CreationTimestamp = metav1.NewTime(creationTime) + return v1 + })) + require.NoError(t, err, "failed to create CSR") + return result +} + +func v1beta1WithCreationTimestamp(t *testing.T, clientset kubernetes.Interface, creationTime time.Time) *wrapper.CSR { + t.Helper() + result, err := wrapper.NewCSR(clientset, modifyValidV1Beta1(t, func(v1beta1 *certv1beta1.CertificateSigningRequest) *certv1beta1.CertificateSigningRequest { + v1beta1.ObjectMeta.CreationTimestamp = metav1.NewTime(creationTime) + return v1beta1 + })) + require.NoError(t, err, "failed to create CSR") + return result +}