Scope deploy/publish/images to dev registry by default#25
Scope deploy/publish/images to dev registry by default#25
Conversation
Renamed E2E_IMAGE_BASENAME to DEV_IMAGE_BASENAME. Split setupKoEnv into Dev/Prod variants. Removed dead localhost fallback. Added user.env override support. Assisted-by: 🤖 Claude Opus/Sonnet 4.6
WalkthroughReassigns the wasm test image basename from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
==========================================
- Coverage 51.39% 50.98% -0.42%
==========================================
Files 10 10
Lines 358 355 -3
==========================================
- Hits 184 181 -3
Misses 154 154
Partials 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Assisted-by: 🤖 Claude Opus/Sonnet 4.6
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build/tasks/test.go`:
- 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).
In `@test/e2e/config.go`:
- Line 62: The error message returned when DevImageBasenameEnv is not set should
mention both user.env and .env (in that order) since the lookup checks user.env
first; update the fmt.Errorf call that currently returns "%s is not set (check
.env or environment)" to instead reference "user.env, .env or the environment"
so the message guides users correctly when the override in user.env is missing;
modify the return where DevImageBasenameEnv is used to include the new wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33dfee47-41ad-4803-a714-6e83189afb43
📒 Files selected for processing (10)
.env.gitignorebuild/tasks/deploy.gobuild/tasks/release.gobuild/tasks/test.gobuild/util/dotenv/load.gopkg/reconciler/wasmmodule/reconciler_test.gopkg/reconciler/wasmmodule/wasmmodule.gotest/e2e/clients.gotest/e2e/config.go
💤 Files with no reviewable changes (1)
- pkg/reconciler/wasmmodule/wasmmodule.go
| // This must be done before building images so they get tagged correctly | ||
| setupE2EImageBasename(a) | ||
| setupKoEnv(a) | ||
| setupKoEnvDev(a) |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
🧩 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!
…essage Assisted-by: 🤖 Claude Opus/Sonnet 4.6
1 participant
Summary
E2E_IMAGE_BASENAMEtoDEV_IMAGE_BASENAMEin.envsetupKoEnvintosetupKoEnvDev(for deploy/publish/images/e2e) andsetupKoEnvProd(for release)./goyek deploynow targets the dev registry by default — production is gated behind./goyek releaseuser.envoverride support (loaded before.env, gitignored)Assisted-by: 🤖 Claude Opus/Sonnet 4.6
Summary by cubic
Defaulted deploy/publish/images to the dev registry via DEV_IMAGE_BASENAME; production pushes now require ./goyek release. KO_DOCKER_REPO is honored (and synced to DEV_IMAGE_BASENAME), user.env overrides are supported, and the localhost registry fallback is removed.
Written for commit 5d07a47. Summary will update on new commits.
Summary by CodeRabbit
Chores
Tests