Skip to content

Commit

Permalink
split cap into rules cap (1024) and parts cap (128)
Browse files Browse the repository at this point in the history
  • Loading branch information
@david-crespo
david-crespo committed Jun 28, 2024
1 parent a9a8755 commit 27bd658
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 39 deletions.
36 changes: 18 additions & 18 deletions nexus/db-model/src/vpc_firewall_rule.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,24 +211,24 @@ pub struct VpcFirewallRule {
pub priority: VpcFirewallRulePriority,
}

/// Cap on the number of rules in a VPC. Also used as a max length on other vecs
/// in the rule body.
/// Cap on the number of rules in a VPC
///
/// We could have 5 constants, but why bother? The value is meant to be big
/// enough that customers will rarely run into it in practice. We just want to
/// prevent a single request from being able to write tons of rows to the DB or
/// blow up OPTE.
const MAX_FW_RULES_PER_VPC: usize = 512;
/// The choice of value is somewhat arbitrary, but the goal is to have a
/// large number that customers are unlikely to actually hit, but which still
/// meaningfully limits the ability to overload the DB with a single request.
const MAX_FW_RULES_PER_VPC: usize = 1024;

fn ensure_max_length<T>(
/// Cap on targets and on each type of filter
const MAX_FW_RULE_PARTS: usize = 128;

fn ensure_max_len<T>(
items: &Vec<T>,
label: &str,
max: usize,
) -> Result<(), external::Error> {
if items.len() > MAX_FW_RULES_PER_VPC {
return Err(external::Error::invalid_value(
label,
format!("max length {}", MAX_FW_RULES_PER_VPC),
));
if items.len() > max {
let msg = format!("max length {}", max);
return Err(external::Error::invalid_value(label, msg));
}
Ok(())
}
Expand All @@ -247,16 +247,16 @@ impl VpcFirewallRule {
},
);

ensure_max_length(&rule.targets, "targets")?;
ensure_max_len(&rule.targets, "targets", MAX_FW_RULE_PARTS)?;

if let Some(hosts) = rule.filters.hosts.as_ref() {
ensure_max_length(&hosts, "filters.hosts")?;
ensure_max_len(&hosts, "filters.hosts", MAX_FW_RULE_PARTS)?;
}
if let Some(ports) = rule.filters.ports.as_ref() {
ensure_max_length(&ports, "filters.ports")?;
ensure_max_len(&ports, "filters.ports", MAX_FW_RULE_PARTS)?;
}
if let Some(protocols) = rule.filters.protocols.as_ref() {
ensure_max_length(&protocols, "filters.protocols")?;
ensure_max_len(&protocols, "filters.protocols", MAX_FW_RULE_PARTS)?;
}

Ok(Self {
Expand Down Expand Up @@ -291,7 +291,7 @@ impl VpcFirewallRule {
params: external::VpcFirewallRuleUpdateParams,
) -> Result<Vec<VpcFirewallRule>, external::Error> {
ensure_no_duplicates(&params)?;
ensure_max_length(&params.rules, "rules")?;
ensure_max_len(&params.rules, "rules", MAX_FW_RULES_PER_VPC)?;
params
.rules
.into_iter()
Expand Down
59 changes: 38 additions & 21 deletions nexus/tests/integration_tests/vpc_firewall.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,37 +353,39 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
format!("/v1/vpc-firewall-rules?vpc=default&project={}", project_name);

// fine with max count
let max: usize = 512;
let max_rules: usize = 1024;
let rules: VpcFirewallRules = object_put(
client,
&url,
&VpcFirewallRuleUpdateParams {
rules: (0..max).map(rule_n).collect::<Vec<_>>(),
rules: (0..max_rules).map(rule_n).collect::<Vec<_>>(),
},
)
.await;
assert_eq!(rules.rules.len(), max);
assert_eq!(rules.rules.len(), max_rules);

// fails with max + 1
let error = object_put_error(
client,
&url,
&VpcFirewallRuleUpdateParams {
rules: (0..max + 1).map(rule_n).collect::<Vec<_>>(),
rules: (0..max_rules + 1).map(rule_n).collect::<Vec<_>>(),
},
StatusCode::BAD_REQUEST,
)
.await;
assert_eq!(error.error_code, Some("InvalidValue".to_string()));
assert_eq!(
error.message,
format!("unsupported value for \"rules\": max length {max}")
format!("unsupported value for \"rules\": max length {max_rules}")
);

///////////////////////
// TARGETS
///////////////////////

let max_parts: usize = 128;

let target = VpcFirewallRuleTarget::Vpc("default".parse().unwrap());

// fine with max
Expand All @@ -392,21 +394,21 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&url,
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
targets: (0..max).map(|_i| target.clone()).collect(),
targets: (0..max_parts).map(|_i| target.clone()).collect(),
..base_rule.clone()
}],
},
)
.await;
assert_eq!(rules.rules[0].targets.len(), max);
assert_eq!(rules.rules[0].targets.len(), max_parts);

