add validation to csr approver with refactoring and near complete integration tests (#182)

* refactored csr v1 & v1beta1 wrapper

* integration tests for csr auto approval

* more tests & refactoring

* test kubelet-serving IPs and DNS match the node

* validate "cast-pool" in node name

* test v1beta1

* adjust old tests

* shorten wait for approval in test

* use require for assertions

* run more go tests in parallel

* run wrapper tests in parallel

* add more t.Parallel

* add t.Parallel

* split integration test

* adjust workflows to run tests in parallel

* slow TestIntegration

* run fewer t.Parallel due to gh actions failure to launch

* Revert "add more t.Parallel"

This reverts commit 18d191c.

* Revert "add t.Parallel"

This reverts commit 1f233c8.

* Revert "slow TestIntegration"

This reverts commit f6bfb80.

* increase 10m to 15m

* sleep less

* GroupVersionKind() returns empty

* tiny optimisation

* dont lock watchStarted write

* allow hostname in dns iff matches node name

* reduce cyclomatic complexity of a func

---------

Co-authored-by: Furkhat Kasymov Genii Uulu <furkhat@cast.ai>

Author: furkhat

.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
 

.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

.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

Makefile

@@ -41,12 +41,12 @@ fix: $(GOLANGCI_LINT)
 .PHONY: fix
 
 test:
-	go test ./... -race
+	go test ./... -race -parallel=20
 .PHONY: test
 
 generate-e2e-client:
 	go generate ./e2e/client
 .PHONY: generate-e2e-client
 
 deploy-loadtest: release
-	IMAGE_REPOSITORY=$(DOCKER_REPOSITORY) IMAGE_TAG=$(VERSION) ./hack/loadtest/deploy.sh
+	IMAGE_REPOSITORY=$(DOCKER_REPOSITORY) IMAGE_TAG=$(VERSION) ./hack/loadtest/deploy.sh

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,30 +119,39 @@ 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) {
 		r := require.New(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{},.