Use gateway url rather than VPC.

[Evrard-Nil]

The previous implementation relied on cloud-api living in a compose file where dstack-service exists. This approach is more generic by relying on the gateway directly rather than using the vpc proxy.

---

Note

Switch Patroni discovery to gateway-based URL and introduce gateway_subdomain in DatabaseConfig, updating construction and tests.

• Database/Patroni Discovery:
  • Change PatroniDiscovery to build URL https://{primary_app_id}-8008.{gateway_subdomain}/cluster (remove VPC proxy headers and dstack-service path).
  • Update PatroniDiscovery::new signature to include gateway_subdomain and wire it through Database::from_config.
• Config:
  • Add gateway_subdomain to config::DatabaseConfig and require GATEWAY_SUBDOMAIN env var in from_env.
• API/Tests:
  • Populate database.gateway_subdomain in test configs across crates/api/src/lib.rs, crates/api/tests/common/mod.rs, and crates/api/tests/e2e_repositories.rs.

^{Written by Cursor Bugbot for commit c41d344. This will update automatically on new commits. Configure here.}

[cursor]

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[cursor]

Bug: HTTP client recreated on every discovery call

A new reqwest::Client is created on every call to discover_cluster(), which is invoked repeatedly by the background refresh task. Creating HTTP clients is expensive as it involves setting up connection pools. The original design stored the client in self.client and reused it. This change loses that efficiency and could cause resource churn during frequent cluster state refreshes.

 

[Copilot]

Pull request overview

This PR migrates the Patroni cluster discovery mechanism from using a VPC proxy with custom headers to directly accessing the gateway URL. The change makes the implementation more generic and removes the dependency on the cloud-api living in the same compose environment as dstack-service.

Key Changes:

• Added gateway_subdomain configuration parameter to construct gateway URLs dynamically
• Updated URL format from http://dstack-service/cluster with custom headers to https://{postgres_app_id}-8008.{gateway_subdomain}/cluster
• Modified HTTP client configuration to handle self-signed certificates

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
crates/config/src/types.rs	Added gateway_subdomain field to DatabaseConfig with corresponding GATEWAY_SUBDOMAIN environment variable
crates/database/src/patroni_discovery.rs	Updated PatroniDiscovery to accept gateway subdomain, construct gateway URLs, and configure TLS for self-signed certificates
crates/database/src/lib.rs	Passed gateway subdomain configuration to PatroniDiscovery constructor

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using danger_accept_invalid_certs(true) disables certificate validation, making the connection vulnerable to man-in-the-middle attacks. This undermines the security benefits of using HTTPS.

If self-signed certificates are expected in the gateway environment, consider using a proper certificate authority or certificate pinning instead. At minimum, add a comment explaining why this is necessary and document the security implications.

Alternative approaches:

1. Configure the gateway with proper TLS certificates from a trusted CA
2. Use add_root_certificate() to trust specific self-signed certificates
3. If this is only for development/testing, guard this with a configuration flag

[Copilot]

The self.client field initialized here is no longer used after the changes in discover_cluster() method (lines 77-82), where a new client is created instead. This creates confusion about which client is being used and wastes resources.

Either remove this client initialization and the field from the struct, or update the discover_cluster() method to use self.client after configuring it with the necessary TLS settings during initialization.

[Copilot]

The URL format https://{app_id}-8008.{gateway_subdomain}/cluster hardcodes the port number (8008) in the URL path. This tightly couples the implementation to a specific port and makes it difficult to change the Patroni API port without code changes.

Consider making the port configurable through the DatabaseConfig or PatroniDiscovery constructor to improve flexibility.

[Copilot]

Creating a new HTTP client on every request is inefficient and defeats the purpose of the existing self.client field. The client should be configured once during initialization in the new() method with the necessary TLS settings, then reused for all requests.

Consider updating the new() method to configure the client with TLS settings:

Self {
    client: Client::builder()
        .timeout(Duration::from_secs(10))
        .tls_info(true)
        .danger_accept_invalid_certs(true)
        .tls_built_in_root_certs(false)
        .build()
        .expect("Failed to create HTTP client"),
    // ... other fields
}

Then use self.client here instead of creating a new one.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The error message should include the URL that failed to connect for better debugging. This is especially important now that the URL is dynamically constructed from configuration rather than being a static value.

Consider updating the error message:

.map_err(|e| anyhow!("Failed to connect to Patroni API at {}: {e}", url))?;

Suggested change

	.map_err(|e| anyhow!("Failed to connect to Patroni API: {e}"))?;
	.map_err(|e| anyhow!("Failed to connect to Patroni API at {}: {e}", url))?;

[Copilot]

This comment is redundant since the same information is already conveyed by the HTTP client configuration at lines 55-56. The comment should be removed or updated to explain why self-signed certificates are being accepted (e.g., "Gateway uses self-signed certificates in the current deployment environment").

Suggested change

// Allow self-signed certificates

[Copilot]

The gateway_subdomain value is not validated after being loaded from the environment variable. Invalid values (e.g., empty string, containing invalid characters, malformed domain) will cause runtime failures when constructing the URL in PatroniDiscovery.

Consider adding validation:

gateway_subdomain: {
    let subdomain = env::var("GATEWAY_SUBDOMAIN")
        .map_err(|_| "GATEWAY_SUBDOMAIN not set".to_string())?;
    if subdomain.trim().is_empty() {
        return Err("GATEWAY_SUBDOMAIN cannot be empty".to_string());
    }
    subdomain
},

[cursor]

Bug: Comment indicates self-signed cert support but not implemented

The comment at line 76 states "Allow self-signed certificates" but the reqwest::Client::builder() at lines 53-56 doesn't actually configure danger_accept_invalid_certs(true). When the code switches from HTTP to HTTPS to connect to the gateway, HTTPS requests will fail with certificate verification errors if the gateway uses self-signed certificates. The comment suggests this was intended to be handled, but the implementation is missing.

Additional Locations (1)

• crates/database/src/patroni_discovery.rs#L51-L56