Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
DEV_IMAGE_BASENAME=quay.io/cardil/knative/serving/wasm/test
IMAGE_BASENAME=ghcr.io/cardil/knative-serving-wasm
E2E_IMAGE_BASENAME=quay.io/cardil/knative/serving/wasm/test
2 changes: 1 addition & 1 deletion .github/workflows/e2e-kind.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
- name: Run e2e tests
run: ./goyek --verbose e2e
env:
E2E_IMAGE_BASENAME: ghcr.io/${{ github.repository }}/e2e-${{ github.event.pull_request.number || github.run_id }}
DEV_IMAGE_BASENAME: ghcr.io/${{ github.repository }}/e2e-${{ github.event.pull_request.number || github.run_id }}
ARTIFACTS: ${{ github.workspace }}/artifacts

- name: Delete e2e images from ghcr.io
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ bazel-*
*.out

build/resources/
user.env

# AI
.ai/
Expand Down
33 changes: 28 additions & 5 deletions build/tasks/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func Deploy(f *goyek.Flow) {
Name: "deploy",
Usage: "Deploys the controller onto Kubernetes",
Action: func(a *goyek.A) {
setupKoEnv(a)
setupKoEnvDev(a)
koApply(a)
},
Deps: goyek.Deps{
Expand Down Expand Up @@ -125,7 +125,7 @@ func Undeploy() goyek.Task {
Name: "undeploy",
Usage: "Removes the controller from Kubernetes",
Action: func(a *goyek.A) {
setupKoEnv(a)
setupKoEnvDev(a)
ko := tools.Ghet(a, "ko")
executil.ExecOrDie(a, spaceJoin(ko, "delete", "-f", "config/"))
},
Expand All @@ -137,7 +137,7 @@ func Publish(f *goyek.Flow) goyek.Task {
Name: "publish",
Usage: "Publish artifacts onto registry",
Action: func(a *goyek.A) {
setupKoEnv(a)
setupKoEnvDev(a)
pushExamples(a)
pushRunnerImage(a)
},
Expand All @@ -152,7 +152,7 @@ func Images() goyek.Task {
Name: "images",
Usage: "Builds OCI images",
Action: func(a *goyek.A) {
setupKoEnv(a)
setupKoEnvDev(a)
buildExamples(a)
buildRunnerImage(a)
},
Expand Down Expand Up @@ -195,7 +195,30 @@ func resolveContainerEngine() string {
return e
}

func setupKoEnv(a *goyek.A) {
// setupKoEnvDev sets KO_DOCKER_REPO from DEV_IMAGE_BASENAME for dev workflows
// (deploy, publish, images). This ensures local development never accidentally
// pushes to the production registry.
func setupKoEnvDev(a *goyek.A) {
a.Helper()

if v, ok := os.LookupEnv(koDockerRepo); ok {
a.Setenv("DEV_IMAGE_BASENAME", v)
return
}

if v := os.Getenv("DEV_IMAGE_BASENAME"); v != "" {
a.Setenv(koDockerRepo, v)
return
}

a.Fatal("DEV_IMAGE_BASENAME is not set. " +
"Set it in .env or as an environment variable to target a dev registry. " +
"Production deploys go through the release task.")
}

// setupKoEnvProd sets KO_DOCKER_REPO from IMAGE_BASENAME for production
// workflows (release).
func setupKoEnvProd(a *goyek.A) {
a.Helper()

if _, ok := os.LookupEnv(koDockerRepo); !ok {
Expand Down
4 changes: 2 additions & 2 deletions build/tasks/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func ReleaseBuild() goyek.Task {
Name: "release-build",
Usage: "Builds release artifacts locally (no registry push), saves version to build/output/release/version.txt",
Action: func(a *goyek.A) {
setupKoEnv(a)
setupKoEnvProd(a)

version := detectReleaseVersion(a)
a.Log("Release version: ", version)
Expand Down Expand Up @@ -131,7 +131,7 @@ func ReleasePerform() goyek.Task {
Name: "release-perform",
Usage: "Pushes images and publishes GitHub release (reads version from build/output/release/version.txt)",
Action: func(a *goyek.A) {
setupKoEnv(a)
setupKoEnvProd(a)

// Read version written by release:build
versionBytes, err := os.ReadFile(versionFile)
Expand Down
61 changes: 1 addition & 60 deletions build/tasks/test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
package tasks

import (
"net"
"os"
"time"

executil "github.com/cardil/knative-serving-wasm/build/util/exec"
"github.com/cardil/knative-serving-wasm/test/util/k8s"
"github.com/goyek/goyek/v2"
"github.com/goyek/x/cmd"
"github.com/joho/godotenv"
)

const (
Expand Down Expand Up @@ -63,10 +60,7 @@ func E2e() goyek.Task {
return
}

// Set up IMAGE_BASENAME and KO_DOCKER_REPO for e2e tests
// This must be done before building images so they get tagged correctly
setupE2EImageBasename(a)
setupKoEnv(a)
setupKoEnvDev(a)
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the e2e runner's basename aligned with the repo you just built.

setupKoEnvDev(a) only guarantees KO_DOCKER_REPO, but the test binary resolves images from DEV_IMAGE_BASENAME. When KO_DOCKER_REPO is pre-set, this task builds and pushes to that override while test/e2e/config.go can still fall back to the basename from dotenv, so goyek e2e may validate the wrong images.

💡 Minimal fix
 			setupKoEnvDev(a)
+			a.Setenv("DEV_IMAGE_BASENAME", os.Getenv(koDockerRepo))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@build/tasks/test.go` at line 63, The e2e runner basename can diverge from the
pushed images because setupKoEnvDev(a) only sets KO_DOCKER_REPO; update the test
task to also set DEV_IMAGE_BASENAME to the same value when KO_DOCKER_REPO is
present (or derive DEV_IMAGE_BASENAME from the repo value) so test/e2e/config.go
resolves the same images; modify the call/site around setupKoEnvDev(a) to export
DEV_IMAGE_BASENAME (using the same string used for KO_DOCKER_REPO) or extend
setupKoEnvDev to set both env vars (referencing setupKoEnvDev, KO_DOCKER_REPO,
DEV_IMAGE_BASENAME, and test/e2e/config.go).

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5d07a47. setupKoEnvDev now syncs DEV_IMAGE_BASENAME from KO_DOCKER_REPO when the latter is explicitly set, so the test binary always resolves the same images that were built and pushed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
git show 5d07a47 --stat
git show 5d07a47 -- build/tasks/deploy.go

Length of output: 1034


@cardil, confirmed — the fix in 5d07a47 looks correct. When KO_DOCKER_REPO is pre-set, setupKoEnvDev now captures its value and propagates it to DEV_IMAGE_BASENAME via a.Setenv before returning, ensuring test/e2e/config.go always resolves the same registry that images were built and pushed to.


🧠 Learnings used
Learnt from: cardil
Repo: cardil/knative-serving-wasm PR: 6
File: test/e2e/config.go:56-86
Timestamp: 2026-02-12T16:56:23.580Z
Learning: In the knative-serving-wasm project, E2E tests are executed via the goyek build system (not directly with `go test`), which sets the current working directory to the repository root before running tasks. This ensures that relative paths like `.env` in `godotenv.Read(".env")` resolve correctly to the project root.

Learnt from: cardil
Repo: cardil/knative-serving-wasm PR: 2
File: build/tasks/deploy.go:0-0
Timestamp: 2025-06-27T20:04:53.653Z
Learning: In the knative-serving-wasm project, environment variables like IMAGE_BASENAME are managed through a `.env` file that provides default values, and `build/util/dotenv/load.go` provides a goyek.Executor wrapper that loads these environment variables before task execution. This ensures environment variables are always available when build tasks run, eliminating the need for validation in individual task functions.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!


// Build images
a.Log("Building images for e2e tests...")
Expand Down Expand Up @@ -95,59 +89,6 @@ func E2e() goyek.Task {
}
}

// setupE2EImageBasename ensures IMAGE_BASENAME is set to a test registry for e2e tests
func setupE2EImageBasename(a *goyek.A) {
a.Helper()

// Load production IMAGE_BASENAME from .env
productionBasename := ""
if envMap, err := godotenv.Read(".env"); err == nil {
productionBasename = envMap["IMAGE_BASENAME"]
}

// Check for explicit E2E_IMAGE_BASENAME
if e2eBasename := os.Getenv("E2E_IMAGE_BASENAME"); e2eBasename != "" {
a.Setenv("IMAGE_BASENAME", e2eBasename)
a.Logf("Using E2E_IMAGE_BASENAME: %s", e2eBasename)
return
}

// Check if IMAGE_BASENAME is already set and differs from production
if currentBasename := os.Getenv("IMAGE_BASENAME"); currentBasename != "" && currentBasename != productionBasename {
a.Logf("Using existing IMAGE_BASENAME: %s", currentBasename)
return
}

// Try to detect and use local registry
localRegistry := "localhost:5001"
if isLocalRegistryAvailable(localRegistry) {
imageBasename := localRegistry + "/knative-serving-wasm"
a.Setenv("IMAGE_BASENAME", imageBasename)
a.Logf("Using local registry: %s", imageBasename)
return
}

// No test registry available - fail to protect production
a.Fatalf(
"E2E tests require a test registry. Options:\n"+
" 1. Set E2E_IMAGE_BASENAME environment variable\n"+
" 2. Set IMAGE_BASENAME to a non-production registry\n"+
" 3. Start local registry on localhost:5001\n"+
"Production registry (%s) cannot be used for e2e tests",
productionBasename,
)
}

// isLocalRegistryAvailable checks if local registry is reachable
func isLocalRegistryAvailable(registry string) bool {
conn, err := net.DialTimeout("tcp", registry, 2*time.Second)
if err != nil {
return false
}
defer conn.Close()
return true
}

func kubeAvailable(a *goyek.A) bool {
a.Helper()

Expand Down
6 changes: 4 additions & 2 deletions build/util/dotenv/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import (

func Load(next goyek.Executor) goyek.Executor {
return func(in goyek.ExecuteInput) error {
if err := godotenv.Load(); err != nil && !os.IsNotExist(err) {
return err
for _, f := range []string{"user.env", ".env"} {
if err := godotenv.Load(f); err != nil && !os.IsNotExist(err) {
return err
}
}

return next(in)
Expand Down
5 changes: 5 additions & 0 deletions pkg/reconciler/wasmmodule/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/tracker"
servingv1 "knative.dev/serving/pkg/apis/serving/v1"
servingfake "knative.dev/serving/pkg/client/clientset/versioned/fake"
servingv1listers "knative.dev/serving/pkg/client/listers/serving/v1"
)

Expand Down Expand Up @@ -170,10 +171,12 @@ func TestReconcileKind_TerminalConfigFailure(t *testing.T) {
const moduleName = "my-wasm"

svc := ksvcWithConfigFailed(ns, moduleName, tt.svcReason, tt.svcMessage)
fakeClient := servingfake.NewSimpleClientset(svc)

r := &wasmmodule.Reconciler{
Tracker: fakeTracker{},
ServiceLister: buildServiceLister(svc),
Client: fakeClient.ServingV1(),
}

module := &api.WasmModule{
Expand All @@ -200,10 +203,12 @@ func TestReconcileKind_TransientNotReady(t *testing.T) {
const moduleName = "my-wasm"

svc := ksvcWithReadyUnknown(ns, moduleName)
fakeClient := servingfake.NewSimpleClientset(svc)

r := &wasmmodule.Reconciler{
Tracker: fakeTracker{},
ServiceLister: buildServiceLister(svc),
Client: fakeClient.ServingV1(),
}

module := &api.WasmModule{
Expand Down
6 changes: 0 additions & 6 deletions pkg/reconciler/wasmmodule/wasmmodule.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,6 @@ func (r *Reconciler) updateService(

serviceName := module.Name

if r.Client == nil {
log.Errorf("Cannot update service %s: client is nil", serviceName)

return existing, nil
}

desired, err := r.buildDesiredService(ctx, module)
if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ var (
)

// ensureClients initializes Kubernetes clients lazily on first use.
func ensureClients(ctx context.Context) error {
func ensureClients(_ context.Context) error {
clientsOnce.Do(func() {
// Verify image basename configuration
imageBasename, err := GetE2EImageBasename(context.WithoutCancel(ctx))
imageBasename, err := GetE2EImageBasename()
if err != nil {
errClientsInit = fmt.Errorf("E2E image basename check failed: %w", err)

Expand Down
65 changes: 14 additions & 51 deletions test/e2e/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package e2e
import (
"context"
"fmt"
"net"
"os"
"strconv"
"time"
Expand All @@ -28,14 +27,8 @@ import (
)

const (
// DefaultLocalRegistry is the default local registry for e2e tests.
DefaultLocalRegistry = "localhost:5001"

// E2EImageBasenameEnv is the environment variable for specifying test image basename.
E2EImageBasenameEnv = "E2E_IMAGE_BASENAME"

// ImageBasenameEnv is the environment variable for image basename.
ImageBasenameEnv = "IMAGE_BASENAME"
// DevImageBasenameEnv is the environment variable for specifying dev/test image basename.
DevImageBasenameEnv = "DEV_IMAGE_BASENAME"

// E2ETestTimeoutEnv is the environment variable for individual test timeout in seconds.
E2ETestTimeoutEnv = "E2E_TEST_TIMEOUT"
Expand All @@ -53,50 +46,20 @@ type Config struct {
ImageBasename string
}

// GetE2EImageBasename returns the image basename to use for e2e tests with safety checks.
func GetE2EImageBasename(ctx context.Context) (string, error) {
// Load .env file to get production IMAGE_BASENAME
productionBasename := ""
if envMap, err := godotenv.Read(".env"); err == nil {
productionBasename = envMap[ImageBasenameEnv]
}

// Check for explicit E2E_IMAGE_BASENAME
if e2eBasename := os.Getenv(E2EImageBasenameEnv); e2eBasename != "" {
return e2eBasename, nil
func GetE2EImageBasename() (string, error) {
if v := os.Getenv(DevImageBasenameEnv); v != "" {
return v, nil
}

// Check if IMAGE_BASENAME has been overridden from production value
currentBasename := os.Getenv(ImageBasenameEnv)
if currentBasename != "" && currentBasename != productionBasename {
// IMAGE_BASENAME is set and differs from production, use it
return currentBasename, nil
}

// Try to detect local registry and construct basename
if isLocalRegistryAvailable(ctx) {
return DefaultLocalRegistry + "/knative-serving-wasm", nil
}

return "", fmt.Errorf(
"e2e tests require %s environment variable or local registry on %s; "+
"alternatively, set %s to a non-production value; "+
"production value is %q",
E2EImageBasenameEnv, DefaultLocalRegistry, ImageBasenameEnv, productionBasename,
)
}

// isLocalRegistryAvailable checks if local registry is reachable.
func isLocalRegistryAvailable(ctx context.Context) bool {
dialer := &net.Dialer{Timeout: 2 * time.Second}

conn, err := dialer.DialContext(ctx, "tcp", DefaultLocalRegistry)
if err != nil {
return false
for _, f := range []string{"user.env", ".env"} {
if envMap, err := godotenv.Read(f); err == nil {
if v := envMap[DevImageBasenameEnv]; v != "" {
return v, nil
}
}
}
defer conn.Close()

return true
return "", fmt.Errorf("%s is not set (check user.env, .env, or environment)", DevImageBasenameEnv)
}

// GetTestTimeout returns the individual test timeout from environment or default.
Expand All @@ -111,8 +74,8 @@ func GetTestTimeout() time.Duration {
}

// NewConfig creates a new e2e test configuration.
func NewConfig(ctx context.Context, namespace string) (*Config, error) {
imageBasename, err := GetE2EImageBasename(ctx)
func NewConfig(_ context.Context, namespace string) (*Config, error) {
imageBasename, err := GetE2EImageBasename()
if err != nil {
return nil, err
}
Expand Down
Loading