feat: support GOOGLE_WORKSPACE_CLI_API_BASE to override Google API base URL#431
feat: support GOOGLE_WORKSPACE_CLI_API_BASE to override Google API base URL#431Drew-Goddyn wants to merge 1 commit intogoogleworkspace:mainfrom
Conversation
|
googleworkspace-bot
added
area: discovery
area: core
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Google Workspace CLI by introducing a mechanism to route all Google API traffic through a user-defined relay URL. This feature is particularly beneficial for users operating in network-restricted environments, such as those behind corporate firewalls or in sandboxed cloud setups, where direct access to Google APIs might be blocked. By providing a simple environment variable, the CLI can now seamlessly adapt to these network configurations, ensuring continued functionality without requiring any modifications to the application's binary. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a GOOGLE_WORKSPACE_CLI_API_RELAY environment variable to route all Google API traffic through a custom URL, which is a valuable feature for sandboxed or proxied environments. The implementation correctly updates discovery document fetching, URL rewriting for both cached and fresh documents, and extends the logic to Gmail helper functions. The changes are well-contained and backward compatible. I've identified one area for improvement regarding code duplication in src/discovery.rs to enhance maintainability.
Drew-Goddyn
force-pushed
the
feat/api-relay-env-var
branch
from
deae628 to
efd9525
Compare
Drew-Goddyn
changed the title
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature to support custom API base URLs via the GOOGLE_WORKSPACE_CLI_API_BASE environment variable, which is particularly useful for environments with network proxies. The implementation correctly applies this override to most API calls, including those in the Gmail and Workflow helpers. However, I've identified a critical issue in how the alternative discovery URL is constructed, which will cause discovery to fail for certain services when the override is active. Additionally, there's an opportunity to improve maintainability by refactoring some duplicated code in the discovery document handling. Addressing these points will ensure the feature is robust and reliable across all Google Workspace services.
Drew-Goddyn
force-pushed
the
feat/api-relay-env-var
branch
from
efd9525 to
189928b
Compare
|
/gemini review |
Drew-Goddyn
force-pushed
the
feat/api-relay-env-var
branch
from
189928b to
2052ba2
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature to override the Google API base URL, which is particularly useful for environments with proxies. The implementation is well-integrated across various helper modules. However, I've identified a couple of critical issues in src/discovery.rs related to code duplication and flawed tests that should be addressed to ensure the correctness and maintainability of the new logic.
I am having trouble creating individual review comments. Click here to see my feedback.
src/discovery.rs (379-410)
This test appears to be re-implementing the logic from fetch_discovery_document rather than testing the function itself. This means you are testing the test's implementation, not the production code. This can lead to tests passing even if the production code is broken.
The test should be refactored to call fetch_discovery_document and assert its output. You can set up a temporary cache directory and file to test the cache-hit path without making network requests.
async fn test_api_base_override_rewrites_cached_doc() {
let _guard = EnvVarGuard::set(
"GOOGLE_WORKSPACE_CLI_API_BASE",
"https://my-proxy.example.com",
);
let temp_dir = tempfile::tempdir().unwrap();
// The `config_dir` function uses this env var.
let _config_dir_guard =
EnvVarGuard::set("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", temp_dir.path().to_str().unwrap());
let cache_dir = temp_dir.path().join("cache");
std::fs::create_dir_all(&cache_dir).unwrap();
let cache_file = cache_dir.join("test_v1.json");
let doc_json = r#"{
"name": "test",
"version": "v1",
"rootUrl": "https://www.googleapis.com/",
"servicePath": "test/v1/"
}"#;
std::fs::write(&cache_file, doc_json).unwrap();
// This will read from cache and should apply the override.
let doc = fetch_discovery_document("test", "v1").await.unwrap();
assert_eq!(doc.root_url, "https://my-proxy.example.com/");
assert_eq!(
doc.base_url.as_deref(),
Some("https://my-proxy.example.com/test/v1/")
);
}src/discovery.rs (414-434)
Similar to the previous test, this test re-implements the logic from fetch_discovery_document instead of testing the function itself. This test is also flawed because it asserts that doc.base_url is None, which is true for the input JSON regardless of the override logic.
To properly test this, you should call fetch_discovery_document and verify that it preserves the original URLs from the cached document when the override is not set. The test should use a cached document that has a baseUrl.
async fn test_api_base_override_unset_preserves_original() {
let _guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_CLI_API_BASE");
let temp_dir = tempfile::tempdir().unwrap();
let _config_dir_guard =
EnvVarGuard::set("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", temp_dir.path().to_str().unwrap());
let cache_dir = temp_dir.path().join("cache");
std::fs::create_dir_all(&cache_dir).unwrap();
let cache_file = cache_dir.join("drive_v3.json");
let doc_json = r#"{
"name": "drive",
"version": "v3",
"rootUrl": "https://www.googleapis.com/",
"baseUrl": "https://www.googleapis.com/drive/v3/",
"servicePath": "drive/v3/"
}"#;
std::fs::write(&cache_file, doc_json).unwrap();
// This will read from cache. Since the override is not set, it should return the original values.
let doc = fetch_discovery_document("drive", "v3").await.unwrap();
assert_eq!(doc.root_url, "https://www.googleapis.com/");
assert_eq!(
doc.base_url.as_deref(),
Some("https://www.googleapis.com/drive/v3/")
);
}src/discovery.rs (264-268)
This logic for applying the API base override is duplicated from lines 212-216. To improve maintainability, consider extracting this into a helper function that can be called from both places. For example:
fn apply_api_base_override(doc: &mut RestDescription, api_base_override: &Option<String>) {
if let Some(ref override_url) = api_base_override {
let base = override_url.trim_end_matches('/');
doc.root_url = format!("{base}/");
doc.base_url = Some(format!("{base}/{}", doc.service_path));
}
}You could then call apply_api_base_override(&mut doc, &api_base_override) here and in the cache-hit logic.
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for overriding the Google API base URL, which is particularly useful for environments with proxies. The implementation is consistent and well-executed across all modified files. My review includes one high-severity suggestion to improve the new tests in src/discovery.rs by refactoring them to remove duplicated logic, which will make them more robust and easier to maintain.
Drew-Goddyn
force-pushed
the
feat/api-relay-env-var
branch
from
2052ba2 to
356e09e
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by allowing the Google API base URL to be overridden via the GOOGLE_WORKSPACE_CLI_API_BASE environment variable. This is a great addition for users in environments with network proxies. The implementation is well-structured, particularly the refactoring in the Gmail helpers to use a new gmail_api_base() function.
My review includes one high-severity comment on src/discovery.rs regarding the construction of the alternative discovery URL. The current implementation uses a query parameter to pass the service name, which contradicts the code comment and may be less robust for generic proxies compared to including the service name in the URL path. The suggested change aligns the code with the comment and improves compatibility.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for overriding the Google API base URL via the GOOGLE_WORKSPACE_CLI_API_BASE environment variable, which is a great feature for environments with network proxies. The implementation correctly handles discovery document fetching and URL rewriting. My review includes one high-severity suggestion to improve performance by caching the environment variable lookup in the Gmail helper functions, as it's currently read on every API call within that module.
Drew-Goddyn
force-pushed
the
feat/api-relay-env-var
branch
from
13a6afc to
4fe877e
Compare
…se URL Override the base URL for all Google API calls via a single env var. When set, discovery document fetches and all API requests (including helper commands like +triage, +reply, +send) route through the specified URL instead of *.googleapis.com. When unset, behavior is unchanged.
Drew-Goddyn
force-pushed
the
feat/api-relay-env-var
branch
from
4fe877e to
75c054d
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for overriding the Google API base URL via the GOOGLE_WORKSPACE_CLI_API_BASE environment variable. This is a valuable feature for users operating in environments with network proxies. The implementation correctly modifies discovery document fetching and parsing in discovery.rs to use the override URL. It also consistently updates the Gmail helper modules to respect the override, replacing hardcoded URLs. The changes are well-structured, and the addition of unit tests for the new logic is commendable. I have reviewed the code and found no issues; the changes are solid and ready for merging.
|
Note: this is related to but distinct from #422 (HTTP proxy support). That issue asks for native |
There was a problem hiding this comment.
Code Review
This pull request introduces the GOOGLE_WORKSPACE_CLI_API_BASE environment variable, allowing users to override the base URL for Google API calls. The changes span discovery document handling and several helper modules, replacing hardcoded URLs with a configurable base URL. The implementation also includes unit tests for the new URL rewriting logic. After a thorough review, I have no specific comments or suggestions for improvement.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #431 +/- ##
==========================================
+ Coverage 64.40% 64.44% +0.03%
==========================================
Files 38 38
Lines 15584 15677 +93
==========================================
+ Hits 10037 10103 +66
- Misses 5547 5574 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Override the base URL for all Google API calls via
GOOGLE_WORKSPACE_CLI_API_BASE.When set, discovery document fetches, all
executor-driven API requests, and Gmail helper commands (+triage,+reply,+send,+forward,+watch) route through the specified URL instead of*.googleapis.com.When unset, behavior is unchanged — no breaking changes.
Why: In locked-down environments (corporate proxies, sandboxed CI, cloud VMs with egress restrictions), direct access to
*.googleapis.comis often blocked. A transparent proxy/relay can bridge the gap, but gws had no way to route traffic through one.What changed:
discovery.rsrootUrl/baseUrlin parsed docs (both fresh fetch and cache hit)helpers/gmail/mod.rsgmail_api_base()reads the env var (Gmail helpers hardcodegmail.googleapis.com)helpers/gmail/triage.rsgmail_api_base()helpers/gmail/reply.rsgmail_api_base()helpers/gmail/watch.rsgmail_api_base()helpers/workflows.rsgmail_api_base()6 files, ~45 insertions, ~16 deletions. Single squashed commit.
Related: #422 — requests native
http_proxy/https_proxysupport in the HTTP client. This PR provides a complementary workaround for environments where a transparent relay is available.