Conversation
1d40c84 to
8a31a21
Compare
d8b031a to
3131910
Compare
There was a problem hiding this comment.
Thanks @yolocs!
It is big PR with a lot of implicit decisions and behaviors. It would help if we could get more documentation about the intended behavior and usage. Could you maybe rewrite your PR description as if it were documentation intended for the user?
For example, the PR adds a new token format but this is documented nowhere currently. The only way to understand the intended usage is to reverse-guess it from the code.
Left a first round of quick feedback.
3d415da to
097fb03
Compare
yolocs
changed the title
cd0907f to
3ad7d11
Compare
3ad7d11 to
f8d8ba8
Compare
f8d8ba8 to
030d3d0
Compare
yolocs
changed the title
5c8926b to
2b5219d
Compare
Approval status: pending
|
2b5219d to
4fa2580
Compare
| setup_docker_auth | ||
|
|
||
| mkdir -p "$HOME/.databricks" | ||
| cp token-cache.json "$HOME/.databricks/token-cache.json" |
There was a problem hiding this comment.
What's the purpose for keeping an existing token cache? What happens if there is no profile (yet)?
There was a problem hiding this comment.
The token cache represents an existing databricks auth login. A logged-in workspace profile is a prerequisite: the helper extracts the workspace ID from the registry hostname, finds the matching profile, and refreshes its cached OAuth token. Without a matching logged-in profile, it fails with guidance to run databricks auth login.
| return runDockerToken(ctx, cmd, loadTokenArgs{ | ||
| authArguments: authArguments, | ||
| tokenTimeout: tokenTimeout, | ||
| // Docker may reuse one credential for a long upload, so maximize its lifetime by refreshing it by default. |
There was a problem hiding this comment.
Is this right? If you do frequent push/pull this will over-refresh.
There was a problem hiding this comment.
It's intentional to work with large image pushes that could span a longer duration.
- (Public) Pulls - Disallowed initially due to an infra restriction. So won't touch this part all.
- Pushes - Much less frequent. And a single
docker pushcommand will only trigger this once and reuse the same credential for all the http requests in that push operation.
4fa2580 to
2a708fb
Compare
2a708fb to
7f52acb
Compare
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
3 participants
🥞 Stacked PR (generated by git ns)
Use this link to review incremental changes.
Changes
Add the experimental
databricks auth docker tokencommand. It implements Docker credential-helperget: reads a registry address from stdin, resolves the matching workspace U2M profile, and returns Docker's credential JSON with a valid OAuth token.Why
Docker supplies only the registry address to a credential helper. Mapping it to a workspace profile lets the CLI refresh short-lived OAuth tokens without storing registry credentials in Docker configuration.
Tests
./task fmt RUFF=ruff,./task checks, and./task lint RUFF=ruff: passgo test ./libs/dockercredentials ./cmd/auth ./cmd/root -count=1: pass./task test: 5,167 tests, 6 skipped, 43 unrelated bundle acceptance snapshot failures on the updated upstream baseThis PR was written by Codex.