From 0264584935fcd01c0d9ed6c764bbeab6a4002106 Mon Sep 17 00:00:00 2001
From: Jordan Jackson <jordan.jackson@ibm.com>
Date: Wed, 23 Nov 2022 13:36:23 +0000
Subject: [PATCH] agent: Update the merge_oci_process function to change cwd

Change the if statement to check if the CWD is set to /
Add unit tests for the correct merging of working directory
in the container and image process

Note: there is an outstanding question about one test case
Format code

Fixes: #5721

Co-authored-by: stevenhorsman <steven@uk.ibm.com>
Signed-off-by: Jordan Jackson <jordan.jackson@ibm.com>
---
 src/agent/src/rpc.rs | 52 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs
index d7e8e4fa9..10d7f0b58 100644
--- a/src/agent/src/rpc.rs
+++ b/src/agent/src/rpc.rs
@@ -172,7 +172,7 @@ fn merge_oci_process(target: &mut oci::Process, source: &oci::Process) {
         target.args.append(&mut source.args.clone());
     }
 
-    if target.cwd.is_empty() && !source.cwd.is_empty() {
+    if target.cwd == "/" && source.cwd != "/" {
         target.cwd = String::from(&source.cwd);
     }
 
@@ -3075,6 +3075,56 @@ COMMIT
         );
     }
 
+    #[tokio::test]
+    async fn test_merge_cwd() {
+        #[derive(Debug)]
+        struct TestData<'a> {
+            container_process_cwd: &'a str,
+            image_process_cwd: &'a str,
+            expected: &'a str,
+        }
+
+        let tests = &[
+            // Image cwd should override blank container cwd
+            // TODO - how can we tell the user didn't specifically set it to `/` vs not setting at all? Is that scenario valid?
+            TestData {
+                container_process_cwd: "/",
+                image_process_cwd: "/imageDir",
+                expected: "/imageDir",
+            },
+            // Container cwd should override image cwd
+            TestData {
+                container_process_cwd: "/containerDir",
+                image_process_cwd: "/imageDir",
+                expected: "/containerDir",
+            },
+            // Container cwd should override blank image cwd
+            TestData {
+                container_process_cwd: "/containerDir",
+                image_process_cwd: "/",
+                expected: "/containerDir",
+            },
+        ];
+
+        for (i, d) in tests.iter().enumerate() {
+            let msg = format!("test[{}]: {:?}", i, d);
+
+            let mut container_process = oci::Process {
+                cwd: d.container_process_cwd.to_string(),
+                ..Default::default()
+            };
+
+            let image_process = oci::Process {
+                cwd: d.image_process_cwd.to_string(),
+                ..Default::default()
+            };
+
+            merge_oci_process(&mut container_process, &image_process);
+
+            assert_eq!(d.expected, container_process.cwd, "{}", msg);
+        }
+    }
+
     #[tokio::test]
     async fn test_merge_env() {
         #[derive(Debug)]