Repository navigation
Conversation
…the credentials `sudo finch login` on Linux writes /root/.docker/config.json, which SAM CLI running as the runner cannot read, so tier1-finch image pulls went out anonymously and hit Public ECR rate limits (surfacing as "no such image"). - Create the runner's own ~/.docker/config.json (atomically, 0600) if missing. docker/cli keeps the owner of an existing config.json when a root-privileged `finch login` rewrites it, so the credentials stay readable by the runner. - Run `finch login` without sudo (setup_finch.sh makes `finch` usable without sudo), and fail fast if the config is not readable afterwards. - Mask the ECR token in the GitHub Actions log.
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.
Which issue(s) does this change fix?
No GitHub issue. Fixes the
tier1-finchintegration failures in the nightly-release run (https://github.com/aws/aws-sam-cli/actions/runs/39232160642):tests/integration/local/execution/test_execution.py::TestLocalExecution::test_tier1_executiontests/integration/local/invoke/test_invoke_durable.py::TestInvokeDurable::test_tier1_durable_invoketests/integration/local/callback/test_callback.py::TestLocalCallback::test_tier1_callbackWhy is this change necessary?
The
tier1-finchjob logs in to Public ECR withsudo finch login. On Linux,finch loginis a plain nerdctl login that writes$DOCKER_CONFIG/config.jsonor~/.docker/config.jsonfor the user it runs as. Undersudothat is/root/.docker/config.json.SAM CLI runs as the runner user and talks to finch-daemon through the Docker SDK, which only reads the runner's Docker client config. finch-daemon never reads a credential file itself; it only uses credentials the client sends. So every image pull in the job went out anonymously, hit the Public ECR rate limit for anonymous pulls, and surfaced as
Failed to pull emulator image ... 404 ... no such image. That broke every test that pulls the durable functions emulator image.How does it address the issue?
The job now logs in the way a non-root Finch user on Linux does, so SAM CLI finds the credentials where it already looks. Only
tests/setup_testing_resources.pychanges:~/.docker/config.json({},0600) if it does not exist, atomically.setup_finch.shmakesfinchsetuid, so the login itself runs as root. docker/cli keeps the owner and mode of an existingconfig.jsonwhen it rewrites it, so the file stays owned by and readable to the runner. Only a first-time root login creates a root-owned file.finch loginwithoutsudo.setup_finch.shalready makesfinchusable without sudo, and this matches the no-sudo setup the job uses.The Docker login path is unchanged apart from the token masking.
Integration test run on this branch: https://github.com/aws/aws-sam-cli/actions/runs/39863937466
tier1-finch(https://github.com/aws/aws-sam-cli/actions/runs/39863937466/job/120121288143): 72 passed, 1 failed, down from 3 failures. The login step writes the credentials to/home/runner/.docker/config.json, the emulator image pulls (Successfully pulled image public.ecr.aws/durable-functions/aws-durable-execution-emulator:latest), and no rate-limit or "no such image" errors remain.test_tier1_durable_invoketest_tier1_callbacktest_tier1_executionRemaining failures in this run (not caused by this change):
test_tier1_executionontier1-finch, and the same assertion on the Docker jobslocal-invoke(https://github.com/aws/aws-sam-cli/actions/runs/39863937466/job/120121288233;test_execution_nonexistent_execution_{get,history,stop}) andtier1-windows-other(https://github.com/aws/aws-sam-cli/actions/runs/39863937466/job/120121288149). The test runssam local execution geton a nonexistent execution ID and compares the error text exactly. It expectsExecution 00000000-0000-0000-0000-000000000000 not found, but the emulator (pulled as:latest) now returnsDurable Execution does not exist. This happens on Docker too, so it is not Finch-specific. Before this change, the pull failure hid it on Finch. To be fixed separately, by relaxing the assertion or pinning the emulator image.build-x86-container-1(https://github.com/aws/aws-sam-cli/actions/runs/39863937466/job/120121288195):TestBuildWithNestedStacks::test_nested_build_invoke_in_container_1_in_containerfailed withJSONDecodeError. This is a Docker job; this change does not touch its login or build path.What side effects does this change have?
~/.docker/config.jsoninstead of/root/.docker/config.json. The runner is ephemeral.finchis no longer usable without sudo), the setup step now fails clearly instead of the tests failing later with misleading pull errors.Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.