Skip to content
Draft
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
57 changes: 42 additions & 15 deletions crates/openshell-cli/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,10 +723,17 @@ pub async fn sandbox_create(
Err(status) => return Err(miette::miette!(status.to_string())),
};
let response = response.into_inner();
let application_service_urls = response.application_service_urls;
let service_urls = response
.service_urls
.into_iter()
.map(|(service, url)| (service, service_url_for_gateway(&url, &effective_server)))
.map(|(service, url)| {
let url = application_service_urls
.get(&service)
.cloned()
.unwrap_or_else(|| service_url_for_gateway(&url, &effective_server));
(service, url)
})
.collect::<HashMap<_, _>>();
let sandbox = response
.sandbox
Expand Down Expand Up @@ -1115,10 +1122,7 @@ pub async fn sandbox_create(
"\u{2713}".green().bold(),
);
if let Some(url) = service_urls.get("").filter(|url| !url.is_empty()) {
eprintln!(
" Access at: {}",
service_url_for_gateway(url, &effective_server)
);
eprintln!(" Access at: {url}");
}
}

Expand Down Expand Up @@ -4361,7 +4365,7 @@ pub async fn service_expose(
);
}
if !response.url.is_empty() {
let url = service_url_for_gateway(&response.url, server);
let url = preferred_service_url(&response, server);
println!(" URL: {}", url.cyan());
}
Ok(())
Expand Down Expand Up @@ -4574,7 +4578,7 @@ fn print_service_endpoint_table(
let url = if response.url.is_empty() {
String::new()
} else {
service_url_for_gateway(&response.url, gateway_endpoint)
preferred_service_url(response, gateway_endpoint)
};
Some((
workspace.to_string(),
Expand Down Expand Up @@ -4665,7 +4669,7 @@ fn service_endpoint_to_json(
let url = if response.url.is_empty() {
String::new()
} else {
service_url_for_gateway(&response.url, gateway_endpoint)
preferred_service_url(response, gateway_endpoint)
};

Some(serde_json::json!({
Expand All @@ -4675,6 +4679,8 @@ fn service_endpoint_to_json(
"target_port": endpoint.target_port,
"authorization_mode": service_authorization_mode_name(endpoint.authorization_mode),
"url": url,
"application_url": response.application_url,
"gateway_url": service_url_for_gateway(&response.gateway_url, gateway_endpoint),
}))
}

Expand All @@ -4689,13 +4695,16 @@ fn service_authorization_mode_name(mode: i32) -> &'static str {
}
}

/// Read gcloud Application Default Credentials from disk.
///
/// Returns `(client_id, client_secret, refresh_token)`.
///
/// Checks `GOOGLE_APPLICATION_CREDENTIALS` first; falls back to
/// `$CLOUDSDK_CONFIG/application_default_credentials.json` when set, then to
/// `~/.config/gcloud/application_default_credentials.json`.
/// Application origins are independent of the gateway transport. Only legacy
/// gateway URLs inherit a caller's external gateway port.
fn preferred_service_url(response: &ServiceEndpointResponse, gateway: &str) -> String {
if response.application_url.is_empty() {
service_url_for_gateway(&response.url, gateway)
} else {
response.application_url.clone()
}
}

fn service_url_for_gateway(service_url: &str, gateway_endpoint: &str) -> String {
let (Ok(mut service_url), Ok(gateway_endpoint)) = (
url::Url::parse(service_url),
Expand Down Expand Up @@ -7324,6 +7333,8 @@ mod tests {
#[test]
fn service_endpoint_json_has_raw_fields_and_normalized_url() {
let response = ServiceEndpointResponse {
application_url: String::new(),
gateway_url: String::new(),
endpoint: Some(ServiceEndpoint {
metadata: Some(ObjectMeta {
workspace: "team-a".to_string(),
Expand All @@ -7349,6 +7360,8 @@ mod tests {
"target_port": 8080,
"authorization_mode": "bearer_passthrough",
"url": "https://api.openshell.localhost:17670/",
"application_url": "",
"gateway_url": "",
})
);
assert!(service_endpoint_to_json(&ServiceEndpointResponse::default(), "unused").is_none());
Expand Down Expand Up @@ -8108,6 +8121,20 @@ mod tests {
assert!(rootfs_tar_sources_supported_for_gateway(None));
}

#[test]
fn application_origin_is_preserved_when_gateway_uses_a_port_forward() {
let response = ServiceEndpointResponse {
application_url: "https://default--demo--web.services.example.com:10443/".into(),
gateway_url: "https://default--demo--web.api.example.com:8080/".into(),
url: "https://default--demo--web.services.example.com:10443/".into(),
..Default::default()
};
assert_eq!(
super::preferred_service_url(&response, "https://localhost:29092"),
response.application_url
);
}

#[test]
fn service_url_for_gateway_uses_external_gateway_port() {
assert_eq!(
Expand Down
2 changes: 2 additions & 0 deletions crates/openshell-cli/tests/provider_commands_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,8 @@ impl OpenShell for TestOpenShell {
let name = request.name;
// Return a minimal sandbox with metadata for CAS operations
Ok(Response::new(SandboxResponse {
application_service_urls: HashMap::new(),
gateway_service_urls: HashMap::new(),
sandbox: Some(Sandbox {
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
id: format!("sb-{name}"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ impl OpenShell for TestOpenShell {
},
);
Ok(Response::new(SandboxResponse {
application_service_urls: HashMap::new(),
gateway_service_urls: HashMap::new(),
sandbox: Some(sandbox),
service_urls,
}))
Expand Down Expand Up @@ -291,6 +293,8 @@ impl OpenShell for TestOpenShell {
};
sandbox.set_phase(SandboxPhase::Ready as i32);
Ok(Response::new(SandboxResponse {
application_service_urls: HashMap::new(),
gateway_service_urls: HashMap::new(),
sandbox: Some(sandbox),
service_urls: HashMap::new(),
}))
Expand Down Expand Up @@ -527,6 +531,8 @@ impl OpenShell for TestOpenShell {
.push(request.into_inner());
Ok(Response::new(
openshell_core::proto::ServiceEndpointResponse {
application_url: String::new(),
gateway_url: String::new(),
url: "https://default--sandbox.openshell.localhost:17670/".to_string(),
..Default::default()
},
Expand All @@ -539,6 +545,8 @@ impl OpenShell for TestOpenShell {
) -> Result<Response<openshell_core::proto::ServiceEndpointResponse>, Status> {
Ok(Response::new(
openshell_core::proto::ServiceEndpointResponse {
application_url: String::new(),
gateway_url: String::new(),
url: "https://default--sandbox.openshell.localhost:17670/".to_string(),
..Default::default()
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ macro_rules! mock_gateway {
async fn get_sandbox(&self, _: Request<proto::GetSandboxRequest>) -> Result<Response<proto::SandboxResponse>, Status> {
self.calls.lock().unwrap().lookups += 1;
Ok(Response::new(proto::SandboxResponse {
application_service_urls: std::collections::HashMap::new(),
gateway_service_urls: std::collections::HashMap::new(),
sandbox: Some(proto::Sandbox {
metadata: Some(proto::datamodel::v1::ObjectMeta {
id: "test-id".to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ impl OpenShell for TestOpenShell {
let name = request.name.clone();
*self.state.last_get_name.lock().await = Some(name.clone());
Ok(Response::new(SandboxResponse {
application_service_urls: std::collections::HashMap::new(),
gateway_service_urls: std::collections::HashMap::new(),
sandbox: Some(Sandbox {
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
id: "test-id".to_string(),
Expand Down
75 changes: 75 additions & 0 deletions crates/openshell-core/src/app_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub struct AppListenerConfig {
pub enabled: bool,
pub port: u16,
pub domain: String,
/// Public HTTPS origin, independently of the supervisor's listener port.
pub public_url: Option<String>,
}

impl Default for AppListenerConfig {
Expand All @@ -21,13 +23,17 @@ impl Default for AppListenerConfig {
enabled: false,
port: 8443,
domain: String::new(),
public_url: None,
}
}
}

impl AppListenerConfig {
pub fn validate(&self) -> Result<(), String> {
if !self.enabled {
if self.public_url.is_some() {
return Err("supervisor.app_listener.public_url requires enabled = true".into());
}
return Ok(());
}
if self.port < 1024 {
Expand All @@ -46,8 +52,40 @@ impl AppListenerConfig {
{
return Err("supervisor.app_listener.domain must be a lowercase DNS name".into());
}
if let Some(origin) = &self.public_url {
let parsed = url::Url::parse(origin)
.map_err(|_| "supervisor.app_listener.public_url must be an HTTPS origin")?;
if parsed.scheme() != "https"
|| parsed.host_str() != Some(self.domain.as_str())
|| !parsed.username().is_empty()
|| parsed.password().is_some()
|| parsed.path() != "/"
|| parsed.query().is_some()
|| parsed.fragment().is_some()
|| parsed.port() == Some(0)
{
return Err("supervisor.app_listener.public_url must be an HTTPS origin matching domain, without credentials, path, query, or fragment".into());
}
}
Ok(())
}

/// Configured address only; this does not imply installation or readiness.
pub fn service_url(&self, workspace: &str, sandbox: &str, service: &str) -> Option<String> {
if !self.enabled {
return None;
}
let mut origin = url::Url::parse(self.public_url.as_deref()?).ok()?;
let label = if service.is_empty() {
format!("{workspace}--{sandbox}")
} else {
format!("{workspace}--{sandbox}--{service}")
};
origin
.set_host(Some(&format!("{label}.{}", self.domain)))
.ok()?;
Some(origin.into())
}
}

/// A synchronous view of installed ingress, evaluated on the sandbox instance
Expand All @@ -60,6 +98,43 @@ pub type InstallationSource = std::sync::Arc<
mod tests {
use super::*;

#[test]
fn public_origin_is_explicit_https_and_matches_listener_domain() {
let mut config = AppListenerConfig {
enabled: true,
domain: "services.example.com".into(),
public_url: Some("https://services.example.com:10443".into()),
..Default::default()
};
config.validate().unwrap();
assert_eq!(
config.service_url("default", "demo", "web").as_deref(),
Some("https://default--demo--web.services.example.com:10443/")
);
config.public_url = Some("https://services.example.com:443/".into());
assert_eq!(
config.service_url("default", "demo", "").as_deref(),
Some("https://default--demo.services.example.com/")
);
for bad in [
"http://services.example.com",
"https://other.example.com",
"https://user:password@services.example.com",
"https://services.example.com/path",
"https://services.example.com/?query",
"https://services.example.com/#fragment",
"https://services.example.com:0",
"not-a-url",
] {
config.public_url = Some(bad.into());
assert!(config.validate().is_err(), "{bad}");
}
config.public_url = Some("https://services.example.com".into());
config.enabled = false;
assert!(config.validate().is_err());
assert!(config.service_url("default", "demo", "web").is_none());
}

#[test]
fn only_explicit_enablement_requires_a_valid_endpoint() {
assert!(!AppListenerConfig::default().enabled);
Expand Down
1 change: 1 addition & 0 deletions crates/openshell-driver-kubernetes/src/app_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ mod tests {
let config = KubernetesAppListenerConfig {
create_service: true,
common: openshell_core::app_listener::AppListenerConfig {
public_url: None,
enabled,
port: 9443,
domain: "services.example.com".into(),
Expand Down
1 change: 1 addition & 0 deletions crates/openshell-driver-kubernetes/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,7 @@ mod tests {
assert!(!config.app_listener.common.enabled);
assert!(config.validate().is_ok());
config.app_listener.common = openshell_core::app_listener::AppListenerConfig {
public_url: None,
enabled: true,
port: 9443,
domain: "services.example.com".into(),
Expand Down
2 changes: 2 additions & 0 deletions crates/openshell-sdk/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1492,6 +1492,8 @@ fn sandbox_from_response(sandbox: Option<proto::Sandbox>) -> Result<SandboxRef>
fn sandbox_from_create_response(response: proto::SandboxResponse) -> Result<SandboxRef> {
let mut sandbox = sandbox_from_response(response.sandbox)?;
sandbox.service_urls = response.service_urls;
sandbox.application_service_urls = response.application_service_urls;
sandbox.gateway_service_urls = response.gateway_service_urls;
Ok(sandbox)
}

Expand Down
6 changes: 6 additions & 0 deletions crates/openshell-sdk/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,10 @@ pub struct SandboxRef {
/// Service URLs returned by sandbox creation, keyed by service name. The
/// empty key identifies the unnamed service. Non-create reads leave this empty.
pub service_urls: HashMap<String, String>,
/// Configured application origins; these do not establish installation readiness.
pub application_service_urls: HashMap<String, String>,
/// Gateway-routed alternatives.
pub gateway_service_urls: HashMap<String, String>,
pub restart_count: u32,
pub next_restart_at_ms: Option<i64>,
pub main_process_started_at_ms: Option<i64>,
Expand Down Expand Up @@ -455,6 +459,8 @@ impl SandboxRef {
.then_some(sandbox.host_key_fingerprint),
created_from_workload_template,
service_urls: HashMap::new(),
application_service_urls: HashMap::new(),
gateway_service_urls: HashMap::new(),
restart_count,
next_restart_at_ms,
main_process_started_at_ms,
Expand Down
Loading
Loading