Skip to content

Add container image substitutions for private registries#47374

Open
pelikhan with Copilot wants to merge 13 commits into
mainfrom
copilot/recreate-pr-47351
Open

Add container image substitutions for private registries#47374
pelikhan with Copilot wants to merge 13 commits into
mainfrom
copilot/recreate-pr-47351

Conversation

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Private-cloud environments can substitute action references through action_pins, but container images still resolve to public registries. This adds equivalent container_pins configuration and documents both substitution mechanisms.

  • Configuration

    • Add container_pins to aw.json parsing and schema validation.
    • Support tagged and digest-pinned replacement images.
  • Resolution

    • Apply substitutions before digest lookup.
    • Cover pre-download manifests, MCP gateway commands, and custom MCP containers.
    • Deduplicate substitution notices across resolution paths.
  • Documentation

    • Add action and container substitution guidance.
    • Extend the self-hosted runner reference with private-registry configuration.
{
  "container_pins": {
    "node:lts-alpine": "registry.example.com/node:lts-alpine",
    "ghcr.io/github/gh-aw-firewall:0.27.22": "registry.example.com/gh-aw-firewall:0.27.22"
  }
}

Generated by 👨‍🍳 PR Sous Chef · gpt54 7.24 AIC · ⌖ 6.95 AIC · ⊞ 7K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/29952896403

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.1 AIC · ⌖ 6.17 AIC · ⊞ 7K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/29952896403

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.1 AIC · ⌖ 6.17 AIC · ⊞ 7K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 6.54 AIC · ⌖ 7.98 AIC · ⊞ 7K ·
Comment /souschef to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title feat: add container_pins to aw.json and document action/container substitutions Add container image substitutions for private registries Jul 22, 2026
Copilot AI requested a review from pelikhan July 22, 2026 17:57
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot must enforce sha in the map

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot must enforce sha in the map

Addressed in be0e488. container_pins replacement values must now use @sha256:<64 lowercase hex characters>; schema and runtime validation reject tag-only or malformed values, with tests and examples updated.

@pelikhan
pelikhan marked this pull request as ready for review July 22, 2026 18:31
Copilot AI review requested due to automatic review settings July 22, 2026 18:31
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds repository-level container image substitutions for private registries, extending the existing action-pin mechanism.

Changes:

  • Parses and validates container_pins from aw.json.
  • Applies mappings during container resolution and MCP rendering.
  • Adds tests and private-registry documentation.
Show a summary per file
File Description
pkg/workflow/workflow_data.go Propagates container mappings into pin contexts.
pkg/workflow/repo_config.go Parses container_pins.
pkg/workflow/repo_config_test.go Tests digest validation.
pkg/workflow/mcp_setup_generator.go Maps the MCP Gateway image.
pkg/workflow/mcp_config_types.go Adds workflow context to MCP rendering.
pkg/workflow/mcp_config_custom.go Maps custom MCP containers.
pkg/workflow/docker.go Maps pre-downloaded images.
pkg/workflow/docker_pin_test.go Tests mapped image resolution.
pkg/workflow/copilot_mcp.go Passes mappings to Copilot rendering.
pkg/workflow/compiler_string_api.go Loads mappings for string compilation.
pkg/workflow/compiler_repo_config.go Copies mappings from repository configuration.
pkg/workflow/compiler_orchestrator_workflow.go Attaches mappings during orchestration.
pkg/workflow/codex_mcp.go Passes mappings to Codex renderers.
pkg/workflow/action_pins.go Adds container mapping helpers.
pkg/parser/schemas/repo_config_schema.json Defines container_pins validation.
pkg/cli/data/agentic_workflows_fallback_aw_files.json Registers substitution guidance.
pkg/actionpins/actionpins.go Implements mapping and notification logic.
pkg/actionpins/actionpins_internal_test.go Tests mapping behavior and deduplication.
docs/src/content/docs/reference/self-hosted-runners.md Documents private-registry configuration.
.github/aw/action-container-substitutions.md Adds focused substitution guidance.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 20/20 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread pkg/parser/schemas/repo_config_schema.json Outdated
Comment thread pkg/workflow/docker.go
Comment on lines +242 to +244
// Apply container_pins mapping from aw.json before digest resolution so that
// redirected registries are pre-downloaded and recorded in the manifest.
img = applyContainerPinMappingFromData(img, workflowData)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the commits following be0e488. All runtime container references now go through resolveGatewayContainerFromMappings: mcp_renderer_github.go at lines 178 and 235, mcp_config_playwright_renderer.go at line 117, mcp_renderer_builtin.go at lines 202 and 370 (DefaultAlpineImage in both TOML and JSON paths), and mcp_config_custom.go's container scalar property. Custom command: docker servers are covered via applyContainerPinMappingFromData in docker.go.

Comment thread pkg/workflow/mcp_config_custom.go Outdated
renderMCPJSONScalar(yaml, renderer, "type", mcpConfig.Type, isLast)
case "container":
renderMCPStringScalar(yaml, renderer, "container", mcpConfig.Container, isLast)
container := applyContainerPinMappingFromData(mcpConfig.Container, renderer.WorkflowData)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the commits following be0e488. renderMCPScalarProperty now routes the container field through resolveGatewayContainerFromMappings, which applies the container_pins redirect and strips any digest component before writing to the MCP Gateway container field. All built-in MCP renderers use the same helper.

@github-actions

Copy link
Copy Markdown
Contributor

Design Decision Gate - ADR Required

This PR makes significant changes to core business logic (379 new lines in pkg/) but does not have a linked Architecture Decision Record (ADR).

Draft ADR committed: docs/adr/47374-add-container-pins-for-private-registry-substitution.md -- review and complete it before merging.

This PR cannot merge until an ADR is linked in the PR body.

What to do next
  1. Review the draft ADR committed to your branch -- it was generated from the PR diff
  2. Complete the missing sections -- add context the AI could not infer, refine the decision rationale, and list real alternatives you considered
  3. Commit the finalized ADR to docs/adr/ on your branch
  4. Reference the ADR in this PR body by adding a line such as:
    ADR: [ADR-47374: Add container_pins for Private-Registry Container Image Substitution](docs/adr/47374-add-container-pins-for-private-registry-substitution.md)

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

Why ADRs Matter

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you.

Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

  • Context -- What is the problem? What forces are at play?
  • Decision -- What did you decide? Why?
  • Alternatives Considered -- What else could have been done?
  • Consequences -- What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · sonnet46 62.6 AIC · ⌖ 9.79 AIC · ⊞ 8.5K ·
Comment /review to run again

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Field Value
Category feature
Risk Medium
Score 72/100
Action fast_track

Score breakdown: Impact 38/50 · Urgency 20/30 · Quality 14/20

Rationale: Adds container image substitution for private registries. Touches 20 files across pkg/workflow, pkg/actionpins, schema, docs. Tests included, CI mostly passing. Code reviewer already posted. Ready for expedited human review given broad cross-package scope.

Labels applied: pr-type:feature · pr-risk:medium · pr-priority:high · pr-action:fast_track · pr-agent:copilot

Generated by 🔧 PR Triage Agent · sonnet46 38.6 AIC · ⌖ 7.92 AIC · ⊞ 5.7K ·

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 85/100 — Excellent

Analyzed 13 test(s): 13 design, 0 implementation, 0 violation(s).

