Skip to content

Commit 914000a

Browse files
committed
fix(gateway): refresh replay URLs and advertise public ingress ports
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
1 parent a4210a9 commit 914000a

20 files changed

Lines changed: 466 additions & 71 deletions

File tree

‎crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3501,30 +3501,32 @@ async fn sandbox_create_json_stdout_is_parseable() {
35013501

35023502
#[tokio::test]
35033503
async fn sandbox_create_json_uses_dedicated_ingress_port() {
3504-
let server = run_server().await;
3505-
server
3506-
.openshell
3507-
.state
3508-
.service_ingress_port
3509-
.store(8082, Ordering::SeqCst);
3510-
let result = run_cli_sandbox_create(
3511-
&server,
3512-
"dedicated-ingress",
3513-
&["--output=json", "--expose=4500", "--detach"],
3514-
)
3515-
.await;
3516-
assert!(
3517-
result.status.success(),
3518-
"{}",
3519-
String::from_utf8_lossy(&result.stderr)
3520-
);
3521-
let value: serde_json::Value = serde_json::from_slice(&result.stdout).unwrap();
3522-
assert_eq!(
3523-
value["service_urls"],
3524-
serde_json::json!({
3525-
"": "https://default--sandbox.openshell.localhost:8082/"
3526-
})
3527-
);
3504+
for port in [8082, 32082] {
3505+
let server = run_server().await;
3506+
server
3507+
.openshell
3508+
.state
3509+
.service_ingress_port
3510+
.store(port, Ordering::SeqCst);
3511+
let result = run_cli_sandbox_create(
3512+
&server,
3513+
"dedicated-ingress",
3514+
&["--output=json", "--expose=4500", "--detach"],
3515+
)
3516+
.await;
3517+
assert!(
3518+
result.status.success(),
3519+
"{}",
3520+
String::from_utf8_lossy(&result.stderr)
3521+
);
3522+
let value: serde_json::Value = serde_json::from_slice(&result.stdout).unwrap();
3523+
assert_eq!(
3524+
value["service_urls"],
3525+
serde_json::json!({
3526+
"": format!("https://default--sandbox.openshell.localhost:{port}/")
3527+
})
3528+
);
3529+
}
35283530
}
35293531

35303532
#[tokio::test]

‎crates/openshell-core/src/config.rs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,10 @@ pub struct Config {
197197
/// primary listener serves control-plane operations only.
198198
pub service_bind_address: Option<SocketAddr>,
199199

200+
/// Optional externally reachable service port, overriding the listener port
201+
/// in generated URLs (for example, a `NodePort` or reverse proxy).
202+
pub service_public_port: Option<u16>,
203+
200204
/// Address to bind the unauthenticated health endpoint to.
201205
///
202206
/// When `None`, the dedicated health listener is disabled.
@@ -304,7 +308,7 @@ pub struct ServiceRoutingConfig {
304308
pub enable_loopback_service_http: bool,
305309
}
306310

307-
/// gRPC response metadata advertising a dedicated service ingress port.
311+
/// gRPC response metadata advertising an explicitly configured service port.
308312
/// Clients use this port for service URLs instead of the control-plane
309313
/// endpoint's externally forwarded port when this is set.
310314
pub const SERVICE_PORT_METADATA_KEY: &str = "openshell-service-port";
@@ -885,6 +889,7 @@ impl Config {
885889
name: DEFAULT_GATEWAY_NAME.to_string(),
886890
bind_address: default_bind_address(),
887891
service_bind_address: None,
892+
service_public_port: None,
888893
health_bind_address: None,
889894
metrics_bind_address: None,
890895
log_level: default_log_level(),
@@ -936,6 +941,14 @@ impl Config {
936941
self.service_bind_address.unwrap_or(self.bind_address)
937942
}
938943

944+
/// Explicit service port clients should use instead of a forwarded
945+
/// control-plane port. Shared mode retains legacy client behavior by default.
946+
#[must_use]
947+
pub fn advertised_service_port(&self) -> Option<u16> {
948+
self.service_public_port
949+
.or_else(|| self.service_bind_address.map(|address| address.port()))
950+
}
951+
939952
#[must_use]
940953
pub const fn with_health_bind_address(mut self, addr: SocketAddr) -> Self {
941954
self.health_bind_address = Some(addr);

‎crates/openshell-server/src/cli.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,8 @@ fn prepare_server_config_with_drivers(
491491
);
492492

493493
validate_listener_ports(bind, service_bind, health_bind, metrics_bind)?;
494+
config.service_public_port = file_gateway.and_then(|g| g.service_public_port);
495+
validate_service_public_port(config.service_public_port)?;
494496
if let Some(addr) = service_bind {
495497
config = config.with_service_bind_address(addr);
496498
}
@@ -1020,6 +1022,7 @@ fn validate_preflight_semantics(
10201022
"service_port",
10211023
|| gateway.service_bind_address,
10221024
);
1025+
validate_service_public_port(gateway.service_public_port)?;
10231026
let health_bind = resolve_aux_listener(
10241027
args.bind_address,
10251028
args.health_port,
@@ -1065,6 +1068,15 @@ fn validate_listener_ports(
10651068
Ok(())
10661069
}
10671070

1071+
fn validate_service_public_port(port: Option<u16>) -> Result<()> {
1072+
if port == Some(0) {
1073+
return Err(miette::miette!(
1074+
"service_public_port requires a nonzero port"
1075+
));
1076+
}
1077+
Ok(())
1078+
}
1079+
10681080
fn resolve_config_path(args: &RunArgs) -> Result<Option<PathBuf>> {
10691081
if let Some(path) = args.config.clone() {
10701082
return Ok(Some(path));
@@ -2373,6 +2385,10 @@ mod tests {
23732385
"service-listener-zero",
23742386
"[openshell]\nversion = 2\n[openshell.gateway]\nservice_bind_address = '0.0.0.0:0'\ndisable_tls = true\n",
23752387
),
2388+
(
2389+
"service-public-port-zero",
2390+
"[openshell]\nversion = 2\n[openshell.gateway]\nservice_public_port = 0\ndisable_tls = true\n",
2391+
),
23762392
(
23772393
"driver-selector",
23782394
"[openshell]\nversion = 2\n[openshell.gateway]\ncompute_driver = 'secret-driver-marker'\ndisable_tls = true\n",
@@ -3170,6 +3186,35 @@ service_bind_address = '0.0.0.0:8082'
31703186
);
31713187
}
31723188

3189+
#[test]
3190+
fn service_public_port_is_loaded_without_rebinding_the_listener() {
3191+
let _lock = ENV_LOCK
3192+
.lock()
3193+
.unwrap_or_else(std::sync::PoisonError::into_inner);
3194+
let dir = tempfile::tempdir().unwrap();
3195+
let path = dir.path().join("gateway.toml");
3196+
std::fs::write(
3197+
&path,
3198+
"[openshell]\nversion = 2\n[openshell.gateway]\nservice_bind_address = '0.0.0.0:8082'\nservice_public_port = 32082\ndisable_tls = true\n",
3199+
)
3200+
.unwrap();
3201+
let _config = EnvVarGuard::set("OPENSHELL_GATEWAY_CONFIG", path.to_str().unwrap());
3202+
let _service_port = EnvVarGuard::remove("OPENSHELL_SERVICE_PORT");
3203+
let _legacy = EnvVarGuard::remove("OPENSHELL_DRIVERS");
3204+
let registry = test_registry("shared", false);
3205+
let (mut args, matches) = parse_with_args(&[
3206+
"openshell-gateway",
3207+
"--db-url",
3208+
"sqlite::memory:",
3209+
"--compute-driver",
3210+
"shared",
3211+
]);
3212+
let prepared =
3213+
super::prepare_server_config_with_drivers(&mut args, &matches, &registry).unwrap();
3214+
assert_eq!(prepared.config.service_address().port(), 8082);
3215+
assert_eq!(prepared.config.advertised_service_port(), Some(32082));
3216+
}
3217+
31733218
#[test]
31743219
fn service_listener_rejects_zero_and_collisions_with_all_other_ports() {
31753220
let primary = "127.0.0.1:8080".parse().unwrap();

‎crates/openshell-server/src/config_file.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ pub struct GatewayFileSection {
9696
#[serde(default)]
9797
pub service_bind_address: Option<SocketAddr>,
9898
#[serde(default)]
99+
pub service_public_port: Option<u16>,
100+
#[serde(default)]
99101
pub health_bind_address: Option<SocketAddr>,
100102
#[serde(default)]
101103
pub metrics_bind_address: Option<SocketAddr>,

‎crates/openshell-server/src/grpc/mutation_replay.rs‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,13 @@ pub(super) trait Mutation: Message + Default + Send + Sync + 'static {
121121
request: Request<Self>,
122122
) -> Result<Response<Self::Output>, Status>;
123123
fn capture(response: &Response<Self::Output>) -> Result<Success, Status>;
124-
async fn restore(store: &Store, success: Success) -> Result<Self::Output, Status>;
124+
/// Preserve the admitted effects and resource identity, while rendering
125+
/// transport-dependent fields from the current gateway configuration.
126+
async fn restore(
127+
store: &Store,
128+
config: &openshell_core::Config,
129+
success: Success,
130+
) -> Result<Self::Output, Status>;
125131
}
126132

127133
/// Empty IDs preserve the existing RPC contract, including cancellation.
@@ -300,7 +306,8 @@ async fn execute_owned<M: Mutation>(
300306
return Err(replay_unavailable());
301307
}
302308
let success = previous.success.ok_or_else(uncertain)?;
303-
let mut response = Response::new(M::restore(&state.store, success).await?);
309+
let mut response =
310+
Response::new(M::restore(&state.store, &state.config, success).await?);
304311
response.metadata_mut().insert(
305312
"openshell-replayed",
306313
"true".parse().expect("static metadata"),
@@ -693,7 +700,11 @@ macro_rules! resource_mutation {
693700
fn capture(response: &Response<Self::Output>) -> Result<Success, Status> {
694701
resource_success(response.get_ref().$field.as_ref())
695702
}
696-
async fn restore(store: &Store, success: Success) -> Result<Self::Output, Status> {
703+
async fn restore(
704+
store: &Store,
705+
_config: &openshell_core::Config,
706+
success: Success,
707+
) -> Result<Self::Output, Status> {
697708
Ok($resp {
698709
$field: Some(restore_resource(store, success).await?),
699710
})
@@ -729,7 +740,11 @@ macro_rules! deletion_mutation {
729740
outcome: response.get_ref().outcome,
730741
})
731742
}
732-
async fn restore(_store: &Store, success: Success) -> Result<Self::Output, Status> {
743+
async fn restore(
744+
_store: &Store,
745+
_config: &openshell_core::Config,
746+
success: Success,
747+
) -> Result<Self::Output, Status> {
733748
let Success::Deletion { outcome } = success else {
734749
return Err(replay_unavailable());
735750
};

0 commit comments

Comments
 (0)