Skip to content

feat(mongodb-runner): authenticate to ECR automatically for DSC clusters - #875

Open
autarch wants to merge 3 commits into
mainfrom
08-28-feat_mongodb-runner_authenticate_to_ecr_automatically_for_dsc_clusters
Open

feat(mongodb-runner): authenticate to ECR automatically for DSC clusters#875
autarch wants to merge 3 commits into
mainfrom
08-28-feat_mongodb-runner_authenticate_to_ecr_automatically_for_dsc_clusters

Conversation

@autarch

@autarch autarch commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Description

Launching a DSC cluster with --slsCompose pulls SLS images from a private ECR registry (664315256653.dkr.ecr.us-east-1.amazonaws.com), but authenticating to it was left entirely to the caller. mongodb-runner now does it itself.

Before starting the compose project, the runner parses the registry host out of the configured image repository and, if it is an ECR registry, shells out to aws ecr get-authorization-token and pipes the decoded password into docker login --password-stdin. Non-ECR repositories are skipped without spawning anything, so nothing changes for existing non-DSC use.

This makes life easier for users and avoids some pitfalls that are easy to fall into when trying to do this manually.

Running aws ecr get-login-password is the wrong command. It mints a token scoped to the caller's own registry, so from a profile outside account 664315256653 docker login rejects it with a bare status: 400 Bad Request and no explanation. The correct form passes --registry-ids explicitly, which is what the runner does.

Extracting the password out of the decoded token is also easy to get wrong.

When the login fails, the tool will produce a useful explanation of why.

This adds a new CLI option, --slsSkipEcrLogin (default false), which disables the new auth behavior. The corresponding change in the code is a new ecrLogin parameter on SLSDisaggregatedStorageSetupOptions which defaults to true.

The DSC docs previously showed the incorrect get-login-password incantation; that section is rewritten to describe the automatic behavior and to give the correct manual fallback.

Open Questions

  • The password is written to the child's stdin and the stream has a no-op error handler, because a spawn failure or early exit would otherwise raise an uncaught EPIPE and kill the runner. The real failure is still reported through the execFile callback — worth a look that swallowing there is not hiding anything else.
  • Login is attempted only for the SLS image repository. An explicitly-set thirdPartyImageRepo pointing at a different ECR registry would not get a login. With the default it resolves to the same host, so this is not reachable today. I'm not sure what the point of this parameter is anyway. Under what circumstances could you set this to a different repo and get a working SLS stack?

Checklist

I work for MongoDB. I don't think I need to sign the CLA.

Copilot AI lite review requested due to automatic review settings September 2, 2026 21:07
@autarch autarch changed the title 08 28 feat mongodb runner authenticate to ecr automatically for dsc clusters feat(mongodb-runner): authenticate to ECR automatically for DSC clusters Sep 2, 2026
@autarch
autarch marked this pull request as ready for review September 2, 2026 21:08
@autarch
autarch requested review from a team as code owners September 2, 2026 21:08
@autarch
autarch requested review from Sgrinfy and dudaschar September 2, 2026 21:08

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.

🟡 Changes recommended

The new docker login error handling currently reads stderr from the error object rather than the execFile callback argument, which can drop the real failure output and undermine the PR’s goal of producing clearer login failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds automatic authentication to Amazon ECR when launching DSC/SLS clusters via mongodb-runner --slsCompose, so pulling private SLS images no longer requires manual docker login setup.

Changes:

  • Add an ECR-aware login helper (maybeLoginToEcr) that shells out to aws ecr get-authorization-token and logs Docker in via --password-stdin.
  • Wire ECR login into the SLS option builder with a new opt-out (--slsSkipEcrLogin / ecrLogin).
  • Add unit tests for ECR parsing/login behavior and update DSC documentation to reflect the new automatic behavior and correct manual fallback.
File summaries
File Description
packages/mongodb-runner/src/sls.ts Introduces an ecrLogin option and performs ECR login before starting the SLS compose project.
packages/mongodb-runner/src/cli.ts Adds --slsSkipEcrLogin and passes the corresponding ecrLogin setting into SLS setup.
packages/mongodb-runner/src/ecr.ts New ECR parsing + login implementation using AWS CLI token retrieval and docker login --password-stdin.
packages/mongodb-runner/src/ecr.spec.ts Adds unit tests covering ECR host parsing and login/skip behavior via injected execFile.
packages/mongodb-runner/src/index.ts Re-exports the new ECR utilities from the package entrypoint.
packages/mongodb-runner/docs/disaggregated-storage.md Updates prerequisites/quick start docs for automatic ECR auth and correct manual login command.
Review details

Suppressed comments (1)

packages/mongodb-runner/src/ecr.ts:112

  • execFile provides stderr as the third callback argument; reading (err as { stderr?: string }).stderr will typically be undefined, so the failure output from docker login may be lost. Capture stderr from the callback args (and consider mentioning the programmatic opt-out as well as the CLI flag).
          const stderr = String(
            (err as { stderr?: string }).stderr ?? '',
          ).trim();
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment on lines +76 to +80
`The AWS CLI is required to authenticate to ${registry.registry}. ` +
`Install it, or authenticate manually with:\n ${manualLoginCommand(
registry,
)}\nIf you have already authenticated another way, pass --slsSkipEcrLogin.`,
),
@autarch
autarch requested a review from mpobrien September 4, 2026 16:15
@autarch
autarch force-pushed the 08-28-feat_mongodb-runner_authenticate_to_ecr_automatically_for_dsc_clusters branch from e4663e8 to 5f9b62d Compare September 11, 2026 19:03
@nbbeeken
nbbeeken removed the request for review from Sgrinfy September 11, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants