Refactor for integration tests (#194)

There are a subset of tests that require real HTTP servers, sleeps, or real external entities. These tests are being designated integration tests and will run as part of their own test suite via `make test-integration`.

This PR changes the `/internal/http` tests into integration tests and introduces Kubernetes integration tests. Additionally, it simplifies the Kubernetes backend construction by having `NewBackend` accept the `Config` object directly instead of requiring an intermediate step. 

Some additional renaming for consistency has been applied through configuration objects.

Closes #170 
Closes #183

.github/workflows/ci.yaml

@@ -32,6 +32,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
+
       - uses: actions/setup-go@v3
         with:
           go-version: "${{ env.GO_VERSION }}"
@@ -43,6 +44,23 @@ jobs:
       - name: Upload coverage report (codcov.io)
         run: bash <(curl -s https://codecov.io/bash)
 
+  integration:
+    name: Test - Integration
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-go@v3
+        with:
+          go-version: "${{ env.GO_VERSION }}"
+          cache: true
+
+      - name: Run integration tests
+        run: make test-integration
+
+      - name: Upload coverage report (codcov.io)
+        run: bash <(curl -s https://codecov.io/bash)
+
   build:
     name: Build
     strategy:
@@ -69,7 +87,7 @@ jobs:
   e2e:
     name: Test - E2E
     runs-on: ubuntu-latest
-    needs: [test]
+    needs: [test, integration]
     steps:
       - uses: actions/checkout@v3
 

.gitignore

@@ -1,7 +1,6 @@
-certs/
-/cmd/hegel/hegel
 .env
 out/
 /hegel-*
 .vscode
 /coverage.out
+/bin

Makefile

@@ -1,3 +1,6 @@
+# Configure the Make shell for recipe invocations.
+SHELL := bash
+
 # Specify the target architecture to build the binary for. (Recipes: build, image)
 GOARCH ?= $(shell go env GOARCH)
 
@@ -35,6 +38,30 @@ test: ## Run unit tests.
 test-e2e: ## Run E2E tests.
 	go test $(GO_TEST_ARGS) -tags=e2e -coverprofile=coverage.out ./internal/e2e
 
+# Version should match with whatever we consume in sources (check the go.mod).
+SETUP_ENVTEST 			:= go run sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+ENVTEST_BIN_DIR 		:= $(shell pwd)/bin
+
+# The kubernetes version to use with envtest. Overridable when invoking make.
+# E.g. make ENVTEST_KUBE_VERSION=1.24 test-integration
+ENVTEST_KUBE_VERSION 	?= 1.25
+
+.PHONY: setup-envtest
+setup-envtest:
+	@echo Installing Kubernetes $(ENVTEST_KUBE_VERSION) binaries into $(ENVTEST_BIN_DIR); \
+	$(SETUP_ENVTEST) use --bin-dir $(ENVTEST_BIN_DIR) $(ENVTEST_KUBE_VERSION)
+
+# Integration tests are located next to unit test. This recipe will search the code base for
+# files including the "//go:build integration" build tag and build them into the test binary.
+# For packages containing both unit and integration tests its recommended to populate 
+# "//go:build !integration" in all unit test sources so as to avoid compiling them in this recipe.
+.PHONY: test-integration
+test-integration: setup-envtest
+test-integration: TEST_DIRS := $(shell grep -R --include="*.go" -l -E "//go:build.*\sintegration" . | xargs dirname | uniq)
+test-integration: ## Run integration tests.
+	source <($(SETUP_ENVTEST) use -p env --bin-dir $(ENVTEST_BIN_DIR) $(ENVTEST_KUBE_VERSION)); \
+	go test $(GO_TEST_ARGS) -tags=integration -coverprofile=coverage.out $(TEST_DIRS)
+
 # When we build the image its Linux based. This means we need a Linux binary hence we need to export
 # GOOS so we have compatible binary.
 .PHONY: image

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/tinkerbell/tink v0.8.0
 	google.golang.org/grpc v1.50.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0
-	k8s.io/apimachinery v0.25.4 // indirect
+	k8s.io/apimachinery v0.25.4
 	k8s.io/client-go v0.25.4
 	sigs.k8s.io/controller-runtime v0.13.1
 )

internal/backend/backend.go

@@ -37,16 +37,12 @@ func New(ctx context.Context, opts Options) (Client, error) {
 		return flatfile.FromYAMLFile(opts.Flatfile.Path)
 
 	case opts.Kubernetes != nil:
-		config, err := kubernetes.NewConfig(
-			opts.Kubernetes.Kubeconfig,
-			opts.Kubernetes.KubeAPI,
-			opts.Kubernetes.KubeNamespace,
-		)
-		if err != nil {
-			return nil, fmt.Errorf("loading kubernetes config: %v", err)
-		}
-
-		kubeclient, err := kubernetes.NewBackend(config)
+		kubeclient, err := kubernetes.NewBackend(kubernetes.BackendConfig{
+			Kubeconfig:       opts.Kubernetes.Kubeconfig,
+			APIServerAddress: opts.Kubernetes.APIServerAddress,
+			Namespace:        opts.Kubernetes.Namespace,
+			Context:          ctx,
+		})
 		if err != nil {
 			return nil, fmt.Errorf("kubernetes client: %v", err)
 		}
@@ -94,15 +90,15 @@ type FlatfileOptions struct {
 
 // KubernetesOptions is the configuration for a Kubernetes backend.
 type KubernetesOptions struct {
-	// KubeAPI is the URL of the Kube API the Kubernetes client talks to.
+	// APIServerAddress is the URL of the Kube API the Kubernetes client talks to.
 	// Optional
-	KubeAPI string
+	APIServerAddress string
 
 	// Kuberconfig is a path to a Kubeconfig file used by the Kubernetes client.
 	// Optional
 	Kubeconfig string
 
 	// KubeNamespace is a namespace override to have Hegel use for reading resources.
 	// Optional
-	KubeNamespace string
+	Namespace string
 }

internal/backend/kubernetes/backend.go

@@ -8,6 +8,9 @@ import (
 	"github.com/tinkerbell/hegel/internal/frontend/ec2"
 	tinkv1 "github.com/tinkerbell/tink/pkg/apis/core/v1alpha1"
 	tinkcontrollers "github.com/tinkerbell/tink/pkg/controllers"
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/clientcmd"
+	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -26,36 +29,70 @@ type Backend struct {
 // NewBackend creates a new Backend instance. It launches a goroutine to perform synchronization
 // between the cluster and internal caches. Consumers can wait for the initial sync using WaitForCachesync().
 // See k8s.io/Backend-go/tools/Backendcmd for constructing *rest.Config objects.
-func NewBackend(cfg Config) (*Backend, error) {
+func NewBackend(cfg BackendConfig) (*Backend, error) {
 	opts := tinkcontrollers.GetServerOptions()
 	opts.Namespace = cfg.Namespace
 
-	// Use a manager from the tink project so we can take advantage of the indexes and caching it configures.
-	// Once started, we don't really need any of the manager capabilities hence we don't store it in the
-	// Backend
-	manager, err := tinkcontrollers.NewManager(cfg.Config, opts)
-	if err != nil {
-		return nil, err
-	}
-
 	// Default the context.
 	if cfg.Context == nil {
 		cfg.Context = context.Background()
 	}
 
+	clientConfig := cfg.ClientConfig
+
+	// If no client was specified, build one and configure the backend with it including waiting
+	// for the caches to sync.
+	if cfg.ClientConfig == nil {
+		restConfig, err := loadConfig(cfg)
+		if err != nil {
+			return nil, err
+		}
+		clientConfig = restConfig
+	}
+
+	// Use a manager from the tink project so we can take advantage of the indexes and caching it
+	// configures. Once started, we don't really need any of the manager capabilities hence we don't
+	// store it in the Backend.
+	manager, err := tinkcontrollers.NewManager(clientConfig, opts)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO(chrisdoherty4) Stop panicing on error. This will likely require exposing Start in
+	// some capacity and allowing the caller to handle the error.
 	go func() {
 		if err := manager.Start(cfg.Context); err != nil {
 			panic(err)
 		}
 	}()
 
-	backend := &Backend{
-		client:           manager.GetClient(),
+	return &Backend{
 		closer:           cfg.Context.Done(),
+		client:           manager.GetClient(),
 		WaitForCacheSync: manager.GetCache().WaitForCacheSync,
+	}, nil
+}
+
+func loadConfig(cfg BackendConfig) (*rest.Config, error) {
+	loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
+	loadingRules.ExplicitPath = cfg.Kubeconfig
+
+	overrides := &clientcmd.ConfigOverrides{
+		ClusterInfo: clientcmdapi.Cluster{
+			Server: cfg.APIServerAddress,
+		},
+		Context: clientcmdapi.Context{
+			Namespace: cfg.Namespace,
+		},
+	}
+
+	loader := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, overrides)
+	config, err := loader.ClientConfig()
+	if err != nil {
+		return nil, err
 	}
 
-	return backend, nil
+	return config, nil
 }
 
 // IsHealthy returns true until the context used to create the Backend is cancelled.

internal/backend/kubernetes/backend_integration_test.go

@@ -0,0 +1,101 @@
+//go:build integration
+
+package kubernetes_test
+
+import (
+	"context"
+	"testing"
+
+	. "github.com/tinkerbell/hegel/internal/backend/kubernetes"
+	tinkv1 "github.com/tinkerbell/tink/pkg/apis/core/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
+)
+
+// TestBackend performs a simple sanity check on the backend initializing constructor to ensure
+// it can in-fact talk to a real API server. More rigerous testing of business logic is performed
+// in unit tests.
+func TestBackend(t *testing.T) {
+	// Configure a test environment and launch it.
+	scheme := runtime.NewScheme()
+	if err := tinkv1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+
+	env := envtest.Environment{
+		Scheme: scheme,
+		CRDDirectoryPaths: []string{
+			// CRDs are not automatically updated and will require manual updates whenever
+			// we bump our Tink repository dependency version.
+			"testdata/integration",
+		},
+	}
+
+	cfg, err := env.Start()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		if err := env.Stop(); err != nil {
+			t.Logf("Stopping test env: %v", err)
+		}
+	}()
+
+	// Build a client and add a Hardware resource.
+	client, err := client.New(cfg, client.Options{Scheme: scheme})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	const ip = "10.10.10.10"
+	const hostname = "foobar"
+
+	hw := tinkv1.Hardware{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "default",
+		},
+		Spec: tinkv1.HardwareSpec{
+			Interfaces: []tinkv1.Interface{
+				{
+					DHCP: &tinkv1.DHCP{
+						IP: &tinkv1.IP{
+							Address: ip,
+							Family:  4,
+						},
+					},
+				},
+			},
+			Metadata: &tinkv1.HardwareMetadata{
+				Instance: &tinkv1.MetadataInstance{
+					Hostname: hostname,
+				},
+			},
+		},
+	}
+
+	if err := client.Create(context.Background(), &hw); err != nil {
+		t.Fatal(err)
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// Construct the backend and attempt to retrieve our test Hardware resource.
+	backend, err := NewBackend(BackendConfig{Context: ctx, ClientConfig: cfg})
+	if err != nil {
+		t.Fatal(err)
+	}
+	backend.WaitForCacheSync(ctx)
+
+	instance, err := backend.GetEC2Instance(ctx, ip)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if instance.Metadata.Hostname != hostname {
+		t.Fatalf("Expected Hostname: %s; Received Hostname: %s\n", instance.Metadata.Hostname, hostname)
+	}
+}

internal/backend/kubernetes/backend_test.go

@@ -1,3 +1,5 @@
+//go:build !integration
+
 package kubernetes_test
 
 import (