fix: fail-fast on missing ECP config file to avoid 30s hang#17377
fix: fail-fast on missing ECP config file to avoid 30s hang#17377nbayati wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a fail-fast mechanism in _agent_identity_utils.py that immediately returns None if the ECP config path is specified but does not exist on a workstation. The tests have been updated to mock os.path.exists to simulate workload environments, and a new test has been added to verify the workstation fail-fast behavior. The reviewer suggested simplifying the nested helper functions exists_side_effect in the tests to concise lambda expressions to improve readability and reduce boilerplate.
| def exists_side_effect(path): | ||
| if path == "/var/run/secrets/workload-spiffe-credentials": | ||
| return True | ||
| return False | ||
| mock_exists.side_effect = exists_side_effect |
There was a problem hiding this comment.
The nested helper function exists_side_effect can be simplified to a concise lambda expression to improve readability and reduce boilerplate.
mock_exists.side_effect = lambda path: path == "/var/run/secrets/workload-spiffe-credentials"| def exists_side_effect(path): | ||
| if path == "/var/run/secrets/workload-spiffe-credentials": | ||
| return True | ||
| return False | ||
| mock_exists.side_effect = exists_side_effect |
There was a problem hiding this comment.
The nested helper function exists_side_effect can be simplified to a concise lambda expression to improve readability and reduce boilerplate.
mock_exists.side_effect = lambda path: path == "/var/run/secrets/workload-spiffe-credentials"If GOOGLE_API_CERTIFICATE_CONFIG is set but the file is missing, and we are not in a workload environment (e.g. on a workstation), exit early and return None. This avoids a 30-second hang trying to poll the well-known SPIFFE path.
f3b2666 to
e63c5ad
Compare
The test test_default_client_encrypted_cert_source mocked open in the test module itself instead of the target module, causing it to write dummy cert_path and key_path files to disk during test runs.
This PR resolves two issues in the
google-authpackage:First, it adds a fast-fail check for ECP configuration. When the
GOOGLE_API_CERTIFICATE_CONFIGenvironment variable is set but the configuration file is missing (common on corporate workstations or clean sandbox test runners), the SDK was falling through to the well-known SPIFFE path and waiting on a 30-second retry loop. We now check if the config path is set but missing, and if we are not in a workload environment (the well-known credentials directory is absent), we immediately returnNoneto fallback to unbound tokens. (Fixes b/512912028)Second, it fixes an incorrect mock in
test_mtls.py.test_default_client_encrypted_cert_sourcewas mockingopenin the test namespace instead of the target module namespace. This caused the test to write actualcert_pathandkey_pathfiles to the local disk during test runs. This is fixed by patchinggoogle.auth.transport.mtls.open.Unit tests have been added to verify the fast-fail behavior, and existing retry tests have been updated to mock the workload directory. All tests now pass without writing files to disk.