chore(firewall): remove obsolete default firewall config path

rs/config/src/config.rs

@@ -90,7 +90,7 @@ pub struct ConfigOptional {
 
 impl Config {
     /// Return a [Config] with default settings that put all paths under a
-    /// 'parent_dir', with the given 'subnet_id'.
+    /// 'parent_dir'.
     ///
     /// It is an alternative way to construct a Config than parsing
     /// a configuration file.
@@ -111,8 +111,10 @@ impl Config {
             csp_vault_logger: logger,
             message_routing: MessageRoutingConfig::default(),
             malicious_behavior: MaliciousBehavior::default(),
-            firewall: ReplicaFirewallConfig::default(),
-            boundary_node_firewall: BoundaryNodeFirewallConfig::default(),
+            firewall: ReplicaFirewallConfig::new(parent_dir.join("replica_firewall")),
+            boundary_node_firewall: BoundaryNodeFirewallConfig::new(
+                parent_dir.join("boundary_node_firewall"),
+            ),
             registration: RegistrationConfig::default(),
             nns_registry_replicator: NnsRegistryReplicatorConfig::default(),
             adapters_config: AdaptersConfig::default(),

rs/config/src/firewall.rs

@@ -7,9 +7,6 @@ use proptest::prelude::{Strategy, any};
 #[cfg(test)]
 use proptest_derive::Arbitrary;
 
-// This path is not used in practice. The code should panic if it is.
-pub const FIREWALL_FILE_DEFAULT_PATH: &str = "/This/must/not/be/a/real/path";
-
 #[derive(Clone, PartialEq, Debug, Deserialize, Serialize)]
 #[serde(rename_all = "snake_case")]
 #[cfg_attr(test, derive(Arbitrary))]
@@ -35,10 +32,11 @@ pub struct ReplicaConfig {
     pub max_simultaneous_connections_per_ip_address: u32,
 }
 
-impl Default for ReplicaConfig {
-    fn default() -> Self {
+impl ReplicaConfig {
+    /// Create a ReplicaConfig from a given path to the config file.
+    pub fn new(config_file: PathBuf) -> Self {
         Self {
-            config_file: PathBuf::from(FIREWALL_FILE_DEFAULT_PATH),
+            config_file,
             file_template: "".to_string(),
             ipv4_tcp_rule_template: "".to_string(),
             ipv6_tcp_rule_template: "".to_string(),
@@ -74,10 +72,11 @@ pub struct BoundaryNodeConfig {
     pub max_simultaneous_connections_per_ip_address: u32,
 }
 
-impl Default for BoundaryNodeConfig {
-    fn default() -> Self {
+impl BoundaryNodeConfig {
+    /// Create a BoundaryNodeConfig from a given path to the config file.
+    pub fn new(config_file: PathBuf) -> Self {
         Self {
-            config_file: PathBuf::from(FIREWALL_FILE_DEFAULT_PATH),
+            config_file,
             file_template: String::default(),
             ipv4_tcp_rule_template: String::default(),
             ipv6_tcp_rule_template: String::default(),

rs/orchestrator/src/firewall.rs

@@ -5,8 +5,7 @@ use crate::{
     registry_helper::RegistryHelper,
 };
 use ic_config::firewall::{
-    BoundaryNodeConfig as BoundaryNodeFirewallConfig, FIREWALL_FILE_DEFAULT_PATH,
-    ReplicaConfig as ReplicaFirewallConfig,
+    BoundaryNodeConfig as BoundaryNodeFirewallConfig, ReplicaConfig as ReplicaFirewallConfig,
 };
 use ic_logger::{ReplicaLogger, debug, info, warn};
 use ic_protobuf::registry::firewall::v1::{FirewallAction, FirewallRule, FirewallRuleDirection};
@@ -18,7 +17,6 @@ use std::{
     collections::BTreeSet,
     convert::TryFrom,
     net::IpAddr,
-    path::PathBuf,
     sync::{Arc, RwLock},
 };
 
@@ -51,8 +49,6 @@ pub(crate) struct Firewall {
     last_applied_version: Arc<RwLock<RegistryVersion>>,
     /// If true, write the file content even if no change was detected in registry, i.e. first time
     must_write: bool,
-    /// If false, do not update the firewall rules (test mode)
-    enabled: bool,
     node_id: NodeId,
 }
 
@@ -66,18 +62,6 @@ impl Firewall {
         local_cup_reader: LocalCUPReader,
         logger: ReplicaLogger,
     ) -> Self {
-        // Disable if the config is the default one (e.g if we're in a test)
-        let enabled = replica_config
-            .config_file
-            .ne(&PathBuf::from(FIREWALL_FILE_DEFAULT_PATH));
-
-        if !enabled {
-            warn!(
-                logger,
-                "Firewall configuration not found. Orchestrator does not update firewall rules."
-            );
-        }
-
         Self {
             registry,
             metrics,
@@ -88,7 +72,6 @@ impl Firewall {
             compiled_config: Default::default(),
             last_applied_version: Default::default(),
             must_write: true,
-            enabled,
             node_id,
         }
     }
@@ -468,9 +451,6 @@ impl Firewall {
     /// Checks for new firewall config, and if found, update local firewall
     /// rules
     pub fn check_and_update(&mut self) {
-        if !self.enabled {
-            return;
-        }
         let registry_version = self.registry.get_latest_version();
         debug!(
             self.logger,
@@ -708,7 +688,10 @@ fn split_ips_by_address_family(ips: &BTreeSet<IpAddr>) -> (Vec<String>, Vec<Stri
 
 #[cfg(test)]
 mod tests {
-    use std::{io::Write, path::Path};
+    use std::{
+        io::Write,
+        path::{Path, PathBuf},
+    };
 
     use config_tool::guestos::generate_ic_config;
     use ic_config::{ConfigOptional, ConfigSource};