// fails with max + 1
let error = object_put_error(
client,
&url,
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
targets: (0..max + 1).map(|_i| target.clone()).collect(),
targets: (0..max_parts + 1).map(|_i| target.clone()).collect(),
..base_rule.clone()
}],
},
Expand All @@ -416,7 +418,7 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
assert_eq!(error.error_code, Some("InvalidValue".to_string()));
assert_eq!(
error.message,
format!("unsupported value for \"targets\": max length {max}")
format!("unsupported value for \"targets\": max length {max_parts}")
);

///////////////////////
Expand All @@ -432,7 +434,9 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
filters: VpcFirewallRuleFilter {
hosts: Some((0..max).map(|_i| host.clone()).collect()),
hosts: Some(
(0..max_parts).map(|_i| host.clone()).collect(),
),
protocols: None,
ports: None,
},
Expand All @@ -441,7 +445,7 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
},
)
.await;
assert_eq!(rules.rules[0].filters.hosts.as_ref().unwrap().len(), max);
assert_eq!(rules.rules[0].filters.hosts.as_ref().unwrap().len(), max_parts);

// fails with max + 1
let error = object_put_error(
Expand All @@ -450,7 +454,9 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
filters: VpcFirewallRuleFilter {
hosts: Some((0..max + 1).map(|_i| host.clone()).collect()),
hosts: Some(
(0..max_parts + 1).map(|_i| host.clone()).collect(),
),
protocols: None,
ports: None,
},
Expand All @@ -463,7 +469,9 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
assert_eq!(error.error_code, Some("InvalidValue".to_string()));
assert_eq!(
error.message,
format!("unsupported value for \"filters.hosts\": max length {max}")
format!(
"unsupported value for \"filters.hosts\": max length {max_parts}"
)
);

///////////////////////
Expand All @@ -479,7 +487,7 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
filters: VpcFirewallRuleFilter {
ports: Some((0..max).map(|_i| port).collect()),
ports: Some((0..max_parts).map(|_i| port).collect()),
protocols: None,
hosts: None,
},
Expand All @@ -488,7 +496,7 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
},
)
.await;
assert_eq!(rules.rules[0].filters.ports.as_ref().unwrap().len(), max);
assert_eq!(rules.rules[0].filters.ports.as_ref().unwrap().len(), max_parts);

// fails with max + 1
let error = object_put_error(
Expand All @@ -497,7 +505,7 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
filters: VpcFirewallRuleFilter {
ports: Some((0..max + 1).map(|_i| port).collect()),
ports: Some((0..max_parts + 1).map(|_i| port).collect()),
protocols: None,
hosts: None,
},
Expand All @@ -510,7 +518,9 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
assert_eq!(error.error_code, Some("InvalidValue".to_string()));
assert_eq!(
error.message,
format!("unsupported value for \"filters.ports\": max length {max}")
format!(
"unsupported value for \"filters.ports\": max length {max_parts}"
)
);

///////////////////////
Expand All @@ -526,7 +536,9 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
filters: VpcFirewallRuleFilter {
protocols: Some((0..max).map(|_i| protocol).collect()),
protocols: Some(
(0..max_parts).map(|_i| protocol).collect(),
),
ports: None,
hosts: None,
},
Expand All @@ -535,7 +547,10 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
},
)
.await;
assert_eq!(rules.rules[0].filters.protocols.as_ref().unwrap().len(), max);
assert_eq!(
rules.rules[0].filters.protocols.as_ref().unwrap().len(),
max_parts
);

// fails with max + 1
let error = object_put_error(
Expand All @@ -544,7 +559,9 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
&VpcFirewallRuleUpdateParams {
rules: vec![VpcFirewallRuleUpdate {
filters: VpcFirewallRuleFilter {
protocols: Some((0..max + 1).map(|_i| protocol).collect()),
protocols: Some(
(0..max_parts + 1).map(|_i| protocol).collect(),
),
ports: None,
hosts: None,
},
Expand All @@ -558,7 +575,7 @@ async fn test_firewall_rules_max_lengths(cptestctx: &ControlPlaneTestContext) {
assert_eq!(
error.message,
format!(
"unsupported value for \"filters.protocols\": max length {max}"
"unsupported value for \"filters.protocols\": max length {max_parts}"
)
);
}

0 comments on commit 27bd658

Please sign in to comment.