Move AuthConfig to types/registry, and implement utilities for encoding/decoding#43885
Move AuthConfig to types/registry, and implement utilities for encoding/decoding#43885tianon merged 15 commits intomoby:masterfrom
Conversation
thaJeztah
added
area/api
thaJeztah
force-pushed
the
auth_header_refactor
branch
2 times, most recently
from
cd52009 to
a2a2d43
Compare
thaJeztah
force-pushed
the
auth_header_refactor
branch
from
a2a2d43 to
990c837
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Making the api types more focused per API type, and the general api/types package somewhat smaller. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Based on EncodeAuthToBase64 in docker/cli; https://github.com/docker/cli/blob/1f4111d2bf3dbd9b6f5252c551621af84401b691/cli/command/registry.go#L30-L37 Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
auth_header_refactor
branch
from
990c837 to
7ca66e3
Compare
| // | ||
| // For details on base64url encoding, see: | ||
| // - RFC4648, section 5: https://tools.ietf.org/html/rfc4648#section-5 | ||
| func EncodeAuthConfig(authConfig AuthConfig) (string, error) { |
There was a problem hiding this comment.
Is the CLI code the intended consumer of this function?
There was a problem hiding this comment.
Currently, yes; we can use this function to replace the EncodeAuthToBase64() that's currently living in docker/cli;
https://github.com/docker/cli/blob/1f4111d2bf3dbd9b6f5252c551621af84401b691/cli/command/registry.go#L30-L37
While admittedly, that'd mostly be shifting some code around, I think it makes sense to have the "encoding" and "decoding" logic to be together, and (as it's effectively part of the API's contract in what format to send this information), I think it makes sense to make it part of the API packages (instead of in "an" implementation of a CLI that uses the API).
w.r.t. the "currently" bit in the above; I'm somewhat learning towards making the encoding/decoding part of the client, so that code like https://github.com/docker/cli/blob/f1615facb1ca44e4336ab20e621315fc2cfb845a/cli/command/container/create.go#L127-L136 wouldn't be needed, and the "as-is" AuthConfig (or perhaps some other construct) could be passed as option.
I haven't worked on that yet (this would require changing some function signatures).
Move AuthConfig to types/registry, and implement utilities for encoding/decoding Signed-off-by: CrazyMax <[email protected]>
4 participants
Had this branch lying around for quite a while, so did some dusting off to open it as a PR. See individual commits for details, but this is mostly to;
There's some bits remaining;
AuthConfigimplemented a Stringer interface (and aMarshalText()), but not entirely sure if that's more convenient.- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)