📊 Metrics (13 tests)
Metric Value
Analyzed 13 (Go: 13, JS: 0)
✅ Design 13 (100%)
⚠️ Implementation 0 (0%)
Edge/error coverage 11 (85%)
Duplicate clusters 0
Inflation YES — docker_pin_test.go (80 added) vs docker.go (3 added) = ~26:1 (functions tested span multiple files; aggregate ratio ~2.5:1)
🚨 Violations 0
Test File Classification Issues
TestApplyContainerPins docker_pin_test.go design_test / high_value 7 table scenarios including nil pins, empty list, embedded pin fallback
TestApplyContainerPins_DefaultFirewallVersion docker_pin_test.go design_test / high_value Regression test for all 4 sidecar images
TestCollectDockerImages_StoresInWorkflowData docker_pin_test.go design_test / high_value Verifies side-effects on WorkflowData
TestCollectDockerImages_SafeOutputsAddsGhAwNodeImage docker_pin_test.go design_test / high_value Negative assertion ensures no fallback to node:lts-alpine
TestMergeDockerImages docker_pin_test.go design_test / medium_value Deduplication happy-path
TestMergeDockerImagePins docker_pin_test.go design_test / high_value Covers duplicate-keeps-first edge case
TestApplyContainerPins_ContainerPinMappings docker_pin_test.go design_test / high_value 3 sub-tests: mapped, unmapped, nil mappings
TestResolveContainerImage_AppliesContainerPinMapping docker_pin_test.go design_test / high_value 3 sub-tests including invariant that mapped image does not inherit source embedded pin
TestLoadRepoConfig_ContainerPinsRequireSHA256Digest repo_config_test.go design_test / high_value 3 scenarios: valid digest, tag-only rejected, short digest rejected
TestLoadActionPinsData_LoadsContainerPins actionpins_internal_test.go design_test / high_value Verifies new containers field loading
TestInitWarnings_InitializesAndPreservesMap actionpins_internal_test.go design_test / medium_value nil-init and preserve-existing sub-tests
TestFormatPinnedActionWithResolution_ConsistentVersionComment actionpins_internal_test.go design_test / high_value 3 table cases covering version comment formatting invariants
TestFindCompatiblePin_SemverFallback actionpins_internal_test.go design_test / high_value 7 table cases including no-match and empty-version edge cases
⚠️ Flagged Tests (1 — inflation notice only)

Inflation notice (docker_pin_test.go) — 80 lines added vs docker.go with only 3 lines added. The mechanical ratio is ~26:1, but functions under test span action_pins.go (+21 lines) and workflow_data.go (+8 lines), bringing the aggregate to ~2.5:1. Not a real concern.

Verdict

Passed. 0% implementation tests (threshold: 30%). All 13 tests verify behavioral contracts with strong error/edge-case coverage. No build-tag or mock violations. The 10-point inflation deduction is mechanical and does not reflect a real quality issue.

References:

🧪 Test quality analysis by Test Quality Sentinel · sonnet46 59.9 AIC · ⌖ 12.3 AIC · ⊞ 7K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 85/100. 0% implementation tests (threshold: 30%). All 13 tests verify behavioral contracts; no violations.

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address the open review feedback below, and rerun checks once the branch is ready.

Open review feedback to address:...

All open review feedback has been addressed:

  • Fragile nil-context test gate — replaced tt.name == "nil context..." string match with nilCtx bool struct field in actionpins_internal_test.go.
  • Container_pins key validation — removed @ from the schema propertyNames.pattern so digest-pinned source keys are rejected at schema validation time; added test coverage.
  • MCPConfigRenderer narrowing — changed WorkflowData *WorkflowData to ContainerPinMappings map[string]string across all callers (custom, copilot, codex, factory).
  • Built-in MCP renderer coveragecontainer_pins mappings now thread through GitHub MCP, Playwright, and AgenticWorkflows renderers (both JSON and TOML paths), including DefaultAlpineImage.
  • MCP Gateway digest stripping — added resolveGatewayContainerFromMappings helper that applies the mapping then strips the digest so MCP Gateway's container field receives only image[:tag]; updated mcp_config_custom.go and all built-in renderers accordingly.
  • Test coverage — added TestCollectDockerImages_DefaultAlpineContainerPinMapping, TestResolveGatewayContainerFromMappings, and TestLoadRepoConfig_ContainerPinsKeyNoDigestAllowed.

All unit tests pass and code review/CodeQL scans are clean. CI re-trigger needed from a maintainer to get green checks.

