Default to gcloud auth#3282
Conversation
Codecov Report
|
53e0172 to
7aafc03
Compare
|
This is failing with gcloud auth errors on integration tests. PTAL @dgageot |
7aafc03 to
bfa86b6
Compare
|
@balopat I cut the PR in two pieces. The other part needs some more work. This part passes the tests |
briandealwis
left a comment
There was a problem hiding this comment.
So one positive from the previous code was that the credential-helpers were only configured if the registry was a gcr.io repo. And we logged that we were auto-configuring it (since it wasn't in their docker config.json).
Should we also checking for docker-credential-gcr too?
| func (credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) { | ||
| cf, err := loadDockerConfig() | ||
| if err != nil { | ||
| return types.AuthConfig{}, err |
There was a problem hiding this comment.
This means a broken config.json will cause skaffold to abort. Maybe we should log an error, but continue on?
There was a problem hiding this comment.
1/ docker-credential-gcloud should be favored. See https://github.com/GoogleCloudPlatform/docker-credential-gcr "For normal development setups, users are encouraged to use gcloud auth configure-docker, instead"
2/ In the docker build code, we ignore the error, like the Docker CLI does it.
3/ We can't just configure it for gcr.io when GetAllAuthConfigs() is used. When GetAuthConfig is used, configuring the helper for *.gcr.io has the exact same effect
bfa86b6 to
ef6fc8b
Compare
ef6fc8b to
4c76fdf
Compare
Signed-off-by: David Gageot <david@gageot.net>
4c76fdf to
91073a0
Compare
nkubala
left a comment
There was a problem hiding this comment.
I won't lie, I'm always a bit hesitant with auth changes, but everything looks fine to my eye. let's merge this and then make sure and monitor the next release to make sure we're not breaking existing users.
Make sure that
gclouddoesn't need to be configured as a helper to be usedto push to/pull from
*.gcr.ioThis used to be the case but using
GetAllAuthConfigs()overGetAuthConfig(registry string)broke this behaviour.Also use the gcloud auth code from
go-containerregistry