feat(bigtable): verify daemon identity before routing accelerated RPCs

[mutianf]

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

[gemini-code-assist]

Code Review

This pull request implements identity and scope verification for the Bigtable accelerator daemon before routing RPCs, ensuring that the daemon and client share the same credentials and scopes. If a mismatch is detected, the client refuses to route RPCs, and if verification is inconclusive, it falls back to the native client. A review comment points out a critical issue where failures during server.read_identity() (such as reading from older daemons or JSON parsing errors) are not caught, which would crash client initialization instead of falling back gracefully. A suggestion was provided to catch these exceptions and raise _AcceleratorUnverified.

[mutianf]

remove the binary path override.

[sushanb]

A few follow-ups from the earlier review — timeout, logging, cache init, and a couple of nits.

[sushanb]

google_auth_requests.Request() is constructed with no timeout, and compute_engine.Credentials.refresh doesn't impose one of its own on the metadata call. On a host where 169.254.169.254 is blackholed (hardened VM, broken route, DNS-shadowed metadata.google.internal) this blocks on connect for the OS default — minutes — and because _start_accelerator runs synchronously under _maybe_start_accelerator, client construction hangs for that long.

Please thread a short, bounded timeout here, ideally on the same order as the daemon's own _DEFAULT_STARTUP_TIMEOUT = 10.0:

from google.auth.transport import requests as google_auth_requests

_PRINCIPAL_RESOLVE_TIMEOUT = 2.0  # seconds; metadata server is link-local

...
try:
    creds.refresh(google_auth_requests.Request(timeout=_PRINCIPAL_RESOLVE_TIMEOUT))
except Exception as exc:
    _LOGGER.debug("Compute Engine credential refresh failed: %s", exc)

That way a non-GCE environment that happens to carry CE credentials degrades to principal = None → fall back to native, instead of a multi-minute BigtableDataClient(...) hang.

[sushanb]

Silent except Exception: pass here hides the reason for refresh failures. If the metadata call fails for a reason other than "no server" (403, token endpoint 5xx, DNS flake), the user ends up with principal = None and sees

Could not verify daemon identity: principal is unknown on this client side

with no breadcrumb back to the real cause. A single debug line costs nothing and makes support bugs tractable:

except Exception as exc:
    _LOGGER.debug("Compute Engine credential refresh failed: %s", exc)

(Can be folded into the same change as the timeout above.)

[sushanb]

Worth leaving a short comment here spelling out the fail-closed-on-ADC behavior change, since this is the point where it becomes visible to users:

# Any dev running under `gcloud auth application-default login` has no
# service_account_email, so _resolve_principal() returns None and we land
# here with "principal is unknown on this client side". Fail-closed to the
# native client is intentional — user ADC is almost never what the daemon
# resolved — but it does mean every user-ADC caller sees this warning once
# per client construction. Call it out in the release notes.
warnings.warn(
    f"Accelerator disabled: {exc}",
    RuntimeWarning,
    stacklevel=2,
)

[sushanb]

Nit: _cached_principal is initialized lazily via getattr(..., _UNSET) on first call, which means vars(self) won't show the attribute until then and two concurrent coroutines that both reach a cold _resolve_principal will each run the (potentially network-touching) CE refresh before the second overwrites the first. Low impact — the refresh is idempotent — but a one-liner in _init_accelerator_config makes the invariant explicit and removes the race:

# In _init_accelerator_config, right after self._accelerator_scopes = ...:
self._cached_principal: str | None | object = _UNSET

Then _resolve_principal can drop the getattr and just read self._cached_principal.

[sushanb]

Nit: the three-line principal = email or None / if principal == "default": principal = None reads a bit indirectly — the second check only exists because the first collapsed the CE sentinel into the result. One expression says it:

principal: str | None = email if (email and email != "default") else None

Same behavior, one branch instead of two.