refactor: more robust E2E tests

Added a new test case for the COSIGN_REPOSITORY variable and made the e2e tests more robust by properly handling timeouts, which weren't working half the time. Signed-off-by: Bruno Bressi <[email protected]>
eumel8 · Jan 28, 2024 · a72b0f1 · a72b0f1
1 parent 9f57e0e
commit a72b0f1
Show file tree

Hide file tree

Showing 11 changed files with 355 additions and 145 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -0,0 +1,105 @@
+linters-settings:
+  dupl:
+    threshold: 100
+  funlen:
+    lines: -1 # the number of lines (code + empty lines) is not a right metric and leads to code without empty line or one-liner.
+    statements: 50
+  goconst:
+    min-len: 2
+    min-occurrences: 3
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      - dupImport # https://github.com/go-critic/go-critic/issues/845
+      - ifElseChain
+      - octalLiteral
+      - whyNoLint
+  gocyclo:
+    min-complexity: 15
+  gofmt:
+    rewrite-rules:
+      - pattern: 'interface{}'
+        replacement: 'any'
+  goimports:
+    local-prefixes: github.com/golangci/golangci-lint
+  gomnd:
+    # don't include the "operation" and "assign"
+    checks:
+      - argument
+      - case
+      - condition
+      - return
+    ignored-numbers:
+      - '0'
+      - '1'
+      - '2'
+      - '3'
+    ignored-functions:
+      - strings.SplitN
+
+  govet:
+    check-shadowing: true
+    settings:
+      printf:
+        funcs:
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+  lll:
+    line-length: 140
+  misspell:
+    locale: US
+  nolintlint:
+    allow-unused: false # report any unused nolint directives
+    require-explanation: false # don't require an explanation for nolint directives
+    require-specific: false # don't require nolint directives to be specific about which linter is being skipped
+  revive:
+    rules:
+      - name: unexported-return
+        disabled: true
+      - name: unused-parameter
+
+linters:
+  disable-all: true
+  enable:
+    - bodyclose
+    - dogsled
+    - dupl
+    - errcheck
+    - exportloopref
+    - funlen
+    - gocheckcompilerdirectives
+    - gochecknoinits
+    - goconst
+    - gocritic
+    - gocyclo
+    - gofmt
+    - goimports
+    - gomnd
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - noctx
+    - nolintlint
+    - revive
+    - staticcheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - whitespace
+
+run:
+  timeout: 5m
+  skip-files:
+    - .*_test\.go
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,17 @@
+repos:
+  - repo: local
+    hooks:
+      - id: go-test
+        name: go-unit-tests
+        entry: make test-unit
+        language: system
+        types: [go]
+  - repo: https://github.com/tekwizely/pre-commit-golang
+    rev: v1.0.0-rc.1
+    hooks:
+      - id: go-mod-tidy-repo
+      - id: go-vet-repo-mod
+      - id: go-fumpt-repo
+        args: [ -l, -w ]
+      - id: golangci-lint-repo-mod
+        args: [ --config, .golangci.yaml, --, --fix ]
diff --git a/Makefile b/Makefile
@@ -37,7 +37,7 @@ e2e-keys:
 
 e2e-images:
 	@echo "Checking for cosign.key..."
-	@test -f cosign.key || (echo "cosign.key not found. Run 'make generate-key' to generate one." && exit 1)
+	@test -f cosign.key || (echo "cosign.key not found. Run 'make e2e-keys' to generate the pairs needed for the tests." && exit 1)
 	@echo "Building test image..."
 	@docker build -t k3d-registry.localhost:5000/cosignwebhook:dev .
 	@echo "Pushing test image..."

diff --git a/README.md b/README.md
@@ -136,6 +136,7 @@ targets. To run the tests the following is required:
 
 To run the E2E tests, the following steps are required (in order):
 
+- create a k3d local cluster for the tests and a local iamge registry (`make e2e-cluster`)
 - signing keys are generated (`make e2e-keys`)
 - a new `cosignwebhook` image is build and signed with a temp key (`make e2e-images`)
 - the image is pushed to a local registry & deployed to the test cluster (`make e2e-deploy`)

