Fix: Google Auth in CLI for non-.app Url #1091
Conversation
WalkthroughThe authentication gate in ZiplineHub initialization now triggers for any HTTPS base URL, not only those ending with ".app". Initialization attempts to obtain an ID token from the environment or Google credentials. When present, subsequent API calls include the Authorization header. No public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Hub as ZiplineHub
Client->>Hub: __init__(base_url)
alt base_url startswith("https")
rect rgb(235, 245, 255)
note right of Hub: Acquire ID token
Hub->>Hub: id_token = env.GCP_ID_TOKEN
opt if missing
Hub->>GoogleAuth: google.auth.default()
GoogleAuth-->>Hub: credentials
Hub->>GoogleAuth: credentials.refresh()
GoogleAuth-->>Hub: credentials.id_token
end
end
else
note right of Hub: No auth attempted
end
Client->>Hub: call_*_api(request)
alt id_token present
Hub->>API Endpoint: HTTP request with Authorization: Bearer {id_token}
else
Hub->>API Endpoint: HTTP request without Authorization
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
api/python/ai/chronon/repo/zipline_hub.py (3)
14-27: Fix ID token minting; don't rely on credentials.id_token (often None) and avoid sending 'Bearer None'.Use google.oauth2.id_token.fetch_id_token with a proper audience (IAP client ID or base_url for Cloud Run). Guard failures and skip auth when token can't be minted. This directly impacts the PR objective for non-.app HTTPS hosts.
- self.base_url = base_url - if self.base_url.startswith("https"): + self.base_url = base_url + if self.base_url.startswith("https"): print("\n 🔐 Using Google Cloud authentication for ZiplineHub.") # First try to get ID token from environment (GitHub Actions) self.id_token = os.getenv('GCP_ID_TOKEN') if self.id_token: print(" 🔑 Using ID token from environment") else: - # Fallback to Google Cloud authentication - print(" 🔑 Generating ID token from default credentials") - credentials, project_id = google.auth.default() - credentials.refresh(Request()) - self.id_token = credentials.id_token + # Fallback: mint ID token for IAP (use IAP_CLIENT_ID) or Cloud Run (use base_url) audience + try: + from google.oauth2 import id_token as google_id_token + request = Request() + audience = os.getenv("IAP_CLIENT_ID") or self.base_url + self.id_token = google_id_token.fetch_id_token(request, audience) + print(f" 🔑 Fetched ID token for audience: {audience}") + except Exception as e: + print(f" ⚠️ Could not fetch ID token; continuing without auth: {e}") + self.id_token = NoneNote: This aligns with the prior CLI behavior where iap_client_id is required for HTTPS hubs (see retrieved learnings). Consider reading IAP_CLIENT_ID from config if available.
35-37: Only attach Authorization when token is truthy.hasattr triggers even when id_token is None, causing "Bearer None" to be sent.
- if hasattr(self, 'id_token'): + if getattr(self, "id_token", None): headers['Authorization'] = f'Bearer {self.id_token}'Apply the same change in all five header blocks.
Also applies to: 54-56, 76-78, 97-99, 122-124
14-27: Require and wire IAP_CLIENT_ID for HTTPS hubsZiplineHub currently ignores IAP client-ID config and uses default credentials without specifying the correct audience. We must:
- In hub_runner.py:
- Extend HubConfig and get_hub_conf() to read “IAP_CLIENT_ID” from metadata or os.environ.
- Pass hub_conf.iap_client_id into each ZiplineHub(...) call.
- In zipline_hub.py:
- Change
__init__(self, base_url)to__init__(self, base_url, iap_client_id: Optional[str] = None).- Enforce: if
base_url.startswith("https")and noiap_client_id, raise ValueError.- Replace default‐credential refresh with explicit ID-token fetch:
class ZiplineHub: - def __init__(self, base_url): + def __init__(self, base_url, iap_client_id: Optional[str] = None): if not base_url: raise ValueError("Base URL for ZiplineHub cannot be empty.") - self.base_url = base_url + self.base_url = base_url + self.iap_client_id = iap_client_id + if self.base_url.startswith("https") and not self.iap_client_id: + raise ValueError("IAP_CLIENT_ID is required for HTTPS hubs.") if self.base_url.startswith("https"): print("\n 🔐 Using Google Cloud authentication for ZiplineHub.") # First try to get ID token from environment (GitHub Actions) self.id_token = os.getenv('GCP_ID_TOKEN') if self.id_token: print(" 🔑 Using ID token from environment") else: # Fallback to Google Cloud authentication - print(" 🔑 Generating ID token from default credentials") - credentials, project_id = google.auth.default() - credentials.refresh(Request()) - self.id_token = credentials.id_token + print(" 🔑 Fetching ID token via IAP client ID") + from google.auth import id_token + self.id_token = id_token.fetch_id_token(Request(), audience=self.iap_client_id)This ensures proper audience wiring for IAP and falls back cleanly for non-HTTPS (Cloud Run) endpoints.
🧹 Nitpick comments (3)
api/python/ai/chronon/repo/zipline_hub.py (3)
13-13: Normalize base_url to avoid double slashes in endpoints.- self.base_url = base_url + self.base_url = base_url.rstrip("/")
104-104: Fix typo: wrong API name in error log.- print(f" ❌ Error calling diff API: {e}") + print(f" ❌ Error calling sync API: {e}")
28-44: DRY header creation + add optional timeout.Factor header construction into a helper (e.g., _headers()) and consider a requests.Session with a default timeout to reduce duplication and hangs.
Happy to provide a small refactor patch if you want it in this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
api/python/ai/chronon/repo/zipline_hub.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: chewy-zlai
PR: zipline-ai/chronon#1054
File: api/python/ai/chronon/repo/hub_runner.py:46-46
Timestamp: 2025-08-11T17:27:40.740Z
Learning: In the Chronon CLI hub_runner.py, iap_client_id can be None for non-HTTPS hubs that don't require authentication. The get_hub_conf function validates that iap_client_id is required only when hub_url starts with "https", raising a ValueError if it's missing for HTTPS endpoints.
📚 Learning: 2025-08-11T17:27:40.740Z
Learnt from: chewy-zlai
PR: zipline-ai/chronon#1054
File: api/python/ai/chronon/repo/hub_runner.py:46-46
Timestamp: 2025-08-11T17:27:40.740Z
Learning: In the Chronon CLI hub_runner.py, iap_client_id can be None for non-HTTPS hubs that don't require authentication. The get_hub_conf function validates that iap_client_id is required only when hub_url starts with "https", raising a ValueError if it's missing for HTTPS endpoints.
Applied to files:
api/python/ai/chronon/repo/zipline_hub.py
Summary
Use Google Auth for all https not just ones ending in app. The cli isn't working currently for the updated urls https://canary-orch.zipline.ai and https://dev-orch.zipline.ai
Checklist
Summary by CodeRabbit