fix(extraction): resolve gcloud ADC HOME for non-nested mounts#786
Conversation
_home_for_adc faked HOME by walking two parents up from gcloud_config_mount, assuming the mount is always shaped <home>/.config/gcloud. That holds for dev compose (mounts the developer's real $HOME/.config/gcloud) but not for stage/prod, where the Vault-sourced kartograph-extraction-runtime secret is mounted flatly at /var/secrets/gcloud. There, the derived HOME (/var) has no .config/gcloud, so openshell's --from-gcloud-adc credential resolution fails with "no credentials resolved for provider type 'google-vertex-ai'" even though the ADC file exists at the configured mount. Stage a scratch HOME per invocation with a symlinked .config/gcloud/application_default_credentials.json instead, so credential resolution works regardless of the mount's actual shape. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 6❌ Failed checks (6 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsRepository analysis: Couldn't refresh Git: Failed to clone repository. Please run the Comment |
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/api/tests/unit/extraction/infrastructure/test_openshell_vertex_provider.py (1)
95-140: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoffNo test covers the concurrent-call HOME race.
Given
_home_for_adcmutates globalos.environ["HOME"], add a test that runs two overlapping_home_for_adccontexts (e.g. via threads) and asserts each sees its own scratch ADC path — this would have caught the CWE-362 issue flagged invertex_provider.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/tests/unit/extraction/infrastructure/test_openshell_vertex_provider.py` around lines 95 - 140, Add a regression test for the concurrent HOME mutation race in _home_for_adc. Create two overlapping _home_for_adc contexts (preferably with threads or another controlled concurrency helper) using different scratch gcloud_config_mount paths, and assert each context resolves _adc_lookup_path_from_home to its own application_default_credentials.json. Keep the test focused on the global os.environ["HOME"] isolation behavior and use the existing _home_for_adc and _adc_lookup_path_from_home helpers to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/api/tests/unit/extraction/infrastructure/test_openshell_vertex_provider.py`:
- Around line 95-140: Add a regression test for the concurrent HOME mutation
race in _home_for_adc. Create two overlapping _home_for_adc contexts (preferably
with threads or another controlled concurrency helper) using different scratch
gcloud_config_mount paths, and assert each context resolves
_adc_lookup_path_from_home to its own application_default_credentials.json. Keep
the test focused on the global os.environ["HOME"] isolation behavior and use the
existing _home_for_adc and _adc_lookup_path_from_home helpers to locate the
code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 28e0d947-86ea-416c-a38f-5049951c6825
📒 Files selected for processing (2)
src/api/extraction/infrastructure/openshell/vertex_provider.pysrc/api/tests/unit/extraction/infrastructure/test_openshell_vertex_provider.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
Summary
openshell provider create ... --from-gcloud-adc ...fails withno credentials resolved for provider type 'google-vertex-ai', even though the ADC secret is correctly mounted._home_for_adc()fakedHOMEfor theopenshellCLI subprocess by walking two parent directories up fromgcloud_config_mount, assuming the mount is always shaped<home>/.config/gcloud. That assumption holds for dev compose (KARTOGRAPH_GCLOUD_CONFIG_MOUNT=${HOME}/.config/gcloud), but not for stage/prod, where the Vault-sourcedkartograph-extraction-runtimesecret is mounted flatly at/var/secrets/gcloud(seehp-fleet-gitopsapps/kartograph/base/api-deployment.yaml). The derivedHOME(/var) has no.config/gcloud, soopenshell's own gcloud-ADC discovery can't find the file, even though it exists at the configured mount.HOMEper invocation with a symlinked.config/gcloud/application_default_credentials.jsonpointing at the actual mounted ADC file, so credential resolution works regardless of the mount's shape. No GitOps manifest changes needed — the prod mount/secret/env wiring is already correct.Test plan
test_home_for_adc_resolves_flat_prod_style_mount, which reproduces the exact stage/prod failure against the old code (fails before the fix, passes after)test_home_for_adc_resolves_nested_dev_style_mountandtest_home_for_adc_restores_previous_homefor regression coverageuv run pytest tests/unit/extraction/infrastructure/test_openshell_vertex_provider.py -v— 6 passeduv run pytest tests/unit/extraction -v— 319 passedMade with Cursor