is_set -> from_env_or_default

Razz4780 · Feb 7, 2025 · e17bc61 · e17bc61
1 parent 3adb319
commit e17bc61
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 14 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -2,7 +2,6 @@
 
 members = [
     "mirrord/*",
-    "mirrord/agent/env",
     "mirrord/layer/tests/apps/fileops",
     "mirrord/layer/tests/apps/outgoing",
     "mirrord/layer/tests/apps/listen_ports",

diff --git a/mirrord/agent/env/Cargo.toml b/mirrord/agent/env/Cargo.toml
@@ -20,3 +20,4 @@ k8s-openapi = ["dep:k8s-openapi"]
 [dependencies]
 k8s-openapi = { workspace = true, optional = true }
 thiserror.workspace = true
+tracing.workspace = true
diff --git a/mirrord/agent/env/src/checked_env.rs b/mirrord/agent/env/src/checked_env.rs
@@ -53,7 +53,6 @@ pub enum ParseEnvError<E> {
 }
 
 /// An environment variable with strict value type checking.
-#[derive(Clone, Copy, PartialEq, Eq)]
 pub struct CheckedEnv<V: EnvValue> {
     /// Name of the variable.
     pub name: &'static str,
@@ -105,15 +104,32 @@ impl<V: EnvValue<IntoReprError = Infallible>> CheckedEnv<V> {
     }
 }
 
-impl CheckedEnv<bool> {
-    /// Convenience method for checking whether this variable is set.
+impl<V> CheckedEnv<V>
+where
+    V: EnvValue + Default + fmt::Debug,
+    V::FromReprError: fmt::Debug,
+{
+    /// Convenience method for getting this variable's value.
     ///
-    /// For variables with [`bool`] values.
-    ///
-    /// Returns `true` if the variable is present and set to true.
-    /// Returns `false` if the variable is missing or set to some other value.
-    pub fn is_set(self) -> bool {
-        self.try_from_env().ok().flatten().unwrap_or_default()
+    /// Reads this variable's value from the process environment.
+    /// If this variable is missing or its value is malformed, returns a [`Default`].
+    pub fn from_env_or_default(self) -> V {
+        match self.try_from_env() {
+            Ok(None) => Default::default(),
+            Ok(Some(value)) => value,
+            Err(error) => {
+                let default_value = V::default();
+
+                tracing::error!(
+                    ?error,
+                    ?default_value,
+                    ?self,
+                    "Failed to read an environment variable, value is malformed. Using a default value.",
+                );
+
+                default_value
+            }
+        }
     }
 }
 
@@ -132,6 +148,29 @@ impl<V: EnvValue> fmt::Display for CheckedEnv<V> {
     }
 }
 
+impl<V: EnvValue> Clone for CheckedEnv<V> {
+    fn clone(&self) -> Self {
+        Self {
+            name: self.name,
+            value_type: PhantomData,
+        }
+    }
+}
+
+impl<V: EnvValue> Copy for CheckedEnv<V> {}
+
+impl<V: EnvValue> PartialEq for CheckedEnv<V> {
+    fn eq(&self, other: &Self) -> bool {
+        self.name.eq(other.name)
+    }
+
+    fn ne(&self, other: &Self) -> bool {
+        self.name.ne(other.name)
+    }
+}
+
+impl<V: EnvValue> Eq for CheckedEnv<V> {}
+
 impl StoredAsString for bool {}
 
 impl StoredAsString for u32 {}

diff --git a/mirrord/agent/src/entrypoint.rs b/mirrord/agent/src/entrypoint.rs
@@ -864,7 +864,7 @@ pub async fn main() -> AgentResult<()> {
     rustls::crypto::CryptoProvider::install_default(rustls::crypto::aws_lc_rs::default_provider())
         .expect("Failed to install crypto provider");
 
-    if envs::JSON_LOG.is_set() {
+    if envs::JSON_LOG.from_env_or_default() {
         tracing_subscriber::registry()
             .with(
                 tracing_subscriber::fmt::layer()

diff --git a/mirrord/agent/src/steal/ip_tables.rs b/mirrord/agent/src/steal/ip_tables.rs
@@ -102,7 +102,7 @@ pub struct IPTablesWrapper {
 
 /// wrapper around iptables::new that uses nft or legacy based on env
 pub fn new_iptables() -> iptables::IPTables {
-    if envs::NFTABLES.is_set() {
+    if envs::NFTABLES.from_env_or_default() {
         iptables::new_with_cmd("/usr/sbin/iptables-nft")
     } else {
         iptables::new_with_cmd("/usr/sbin/iptables-legacy")
@@ -112,7 +112,7 @@ pub fn new_iptables() -> iptables::IPTables {
 
 /// wrapper around iptables::new that uses nft or legacy based on env
 pub fn new_ip6tables() -> iptables::IPTables {
-    if envs::NFTABLES.is_set() {
+    if envs::NFTABLES.from_env_or_default() {
         iptables::new_with_cmd("/usr/sbin/ip6tables-nft")
     } else {
         iptables::new_with_cmd("/usr/sbin/ip6tables-legacy")

diff --git a/mirrord/agent/src/steal/ip_tables/mesh.rs b/mirrord/agent/src/steal/ip_tables/mesh.rs
@@ -137,7 +137,7 @@ pub(super) trait MeshVendorExt: Sized {
 
 impl MeshVendorExt for MeshVendor {
     fn detect<IPT: IPTables>(ipt: &IPT) -> AgentResult<Option<Self>> {
-        if envs::ISTIO_CNI.is_set() {
+        if envs::ISTIO_CNI.from_env_or_default() {
             return Ok(Some(MeshVendor::IstioCni));
         }