-
Notifications
You must be signed in to change notification settings - Fork 398
Document and possibly clean up gcr.io authentication support #200
Copy link
Copy link
Closed
Description
As discussed in more detail #195, the code in #191 is non-obvious, it neither documents why it is needed nor carries a test case; it is fairly risky that we will break it over time just because we don’t know what to maintain.
- Most importantly, what is the way the original code failed?
- It is non-obvious that blindly sending a
Basicauth without checking theWWW-Authenticateheader contents at all is necessary; can’t we tell exactly that this is what the server needs? https://bugzilla.redhat.com/show_bug.cgi?id=1409873 (note, not the original bug motivating docker: set basic auth when requesting bearer token #191) mentionsUnimplemented: WWW-Authenticate Bearer replaced by "basic", which suggests there may in fact be some data to base the decision on. - Also, if using
/v2/ping results and assuming that the ping’sWWW-Authenticatevalue is relevant for other URLs is unsound in general (as it seems to be, per docker: fix unauthenticated pulls from gcr.io #195), is there another cleaner approach we could take? E.g. pinging only to determine http/https (then we don’t need care about HTTP response codes at all), and reacting toWWW-Authenticatefor each individual URL separately? (The difficulty with this would be delaying a streamed blob upload until we know it would be accepted—we need the server to give us an “auth OK, go ahead” response.)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels