Skip to content

Commit f97e45c

Browse files
authored
fix(iac): converge empty tcpProxies and import database networking (#1169)
* fix(iac): converge empty tcpProxies and import database networking `tcp: []` compiles to `tcpProxies: {}` while Railway serializes a proxy-less service with no `tcpProxies` key, so a database (or service) declared private re-planned `Update <name> networking` after every successful apply. Empty networking maps now normalize away before the diff. Database nodes also import their live networking through the same helper services use, `railway config pull` writes it back as `db.networking = { tcpProxies: { "5432": {} } }` when a proxy exists, and a database node without a networking block keeps the exposure it has. Fixes #1168 * fix(iac): plan the effect of an authored networking block, not the block `environmentApplyChangeSet` reads `networking` as a sparse patch: a key the author leaves out stays as Railway has it, and inside `tcpProxies` an entry keeps or creates a proxy, a `null` entry deletes one, and an unmentioned port is left alone — so an empty map changes nothing. Diffing the block as if it were the whole desired state promised three changes the apply never makes, and re-planned every one of them after each apply: `tcpProxies` left out, `privateNetworkEndpoint` left out, and `tcpProxies: {}` against a live proxy. The plan now diffs the block's effect, so a change is planned only when the apply will move something, while the change set still carries the block as written — that is the payload the apply sends, and a `null` entry is how a proxy gets deleted. `tcpProxies: { "5432": null }` converges once the apply has removed the proxy, and an empty map warns that it cannot remove one instead of planning a removal that never lands.
1 parent 661baa2 commit f97e45c

4 files changed

Lines changed: 429 additions & 15 deletions

File tree

src/commands/config/mod.rs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,12 @@ fn render_graph_as_railway(
738738
lang,
739739
&mut out,
740740
);
741+
render_database_networking_overrides(
742+
resource.networking.as_ref(),
743+
&var_name,
744+
lang,
745+
&mut out,
746+
);
741747
}
742748
}
743749
"service" => {
@@ -1138,6 +1144,42 @@ fn render_database_deploy_overrides(
11381144
}
11391145
}
11401146

1147+
/// A database's public exposure is authored on the helper result, e.g.
1148+
/// `db.networking = { tcpProxies: { "5432": {} } }`, so a pulled file keeps the
1149+
/// proxy it found and a plan sees the drift when one appears or disappears.
1150+
/// Service domains are platform-generated and stay out of the file.
1151+
fn render_database_networking_overrides(
1152+
networking: Option<&serde_json::Value>,
1153+
var_name: &str,
1154+
lang: AuthoringLang,
1155+
out: &mut String,
1156+
) {
1157+
let Some(networking) = networking.and_then(|value| value.as_object()) else {
1158+
return;
1159+
};
1160+
let mut overrides = networking.clone();
1161+
overrides.remove("serviceDomains");
1162+
overrides.retain(|_, value| !value.as_object().is_some_and(|map| map.is_empty()));
1163+
if overrides.is_empty() {
1164+
return;
1165+
}
1166+
let value = serde_json::Value::Object(overrides);
1167+
match lang {
1168+
AuthoringLang::TypeScript => out.push_str(&format!(
1169+
" {var_name}.networking = {};\n",
1170+
ts_value(&value)
1171+
)),
1172+
AuthoringLang::Python => out.push_str(&format!(
1173+
" # networking overrides: {}\n",
1174+
code_value(&value, lang)
1175+
)),
1176+
AuthoringLang::Go => out.push_str(&format!(
1177+
" // networking overrides: {}\n",
1178+
code_value(&value, lang)
1179+
)),
1180+
}
1181+
}
1182+
11411183
fn render_volume_attachments(
11421184
attachments: Option<&serde_json::Map<String, serde_json::Value>>,
11431185
resource_names: &std::collections::HashMap<String, String>,
@@ -2044,6 +2086,64 @@ mod tests {
20442086
}
20452087
}
20462088

