Reference declarative agents by tag; digests only for sandbox agents#2242
Reference declarative agents by tag; digests only for sandbox agents#2242iplay88keys wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates declarative agent runtime image resolution to reference standard agents by tag (honoring --image-tag / Helm IMAGE_TAG) while keeping digest-pinned image references for sandbox agents due to Substrate ActorTemplate validation requirements.
Changes:
- Add controller flags to override baked-in sandbox runtime image digests (
--python-adk-*-image-digest,--golang-adk-*-image-digest). - Switch regular (non-sandbox) agent runtime images from digest-pinned references to tag-based references, using
-fulltag suffix for full variants. - Update translator golden test outputs and unit tests to reflect tag-based image refs for regular agents and digest pinning for sandbox agents.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| go/core/pkg/app/app.go | Adds new CLI flags for overriding sandbox runtime image digests. |
| go/core/internal/controller/translator/agent/deployments.go | Implements shared image resolution helper that chooses tag vs digest pinning based on sandbox mode. |
| go/core/internal/controller/translator/agent/adk_api_translator.go | Updates documentation around digest defaults/overrides and when they apply. |
| go/core/internal/controller/translator/agent/runtime_test.go | Adjusts runtime tests to assert tag refs for regular agents and prohibit digest pinning. |
| go/core/internal/controller/translator/agent/imageconfig_test.go | Updates digest-related tests and adds coverage for tag-based resolution and sandbox pinning behavior. |
| go/core/internal/controller/translator/agent/testdata/outputs/*.json | Updates golden manifests from @sha256:... to :tag for regular agent images (and :tag-full for full variants). |
Comments suppressed due to low confidence (1)
go/core/pkg/app/app.go:1
- These flag descriptions imply the value must be in "sha256:..." form, but normalizeImageDigest() accepts values both with and without the "sha256:" prefix. Consider updating the help text to explicitly allow both formats (and optionally mention the env var equivalents if that's the intended primary config path in Helm) to reduce operator confusion.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
32574bd to
341d294
Compare
Description
Declarative agent images are now referenced by tag (
registry/repository:tag), honoring--image-tag/ HelmIMAGE_TAG. Digest-pinned references (registry/repository@sha256:...) are only used for sandbox agents, where Substrate ActorTemplate validation rejects tag refs.resolvePythonRuntimeImage/resolveGoRuntimeImageemitregistry/repo:tag; full variants (skills / SRT) use the<tag>-fullconvention the Makefile publishes (APP_FULL_IMAGE_TAG,GOLANG_ADK_FULL_IMAGE_TAG).SandboxAgent, workload modesandbox): unchanged digest pinning from the link-time digests.--app-image-digest,--app-full-image-digest,--golang-adk-image-digest,--golang-adk-full-image-digest(env:APP_IMAGE_DIGEST, etc.). Defaults are the digests baked in at build time.IMAGE_TAGwas already plumbed through the controller ConfigMap; the digest overrides are settable via controller env.Supersedes #2076, following the approach requested in its review: tags for standard deployments, digests kept only where Substrate requires them, rather than an opt-out flag.
Fixes #2055
Test plan
go test ./core/internal/controller/translator/agent/...— regular-agent tests now assert tag refs (and no@sha256:), sandbox-agent test asserts digest refs, new unit tests cover tag resolution including the-fullsuffix and the missing-digest error.UPDATE_GOLDEN=true): all 35 regular-agent outputs changeapp@sha256:...→app:dev.