Stabilize classic kubernetes e2e path

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

Makefile

@@ -250,7 +250,7 @@ run: generate ## Run app locally (without a k8s cluster)
 
 manifests: helm.generate ## Generate manifests from helm chart
 	mkdir -p $(OUTPUT_DIR)/deploy/manifests
-	helm dependency build $(HELM_DIR)
+	./hack/helm.dependency.ensure.sh $(HELM_DIR)
 	helm template external-secrets $(HELM_DIR) -f deploy/manifests/helm-values.yaml > $(OUTPUT_DIR)/deploy/manifests/external-secrets.yaml
 
 crds.install: generate ## Install CRDs into a cluster. This is for convenience

e2e/Makefile

@@ -53,17 +53,11 @@ stop-kind: ## Stop kind cluster
 		--name "$(KIND_CLUSTER_NAME)" \
 
 test: start-kind e2e-image ## Run e2e tests against current kube context
-	$(MAKE) -C ../ docker.build \
-		IMAGE_NAME=$(IMAGE_NAME) \
-		VERSION=$(VERSION) \
-		ARCH=amd64 \
-		DOCKER_BUILD_ARGS="${DOCKER_BUILD_ARGS} --build-arg TARGETARCH=amd64 --build-arg TARGETOS=linux"
-	$(MAKE) -C ../ docker.build \
+	$(MAKE) -C ../ docker.build.controller.e2e \
 		IMAGE_NAME=$(OCI_IMAGE_NAME) \
 		VERSION=$(VERSION) \
 		ARCH=amd64 \
 		DOCKER_BUILD_ARGS="${DOCKER_BUILD_ARGS} --build-arg TARGETARCH=amd64 --build-arg TARGETOS=linux"
-	kind load docker-image --name="$(KIND_CLUSTER_NAME)" $(IMAGE_NAME):$(VERSION)
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" $(OCI_IMAGE_NAME):$(VERSION)
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" $(E2E_IMAGE_NAME):$(VERSION)
 	KUBECTL_CONTEXT="$(KIND_CONTEXT)" E2E_SKIP_HELM_DEPENDENCY_UPDATE="true" ./run.sh
@@ -114,7 +108,7 @@ e2e-image: e2e-bin
 	-rm -rf ./k8s/deploy
 	mkdir -p k8s
 	$(MAKE) -C ../ helm.generate
-	helm dependency build ../deploy/charts/external-secrets
+	../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets
 	cp -r ../deploy ./k8s
 	docker build $(DOCKER_BUILD_ARGS) -t $(E2E_IMAGE_NAME):$(VERSION) -f Dockerfile ..
 

e2e/makefile_test.go

@@ -24,39 +24,64 @@ import (
 	"testing"
 )
 