2089+
fn database_resource(
2090+
name: &str,
2091+
engine: &str,
2092+
networking: Option<serde_json::Value>,
2093+
) -> runner::DesiredResource {
2094+
runner::DesiredResource {
2095+
address: Some(format!("database.{name}")),
2096+
r#type: "database".to_string(),
2097+
name: name.to_string(),
2098+
engine: Some(engine.to_string()),
2099+
variables: None,
2100+
source: None,
2101+
build: None,
2102+
deploy: None,
2103+
networking,
2104+
volume_attachments: None,
2105+
config: None,
2106+
group_id: None,
2107+
}
2108+
}
2109+
2110+
fn render_database(networking: Option<serde_json::Value>) -> String {
2111+
let graph = runner::DesiredGraph {
2112+
project: Some(runner::DesiredProject { name: "app".into() }),
2113+
resources: vec![database_resource("postgres", "postgres", networking)],
2114+
};
2115+
render_graph_as_railway(&graph, true, AuthoringLang::TypeScript)
2116+
}
2117+
2118+
#[test]
2119+
fn pull_renderer_authors_a_database_tcp_proxy() {
2120+
let rendered = render_database(Some(json!({
2121+
"tcpProxies": { "5432": {} },
2122+
"serviceDomains": { "postgres.up.railway.app": { "port": 5432 } }
2123+
})));
2124+
let helper = rendered
2125+
.lines()
2126+
.position(|line| line.contains("= postgres(\"postgres\")"))
2127+
.expect("database helper call");
2128+
assert_eq!(
2129+
rendered.lines().nth(helper + 1).unwrap().trim(),
2130+
"postgresDatabase.networking = { tcpProxies: { \"5432\": {} } };"
2131+
);
2132+
assert!(!rendered.contains("serviceDomains"));
2133+
}
2134+
2135+
#[test]
2136+
fn pull_renderer_leaves_a_private_database_without_networking() {
2137+
assert!(!render_database(None).contains(".networking"));
2138+
assert!(
2139+
!render_database(Some(json!({
2140+
"serviceDomains": { "postgres.up.railway.app": { "port": 5432 } }
2141+
})))
2142+
.contains(".networking")
2143+
);
2144+
assert!(!render_database(Some(json!({ "tcpProxies": {} }))).contains(".networking"));
2145+
}
2146+
20472147
#[test]
20482148
fn pull_renderer_preserves_single_region_placement() {
20492149
assert_eq!(

src/iac/change_set.rs

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,12 @@ fn diff_networking(
337337
) {
338338
let before = previous.get("networking");
339339
let after = resource.get("networking");
340+
// The database helpers take no networking, and pulled files only author it
341+
// when a proxy exists, so a database node without a networking block keeps
342+
// whatever exposure it has.
343+
if resource_type(resource) == "database" && after.is_none() {
344+
return;
345+
}
340346
let before_domains = before.and_then(|n| n.get("customDomains"));
341347
diagnose_unsupported_custom_domains(resource, diagnostics, before_domains);
342348
let mut before_copy = before.cloned().unwrap_or(json!({}));
@@ -349,9 +355,18 @@ fn diff_networking(
349355
obj.remove("customDomains");
350356
obj.remove("serviceDomains");
351357
}
358+
diagnose_unremovable_tcp_proxies(resource, diagnostics, &before_copy, &after_copy);
352359
let normalized_before = normalize_for_diff("networking", &before_copy);
353360
let normalized_after = normalize_for_diff("networking", &after_copy);
354-
if stable_stringify(&normalized_before) != stable_stringify(&normalized_after) {
361+
// Plan the change only when applying the authored block moves something.
362+
// The block is a sparse patch, so what the plan promises is its effect,
363+
// while the change set still carries the block as written: that is the
364+
// payload the apply sends, and a `null` entry is how a proxy is deleted.
365+
let applied = normalize_for_diff(
366+
"networking",
367+
&networking_after_apply(&before_copy, &after_copy),
368+
);
369+
if stable_stringify(&normalized_before) != stable_stringify(&applied) {
355370
changes.push(update(
356371
&resource_addr(resource),
357372
"networking",
@@ -364,6 +379,85 @@ fn diff_networking(
364379
}
365380
}
366381

382+
/// What `networking` looks like once Railway has applied the authored block.
383+
///
384+
/// `environmentApplyChangeSet` reads the block as a sparse patch: a key the
385+
/// author leaves out stays as Railway has it, and inside `tcpProxies` an entry
386+
/// keeps or creates a proxy, a `null` entry deletes one, and an unmentioned port
387+
/// is left alone — so an empty map changes nothing. The plan diffs against that
388+
/// same effect, because a plan that plans anything else promises work the apply
389+
/// will not do and then plans it again after every apply.
390+
fn networking_after_apply(before: &Value, after: &Value) -> Value {
391+
let mut effective = before.as_object().cloned().unwrap_or_default();
392+
let Some(authored) = after.as_object() else {
393+
return Value::Object(effective);
394+
};
395+
for (key, value) in authored {
396+
if key != "tcpProxies" {
397+
effective.insert(key.clone(), value.clone());
398+
continue;
399+
}
400+
let mut proxies = before
401+
.get("tcpProxies")
402+
.and_then(Value::as_object)
403+
.cloned()
404+
.unwrap_or_default();
405+
for (port, entry) in value.as_object().into_iter().flatten() {
406+
if entry.is_null() {
407+
proxies.remove(port);
408+
} else {
409+
proxies.entry(port.clone()).or_insert_with(|| entry.clone());
410+
}
411+
}
412+
effective.insert(key.clone(), Value::Object(proxies));
413+
}
414+
Value::Object(effective)
415+
}
416+
417+
/// An empty `tcpProxies` map — what `tcp: []` compiles to — cannot take a proxy
418+
/// away, because the apply leaves every port the block does not mention alone.
419+
/// Say that out loud instead of leaving the author with a file that reads
420+
/// "private" next to a proxy that is still serving traffic.
421+
fn diagnose_unremovable_tcp_proxies(
422+
resource: &Value,
423+
diagnostics: &mut Vec<Diagnostic>,
424+
before: &Value,
425+
after: &Value,
426+
) {
427+
let authored_empty = after
428+
.get("tcpProxies")
429+
.and_then(Value::as_object)
430+
.is_some_and(Map::is_empty);
431+
if !authored_empty {
432+
return;
433+
}
434+
let live = before.get("tcpProxies").and_then(Value::as_object);
435+
let Some(live) = live.filter(|proxies| !proxies.is_empty()) else {
436+
return;
437+
};
438+
let ports = live
439+
.keys()
440+
.map(String::as_str)
441+
.collect::<Vec<_>>()
442+
.join(", ");
443+
let removals = live
444+
.keys()
445+
.map(|port| format!("\"{port}\": null"))
446+
.collect::<Vec<_>>()
447+
.join(", ");
448+
diagnostics.push(Diagnostic {
449+
severity: "warning".into(),
450+
path: format!(
451+
"resources.{}.networking.tcpProxies",
452+
resource_addr(resource)
453+
),
454+
message: format!(
455+
"{} has a public TCP proxy on port {ports}, and an empty tcpProxies map does not remove it. Author networking.tcpProxies = {{ {removals} }} to remove the proxy, or remove it from the dashboard.",
456+
resource_name(resource)
457+
),
458+
});
459+
}
460+
367461
fn diagnose_unsupported_custom_domains(
368462
resource: &Value,
369463
diagnostics: &mut Vec<Diagnostic>,
@@ -1100,6 +1194,11 @@ fn normalize_for_diff(field_name: &str, value: &Value) -> Value {
11001194
copy.remove("dockerfilePath");
11011195
}
11021196
}
1197+
if field_name == "networking" {
1198+
// `tcp: []` compiles to `tcpProxies: {}`; Railway serializes a service
1199+
// with no proxy as no `tcpProxies` at all. Both mean the same thing.
1200+
copy.retain(|_, child| !child.is_null() && !child.as_object().is_some_and(Map::is_empty));
1201+
}
11031202
if field_name == "deploy" {
11041203
if copy.get("useLegacyStacker") == Some(&json!(false)) {
11051204
copy.remove("useLegacyStacker");

src/iac/compiler.rs

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,11 @@ pub fn environment_config_to_graph(
656656
"deploy": service.get("deploy").cloned().unwrap_or(Value::Null),
657657
"volumeMounts": service.get("volumeMounts").cloned().unwrap_or(Value::Null),
658658
}));
659+
if let Some(networking) =
660+
networking_from_environment_config(service, service_id, options)
661+
{
662+
node["networking"] = networking;
663+
}
659664
if let Some(group_id) = field_str(service, "groupId") {
660665
node["groupId"] = json!(
661666
group_names_by_id
@@ -710,20 +715,8 @@ pub fn environment_config_to_graph(
710715
if let Some(variables) = service.get("variables") {
711716
node["variables"] = variables_from_environment_config(variables);
712717
}
713-
let mut networking = service.get("networking").cloned().unwrap_or(json!({}));
714-
if let Some(domains) = options.custom_domains_by_service_id.get(service_id) {
715-
networking["customDomains"] = domains.clone();
716-
} else if let Some(existing) = service
717-
.get("networking")
718-
.and_then(|n| n.get("customDomains"))
719-
{
720-
networking["customDomains"] = existing.clone();
721-
}
722-
let networking = prune_empty(networking);
723-
if networking.as_object().is_some_and(|obj| !obj.is_empty())
724-
|| options
725-
.custom_domains_by_service_id
726-
.contains_key(service_id)
718+
if let Some(networking) =
719+
networking_from_environment_config(service, service_id, options)
727720
{
728721
node["networking"] = networking;
729722
}
@@ -820,6 +813,32 @@ pub fn environment_config_to_graph(
820813
}))
821814
}
822815

816+
/// Import a service's live networking (TCP proxies, domains) as graph state.
817+
///
818+
/// Databases go through this too: a public TCP proxy on a database is exposure
819+
/// the plan has to be able to see, and the imported graph is what
820+
/// `railway config pull` renders.
821+
fn networking_from_environment_config(
822+
service: &Value,
823+
service_id: &str,
824+
options: &EnvironmentConfigToGraphOptions,
825+
) -> Option<Value> {
826+
let mut networking = service.get("networking").cloned().unwrap_or(json!({}));
827+
if let Some(domains) = options.custom_domains_by_service_id.get(service_id) {
828+
networking["customDomains"] = domains.clone();
829+
}
830+
let networking = prune_empty(networking);
831+
if networking.as_object().is_some_and(|obj| !obj.is_empty())
832+
|| options
833+
.custom_domains_by_service_id
834+
.contains_key(service_id)
835+
{
836+
Some(networking)
837+
} else {
838+
None
839+
}
840+
}
841+
823842
fn variables_from_environment_config(variables: &Value) -> Value {
824843
let Some(map) = variables.as_object() else {
825844
return json!({});

0 commit comments

Comments
 (0)