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.

Sign up for GitHub

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

Jump to bottom

Add more context to errors when obtaining a registry access token #2454

Merged
merged 3 commits into from
Jun 24, 2024
Merged

Add more context to errors when obtaining a registry access token #2454

merged 3 commits into from
Jun 24, 2024

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Jun 17, 2024

It seems that they originate from the authentication endpoint. Add more context to the error there to tell whether it is so.

See individual commits for details.

@mtrmac mtrmac mentioned this pull request Jun 17, 2024
Merged
@mtrmac
Copy link
Collaborator Author

mtrmac commented Jun 18, 2024

For the record, failures with k8s.gcr.io:

  • initializing source docker://k8s.gcr.io/coredns/coredns:sha256-1eeb4c7316bacb1d4c8ead65571cd92dd21e27359f0d4917f1a5822a73b75db1.sig: reading manifest sha256-1eeb4c7316bacb1d4c8ead65571cd92dd21e27359f0d4917f1a5822a73b75db1.sig in k8s.gcr.io/coredns/coredns: manifest unknown: Failed to fetch \"sha256-1eeb4c7316bacb1d4c8ead65571cd92dd21e27359f0d4917f1a5822a73b75db1.sig\" from request \"/v2/coredns/coredns/manifests/sha256-1eeb4c7316bacb1d4c8ead65571cd92dd21e27359f0d4917f1a5822a73b75db1.sig\" (unrelated to this PR)
  • Error copying ref \"docker://k8s.gcr.io/pause:3.1\": initializing source docker://k8s.gcr.io/pause:3.1: decoding bearer token (last URL \"https://accounts.google.com/v3/signin/identifier?continue=https%3A%2F%2Fconsole.cloud.google.com%2Fartifacts%2Fdocker%2Fv2%2Fus-central1%2Fk8s-artifacts-prod%2Fimages%252Ftoken&followup=https%3A%2F%2Fconsole.cloud.google.com%2Fartifacts%2Fdocker%2Fv2%2Fus-central1%2Fk8s-artifacts-prod%2Fimages%252Ftoken&ifkv=…&osid=1&passive=…&service=cloudconsole&flowName=WebLiteSignIn&flowEntry=ServiceLogin&dsh=…\", body start \"<!doctype html><html lang=\\\"en-US\\\" dir=\\\"ltr\\\"><head>\"): invalid character '<' looking for beginning of value" (with this PR, the error message contains the redirected URL and start of the body)

@mtrmac mtrmac changed the title Confirm a hypothesis about the k8s.io failures we see Add more context to errors when obtaining a registry access token Jun 19, 2024
@mtrmac
Copy link
Collaborator Author

mtrmac commented Jun 19, 2024

We seem to have resolved the test failures via containers/skopeo#2356 ; the error reporting improvement is still valuable.

@mtrmac mtrmac marked this pull request as ready for review June 19, 2024 16:01
@mtrmac
Refactor tests of newBearerTokenFromJSONBlob
b47e339 
Make them table-driven where possible. Use testify.

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac
Turn newBearerTokenFromJSON into newBearerTokenFromHTTPResponseBody
b980610 
... so that we have more context for error reporting.

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac
Include more data when we fail to fetch a token
2044bb9 
Signed-off-by: Miloslav Trmač <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Jun 24, 2024

LGTM

@rhatdan rhatdan merged commit 619f28e into containers:main Jun 24, 2024
10 checks passed
@mtrmac mtrmac deleted the confirm-k8s-hypothesis branch June 24, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mtrmac @rhatdan