Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(proxy): specify hosts for proxy exclusion instead of complete URLs #6957

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions config/config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ certificate = "/path/to/certificate.pem"
# Proxy server configuration for connecting to payment gateways.
# Don't define the fields if a Proxy isn't needed. Empty strings will cause failure.
[proxy]
# http_url = "http proxy url" # Proxy all HTTP traffic via this proxy
# https_url = "https proxy url" # Proxy all HTTPS traffic via this proxy
idle_pool_connection_timeout = 90 # Timeout for idle pool connections (defaults to 90s)
bypass_proxy_urls = [] # A list of URLs that should bypass the proxy

# http_url = "http proxy url" # Proxy all HTTP traffic via this proxy
# https_url = "https proxy url" # Proxy all HTTPS traffic via this proxy
idle_pool_connection_timeout = 90 # Timeout for idle pool connections (defaults to 90s)
bypass_proxy_hosts = "localhost, cluster.local" # A comma-separated list of domains or IP addresses that should not use the proxy. Whitespace between entries would be ignored.

# Configuration for the Key Manager Service
[key_manager]
Expand Down Expand Up @@ -468,8 +467,8 @@ bank_redirect.giropay = { connector_list = "adyen,globalpay" }


[mandates.update_mandate_supported]
card.credit = { connector_list = "cybersource" } # Update Mandate supported payment method type and connector for card
card.debit = { connector_list = "cybersource" } # Update Mandate supported payment method type and connector for card
card.credit = { connector_list = "cybersource" } # Update Mandate supported payment method type and connector for card
card.debit = { connector_list = "cybersource" } # Update Mandate supported payment method type and connector for card

# Required fields info used while listing the payment_method_data
[required_fields.pay_later] # payment_method = "pay_later"
Expand Down
10 changes: 5 additions & 5 deletions config/deployments/env_specific.toml
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ pm_auth_key = "pm_auth_key" # Payment method auth key used for authorization
redis_expiry = 900 # Redis expiry time in milliseconds

[proxy]
http_url = "http://proxy_http_url" # Outgoing proxy http URL to proxy the HTTP traffic
https_url = "https://proxy_https_url" # Outgoing proxy https URL to proxy the HTTPS traffic
bypass_proxy_urls = [] # A list of URLs that should bypass the proxy
http_url = "http://proxy_http_url" # Proxy all HTTP traffic via this proxy
https_url = "https://proxy_https_url" # Proxy all HTTPS traffic via this proxy
bypass_proxy_hosts = "localhost, cluster.local" # A comma-separated list of domains or IP addresses that should not use the proxy. Whitespace between entries would be ignored.

# Redis credentials
[redis]
Expand Down Expand Up @@ -306,8 +306,8 @@ enabled = false
global_tenant = { schema = "public", redis_key_prefix = "", clickhouse_database = "default"}

[multitenancy.tenants.public]
base_url = "http://localhost:8080"
schema = "public"
base_url = "http://localhost:8080"
schema = "public"
redis_key_prefix = ""
clickhouse_database = "default"

