WIP

src/config.rs

@@ -139,6 +139,7 @@ impl RecertConfig {
                 kubeadmin_password_hash: None,
                 pull_secret: None,
                 additional_trust_bundle: None,
+                proxy: None,
             },
             threads: None,
             regenerate_server_ssh_keys: None,

src/ocp_postprocess/additional_trust_bundle/filesystem_rename.rs

@@ -11,7 +11,6 @@ pub(crate) async fn fix_filesystem_ca_trust_anchors(additional_trust_bundle: &st
         file_utils::globvec(dir, "**/anchors/openshift-config-user-ca-bundle.crt")?
             .into_iter()
             .map(|file_path| {
-                let crt_file_path = file_path.clone();
                 let additional_trust_bundle = additional_trust_bundle.to_string();
                 tokio::spawn(async move {
                     async move {

src/ocp_postprocess/proxy_rename.rs

@@ -68,7 +68,7 @@ async fn fix_etcd_resources(etcd_client: &Arc<InMemoryK8sEtcd>, proxy: &Proxy) -
 
     etcd_rename::fix_controllerconfigs(etcd_client, proxy)
         .await
-        .context("fixing machineconfigs")?;
+        .context("fixing controllerconfigs")?;
 
     Ok(())
 }

src/ocp_postprocess/proxy_rename/etcd_rename.rs

@@ -108,17 +108,21 @@ pub(crate) async fn fix_controllerconfigs(etcd_client: &InMemoryK8sEtcd, proxy:
                     .await?
                     .context("no machineconfig")?;
 
-                let spec_proxy = cluster_proxy
-                    .pointer_mut("/spec/proxy")
-                    .context("no /spec/proxy")?
-                    .as_object_mut()
-                    .context("/spec/proxy not an object")?;
-
-                spec_proxy.insert("httpProxy".to_string(), Value::String(proxy.http_proxy.clone()));
-                spec_proxy.insert("httpsProxy".to_string(), Value::String(proxy.https_proxy.clone()));
-                spec_proxy.insert("noProxy".to_string(), Value::String(proxy.no_proxy.clone()));
-
-                put_etcd_yaml(etcd_client, &k8s_resource_location, cluster_proxy).await?;
+                let object_mut = cluster_proxy.pointer_mut("/spec/proxy").context("no /spec/proxy")?.as_object_mut();
+
+                match object_mut {
+                    None => {
+                        // This is simply null when the proxy is not set
+                        return Ok(())
+                    },
+                    Some(spec_proxy) => {
+                        spec_proxy.insert("httpProxy".to_string(), Value::String(proxy.http_proxy.clone()));
+                        spec_proxy.insert("httpsProxy".to_string(), Value::String(proxy.https_proxy.clone()));
+                        spec_proxy.insert("noProxy".to_string(), Value::String(proxy.no_proxy.clone()));
+
+                        put_etcd_yaml(etcd_client, &k8s_resource_location, cluster_proxy).await?;
+                    }
+                }
 
                 Ok(())
             }),

src/ocp_postprocess/proxy_rename/filesystem_rename.rs

@@ -1,6 +1,6 @@
 use super::{
     args::Proxy,
-    utils::{self, fix_containers_proxy, fix_machineconfig},
+    utils::{self, fix_containers, fix_machineconfig},
 };
 use crate::file_utils::{self, commit_file, read_file_to_string};
 use anyhow::{self, Context, Result};
@@ -108,7 +108,7 @@ pub(crate) async fn fix_pods_yaml(proxy: &Proxy, dir: &Path) -> Result<()> {
 
                 let mut config: Value = serde_yaml::from_str(&contents).context("parsing pods.yaml")?;
 
-                fix_containers_proxy(&mut config, &proxy)?;
+                fix_containers(&mut config, &proxy).context("fixing containers")?;
 
                 commit_file(file_path, serde_yaml::to_string(&config).context("serializing pods.yaml")?)
                     .await

src/ocp_postprocess/proxy_rename/utils.rs

@@ -1,3 +1,5 @@
+use std::ops::Not;
+
 use super::args::Proxy;
 use crate::file_utils;
 use anyhow::{Context, Result};
@@ -70,10 +72,18 @@ pub(crate) fn fix_machineconfig(machineconfig: &mut Value, proxy: &Proxy) -> Res
     Ok(())
 }
 
-pub(crate) fn fix_containers_proxy(config: &mut Value, proxy: &Proxy) -> Result<()> {
-    let prefix = match config.pointer("kind").context("no /kind")?.as_str().context("kind not a string")? {
+pub(crate) fn fix_containers(config: &mut Value, proxy: &Proxy) -> Result<()> {
+    let kind = config.pointer("kind");
+
+    let kind = match kind {
+        Some(kind) => kind,
+        None => return Ok(()),
+    };
+
+    let prefix = match kind.as_str().context("kind not a string")? {
         "Deployment" | "DaemonSet" | "StatefulSet" | "ReplicaSet" => "/spec/template/spec/",
         "Pod" => "/spec/",
+        _ => return Ok(()),
     };
 
     let suffixes = &["containers", "initContainers"];
@@ -93,57 +103,122 @@ pub(crate) fn fix_containers_proxy(config: &mut Value, proxy: &Proxy) -> Result<
                 .as_array_mut()
                 .context("env not an array")?;
 
-            let proxies = [
-                (
-                    "HTTP_PROXY",
-                    if !proxy.http_proxy.is_empty() {
-                        Some(proxy.http_proxy.as_str())
-                    } else {
-                        None
-                    },
-                ),
-                (
-                    "HTTPS_PROXY",
-                    if !proxy.https_proxy.is_empty() {
-                        Some(proxy.https_proxy.as_str())
-                    } else {
-                        None
-                    },
-                ),
-                (
-                    "NO_PROXY",
-                    if !proxy.no_proxy.is_empty() {
-                        Some(proxy.no_proxy.as_str())
-                    } else {
-                        None
-                    },
-                ),
-            ];
-
-            for (proxy_env_name, desired_proxy_env_value) in proxies.iter() {
-                match desired_proxy_env_value {
-                    Some(desired_proxy_env_value) => {
-                        let found_env = container_env
-                            .iter_mut()
-                            .find_map(|env| (env.pointer("/name")? == proxy_env_name).then_some(env));
-
-                        match found_env {
-                            Some(env) => {
-                                env["value"] = serde_json::Value::String(desired_proxy_env_value.to_string());
-                            }
-                            None => {
-                                container_env.push(serde_json::json!({"name": proxy_env_name, "value": desired_proxy_env_value}));
-                            }
-                        }
-                    }
-                    None => {
-                        // Remove the env var if it exists
-                        container_env.retain(|env| env.pointer("/name")? != proxy_env_name);
-                    }
+            let desired_proxies = [
+                proxy
+                    .http_proxy
+                    .is_empty()
+                    .not()
+                    .then_some(("HTTP_PROXY", proxy.http_proxy.as_str())),
+                proxy
+                    .https_proxy
+                    .is_empty()
+                    .not()
+                    .then_some(("HTTPS_PROXY", proxy.https_proxy.as_str())),
+                proxy.no_proxy.is_empty().not().then_some(("NO_PROXY", proxy.no_proxy.as_str())),
+            ]
+            .iter()
+            .filter_map(|x| *x)
+            .map(|(k, v)| serde_json::json!({"name": k, "value": v}))
+            .collect::<Vec<_>>();
+
+            let insertion_index = remove_existing_proxy_env_vars(container_env)?;
+
+            match insertion_index {
+                Some(i) => {
+                    container_env.splice(i..i, desired_proxies);
                 }
+                None => continue,
             }
         }
     }
 
     Ok(())
 }
+
+// Remove all existing proxy env vars from the container's env and return the index of where
+// the first proxy env var should be inserted
+fn remove_existing_proxy_env_vars(container_env: &mut Vec<Value>) -> Result<Option<usize>> {
+    let indices_to_remove = container_env
+        .iter()
+        .enumerate()
+        .filter_map(|(i, env)| {
+            let name = env.pointer("/name").context("no /name").ok()?;
+            if name == "HTTP_PROXY" || name == "HTTPS_PROXY" || name == "NO_PROXY" {
+                Some(i)
+            } else {
+                None
+            }
+        })
+        .collect::<Vec<_>>();
+
+    // Run backwards so we don't have to adjust the indices as we remove elements
+    for i in indices_to_remove.iter().rev() {
+        container_env.remove(*i);
+    }
+
+    if indices_to_remove.is_empty() {
+        Ok(None)
+    } else {
+        Ok(Some(indices_to_remove[0]))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::json;
+
+    #[test]
+    fn test_remove_existing_proxy_env_vars() {
+        let mut env = vec![
+            json!({"name": "SOME", "value": "value"}),
+            json!({"name": "HTTP_PROXY", "value": "http://proxy.example.com"}),
+            json!({"name": "HTTPS_PROXY", "value": "http://proxy.example.com"}),
+            json!({"name": "NO_PROXY", "value": "localhost"}),
+            json!({"name": "OTHER", "value": "value"}),
+        ];
+
+        let insertion_index = remove_existing_proxy_env_vars(&mut env).unwrap();
+
+        assert_eq!(
+            env,
+            vec![
+                json!({"name": "SOME", "value": "value"}),
+                json!({"name": "OTHER", "value": "value"})
+            ]
+        );
+        assert_eq!(insertion_index, Some(1));
+
+        ////////////////////////////////////////////////////
+
+        let mut env = vec![
+            json!({"name": "HTTP_PROXY", "value": "http://proxy.example.com"}),
+            json!({"name": "HTTPS_PROXY", "value": "http://proxy.example.com"}),
+            json!({"name": "NO_PROXY", "value": "localhost"}),
+        ];
+
+        let insertion_index = remove_existing_proxy_env_vars(&mut env).unwrap();
+
+        assert!(env.is_empty());
+        assert_eq!(insertion_index, Some(0));
+
+        ////////////////////////////////////////////////////
+
+        let mut env = vec![
+            json!({"name": "SOME", "value": "value"}),
+            json!({"name": "HTTPS_PROXY", "value": "http://proxy.example.com"}),
+            json!({"name": "OTHER", "value": "value"}),
+        ];
+
+        let insertion_index = remove_existing_proxy_env_vars(&mut env).unwrap();
+
+        assert_eq!(
+            env,
+            vec![
+                json!({"name": "SOME", "value": "value"}),
+                json!({"name": "OTHER", "value": "value"})
+            ]
+        );
+        assert_eq!(insertion_index, Some(1));
+    }
+}

src/recert.rs

@@ -2,7 +2,7 @@ use crate::{
     cluster_crypto::{crypto_utils::ensure_openssl_version, scanning, ClusterCryptoObjects},
     config::{ClusterCustomizations, CryptoCustomizations, RecertConfig},
     k8s_etcd::InMemoryK8sEtcd,
-    ocp_postprocess::{ocp_postprocess, proxy_rename::args::Proxy},
+    ocp_postprocess::ocp_postprocess,
     rsa_key_pool, server_ssh_keys,
 };
 use anyhow::{Context, Result};