Skip to content

Conversation

@lsierant
Copy link
Contributor

@lsierant lsierant commented Jul 22, 2025

Summary

This PR adds docker credentials verification in make aws_login to reliably refresh docker credentials only if needed.

Previous method of skipping was checking if the ~/.docker/config.json was modified in the last six hours, but it wasn't reliable enough. If you performed docker login to any other registry or just modified the file, you won't get ECR credentials refreshed.

Proof of Work

breaking docker credentials (simulating expired):

$ cat > ~/.docker/config.json <<EOF
{
  "auths": {
    "268558157000.dkr.ecr.eu-west-1.amazonaws.com": {
      "auth": "INVALID"
    },
    "268558157000.dkr.ecr.us-east-1.amazonaws.com": {
      "auth": "INVALID"
    }
  },
  "currentContext": "desktop-linux"
}
EOF

$ docker pull 268558157000.dkr.ecr.us-east-1.amazonaws.com/dev/mongodb-kubernetes/latest
WARNING: Error parsing config file (/Users/lukasz.sierant/.docker/config.json): illegal base64 data at input byte 4
Using default tag: latest
Error response from daemon: failed to resolve reference "268558157000.dkr.ecr.us-east-1.amazonaws.com/dev/mongodb-kubernetes/latest:latest": pull access denied, repository does not exist or may require authorization: authorization failed: no basic auth credentials

from master - auth is skipped due to file modification time check only

$ make aws_login
Skipping docker daemon check when not running in Linux
Docker credentials are up to date - not performing the new login!

from this PR:

$ make aws_login
Skipping docker daemon check when not running in Linux
Checking if Docker credentials are valid...
Docker login required (HTTP status: 401)
=> Performing docker login to ECR registries
aws-cli/2.27.46 Python/3.13.5 Darwin/24.5.0 source/arm64}
[...]
Login Succeeded
[...]

$ docker pull 268558157000.dkr.ecr.us-east-1.amazonaws.com/dev/mongodb-kubernetes:latest
docker pull 268558157000.dkr.ecr.us-east-1.amazonaws.com/dev/mongodb-kubernetes:latest
latest: Pulling from dev/mongodb-kubernetes
671b7b89b752: Pulling fs layer
[....]

running again skips properly:

$ make aws_login
Skipping docker daemon check when not running in Linux
Checking if Docker credentials are valid...
Docker credentials are up to date - not performing the new login!

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you checked for release_note changes?

@lsierant lsierant self-assigned this Jul 22, 2025
@lsierant lsierant force-pushed the lsierant/check-docker-auth branch from d8212f3 to 31c9dd2 Compare July 22, 2025 09:13
@lsierant lsierant marked this pull request as ready for review July 22, 2025 09:14
@lsierant lsierant requested a review from a team as a code owner July 22, 2025 09:14
ecr_auth=$(jq -r '.auths."268558157000.dkr.ecr.us-east-1.amazonaws.com".auth // empty' ~/.docker/config.json)

if [[ -n "${ecr_auth}" ]]; then
http_status=$(curl -s -o /dev/null -w "%{http_code}" -I --max-time 3 "https://268558157000.dkr.ecr.us-east-1.amazonaws.com/v2/dev/mongodb-kubernetes/manifests/latest" \
Copy link
Contributor

@viveksinghggits viveksinghggits Jul 22, 2025

Choose a reason for hiding this comment

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

nit: Should we use --head instead of -I to make the command more descriptive/readable? I didn't initially understand which flag is making this request a HEAD request, but having --head would help.

@lsierant lsierant force-pushed the lsierant/check-docker-auth branch from b68a2ab to 56e4a34 Compare July 23, 2025 06:34
@lsierant lsierant force-pushed the lsierant/check-docker-auth branch from 56e4a34 to 8bd25af Compare July 25, 2025 07:45
@lsierant lsierant enabled auto-merge (squash) July 25, 2025 07:46
@lsierant lsierant merged commit e2781ef into master Jul 25, 2025
6 of 7 checks passed
@lsierant lsierant deleted the lsierant/check-docker-auth branch July 25, 2025 07:55
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.

5 participants