Expand Down
2 changes: 2 additions & 0 deletions crates/common_enums/src/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ pub enum ApiClientError {
#[error("Unexpected state reached/Invariants conflicted")]
UnexpectedState,

#[error("Failed to parse URL")]
UrlParsingFailed,
#[error("URL encoding of request payload failed")]
UrlEncodingFailed,
#[error("Failed to send request to connector {0}")]
Expand Down
11 changes: 2 additions & 9 deletions crates/router/src/bin/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,8 @@ async fn main() -> CustomResult<(), ProcessTrackerError> {
let conf = Settings::with_config_path(cmd_line.config_path)
.expect("Unable to construct application configuration");
let api_client = Box::new(
services::ProxyClient::new(
conf.proxy.clone(),
services::proxy_bypass_urls(
conf.key_manager.get_inner(),
&conf.locker,
&conf.proxy.bypass_proxy_urls,
),
)
.change_context(ProcessTrackerError::ConfigurationError)?,
services::ProxyClient::new(&conf.proxy)
.change_context(ProcessTrackerError::ConfigurationError)?,
);
// channel for listening to redis disconnect events
let (redis_shutdown_signal_tx, redis_shutdown_signal_rx) = oneshot::channel();
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/configs/defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl Default for super::settings::Proxy {
http_url: Default::default(),
https_url: Default::default(),
idle_pool_connection_timeout: Some(90),
bypass_proxy_urls: Vec::new(),
bypass_proxy_hosts: Default::default(),
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions crates/router/src/configs/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ pub struct Proxy {
pub http_url: Option<String>,
pub https_url: Option<String>,
pub idle_pool_connection_timeout: Option<u64>,
pub bypass_proxy_urls: Vec<String>,
pub bypass_proxy_hosts: Option<String>,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not using a Vec or HashSet here (and splitting the string on commas), we pass the string directly to NoProxy::from_string(), which handles the splitting by comma and trimming whitespaces.

}

#[derive(Debug, Deserialize, Clone)]
Expand Down Expand Up @@ -804,7 +804,6 @@ impl Settings<SecuredSecret> {
.with_list_parse_key("log.telemetry.route_to_trace")
.with_list_parse_key("redis.cluster_urls")
.with_list_parse_key("events.kafka.brokers")
.with_list_parse_key("proxy.bypass_proxy_urls")
.with_list_parse_key("connectors.supported.wallets")
.with_list_parse_key("connector_request_reference_id_config.merchant_ids_send_payment_id_as_connector_request_id"),

Expand Down
16 changes: 3 additions & 13 deletions crates/router/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,19 +236,9 @@ pub async fn start_server(conf: settings::Settings<SecuredSecret>) -> Applicatio
logger::debug!(startup_config=?conf);
let server = conf.server.clone();
let (tx, rx) = oneshot::channel();
let api_client = Box::new(
services::ProxyClient::new(
conf.proxy.clone(),
services::proxy_bypass_urls(
conf.key_manager.get_inner(),
&conf.locker,
&conf.proxy.bypass_proxy_urls,
),
)
.map_err(|error| {
errors::ApplicationError::ApiClientError(error.current_context().clone())
})?,
);
let api_client = Box::new(services::ProxyClient::new(&conf.proxy).map_err(|error| {
errors::ApplicationError::ApiClientError(error.current_context().clone())
})?);
let state = Box::pin(AppState::new(conf, tx, api_client)).await;
let request_body_limit = server.request_body_limit;

Expand Down
26 changes: 4 additions & 22 deletions crates/router/src/services/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use actix_web::{
http::header::{HeaderName, HeaderValue},
web, FromRequest, HttpRequest, HttpResponse, Responder, ResponseError,
};
pub use client::{proxy_bypass_urls, ApiClient, MockApiClient, ProxyClient};
pub use client::{ApiClient, MockApiClient, ProxyClient};
pub use common_enums::enums::PaymentAction;
pub use common_utils::request::{ContentType, Method, Request, RequestBuilder};
use common_utils::{
Expand Down Expand Up @@ -415,29 +415,11 @@ pub async fn send_request(
) -> CustomResult<reqwest::Response, errors::ApiClientError> {
logger::info!(method=?request.method, headers=?request.headers, payload=?request.body, ?request);

let url = reqwest::Url::parse(&request.url)
.change_context(errors::ApiClientError::UrlEncodingFailed)?;

#[cfg(feature = "dummy_connector")]
let should_bypass_proxy = url
.as_str()
.starts_with(&state.conf.connectors.dummyconnector.base_url)
|| proxy_bypass_urls(
state.conf.key_manager.get_inner(),
&state.conf.locker,
&state.conf.proxy.bypass_proxy_urls,
)
.contains(&url.to_string());
#[cfg(not(feature = "dummy_connector"))]
let should_bypass_proxy = proxy_bypass_urls(
&state.conf.key_manager.get_inner(),
&state.conf.locker,
&state.conf.proxy.bypass_proxy_urls,
)
.contains(&url.to_string());
let url =
url::Url::parse(&request.url).change_context(errors::ApiClientError::UrlParsingFailed)?;

let client = client::create_client(
&state.conf.proxy,
should_bypass_proxy,
request.certificate,
request.certificate_key,
)?;
Expand Down
Loading
Loading