-func TestV2MakeTargetCanSkipKubernetesProviderBuild(t *testing.T) {
+func TestClassicMakeTargetBuildsOnlyControllerImageOnce(t *testing.T) {
 	t.Parallel()
 
-	defaultDryRun := runMakeDryRun(t, "VERSION=test-version")
-	if !strings.Contains(defaultDryRun, "helm dependency build ../deploy/charts/external-secrets") {
-		t.Fatalf("expected default test.v2 dry-run to vendor helm chart dependencies before building the e2e image, output:\n%s", defaultDryRun)
+	dryRun := runMakeDryRun(t, "test", "VERSION=test-version", `TEST_SUITES=provider`, `GINKGO_LABELS=kubernetes && !v2`)
+
+	if !strings.Contains(dryRun, "docker.build.controller.e2e") {
+		t.Fatalf("expected classic test dry-run to build the controller image via docker.build.controller.e2e, output:\n%s", dryRun)
+	}
+	if strings.Contains(dryRun, "docker.build.provider.kubernetes") {
+		t.Fatalf("expected classic test dry-run to omit kubernetes provider image builds, output:\n%s", dryRun)
+	}
+	if strings.Contains(dryRun, "docker.build.provider.aws") {
+		t.Fatalf("expected classic test dry-run to omit aws provider image builds, output:\n%s", dryRun)
+	}
+	if strings.Contains(dryRun, "docker.build.provider.fake") {
+		t.Fatalf("expected classic test dry-run to omit fake provider image builds, output:\n%s", dryRun)
+	}
+	if count := strings.Count(dryRun, `kind load docker-image --name="external-secrets" ghcr.io/external-secrets/external-secrets:test-version`); count != 1 {
+		t.Fatalf("expected classic test dry-run to load the controller image once, got %d occurrences, output:\n%s", count, dryRun)
+	}
+	if strings.Contains(dryRun, "ghcr.io/external-secrets/provider-kubernetes:test-version") {
+		t.Fatalf("expected classic test dry-run to avoid loading the kubernetes provider image, output:\n%s", dryRun)
 	}
+	if !strings.Contains(dryRun, "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets") {
+		t.Fatalf("expected classic test dry-run to ensure helm dependencies before copying the chart, output:\n%s", dryRun)
+	}
+}
+
+func TestV2MakeTargetCanSkipKubernetesProviderBuild(t *testing.T) {
+	t.Parallel()
+
+	defaultDryRun := runMakeDryRun(t, "test.v2", "VERSION=test-version")
 	if !strings.Contains(defaultDryRun, "docker.build.provider.kubernetes") {
 		t.Fatalf("expected default test.v2 dry-run to build the kubernetes provider image, output:\n%s", defaultDryRun)
 	}
 	if !strings.Contains(defaultDryRun, "ghcr.io/external-secrets/provider-kubernetes:test-version") {
 		t.Fatalf("expected default test.v2 dry-run to still load the kubernetes provider image, output:\n%s", defaultDryRun)
 	}
-	if !strings.Contains(defaultDryRun, `E2E_SKIP_HELM_DEPENDENCY_UPDATE="true"`) {
-		t.Fatalf("expected default test.v2 dry-run to skip helm dependency updates inside the e2e pod, output:\n%s", defaultDryRun)
+	if !strings.Contains(defaultDryRun, "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets") {
+		t.Fatalf("expected default test.v2 dry-run to ensure helm dependencies before copying the chart, output:\n%s", defaultDryRun)
 	}
 
-	skippedDryRun := runMakeDryRun(t, "VERSION=test-version", "SKIP_PROVIDER_KUBERNETES_BUILD=true")
+	skippedDryRun := runMakeDryRun(t, "test.v2", "VERSION=test-version", "SKIP_PROVIDER_KUBERNETES_BUILD=true")
 	if strings.Contains(skippedDryRun, "docker.build.provider.kubernetes") {
 		t.Fatalf("expected skipped test.v2 dry-run to omit the kubernetes provider build, output:\n%s", skippedDryRun)
 	}
 	if !strings.Contains(skippedDryRun, "ghcr.io/external-secrets/provider-kubernetes:test-version") {
 		t.Fatalf("expected skipped test.v2 dry-run to still load the kubernetes provider image, output:\n%s", skippedDryRun)
 	}
-	if !strings.Contains(skippedDryRun, `E2E_SKIP_HELM_DEPENDENCY_UPDATE="true"`) {
-		t.Fatalf("expected skipped test.v2 dry-run to skip helm dependency updates inside the e2e pod, output:\n%s", skippedDryRun)
+	if !strings.Contains(skippedDryRun, "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets") {
+		t.Fatalf("expected skipped test.v2 dry-run to ensure helm dependencies before copying the chart, output:\n%s", skippedDryRun)
 	}
 }
 
-func runMakeDryRun(t *testing.T, extraArgs ...string) string {
+func runMakeDryRun(t *testing.T, target string, extraArgs ...string) string {
 	t.Helper()
 
-	args := append([]string{"-n", "test.v2"}, extraArgs...)
+	args := append([]string{"-n", target}, extraArgs...)
 	cmd := exec.Command("make", args...)
 	cmd.Dir = "."
 	cmd.Env = os.Environ()

hack/helm.dependency.ensure.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+chart_dir="${1:?usage: helm.dependency.ensure.sh <chart-dir>}"
+helm_bin="${HELM_BIN:-helm}"
+
+if "${helm_bin}" dependency list "${chart_dir}" 2>/dev/null | awk 'NR == 1 { next } NF && $NF != "ok" && $NF != "unpacked" { status = 1 } END { exit status }'; then
+  echo "Helm dependencies already present for ${chart_dir}; skipping dependency build"
+  exit 0
+fi
+
+"${helm_bin}" dependency build "${chart_dir}"

hack/helm_dependency_ensure_test.go

@@ -0,0 +1,79 @@
+package hack
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+)
+
+func TestHelmDependencyEnsureSkipsBuildWhenDependenciesAreReady(t *testing.T) {
+	t.Parallel()
+
+	workdir := t.TempDir()
+	marker := filepath.Join(workdir, "build-called")
+	fakeHelm := writeFakeHelm(t, workdir, "#!/usr/bin/env bash\nset -euo pipefail\n\nif [[ \"$1\" == \"dependency\" && \"$2\" == \"list\" ]]; then\n  cat <<'EOF'\nNAME                \tVERSION\tREPOSITORY                           \tSTATUS\nbitwarden-sdk-server\tv0.5.2 \toci://ghcr.io/external-secrets/charts\tok\nEOF\n  exit 0\nfi\n\nif [[ \"$1\" == \"dependency\" && \"$2\" == \"build\" ]]; then\n  touch \""+marker+"\"\n  exit 0\nfi\n\necho \"unexpected helm invocation: $*\" >&2\nexit 1\n")
+
+	cmd := exec.Command("bash", "helm.dependency.ensure.sh", "deploy/charts/external-secrets")
+	cmd.Dir = "."
+	cmd.Env = append(os.Environ(), "HELM_BIN="+fakeHelm)
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("helper failed: %v\n%s", err, string(output))
+	}
+
+	if _, err := os.Stat(marker); !os.IsNotExist(err) {
+		t.Fatalf("expected helper to skip helm dependency build when dependencies are already available")
+	}
+}
+
+func TestHelmDependencyEnsureSkipsBuildWhenDependenciesAreUnpacked(t *testing.T) {
+	t.Parallel()
+
+	workdir := t.TempDir()
+	marker := filepath.Join(workdir, "build-called")
+	fakeHelm := writeFakeHelm(t, workdir, "#!/usr/bin/env bash\nset -euo pipefail\n\nif [[ \"$1\" == \"dependency\" && \"$2\" == \"list\" ]]; then\n  cat <<'EOF'\nNAME                \tVERSION\tREPOSITORY                           \tSTATUS\nbitwarden-sdk-server\tv0.6.0 \toci://ghcr.io/external-secrets/charts\tunpacked\nEOF\n  exit 0\nfi\n\nif [[ \"$1\" == \"dependency\" && \"$2\" == \"build\" ]]; then\n  touch \""+marker+"\"\n  exit 0\nfi\n\necho \"unexpected helm invocation: $*\" >&2\nexit 1\n")
+
+	cmd := exec.Command("bash", "helm.dependency.ensure.sh", "deploy/charts/external-secrets")
+	cmd.Dir = "."
+	cmd.Env = append(os.Environ(), "HELM_BIN="+fakeHelm)
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("helper failed: %v\n%s", err, string(output))
+	}
+
+	if _, err := os.Stat(marker); !os.IsNotExist(err) {
+		t.Fatalf("expected helper to skip helm dependency build when dependencies are unpacked locally")
+	}
+}
+
+func TestHelmDependencyEnsureBuildsWhenDependenciesAreMissing(t *testing.T) {
+	t.Parallel()
+
+	workdir := t.TempDir()
+	marker := filepath.Join(workdir, "build-called")
+	fakeHelm := writeFakeHelm(t, workdir, "#!/usr/bin/env bash\nset -euo pipefail\n\nif [[ \"$1\" == \"dependency\" && \"$2\" == \"list\" ]]; then\n  cat <<'EOF'\nNAME                \tVERSION\tREPOSITORY                           \tSTATUS\nbitwarden-sdk-server\tv0.5.2 \toci://ghcr.io/external-secrets/charts\tmissing\nEOF\n  exit 0\nfi\n\nif [[ \"$1\" == \"dependency\" && \"$2\" == \"build\" ]]; then\n  touch \""+marker+"\"\n  exit 0\nfi\n\necho \"unexpected helm invocation: $*\" >&2\nexit 1\n")
+
+	cmd := exec.Command("bash", "helm.dependency.ensure.sh", "deploy/charts/external-secrets")
+	cmd.Dir = "."
+	cmd.Env = append(os.Environ(), "HELM_BIN="+fakeHelm)
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("helper failed: %v\n%s", err, string(output))
+	}
+
+	if _, err := os.Stat(marker); err != nil {
+		t.Fatalf("expected helper to invoke helm dependency build when dependencies are missing: %v", err)
+	}
+}
+
+func writeFakeHelm(t *testing.T, dir, content string) string {
+	t.Helper()
+
+	path := filepath.Join(dir, "helm")
+	if err := os.WriteFile(path, []byte(content), 0o755); err != nil {
+		t.Fatalf("write fake helm: %v", err)
+	}
+
+	return path
+}