diff --git a/rs/config/src/config.rs b/rs/config/src/config.rs index 56cc94db1e46..60c3f4d8b0a1 100644 --- a/rs/config/src/config.rs +++ b/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(), diff --git a/rs/config/src/firewall.rs b/rs/config/src/firewall.rs index e9134a29e789..663792e44cc1 100644 --- a/rs/config/src/firewall.rs +++ b/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(), diff --git a/rs/orchestrator/src/firewall.rs b/rs/orchestrator/src/firewall.rs index c1113cc4d4fe..923b02a54413 100644 --- a/rs/orchestrator/src/firewall.rs +++ b/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>, /// 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) -> (Vec, Vec