Copilot AI requested a review from gh-aw-bot July 22, 2026 19:24
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

Open review feedback to address:

  • github-actions requested changes

Generated by 👨‍🍳 PR Sous Chef · gpt54 7.24 AIC · ⌖ 6.95 AIC · ⊞ 7K ·
Comment /souschef to run again

Copilot AI and others added 2 commits July 22, 2026 19:48
…inContext invariant

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

Open review feedback to address:...

All unresolved review threads addressed:

  • github-actions CHANGES_REQUESTED (Matt Pocock Skills Reviewer): Both blocking items fixed — DefaultAlpineImage redirect is applied in both TOML and JSON Agentic Workflows renderers; nilCtx bool struct field was already in place from a prior commit.
  • ApplyContainerPinMapping silent skip (#discussion_r3632907990): Invalid container_pins values now emit a visible warning to stderr via console.FormatWarningMessage instead of a silent debug-only log.
  • PinContext() allocation invariant (#discussion_r3632907995): Added explicit doc comment documenting the shared-Warnings invariant.
  • Outdated threads (MCP Gateway digest stripping, nilCtx struct field): Replied confirming they were fixed in prior commits.
  • Schema tagged-replacement thread: Replied confirming the contract is now digest-only and the PR description example is stale from the initial draft.
  • MCP runtime coverage thread: Replied confirming all renderers now apply mappings via resolveGatewayContainerFromMappings.

Local make fmt + go test ./pkg/actionpins/... ./pkg/workflow/... pass. CI re-trigger needed from a maintainer.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.1 AIC · ⌖ 6.17 AIC · ⊞ 7K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once the branch is up to date.

...

All open review items were addressed in prior commits; local validation is now fully clean:

  • Reviews — All 4 blocking items from Matt Pocock Skills Reviewer (CHANGES_REQUESTED) fixed: DefaultAlpineImage redirect applied in both TOML and JSON renderers; nilCtx bool struct field used in place of fragile name-string gate; invalid container_pins values now emit a visible warning to stderr; PinContext() doc comment documents the shared-Warnings invariant. All 6 review threads have replies.
  • Checks (local)make fmt, make agent-report-progress (lint + compile + test-unit), and parallel_validation (code review + CodeQL) all pass with no findings.
  • Checks (CI) — Stale after agent push; needs human re-trigger. Prior CHANGES_REQUESTED issues are fixed at the root cause.
  • Mergeable — PR is open, not draft; no conflicts.

Hand-off: CI must be re-triggered by a maintainer (close/reopen the PR, workflow_dispatch, or a push) before merge.

@pelikhan pelikhan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot the container pins map should hold the sha digest AND ref name (as object) so that the mapping can be validated as well.

@pelikhan pelikhan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot the container pins map should hold the sha digest AND ref name (as object) so that the mapping can be validated as well.

Copilot AI and others added 2 commits July 22, 2026 21:04
…elds

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…elds

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot the container pins map should hold the sha digest AND ref name (as object) so that the mapping can be validated as well.

Done in the latest commit. container_pins values are now objects with separate image and digest fields — each validated independently by the JSON schema:

{
  "container_pins": {
    "ghcr.io/owner/image:tag": {
      "image": "registry.acme.com/image:tag",
      "digest": "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
    }
  }
}

Changes:

  • Schema (repo_config_schema.json): additionalProperties is now an object with required: [image, digest]; image pattern rejects digest components, digest pattern requires sha256: prefix.
  • RepoConfig: new ContainerPinTarget struct with Image and Digest fields; ContainerPins is now map[string]ContainerPinTarget.
  • getContainerPinMappings: combines the two fields into image@digest for the internal map[string]string used by the resolution machinery — no changes needed downstream.
  • Tests and docs updated to use the new format.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, review the latest open feedback and comments on this PR, address any unresolved items, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · gpt54 6.54 AIC · ⌖ 7.98 AIC · ⊞ 7K ·
Comment /souschef to run again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants