Skip to content

Document and possibly clean up gcr.io authentication support #200

Closed
@mtrmac

Description

@mtrmac

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 Basic auth without checking the WWW-Authenticate header 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) mentions Unimplemented: 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’s WWW-Authenticate value 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 to WWW-Authenticate for 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.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions