diff --git a/.env b/.env index 40b6446f9..e4e2704ab 100644 --- a/.env +++ b/.env @@ -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 diff --git a/.github/workflows/e2e-kind.yaml b/.github/workflows/e2e-kind.yaml index d39fc05a8..0fcb4972b 100644 --- a/.github/workflows/e2e-kind.yaml +++ b/.github/workflows/e2e-kind.yaml @@ -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 diff --git a/.gitignore b/.gitignore index c81213042..d7f764979 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ bazel-* *.out build/resources/ +user.env # AI .ai/ diff --git a/build/tasks/deploy.go b/build/tasks/deploy.go index 05d3774e8..1f051a8a9 100644 --- a/build/tasks/deploy.go +++ b/build/tasks/deploy.go @@ -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{ @@ -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/")) }, @@ -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) }, @@ -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) }, @@ -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 { diff --git a/build/tasks/release.go b/build/tasks/release.go index fa1709806..fd029f8be 100644 --- a/build/tasks/release.go +++ b/build/tasks/release.go @@ -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) @@ -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) diff --git a/build/tasks/test.go b/build/tasks/test.go index 2ba0fa5f9..fc881bd9b 100644 --- a/build/tasks/test.go +++ b/build/tasks/test.go @@ -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 ( @@ -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) // Build images a.Log("Building images for e2e tests...") @@ -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() diff --git a/build/util/dotenv/load.go b/build/util/dotenv/load.go index d7ff54734..82bd86d3d 100644 --- a/build/util/dotenv/load.go +++ b/build/util/dotenv/load.go @@ -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) diff --git a/pkg/reconciler/wasmmodule/reconciler_test.go b/pkg/reconciler/wasmmodule/reconciler_test.go index 0cb78ef32..8052919e9 100644 --- a/pkg/reconciler/wasmmodule/reconciler_test.go +++ b/pkg/reconciler/wasmmodule/reconciler_test.go @@ -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" ) @@ -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{ @@ -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{ diff --git a/pkg/reconciler/wasmmodule/wasmmodule.go b/pkg/reconciler/wasmmodule/wasmmodule.go index a00145ab4..bf2a8e691 100644 --- a/pkg/reconciler/wasmmodule/wasmmodule.go +++ b/pkg/reconciler/wasmmodule/wasmmodule.go @@ -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 diff --git a/test/e2e/clients.go b/test/e2e/clients.go index 91a545e87..b1bcc738c 100644 --- a/test/e2e/clients.go +++ b/test/e2e/clients.go @@ -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) diff --git a/test/e2e/config.go b/test/e2e/config.go index ee12df594..a72ef27d2 100644 --- a/test/e2e/config.go +++ b/test/e2e/config.go @@ -19,7 +19,6 @@ package e2e import ( "context" "fmt" - "net" "os" "strconv" "time" @@ -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" @@ -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. @@ -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 }