Skip to content

Commit ba0fdf6

Browse files
committed
feat(config): Allow static + stateful NAT, on opposite ends
Now that we support using static NAT on one side of a peering and masquerading or port forwarding on the other end, let's allow users to deploy such configurations. Also adjust relevant tests accordingly. Signed-off-by: Quentin Monnet <qmo@qmon.net>
1 parent f7bfa85 commit ba0fdf6

4 files changed

Lines changed: 15 additions & 52 deletions

File tree

config/src/external/overlay/tests.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -420,11 +420,7 @@ pub mod test {
420420

421421
// Build overlay object and validate it
422422
let overlay = Overlay::new(vpc_table, peering_table);
423-
assert!(
424-
overlay
425-
.validate()
426-
.is_err_and(|e| e == ConfigError::IncompatibleNatModes("Peering-1".to_owned()))
427-
);
423+
assert!(overlay.validate().is_ok());
428424
}
429425

430426
#[test]

config/src/external/overlay/validation_tests.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,7 +1209,7 @@ mod test {
12091209
assert!(validate_overlay_with_peering(peering).is_ok());
12101210
}
12111211

1212-
// Stateless + stateful rejected
1212+
// Stateless + stateful passes
12131213
#[test]
12141214
fn test_stateless_plus_stateful_rejected() {
12151215
let peering = VpcPeering::with_default_group(
@@ -1237,15 +1237,10 @@ mod test {
12371237
],
12381238
),
12391239
);
1240-
let result = validate_overlay_with_peering(peering);
1241-
assert_eq!(
1242-
result,
1243-
Err(ConfigError::IncompatibleNatModes("Peering-1".to_owned())),
1244-
"{result:?}",
1245-
);
1240+
assert!(validate_overlay_with_peering(peering).is_ok());
12461241
}
12471242

1248-
// Stateless + port forwarding rejected
1243+
// Stateless + port forwarding passes
12491244
#[test]
12501245
fn test_stateless_plus_port_forwarding_rejected() {
12511246
let peering = VpcPeering::with_default_group(
@@ -1273,12 +1268,7 @@ mod test {
12731268
],
12741269
),
12751270
);
1276-
let result = validate_overlay_with_peering(peering);
1277-
assert_eq!(
1278-
result,
1279-
Err(ConfigError::IncompatibleNatModes("Peering-1".to_owned())),
1280-
"{result:?}",
1281-
);
1271+
assert!(validate_overlay_with_peering(peering).is_ok());
12821272
}
12831273

12841274
// Stateful + stateful rejected (across peering sides)

config/src/external/overlay/vpc.rs

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -122,55 +122,39 @@ impl ValidatedPeering {
122122
fn validate_nat_combinations(&self) -> ConfigResult {
123123
// If stateful NAT is set up on one side of the peering, we don't support NAT (stateless or
124124
// stateful) on the other side.
125-
let mut local_has_stateless_nat = false;
126-
let mut local_has_stateful_nat = false;
125+
let mut local_has_masquerading = false;
127126
let mut local_has_port_forwarding = false;
128127
for expose in self.local.valexp() {
129128
match expose.nat_config() {
130129
Some(VpcExposeNatConfig::Stateful { .. }) => {
131-
local_has_stateful_nat = true;
132-
}
133-
Some(VpcExposeNatConfig::Stateless { .. }) => {
134-
local_has_stateless_nat = true;
130+
local_has_masquerading = true;
135131
}
136132
Some(VpcExposeNatConfig::PortForwarding { .. }) => {
137133
local_has_port_forwarding = true;
138134
}
139-
None => {}
135+
Some(VpcExposeNatConfig::Stateless { .. }) | None => {}
140136
}
141137
}
142-
let local_has_nat =
143-
local_has_stateless_nat || local_has_stateful_nat || local_has_port_forwarding;
144138

145-
if !local_has_nat {
139+
// No NAT or static NAT only is compatible with all other modes on the other side
140+
if !(local_has_masquerading || local_has_port_forwarding) {
146141
return Ok(());
147142
}
148143

149-
let local_has_stateless_nat_only =
150-
local_has_stateless_nat && !local_has_stateful_nat && !local_has_port_forwarding;
151-
152144
// Allowed:
153145
//
154146
// - no NAT ------------ *
155-
// - stateless NAT ----- stateless NAT
147+
// - stateless NAT ----- *
156148
//
157149
// Disallowed (some of them may be supported in the future):
158150
//
159-
// - stateful NAT ------ stateless NAT
160-
// - stateful NAT ------ stateful NAT
161-
// - stateful NAT ------ port forwarding
151+
// - masquerading ------ masquerading
152+
// - masquerading ------ port forwarding
162153
// - port forwarding --- port forwarding
163-
// - port forwarding --- stateless NAT
164-
165154
for remote_expose in self.remote.valexp() {
166-
if !remote_expose.has_nat() {
167-
continue;
168-
}
169-
if local_has_stateless_nat_only && remote_expose.has_stateless_nat() {
170-
continue;
155+
if remote_expose.has_stateful_nat() || remote_expose.has_port_forwarding() {
156+
return Err(ConfigError::IncompatibleNatModes(self.name.clone()));
171157
}
172-
// Other combinations are rejected
173-
return Err(ConfigError::IncompatibleNatModes(self.name.clone()));
174158
}
175159
Ok(())
176160
}

config/src/external/overlay/vpcpeering.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -535,13 +535,6 @@ impl ValidatedExpose {
535535
)
536536
}
537537

538-
#[must_use]
539-
pub(crate) fn has_nat(&self) -> bool {
540-
self.nat
541-
.as_ref()
542-
.is_some_and(|nat| !nat.as_range.is_empty())
543-
}
544-
545538
#[must_use]
546539
pub fn has_stateful_nat(&self) -> bool {
547540
self.nat.as_ref().is_some_and(VpcExposeNat::is_stateful)

0 commit comments

Comments
 (0)