-
Notifications
You must be signed in to change notification settings - Fork 258
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
Added docker_config_json parameter #317
base: master
Are you sure you want to change the base?
Conversation
This means you can specify a full ~/.docker/config.json file to use for authentication, which is useful to work around Docker Hub's rate-limiting. You can pass this in (e.g. from a secrets source such as Credhub or k8s secrets), and authenticate against multiple registries. Also added GODEBUG="x509ignoreCN=0" env var to work around "legacy CN" format SSL certificates warning, introduced with latest version of Go SSL library.
This is a work-around for #278 which has recently bitten me due to Docker Hub's rate-limiting. I needed to be able to drop a raw docker Hope this helps someone in any case! |
Hi, thx for the PR. Have you tried https://github.com/concourse/registry-image-resource? We encourage users to move to If that works in your case I'd rather hold on this PR since all your credetials will be stored plain text in the config file, refer to #243 |
@xtremerui The registry-image-resource doesn't really work for me, as it doesn't deal with building containers using public base images. Which is where I was being badly bitten by Docker Hub's rate-limiting when not authenticating as I'm behind CGNAT and presumably sharing my public IP with 100s/1000s of other users. I could I guess hook registry-image up to the oci-build resource but that's somewhat in flux at the moment and doesn't really offer anything at the moment over this resource as far as I can see (e.g. it still requires I understand the hesitation to merge given the config file being written to disk, but then as far as I understand it, that's still what happens with the "docker login" approach. Although I agree it's not best practice to leave credentials floating around on disk. I'm fine with whatever you choose to do though :) The fact that this PR exists and is linked in the associated issue is enough for me, hopefully it'll help someone else with a workaround in the worst-case scenario. |
@@ -27,6 +27,8 @@ Note: docker registry must be [v2](https://docs.docker.com/registry/spec/api/). | |||
* `aws_access_key_id`: *Optional.* AWS access key to use for acquiring ECR | |||
credentials. | |||
|
|||
* `docker_config_json` : *Optional.* The raw `config.json` file used for authenticating with Docker registries. If specified, `username` and `password` parameters will be ignored. You may find this useful if you need to be authenticated against multiple registries (e.g. pushing to a private registry, but you also also need to pull authenticate to pull images from Docker Hub without being rate-limited). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `docker_config_json` : *Optional.* The raw `config.json` file used for authenticating with Docker registries. If specified, `username` and `password` parameters will be ignored. You may find this useful if you need to be authenticated against multiple registries (e.g. pushing to a private registry, but you also also need to pull authenticate to pull images from Docker Hub without being rate-limited). | |
* `docker_config_json` : *Optional.* The raw `config.json` file used for authenticating with Docker registries. If specified, `username` and `password` parameters will be ignored. You may find this useful if you need to be authenticated against multiple registries (e.g. pushing to a private registry, but you also need to pull authenticate to pull images from Docker Hub without being rate-limited). |
log_in "$username" "$password" "$registry" | ||
else | ||
docker_config_json_to_file "$docker_config_json" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an out test to cover this feature?
@@ -2,6 +2,9 @@ LOG_FILE=${LOG_FILE:-/tmp/docker.log} | |||
SKIP_PRIVILEGED=${SKIP_PRIVILEGED:-false} | |||
STARTUP_TIMEOUT=${STARTUP_TIMEOUT:-120} | |||
|
|||
# Otherwise we get "certificate relies on legacy Common Name field" | |||
export GODEBUG="x509ignoreCN=0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what user case that you need this env var? Are you seeing the error when check/in/out?
From what I understand in this resource, only the check cmd is using Go, so in/out that uses bash script should not be affected?
This means you can specify a full ~/.docker/config.json file to use for
authentication, which is useful to work around Docker Hub's rate-limiting.
You can pass this in (e.g. from a secrets source such as Credhub or k8s
secrets), and authenticate against multiple registries.
Also added GODEBUG="x509ignoreCN=0" env var to work around "legacy CN"
format SSL certificates warning, introduced with latest version of Go SSL
library.