GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add ECR auto-refreshing upstream authentication by andrew · Pull Request #278 · git-pkgs/proxy

Conversation

@andrew

@andrew andrew commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Adds type: ecr to upstream.auth so the proxy can front private AWS ECR registries without a static credential in config. On first request (and again shortly before the 12-hour expiry) it calls ecr:GetAuthorizationToken via the AWS SDK default credential chain and sends the result as HTTP Basic to the ECR host; the existing OCI Bearer-challenge handling in internal/httpclient takes it from there.

upstream:
  oci:
    ecr: "https://123456789012.dkr.ecr.eu-west-1.amazonaws.com"
  auth:
    "https://123456789012.dkr.ecr.eu-west-1.amazonaws.com":
      type: ecr
      region: eu-west-1

AWS credentials come from the SDK default chain (IRSA on EKS, EC2/ECS instance profiles, AWS_* env vars, ~/.aws/credentials). The identity needs ecr:GetAuthorizationToken plus pull permissions on the target repositories. region is optional and falls back to the SDK default resolution.

Implementation: AuthConfig gains a Region field; Server.authForURL routes type: ecr to a small per-region token cache in internal/server/ecr_auth.go that mirrors the expiry-with-skew pattern already used by the OCI token cache in internal/httpclient/transport.go. AuthConfig.Header() and the AuthFunc signature are unchanged; on SDK error the provider logs and returns no header so the upstream 401 surfaces normally.

Dependency impact: aws-sdk-go-v2 is already linked via gocloud.dev/blob/s3blob, so this only adds github.com/aws/aws-sdk-go-v2/service/ecr. Stripped linux binary grows from 35,848,352 to 36,044,960 bytes (+192 KB, +0.55%); go.sum net +2 lines.

Fixes #276.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@andrew andrew mentioned this pull request Aug 21, 2026
ECR authentication #276
Closed
@andrew
andrew requested a lite review from Copilot August 21, 2026 08:26
Copilot started reviewing on behalf of andrew August 21, 2026 08:26
View session
Copilot AI reviewed Aug 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull request overview

Adds automatic AWS ECR authentication for OCI upstreams with region-scoped token caching and refresh.

Changes:

  • Adds type: ecr with optional AWS region configuration.
  • Implements ECR token retrieval through the AWS SDK credential chain.
  • Adds tests, dependencies, documentation, and configuration examples.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Summary
internal/server/server.go Integrates ECR authentication routing.
internal/server/ecr_auth.go Fetches and caches ECR tokens. Critical: error handling can trigger an incorrect Bearer retry instead of preserving the original 401 (1 vote). Moderate: concurrent cache misses can cause redundant token requests; refreshes should be coordinated (4 votes).
internal/server/ecr_auth_test.go Tests ECR token behavior, caching, refresh, and routing.
internal/config/config.go Adds ECR authentication and region configuration.
go.sum Updates dependency checksums.
go.mod Adds AWS ECR SDK dependencies.
docs/configuration.md Documents ECR configuration and permissions.
config.example.yaml Adds an ECR configuration example.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread internal/server/ecr_auth.go Outdated
Comment on lines +62 to +64
if err != nil {
e.logger.Error("fetching ECR authorization token", "region", region, "error", err)
return "", ""

@andrew andrew Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is the existing behaviour of internal/httpclient.Transport.RoundTrip for every auth type, not something the ECR provider introduces: on a 401 with a Bearer challenge it drains the original response and returns registry authentication: %w when the token fetch fails (transport.go:89-93). A misconfigured type: basic against ECR or GHCR takes the identical path today. The provider logs the SDK error (no credentials, AccessDenied, etc) before returning empty, which is more actionable than the bare 401 would be. Changing the surfaced status means changing RoundTrip to return the original 401 when the token fetch fails, which affects every registry and is out of scope here; updated the doc comment on header() to describe the actual failure path.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread internal/server/ecr_auth.go Resolved

Uh oh!

There was an error while loading. Please reload this page.
andrew added 2 commits August 21, 2026 09:34
@andrew
Add ECR auto-refreshing upstream authentication
7b56989 
- Add "ecr" auth type to upstream.auth config with optional region
- Cache ecr:GetAuthorizationToken results per region and refresh
  shortly before expiry via the AWS SDK default credential chain
- Route type: ecr through the token cache in Server.authForURL
- Document the new type in config.example.yaml and docs/configuration.md

Fixes #276
@andrew
Collapse ecrTokens.header to a single return path
b907438 
Drops the internal/server package below the goconst min-occurrences
threshold for the Authorization literal.
@andrew
andrew force-pushed the andrew/ecr-auth branch from 62b5364 to b907438 Compare August 21, 2026 08:36
@andrew
Coalesce concurrent ECR token fetches with singleflight
4a1cee7 
Concurrent cache misses for the same region now share a single
GetAuthorizationToken call instead of each issuing their own, avoiding
a request burst against the ECR API at cold start and at each 12-hour
refresh. golang.org/x/sync is already a direct dependency.
er0k
er0k reviewed Aug 23, 2026
View reviewed changes
Comment thread config.example.yaml Outdated
# chain, so IRSA / instance profiles / AWS_* env vars all work)
# "https://123456789012.dkr.ecr.eu-west-1.amazonaws.com":
# type: ecr
# region: eu-west-1

@er0k er0k Aug 23, 2026

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

it would be nice if this fell back to the region in the URL if not set in config

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
er0k
er0k approved these changes Aug 23, 2026
View reviewed changes

@er0k er0k left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

works as expected, looks good to me 🚀 thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
andrew added 2 commits August 24, 2026 10:01
@andrew
Improve ECR token refresh and region inference
b5e1ffe
@andrew
Back off failed ECR token refreshes
c62a57d
@andrew
andrew merged commit b5ee6dd into main Sep 2, 2026
8 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@andrew
andrew deleted the andrew/ecr-auth branch September 2, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@er0k er0k er0k approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ECR authentication

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@andrew @er0k
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.