Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Amazon ECR Docker Credential Helper #676

Open
pda opened this issue Mar 11, 2020 · 4 comments
Open

Add Amazon ECR Docker Credential Helper #676

pda opened this issue Mar 11, 2020 · 4 comments
Labels
docker Relates to the use of Docker on stack agents

Comments

@pda
Copy link
Member

pda commented Mar 11, 2020

We could perhaps use this instead of the ECR plugin:
https://github.com/awslabs/amazon-ecr-credential-helper

I think it would be a more elegant solution, but there might be other considerations…?

@toolmantim
Copy link
Contributor

I don’t see why not

@bshelton229
Copy link
Contributor

I got a PR started (#763). I'm not sure if it's the correct approach, but it's a start. We'd like to explore this as maintaining a cache between steps would help us not run into rate limits occasionally as well as speed things up. It also means we don't need to know all the ECR registry IDS ahead of time or put them explicitly in a separate plugin definition in our steps.

@keithduncan keithduncan added the docker Relates to the use of Docker on stack agents label Sep 8, 2021
@keithduncan keithduncan changed the title Consider adding Amazon ECR Docker Credential Helper Add Amazon ECR Docker Credential Helper Sep 8, 2021
@jamestelfer
Copy link

At the moment if the ecr-login helper is configured at the credsStore or credHelper level, an explicit docker login causes it to fail.

We could work around this:

  1. Require steps to switch from explicit login to using the helper (via an explicit plugin reference)
  2. Work around the issue via a helper script as suggested in the ecr-credential-helper issue
  3. Allow the helper to behave differently and ignore these hooks.

We're working on (3) by getting this behaviour changed in the ecr-helper itself (awslabs/amazon-ecr-credential-helper#847)

This should allow for the elastic stack to install the helper in the base AMI, and optionally allow it to be configured in the Docker config.json.

I recommend configuring it for an allow list of ECR repos, rather than as the default. Configuring ecr-login using credStore isn't the optimal way to work with this plugin, as it would compromise other repo usage.

It would be better to install it for an allow list of repositories, like:

  "credHelpers": {
    "<account_id_here>.dkr.ecr.us-west-2.amazonaws.com": "ecr-login",
    "public.ecr.aws": "ecr-login"
  },

This could perhaps be configured in the Elastic stack parameters, or become part of the default environment hook.

Ref: awslabs/amazon-ecr-credential-helper#847

@jamestelfer
Copy link

jamestelfer commented Sep 19, 2024

AWS have just released v0.9.0 with the referenced issue fixed.

This means that if the environment variable AWS_ECR_IGNORE_CREDS_STORAGE is set to true, the plugin will ignore requests to store credentials. This makes it compatible with the ecr plugin.

For Culture Amp builds at the moment we install the plugin by default and allow builds to opt-in to use it (because of the compatibility issue). If a build exposes an ENABLE_ECR_LOGIN environment variable in their build, we'll register it for our ECR locations. We use a helper script (included below) to make the necessary changes.

Now, though, we can enable it by default and start removing references to the ECR login from our pipelines.

If this were to be included in the Elastic Stack:

  • it should be enabled at credHelper level (per ECR URL) rather than credStore level (globally). While the environment variable allows the login to succeed, it doesn't actually store anything. This would cause incompatibility with other uses of docker login, in particular when logging in to Docker Hub.
  • The set of URLs enabled for a stack could be supplied as a parameter. Configuration changes would need to be per build, as the DOCKER_CONFIG variable is local to the build. I haven't looked around, but perhaps the bootstrap could create a default configuration that every build copies when setting up?
Expand for register_docker_credential_helper script
#!/bin/bash
set -euo pipefail

# USAGE:
# register_docker_credential_helper <URL> <HELPER>
#
# e.g. register ecr login helper for the CI account:
#  register_docker_credential_helper "226140413739.dkr.ecr.us-west-2.amazonaws.com" "ecr-login"
#
# e.g. register the env login helper for the public docker registry:
#  register_docker_credential_helper "https://index.docker.io/v1/" "env"

main() {
  local url="$1"
  local helper="$2"

  register "${url}" "${helper}"
}

register() {
  local url="$1"
  local target="$2"

  # find config location
  docker_config=${DOCKER_CONFIG:-$HOME/.docker}
  config_file="$docker_config/config.json"

  # ensure the configuration exists and has valid but empty content
  mkdir -p "$docker_config"
  if [ ! -f "${config_file}" ]; then
    echo '{}' > "${config_file}"
  fi

  # register credential helper in docker config
  temp_json="$(mktemp)"

  jq \
    --arg url "${url}" \
    --arg target "${target}" \
    '.credHelpers += { ($url): $target  }' \
    "${config_file}" > "${temp_json}"

  mv "${temp_json}" "${config_file}"

  # show configuration in build output
  cat "${config_file}"
}

main "$@"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Relates to the use of Docker on stack agents
Projects
None yet
Development

No branches or pull requests

5 participants