Add opt-in ADC reauth preflight (--reauth=off|auto) - #93
Merged
Merged
Conversation
Detect invalid_grant with error_subtype invalid_rapt or rapt_required on a token fetch before creating the Spanner client. --reauth=off (default) only appends a google-auth-style hint. --reauth=auto may run gcloud application-default login once when user ADC is applicable, then reload credentials. Never replay SQL, DML, or already-started RPCs. Skip automatic login when emulator/GAC/CLOUDSDK_CONFIG is set, ADC is not a writable authorized_user file, stdin/stderr are not terminals, gcloud is missing, or tests inject client options. Login is human-paced and does not consume --timeout. Validation: go build, package tests, go test -run Reauth|Auth, full emulator suite, and golangci-lint with GOTOOLCHAIN=go1.25.13.
signal.NotifyContext keeps capturing SIGINT until stop() is called, so a second Ctrl-C would be swallowed while a cancelled login or query is still unwinding. Call stop() as soon as ctx is cancelled so the second interrupt terminates the process as before.
A review suggested that passing the resolved gcloud.cmd path to exec.CommandContext cannot start the batch wrapper on Windows. It can: CreateProcess launches .cmd/.bat files through cmd.exe implicitly, which is why exec.LookPath resolves PATHEXT batch extensions and why os/exec documents cmd.exe quoting as a caveat rather than a limitation. Record that reasoning at the call site, together with why the cmd.exe unquoting differences are irrelevant for this fixed argument vector, so future reviews do not repeat the concern. Windows runtime behavior remains unverified in CI.
mergedParams and GenerateParams ran after maybeAuthPreflight, so a parameter file edited while the user completed a browser login could change the executed statement. Build the spanner.Statement (SQL and parameters) before any interactive step, as the design note's frozen execution spec requires. Behavior is unchanged when --reauth=off.
Resolve the runCLI overlap with the split-output change: freeze SQL and parameters, open the output sinks (destination validation and temp files), then run the reauth preflight and start the query timeout. go.mod/go.sum re-tidied on top of main (x/term v0.43.0; x/sys stays at main's v0.45.0).
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.
Summary
--reauth=off|auto(defaultoff, envEXECSPANSQL_REAUTH) so Workspace session-policy refresh failures (invalid_grant+invalid_rapt/rapt_required) can be classified before any Spanner RPC.offkeeps today's client construction (noDetectDefault, noWithAuthCredentials) and only appends a google-auth-style hint on classified or message-only reauth errors.autois explicit consent for one interactivegcloud auth application-default loginduring a preflight token fetch, then ADC reload and a second token check, then client creation with the reloaded credentials.Flag contract
off(default)auto--timeoutstarts after authentication. Login is human-paced and does not consume or extend the query deadline. SIGINT still cancels the login and later execution.automeans continue as whoever completes the login. The previous ADC principal is not compared.Safety boundaries / what is never retried
runCLIloop after a failed RPC.Unauthenticatedduring a long-running statement is reported with the hint and a non-zero exit.invalid_rapt/rapt_requiredare hint-only; they never trigger login.exec.CommandContext(stdout attached to stderr). No shell string is built.Applicability conditions
Automatic login runs only when all of the following hold:
--reauth=autoSPANNER_EMULATOR_HOSTunsetGOOGLE_APPLICATION_CREDENTIALSunsetCLOUDSDK_CONFIGunset (gcloud honors it; the Go auth library always reads the well-known ADC path)"type"isauthorized_usergolang.org/x/term.IsTerminal; stdout may be a pipe)gcloudis found byexec.LookPathrunCLIclient-option list skips preflight)If
CLOUDSDK_CONFIGorGOOGLE_APPLICATION_CREDENTIALSis set, the hint names that variable.Tests run
go build ./...go test ./params/... ./jqresult/... ./resultset/...go test -count=1 -run 'Reauth|Auth' .(Colima emulator env)golangci-lint runGOTOOLCHAIN=go1.25.13 golangci-lint runDOCKER_HOST=unix://$HOME/.colima/default/docker.sock TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE=/var/run/docker.sock go test ./...go run . --help--reauth="off"and$EXECSPANSQL_REAUTHshown)go mod tidycloud.google.com/go/auth,golang.org/x/oauth2,golang.org/x/term) and thegolang.org/x/sysindirect bump required byx/termCoverage includes typed classifier cases (
auth.Error,oauth2.RetrieveError, revokedinvalid_grant, non-JSON body, hint-only gRPCUnauthenticated), orchestration fakes (off/auto, login failure, unchanged/wrong-type/unreadable reload, second-fetch failure, spent budget, quota-project change), applicability viat.Setenvand tempHOME, a PATH-first fakegcloudthat must not write to process stdout, and a transport test that--reauth=autoplus injected client options skipsDetectDefaultand leaves ExecuteSql count at 1.Known limitations
auth.TokenProviderand no read-only re-attempt after an RPC has started. Mid-run expiry after the preflight remains hint-only.promptvalue; default staysoff.gcloud.cmdlaunch is untested; the fake-gcloud script test is POSIX-only.