Repository navigation
fix(auth): report why credentials failed, and as whom (0.22.3) - #183
Merged
Merged
Conversation
`dt auth credentials install bcarc_wts` answered a real failure with:
Error: No credentials were installed (no usable secrets found).
Five causes collapse into that sentence -- denied, absent, empty, legacy
format, unrecognised format -- and every one was printed only under -v.
The reason was known and discarded, so diagnosing it took a round trip
through a collaborator on another continent.
A denial is also uninterpretable on its own. GCP returns PERMISSION_DENIED
for secrets that do not exist, and dt may be authenticating as a service
account inherited from GOOGLE_APPLICATION_CREDENTIALS rather than as the
human, in which case "I already granted her access" and "permission denied"
are both true. So report the identity on every fetch, not on request:
Authenticating to GCP as: service account ctp-archive@... (from
$GOOGLE_APPLICATION_CREDENTIALS=/g/data/a56/dvc/config/...json)
Reading secrets from project: bcarc-489101
! This is a service account, not your user account -- IAM grants
made to your email do not apply to it.
Changes:
* Per-repo failure reasons print unconditionally. A failure reason is not
verbose-level information.
* The summary AuthError repeats every reason and the identity, so the
exception alone is enough to diagnose from -- stdout gets piped away,
and collaborators paste the last line.
* GCPIdentity resolves who we authenticate as in true ADC precedence
(GOOGLE_APPLICATION_CREDENTIALS, then the ADC file, then the gcloud CLI
account used by the fallback), from files only, no network call. It
admits when the email is unknowable rather than showing the gcloud CLI
account, which is a different identity and often a different person.
* _get_secret_backend hands its already-resolved gcloud account to the
backend, so reporting the identity costs no second `gcloud auth list`
(~20s cold on NCI).
* auth_setup no longer prints "No S3 endpoints -- credential install
skipped" when endpoints exist but GCP auth failed. It said the repo had
no S3 remote when it plainly did, inviting users to dismiss the real
error printed underneath.
* The GCP auth warning in auth_setup prints without -v, matching the
error already surfaced in the report.
2208 passed, 2 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two ways a GCP denial stayed invisible.
1. The Python client's PermissionDenied was swallowed whenever the gcloud
CLI fallback succeeded. Verified against the real backend: reading
dvc-remote-bcarc_wts from bcarc-489101 as the ctp-archive service
account is refused, the fallback rescues it as the human, and the user
sees nothing at all. The broken ADC identity stays in place until it is
used somewhere with no fallback -- a container without gcloud -- where
it resurfaces as a mystery.
2. When the fallback was *also* refused, only the CLI's denial was raised.
The library denial, under a different identity, was dropped. Showing
one account invites "but I granted that account access" about the
other.
Now every denial is echoed as it happens, naming the identity refused:
! Permission denied reading secret 'dvc-remote-bcarc_wts' from GCP
project 'bcarc-489101'
authenticated as: service account ctp-archive@... (from
$GOOGLE_APPLICATION_CREDENTIALS=/g/data/a56/dvc/config/...json)
This is a service account, not your user account -- IAM grants
made to your email do not apply to it.
Unset GOOGLE_APPLICATION_CREDENTIALS to authenticate as
yourself instead.
retrying via the gcloud CLI, which may use a different account ...
and when both are refused the error names both, via "Refused for:" and
"Also refused for:".
_cli_identity_desc reports the *CLI* account, which is deliberately
distinct from GCPIdentity: when both exist they are often different
accounts, and that divergence is the whole reason to attach an identity
to a denial. It reuses the cached account where possible so an error path
costs no extra `gcloud auth list`.
NOT_FOUND is untouched -- it is not a denial and does not echo one.
2222 passed, 2 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things.
1. Denial echoes and per-repo failure reasons move to stderr. They are
diagnostics and must survive `dt ... > out` or `| head`, which is how
they get lost before reaching whoever is helping.
2. `dt auth credentials list` is now suggested wherever a read fails --
the denied message, both NOT_FOUND messages, and the install summary.
It answers the question the messages structurally cannot: a denial and
an absence are the same PERMISSION_DENIED, so a secret named
differently from its repo, or a typo, is indistinguishable from a
missing grant until you list what is actually visible.
3. Audited the rest of `dt auth credentials`. migrate, set, status and
uninstall already converted SecretError to AuthError and were fine.
Two were not:
* `list` raised a bare "gcloud error listing secrets: <stderr>" with no
project, no identity, and no handling of PERMISSION_DENIED. It now
names both and points out that secretmanager.secrets.list is a
separate grant from reading any single secret -- being able to read
one does not imply being able to list.
* `check` reported "secret not found" whenever existence could not be
determined, including denials. That sends people off to create a
secret that already exists. It now distinguishes "could not confirm
the secret exists" from a genuine absence, and suggests listing in
the latter case.
`list` output also names the project it read, because secrets.gcp.project
can differ by config scope between two people on one machine, and a bare
list of names hides which project produced it.
`install`'s summary said "Not found" for every failure; failures may be
denials, empty secrets, or wrong formats. Now "Failed".
2239 passed, 2 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The report that prompted this
That is the entire diagnostic output. Five distinct causes collapse into that one sentence — denied, absent, empty, legacy DVC-INI format, unrecognised format — and each was printed only under
-v. The reason was known at the point of failure and thrown away.A denial is not self-explanatory either. GCP returns
PERMISSION_DENIEDfor secrets that do not exist, and dt may be authenticating as a service account inherited fromGOOGLE_APPLICATION_CREDENTIALSrather than as the human — in which case "I already granted her access" and "permission denied" are simultaneously true. We lost days to exactly that.Now
No
-vrequired.Changes
AuthErrorrepeats every reason and the identity. stdout gets piped away, scrolled past, and pasted selectively by collaborators — the exception alone now suffices.GCPIdentityresolves who we authenticate as in true ADC precedence:GOOGLE_APPLICATION_CREDENTIALS→ ADC file → the gcloud CLI account used by the fallback. Files and environment only, no network call, so it is cheap enough to report every time._get_secret_backendhands over the account it already resolved, so reporting costs no secondgcloud auth list(~20s cold on NCI).auth_setupno longer claims "No S3 endpoints" when endpoints exist but GCP auth failed. It told users their repo had no S3 remote when it plainly did, and invited them to dismiss the real error printed directly underneath.auth_setupprints without-v, matching the error already surfaced in the report.A deliberate omission
For user ADC we report
user login (from ADC file ...; the file does not record which account)rather than an email.gcloud auth application-default logingenuinely does not record one. The tempting fix — showing thegcloud auth listaccount — would be wrong: that is a different identity, and the case where they diverge is precisely the bug worth catching.Verification
tests/unit/test_credential_error_reporting.pyXDG_CONFIG_DIRS, isolatedHOME, no gcloud login) and against all three real identity paths on Gadi: GAC service account, user ADC, and CLI fallback.🤖 Generated with Claude Code