diff --git a/main.go b/main.go
@@ -5,13 +5,13 @@ import (
 	"crypto/tls"
 	"flag"
 	"fmt"
+	"github.com/gookit/slog"
 	"net/http"
 	"os"
 	"os/signal"
 	"syscall"
 
 	"github.com/eumel8/cosignwebhook/webhook"
-	log "github.com/gookit/slog"
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
@@ -34,26 +34,26 @@ func main() {
 	// set log level
 	switch *logLevel {
 	case "fatal":
-		log.SetLogLevel(log.FatalLevel)
+		slog.SetLogLevel(slog.FatalLevel)
 	case "trace":
-		log.SetLogLevel(log.TraceLevel)
+		slog.SetLogLevel(slog.TraceLevel)
 	case "debug":
-		log.SetLogLevel(log.DebugLevel)
+		slog.SetLogLevel(slog.DebugLevel)
 	case "error":
-		log.SetLogLevel(log.ErrorLevel)
+		slog.SetLogLevel(slog.ErrorLevel)
 	case "warn":
-		log.SetLogLevel(log.WarnLevel)
+		slog.SetLogLevel(slog.WarnLevel)
 	case "info":
-		log.SetLogLevel(log.InfoLevel)
+		slog.SetLogLevel(slog.InfoLevel)
 	default:
-		log.SetLogLevel(log.InfoLevel)
+		slog.SetLogLevel(slog.InfoLevel)
 	}
 
-	log.GetFormatter().(*log.TextFormatter).SetTemplate(logTemplate)
+	slog.GetFormatter().(*slog.TextFormatter).SetTemplate(logTemplate)
 
 	certs, err := tls.LoadX509KeyPair(tlscert, tlskey)
 	if err != nil {
-		log.Errorf("Failed to load key pair: ", err)
+		slog.Error("failed to load key pair", "error", err)
 	}
 
 	server := &http.Server{
@@ -79,23 +79,23 @@ func main() {
 	// start webhook server in new rountine
 	go func() {
 		if err := server.ListenAndServeTLS("", ""); err != nil {
-			log.Errorf("Failed to listen and serve webhook server %v", err)
+			slog.Error("Failed to listen and serve webhook server", "error", err)
 		}
 	}()
 	go func() {
 		if err := mserver.ListenAndServe(); err != nil {
-			log.Errorf("Failed to listen and serve monitor server %v", err)
+			slog.Error("Failed to listen and serve monitor server %v", "error", err)
 		}
 	}()
 
-	log.Infof("Server running listening in port: %s,%s", port, mport)
+	slog.Info("Webhook server running", "port", port, "metricsPort", mport)
 
 	// listening shutdown signal
 	signalChan := make(chan os.Signal, 1)
 	signal.Notify(signalChan, syscall.SIGINT, syscall.SIGTERM)
 	<-signalChan
 
-	log.Info("Got shutdown signal, shutting down webhook server gracefully...")
-	server.Shutdown(context.Background())
-	mserver.Shutdown(context.Background())
+	slog.Info("Got shutdown signal, shutting down webhook server gracefully...")
+	_ = server.Shutdown(context.Background())
+	_ = mserver.Shutdown(context.Background())
 }
diff --git a/test/framework/client.go b/test/framework/client.go
@@ -3,14 +3,15 @@ package framework
 import (
 	"context"
 	"fmt"
+	"os"
+	"testing"
+	"time"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
-	"os"
-	"testing"
-	"time"
 )
 
 // Framework is a helper struct for testing
@@ -61,13 +62,11 @@ func (f *Framework) Cleanup(t testing.TB) {
 // cleanupDeployments removes all deployments from the testing namespace
 // if they exist
 func (f *Framework) cleanupDeployments(t testing.TB) {
-
 	t.Logf("cleaning up deployments")
 	deployments, err := f.k8s.AppsV1().Deployments("test-cases").List(context.Background(), metav1.ListOptions{})
 	if err != nil {
 		f.Cleanup(t)
 		t.Fatal(err)
-
 	}
 	for _, d := range deployments.Items {
 		err = f.k8s.AppsV1().Deployments("test-cases").Delete(context.Background(), d.Name, metav1.DeleteOptions{})
@@ -100,7 +99,6 @@ func (f *Framework) cleanupDeployments(t testing.TB) {
 
 // cleanupSecrets removes all secrets from the testing namespace
 func (f *Framework) cleanupSecrets(t testing.TB) {
-
 	t.Logf("cleaning up secrets")
 	secrets, err := f.k8s.CoreV1().Secrets("test-cases").List(context.Background(), metav1.ListOptions{})
 	if err != nil {
@@ -130,25 +128,24 @@ func (f *Framework) CreateDeployment(t testing.TB, d appsv1.Deployment) {
 
 // WaitForDeployment waits until the deployment is ready
 func (f *Framework) WaitForDeployment(t *testing.T, d appsv1.Deployment) {
-
 	t.Logf("waiting for deployment %s to be ready", d.Name)
 	// wait until the deployment is ready
-	w, err := f.k8s.AppsV1().Deployments(d.Namespace).Watch(context.Background(), metav1.ListOptions{
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	w, err := f.k8s.AppsV1().Deployments(d.Namespace).Watch(ctx, metav1.ListOptions{
 		FieldSelector: fmt.Sprintf("metadata.name=%s", d.Name),
 	})
-
 	if err != nil {
 		f.Cleanup(t)
 		t.Fatal(err)
 	}
 
-	timeout := time.After(30 * time.Second)
-	for event := range w.ResultChan() {
+	for {
 		select {
-		case <-timeout:
+		case <-ctx.Done():
 			f.Cleanup(t)
 			t.Fatal("timeout reached while waiting for deployment to be ready")
-		default:
+		case event := <-w.ResultChan():
 			deployment, ok := event.Object.(*appsv1.Deployment)
 			if !ok {
 				time.Sleep(5 * time.Second)
@@ -162,9 +159,6 @@ func (f *Framework) WaitForDeployment(t *testing.T, d appsv1.Deployment) {
 			time.Sleep(5 * time.Second)
 		}
 	}
-
-	f.Cleanup(t)
-	t.Fatal("failed to wait for deployment to be ready")
 }
 
 // CreateSecret creates a secret in the testing namespace
@@ -180,7 +174,6 @@ func (f *Framework) CreateSecret(t *testing.T, secret corev1.Secret) {
 
 // AssertDeploymentFailed asserts that the deployment cannot start
 func (f *Framework) AssertDeploymentFailed(t *testing.T, d appsv1.Deployment) {
-
 	t.Logf("waiting for deployment %s to fail", d.Name)
 
 	// watch for replicasets of the deployment
@@ -222,7 +215,6 @@ func (f *Framework) AssertDeploymentFailed(t *testing.T, d appsv1.Deployment) {
 
 // AssertEventForPod asserts that a PodVerified event is created
 func (f *Framework) AssertEventForPod(t *testing.T, reason string, p corev1.Pod) {
-
 	t.Logf("waiting for %s event to be created for pod %s", reason, p.Name)
 
 	// watch for events of deployment's namespace and check if the podverified event is created

diff --git a/test/framework/cosign.go b/test/framework/cosign.go
@@ -2,15 +2,15 @@ package framework
 
 import (
 	"fmt"
-	"github.com/sigstore/cosign/v2/cmd/cosign/cli"
 	"os"
 	"regexp"
 	"testing"
+
+	"github.com/sigstore/cosign/v2/cmd/cosign/cli"
 )
 
 // cleanupKeys removes all keypair files from the testing directory
 func cleanupKeys(t testing.TB) {
-
 	t.Logf("cleaning up keypair files")
 	files, err := os.ReadDir(".")
 	if err != nil {
@@ -87,5 +87,6 @@ func (f *Framework) SignContainer(t *testing.T, priv, img string) {
 	err := cmd.Execute()
 	if err != nil {
 		f.Cleanup(t)
+		t.Fatal(err)
 	}
 }
diff --git a/test/main_test.go b/test/main_test.go
@@ -6,7 +6,6 @@ import (
 
 // TestPassingDeployments tests deployments that should pass signature verification
 func TestPassingDeployments(t *testing.T) {
-
 	testFuncs := map[string]func(t *testing.T){
 		"OneContainerSinglePubKeyEnvRef":            testOneContainerSinglePubKeyEnvRef,
 		"TwoContainersSinglePubKeyEnvRef":           testTwoContainersSinglePubKeyEnvRef,
@@ -16,6 +15,7 @@ func TestPassingDeployments(t *testing.T) {
 		"TwoContainersSingleWithInitPubKeyMixedRef": testTwoContainersWithInitSinglePubKeyMixedRef,
 		"EventEmittedOnSignatureVerification":       testEventEmittedOnSignatureVerification,
 		"EventEmittedOnNoSignatureVerification":     testEventEmittedOnNoSignatureVerification,
+		"OneContainerWithCosingRepoVariable":        testOneContainerWithCosingRepoVariable,
 	}
 
 	for name, tf := range testFuncs {
@@ -25,7 +25,6 @@ func TestPassingDeployments(t *testing.T) {
 
 // TestFailingDeployments tests deployments that should fail signature verification
 func TestFailingDeployments(t *testing.T) {
-
 	testFuncs := map[string]func(t *testing.T){
 		"OneContainerSinglePubKeyMalformedEnvRef":  testOneContainerSinglePubKeyMalformedEnvRef,
 		"TwoContainersSinglePubKeyMalformedEnvRef": testTwoContainersSinglePubKeyMalformedEnvRef,