feat(mongodb-runner): authenticate to ECR automatically for DSC clusters - #864
Draft
autarch wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds automatic Amazon ECR authentication to mongodb-runner when launching DSC/SLS compose stacks, removing the need for users to manually run docker login for the default private ECR registry.
Changes:
- Introduces an ECR helper module to detect ECR registries and perform
aws ecr get-authorization-token+docker login --password-stdin. - Wires the login step into SLS option creation with an opt-out (
--slsSkipEcrLogin/ecrLogin). - Updates DSC documentation to describe the automatic behavior and the correct manual fallback.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mongodb-runner/src/sls.ts | Calls ECR login before starting the SLS compose project; adds ecrLogin option. |
| packages/mongodb-runner/src/index.ts | Re-exports ECR helpers/types from the package entrypoint. |
| packages/mongodb-runner/src/ecr.ts | Implements ECR registry parsing and docker login via AWS CLI token. |
| packages/mongodb-runner/src/ecr.spec.ts | Adds unit tests covering ECR parsing and login flows (including failure messaging). |
| packages/mongodb-runner/src/cli.ts | Adds --slsSkipEcrLogin and forwards it into SLS options. |
| packages/mongodb-runner/docs/disaggregated-storage.md | Updates prerequisites/quickstart to reflect automatic login and correct manual command. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+14
to
+15
| const ECR_HOST_RE = | ||
| /^(?<registryId>\d+)\.dkr\.ecr\.(?<region>[a-z0-9-]+)\.amazonaws\.com$/; |
| return { registry: host, registryId, region }; | ||
| } | ||
|
|
||
| /** Injection point for tests; not part of the public API. */ |
Comment on lines
+142
to
+152
| const token = await getAuthorizationToken(registry, execFile); | ||
| const decoded = Buffer.from(token, 'base64').toString('utf8'); | ||
| // The token decodes to `AWS:<password>`, and the password contains colons, | ||
| // so only the first one separates the two fields. | ||
| const separator = decoded.indexOf(':'); | ||
| if (separator === -1) { | ||
| throw new Error( | ||
| `Unexpected ECR authorization token format for ${registry.registry}`, | ||
| ); | ||
| } | ||
| await dockerLogin(registry, decoded.slice(separator + 1), execFile); |
Comment on lines
298
to
302
| const sls = await createSLSMultiCellEnvironment(options); | ||
| // Read the repository back out of the environment we just built so the login | ||
| // target cannot drift from the repository the images are actually pulled from. | ||
| await maybeLoginToEcr(sls.env.SLS_IMAGE_REPO, options.ecrLogin ?? true); | ||
| const projectName = options.projectName ?? `mongodb-runner-sls-${uuid()}`; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Launching a DSC cluster with
--slsComposepulls 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-tokenand pipes the decoded password intodocker login --password-stdin. Non-ECR repositories are skipped without spawning anything, so nothing changes for existing non-DSC use.This makes life much easier for users, and avoids some pitfalls that are easy to fall into when trying to do this manually.
Running
aws ecr get-login-passwordis the wrong It mints a token scoped to the caller's own registry, so from a profile outside account664315256653docker loginrejects it with a barestatus: 400 Bad Requestand no explanation. The correct form passes--registry-idsexplicitly, 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 a new CLI option,
--slsSkipEcrLogin(defaultfalse) to opt out if you have already authenticated another way.The corresponding change in the code is a new
ecrLoginparameter onSLSDisaggregatedStorageSetupOptionswhich defaults totrue.The DSC docs previously showed the incorrect
get-login-passwordincantation; that section is rewritten to describe the automatic behavior and to give the correct manual fallback.Open Questions
errorhandler, because a spawn failure or early exit would otherwise raise an uncaughtEPIPEand kill the runner. The real failure is still reported through theexecFilecallback — worth a look that swallowing there is not hiding anything else.thirdPartyImageRepopointing 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