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

"https://" prefix on push config is required on some systems and not on others #472

Closed
AaronFriel opened this issue Jan 28, 2023 · 5 comments
Assignees
Labels
4.x.x kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Milestone

Comments

@AaronFriel
Copy link
Contributor

I found that when using ECR login using the credential tool, I received a confusing error message: "no basic auth credentials".

Steps to reproduce

  1. Log in via aws ecr get-login-password.
aws ecr get-login-password --region us-west-2 | docker login --username AWS --password-stdin [registry hostname]

This created an authconfig entry in docker/config.json for [registry hostname] without https://.

  1. Use a program to construct an ECR Registry and v4 Image

Expected behavior

Successful push.

Actual behavior

Error "no basic auth credentials".

Root causing

The registryServer on line 210 here began with https://, but the authConfigs map contained only hostnames without https://:

registryServer, err := getRegistryAddrForAuth(img.Registry.Server, img.Name)
if err != nil {
return "", nil, err
}
// we use the credentials for the server declared in the program, looking them up from the host authConfigs.
pushAuthConfig = authConfigs[registryServer]

@mikhailshilkov
Copy link
Member

@AaronFriel Could you please add a kind/* label? That helps keep our reporting clean. Thank you!

@AaronFriel AaronFriel added kind/enhancement Improvements or new features 4.x.x labels Feb 1, 2023
@guineveresaenger
Copy link
Contributor

For ACR and Dockerhub, the authConfigs map does need the https:// prefix for lookup.

A side note: I am fairly certain most users would pass in the registry username and password/token-as-password directly from creating the registry inline in the program.

@guineveresaenger
Copy link
Contributor

Update:

This appears to be a subtle difference in the credential helpers used to call GetAllCredentials between either OSes or docker engine versions.

Solution to move forward: Try both lookups in authConfig.

@guineveresaenger guineveresaenger changed the title Pushing to ECR requires errors due to "https://" prefix on push config "https://" prefix on push config is required on some systems and not on others Feb 2, 2023
@guineveresaenger guineveresaenger self-assigned this Feb 2, 2023
@guineveresaenger guineveresaenger added this to the 0.84 milestone Feb 3, 2023
@guineveresaenger
Copy link
Contributor

Fixed in #480

@pulumi-bot
Copy link
Contributor

Cannot close issue without required labels: resolution/

@guineveresaenger guineveresaenger added the resolution/fixed This issue was fixed label Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x.x kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants