More WIP

- Add database CHECK constraint, that array is _really_ not going to be
  empty
- Avoid unwrap when modifying VPC firewall rules to insert allowlist

nexus/networking/src/firewall_rules.rs

@@ -200,8 +200,7 @@ pub async fn resolve_firewall_rules_for_sled_agent(
     // on port 53. Adding a _new_ firewall rule for the allowlist with higher
     // priority would remove this port / protocol requirement. Instead, we
     // modify the rules in-place.
-    let should_apply_allowlist = allowlist_applies_to_vpc(vpc);
-    let allowed_ips = if should_apply_allowlist {
+    let allowed_ips = if allowlist_applies_to_vpc(vpc) {
         let allowed_ips =
             lookup_allowed_source_ips(datastore, opctx, log).await?;
         match &allowed_ips {
@@ -309,15 +308,15 @@ pub async fn resolve_firewall_rules_for_sled_agent(
         }
 
         // Construct the set of filter hosts from the DB rule.
-        let filter_hosts = match (&rule.filter_hosts, should_apply_allowlist) {
+        let filter_hosts = match (&rule.filter_hosts, &allowed_ips) {
             // No host filters, but we need to insert the allowlist entries.
             //
             // This is the expected case when applying rules for the built-in
             // Oxide-services VPCs, which do not contain host filters. (See
             // `nexus_db_queries::fixed_data::vpc_firewall_rule` for those
             // rules.) If those rules change to include any filter hosts, this
             // logic needs to change as well.
-            (None, true) => match allowed_ips.as_ref().unwrap() {
+            (None, Some(allowed_ips)) => match allowed_ips {
                 AllowedSourceIps::Any => None,
                 AllowedSourceIps::List(list) => Some(
                     list.iter()
@@ -329,9 +328,9 @@ pub async fn resolve_firewall_rules_for_sled_agent(
 
             // No rules exist, and we don't need to add anything for the
             // allowlist.
-            (None, false) => None,
+            (None, None) => None,
 
-            (Some(_), true) => {
+            (Some(_), Some(_)) => {
                 return Err(Error::internal_error(
                     "While trying to apply the user-facing services allowlist, \
                     we found unexpected host filters already in the rules. These \
@@ -344,7 +343,7 @@ pub async fn resolve_firewall_rules_for_sled_agent(
 
             // There are host filters, but we don't need to apply the allowlist
             // to this VPC either, so insert the rules as-is.
-            (Some(hosts), false) => {
+            (Some(hosts), None) => {
                 let mut host_addrs = Vec::with_capacity(hosts.len());
                 for host in hosts {
                     match &host.0 {

schema/crdb/add-allowed-source-ips/up.sql

@@ -2,5 +2,5 @@ CREATE TABLE IF NOT EXISTS omicron.public.allowed_source_ip (
     id UUID PRIMARY KEY,
     time_created TIMESTAMPTZ NOT NULL,
     time_modified TIMESTAMPTZ NOT NULL,
-    allowed_ips INET[]
+    allowed_ips INET[] CHECK (array_length(allowed_ips, 1) > 0)
 );

schema/crdb/dbinit.sql

@@ -3770,7 +3770,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.allowed_source_ip (
     -- NULL is used to indicate _any_ source IP is allowed. A _non-empty_ list
     -- represents an explicit allow list of IPs or IP subnets. Note that the
     -- list itself may never be empty.
-    allowed_ips INET[]
+    allowed_ips INET[] CHECK (array_length(allowed_ips, 1) > 0)
 );
 
 /*