Solved multiple Buildah-related issues in the Buildah SAL + fixed test suite for Buildah

rhai_tests/buildah/01_builder_pattern.rhai

@@ -22,7 +22,8 @@ fn assert_eq(actual, expected, message) {
 // Helper function to check if buildah is available
 fn is_buildah_available() {
     try {
-        let result = run("which buildah");
+        let command = run("which buildah");
+        let result = command.execute();
         return result.success;
     } catch(err) {
         return false;
@@ -121,7 +122,7 @@ try {
     // Test committing to an image
     print("Testing commit()...");
     let image_name = "rhai_test_image:latest";
-    builder.commit(image_name);
+    builder.commit(image_name, []);
     print("✓ commit(): Container committed to image successfully");
     
     // Test removing the container
@@ -154,19 +155,21 @@ try {
     // Clean up in case of error
     try {
         // Remove test container if it exists
-        run("buildah rm rhai_test_container");
+        let command = run("buildah rm rhai_test_container");
-    } catch(_) {}
+        command.execute();
+    } catch(err) {}
     
     try {
         // Remove test image if it exists
-        run("buildah rmi rhai_test_image:latest");
+        let command = run("buildah rmi alpine");
-    } catch(_) {}
+        command.execute();
+    } catch(err) {}
     
     try {
         // Remove test files if they exist
         delete("test_add_file.txt");
         delete("test_copy_file.txt");
-    } catch(_) {}
+    } catch(err) {}
     
     throw err;
 }

rhai_tests/buildah/02_image_operations.rhai

@@ -22,7 +22,8 @@ fn assert_eq(actual, expected, message) {
 // Helper function to check if buildah is available
 fn is_buildah_available() {
     try {
-        let result = run("which buildah");
+        let command = run("which buildah");
+        let result = command.execute();
         return result.success;
     } catch(err) {
         return false;
@@ -32,8 +33,18 @@ fn is_buildah_available() {
 // Helper function to check if an image exists
 fn image_exists(image_name) {
     try {
-        let result = run(`buildah images -q ${image_name}`);
+        // First, check for the exact image name
-        return result.success && result.stdout.trim() != "";
+        let command = run(`buildah images -q ${image_name}`);
+        let result = command.execute();
+        if result.success && result.stdout.trim() != "" {
+            return true;
+        }
+
+        // If not found, check for the localhost-prefixed version
+        let prefixed_image_name = `localhost/${image_name}`;
+        let command = run(`buildah images -q ${prefixed_image_name}`);
+        let result_prefixed = command.execute();
+        return result_prefixed.success && result_prefixed.stdout.trim() != "";
     } catch(err) {
         return false;
     }
@@ -82,8 +93,10 @@ try {
     
     // Find our tagged image
     let found_tag = false;
+    let expected_tag = "rhai_test_tag:latest";
     for image in images {
-        if image.names.contains("rhai_test_tag:latest") {
+        // The tag might be prefixed with 'localhost/' if no registry is specified.
+        if image.names.contains(expected_tag) || image.names.contains("localhost/" + expected_tag) {
             found_tag = true;
             break;
         }
@@ -95,10 +108,11 @@ try {
     print("Testing build()...");
     
     // Create a simple Dockerfile
-    let dockerfile_content = `FROM alpine:latest
+    let dockerfile_content = `
-RUN echo "Hello from Dockerfile" > /hello.txt
+        FROM alpine:latest
-CMD ["cat", "/hello.txt"]
+        RUN echo "Hello from Dockerfile" > /hello.txt
-`;
+        CMD ["cat", "/hello.txt"]
+    `;
     file_write(`${test_dir}/Dockerfile`, dockerfile_content);
     
     // Build the image
@@ -133,18 +147,23 @@ CMD ["cat", "/hello.txt"]
     // Clean up in case of error
     try {
         // Remove test container if it exists
-        run("buildah rm rhai_test_container");
+        let command = run("buildah rm rhai_test_container");
-    } catch(_) {}
+        command.execute();
+    } catch(err) {}
     
     try {
         // Remove test images if they exist
-        run("buildah rmi rhai_test_tag:latest");
+        let command = run("buildah rmi rhai_test_tag:latest");
-        run("buildah rmi rhai_test_build:latest");
+        command.execute();
-    } catch(_) {}
+        let command = run("buildah rmi rhai_test_build:latest");
+        command.execute();
+    } catch(err) {}
+
+    try {
+        // Remove test directory if it exists
+        delete(test_dir);
+        print("✓ Cleanup: Test directory removed");
+    } catch (err) {}
     
     throw err;
-} finally {
-    // Clean up test directory
-    delete(test_dir);
-    print("✓ Cleanup: Test directory removed");
 }

rhai_tests/buildah/03_container_operations.rhai

@@ -22,7 +22,8 @@ fn assert_eq(actual, expected, message) {
 // Helper function to check if buildah is available
 fn is_buildah_available() {
     try {
-        let result = run("which buildah");
+        let command = run("which buildah");
+        let result = command.execute();
         return result.success;
     } catch(err) {
         return false;
@@ -59,10 +60,12 @@ try {
     
     // Test config
     print("Testing config()...");
-    let config_options = #{
+    let config_options = [
-        "LABEL": "rhai_test=true",
+        ["label", "rhai_test_true"],
-        "ENV": "TEST_VAR=test_value"
+        ["env", "TEST_VAR=test_value"],
-    };
+        ["env", "ANOTHER_VAR=another_value"],
+        ["author", "Rhai Test With Spaces"]
+    ];
     builder.config(config_options);
     print("✓ config(): Container configured successfully");
     
@@ -77,9 +80,10 @@ try {
     print("Testing content operations...");
     
     // Write content to a file
-    let script_content = `#!/bin/sh
+    let script_content = `
-echo "Hello from script"
+        #!/bin/sh
-`;
+        echo "Hello from script"
+    `;
     builder.write_content(script_content, "/script.sh");
     
     // Make the script executable
@@ -91,14 +95,10 @@ echo "Hello from script"
     assert_true(script_result.stdout.contains("Hello from script"), "Script output should contain expected text");
     print("✓ Content operations: Script created and executed successfully");
     
-    // Test commit with config
+    // Test commit
-    print("Testing commit with config...");
+    print("Testing commit...");
-    let commit_options = #{
+    builder.commit("rhai_test_commit:latest", [["q", ""]]);
-        "author": "Rhai Test",
+    print("✓ commit(): Container committed successfully");
-        "message": "Test commit"
-    };
-    builder.commit("rhai_test_commit:latest", commit_options);
-    print("✓ commit(): Container committed with config successfully");
     
     // Clean up
     builder.remove();
@@ -115,13 +115,15 @@ echo "Hello from script"
     // Clean up in case of error
     try {
         // Remove test container if it exists
-        run("buildah rm rhai_test_container");
+        let command = run("buildah rm rhai_test_container");
-    } catch(_) {}
+        command.execute();
+    } catch(err) {}
     
     try {
         // Remove test image if it exists
-        run("buildah rmi rhai_test_commit:latest");
+        let command = run("buildah rmi rhai_test_commit:latest");
-    } catch(_) {}
+        command.execute();
+    } catch(err) {}
     
     throw err;
 }

rhai_tests/buildah/run_all_tests.rhai

@@ -1,155 +0,0 @@
-// run_all_tests.rhai
-// Runs all Buildah module tests
-
-print("=== Running Buildah Module Tests ===");
-
-// Custom assert function
-fn assert_true(condition, message) {
-    if !condition {
-        print(`ASSERTION FAILED: ${message}`);
-        throw message;
-    }
-}
-
-// Helper function to check if buildah is available
-fn is_buildah_available() {
-    try {
-        let result = run("which buildah");
-        return result.success;
-    } catch(e) {
-        return false;
-    }
-}
-
-// Run each test directly
-let passed = 0;
-let failed = 0;
-let skipped = 0;
-let total = 0;
-
-// Check if buildah is available
-let buildah_available = is_buildah_available();
-if !buildah_available {
-    print("Buildah is not available. Skipping all Buildah tests.");
-    skipped = 3; // Skip all three tests
-    total = 3;
-} else {
-    // Test 1: Builder Pattern
-    print("\n--- Running Builder Pattern Tests ---");
-    try {
-        // Create a builder
-        let builder = bah_new("rhai_test_container", "alpine:latest");
-        
-        // Test basic properties
-        assert_true(builder.container_id != "", "Container ID should not be empty");
-        assert_true(builder.name == "rhai_test_container", "Container name should match");
-        
-        // Run a simple command
-        let result = builder.run("echo 'Hello from container'");
-        assert_true(result.success, "Command should succeed");
-        
-        // Clean up
-        builder.remove();
-        
-        print("--- Builder Pattern Tests completed successfully ---");
-        passed += 1;
-    } catch(err) {
-        print(`!!! Error in Builder Pattern Tests: ${err}`);
-        failed += 1;
-        
-        // Clean up in case of error
-        try {
-            run("buildah rm rhai_test_container");
-        } catch(e) {
-            // Ignore errors during cleanup
-        }
-    }
-    total += 1;
-    
-    // Test 2: Image Operations
-    print("\n--- Running Image Operations Tests ---");
-    try {
-        // Create a temporary directory for testing
-        let test_dir = "rhai_test_buildah";
-        mkdir(test_dir);
-        
-        // Create a builder
-        let builder = bah_new("rhai_test_container", "alpine:latest");
-        
-        // List images
-        let images = builder.images();
-        assert_true(images.len() > 0, "There should be at least one image");
-        
-        // Clean up
-        builder.remove();
-        delete(test_dir);
-        
-        print("--- Image Operations Tests completed successfully ---");
-        passed += 1;
-    } catch(err) {
-        print(`!!! Error in Image Operations Tests: ${err}`);
-        failed += 1;
-        
-        // Clean up in case of error
-        try {
-            run("buildah rm rhai_test_container");
-            delete("rhai_test_buildah");
-        } catch(e) {
-            // Ignore errors during cleanup
-        }
-    }
-    total += 1;
-    
-    // Test 3: Container Operations
-    print("\n--- Running Container Operations Tests ---");
-    try {
-        // Create a builder
-        let builder = bah_new("rhai_test_container", "alpine:latest");
-        
-        // Test reset
-        builder.reset();
-        
-        // Create a new container
-        builder = bah_new("rhai_test_container", "alpine:latest");
-        
-        // Run a command
-        let result = builder.run("echo 'Hello from container'");
-        assert_true(result.success, "Command should succeed");
-        
-        // Clean up
-        builder.remove();
-        
-        print("--- Container Operations Tests completed successfully ---");
-        passed += 1;
-    } catch(err) {
-        print(`!!! Error in Container Operations Tests: ${err}`);
-        failed += 1;
-        
-        // Clean up in case of error
-        try {
-            run("buildah rm rhai_test_container");
-        } catch(e) {
-            // Ignore errors during cleanup
-        }
-    }
-    total += 1;
-}
-
-print("\n=== Test Summary ===");
-print(`Passed: ${passed}`);
-print(`Failed: ${failed}`);
-print(`Skipped: ${skipped}`);
-print(`Total: ${total}`);
-
-if failed == 0 {
-    if skipped > 0 {
-        print("\n⚠️ All tests skipped or passed!");
-    } else {
-        print("\n✅ All tests passed!");
-    }
-} else {
-    print("\n❌ Some tests failed!");
-}
-
-// Return the number of failed tests (0 means success)
-failed;

virt/src/buildah/builder.rs

@@ -249,8 +249,32 @@ impl Builder {
     /// # Returns
     ///
     /// * `Result<CommandResult, BuildahError>` - Command result or error
-    pub fn commit(&self, image_name: &str) -> Result<CommandResult, BuildahError> {
+    pub fn commit(&self, image_name: &str, options: Option<Vec<(String, String)>>) -> Result<CommandResult, BuildahError> {
         if let Some(container_id) = &self.container_id {
+            let mut args_owned: Vec<String> = Vec::new();
+            args_owned.push("commit".to_string());
+
+            // Process options
+            if let Some(options_vec) = options {
+                for (key, value) in options_vec.iter() {
+                    let option_name = if key.len() == 1 {
+                        format!("-{}", key)
+                    } else {
+                        format!("--{}", key)
+                    };
+                    args_owned.push(option_name);
+                    if !value.is_empty() {
+                        args_owned.push(value.clone());
+                    }
+                }
+            }
+
+            args_owned.push(container_id.clone());
+            args_owned.push(image_name.to_string());
+
+            // Convert Vec<String> to Vec<&str> for execute_buildah_command
+            let args: Vec<&str> = args_owned.iter().map(|s| s.as_str()).collect();
+
             // Save the current debug flag
             let previous_debug = thread_local_debug();
 
@@ -258,7 +282,7 @@ impl Builder {
             set_thread_local_debug(self.debug);
 
             // Execute the command
-            let result = execute_buildah_command(&["commit", container_id, image_name]);
+            let result = execute_buildah_command(&args);
 
             // Restore the previous debug flag
             set_thread_local_debug(previous_debug);
@@ -336,16 +360,22 @@ impl Builder {
     /// # Returns
     ///
     /// * `Result<CommandResult, BuildahError>` - Command result or error
-    pub fn config(&self, options: HashMap<String, String>) -> Result<CommandResult, BuildahError> {
+    pub fn config(&self, options: Vec<(String, String)>) -> Result<CommandResult, BuildahError> {
         if let Some(container_id) = &self.container_id {
             let mut args_owned: Vec<String> = Vec::new();
             args_owned.push("config".to_string());
 
-            // Process options map
+            // Process options
             for (key, value) in options.iter() {
-                let option_name = format!("--{}", key);
+                let option_name = if key.len() == 1 {
+                    format!("-{}", key)
+                } else {
+                    format!("--{}", key)
+                };
                 args_owned.push(option_name);
-                args_owned.push(value.clone());
+                if !value.is_empty() {
+                    args_owned.push(value.clone());
+                }
             }
 
             args_owned.push(container_id.clone());
@@ -380,8 +410,19 @@ impl Builder {
     /// # Returns
     ///
     /// * `Result<CommandResult, BuildahError>` - Command result or error
-    pub fn set_entrypoint(&self, entrypoint: &str) -> Result<CommandResult, BuildahError> {
+    pub fn set_entrypoint(&self, entrypoint: Vec<String>) -> Result<CommandResult, BuildahError> {
         if let Some(container_id) = &self.container_id {
+            // Serialize the entrypoint vector to a JSON string
+            let entrypoint_json = match serde_json::to_string(&entrypoint) {
+                Ok(json) => json,
+                Err(e) => {
+                    return Err(BuildahError::JsonParseError(format!(
+                        "Failed to serialize entrypoint to JSON: {}",
+                        e
+                    )));
+                }
+            };
+
             // Save the current debug flag
             let previous_debug = thread_local_debug();
 
@@ -390,7 +431,7 @@ impl Builder {
 
             // Execute the command
             let result =
-                execute_buildah_command(&["config", "--entrypoint", entrypoint, container_id]);
+                execute_buildah_command(&["config", "--entrypoint", &entrypoint_json, container_id]);
 
             // Restore the previous debug flag
             set_thread_local_debug(previous_debug);
@@ -410,8 +451,19 @@ impl Builder {
     /// # Returns
     ///
     /// * `Result<CommandResult, BuildahError>` - Command result or error
-    pub fn set_cmd(&self, cmd: &str) -> Result<CommandResult, BuildahError> {
+    pub fn set_cmd(&self, cmd: Vec<String>) -> Result<CommandResult, BuildahError> {
         if let Some(container_id) = &self.container_id {
+            // Serialize the cmd vector to a JSON string
+            let cmd_json = match serde_json::to_string(&cmd) {
+                Ok(json) => json,
+                Err(e) => {
+                    return Err(BuildahError::JsonParseError(format!(
+                        "Failed to serialize cmd to JSON: {}",
+                        e
+                    )));
+                }
+            };
+
             // Save the current debug flag
             let previous_debug = thread_local_debug();
 
@@ -419,7 +471,7 @@ impl Builder {
             set_thread_local_debug(self.debug);
 
             // Execute the command
-            let result = execute_buildah_command(&["config", "--cmd", cmd, container_id]);
+            let result = execute_buildah_command(&["config", "--cmd", &cmd_json, container_id]);
 
             // Restore the previous debug flag
             set_thread_local_debug(previous_debug);

virt/src/buildah/content.rs

@@ -55,34 +55,44 @@ impl ContentOperations {
     ///
     /// * `Result<String, BuildahError>` - File content or error
     pub fn read_content(container_id: &str, source_path: &str) -> Result<String, BuildahError> {
-        // Create a temporary file
+        // Create a temporary file to store the content from the container.
         let temp_file = NamedTempFile::new()
             .map_err(|e| BuildahError::Other(format!("Failed to create temporary file: {}", e)))?;
-
         let temp_path = temp_file.path().to_string_lossy().to_string();
 
-        // Copy the file from the container to the temporary file
+        // In rootless mode, `buildah mount` must run inside `buildah unshare`.
-        // Use mount to access the container's filesystem
+        // We create a script to mount, copy the file, and unmount, all within the unshare session.
-        let mount_result = execute_buildah_command(&["mount", container_id])?;
+        let script = format!(
-        let mount_point = mount_result.stdout.trim();
+            r#"
+                set -e
+                mount_point=$(buildah mount '{container_id}')
+                if [ -z "$mount_point" ]; then
+                    echo "Error: Failed to mount container '{container_id}'." >&2
+                    exit 1
+                fi
+                trap 'buildah umount '{container_id}'' EXIT
+                cp "${{mount_point}}{source_path}" '{temp_path}'
+            "#,
+            container_id = container_id,
+            source_path = source_path,
+            temp_path = &temp_path
+        );
 
-        // Construct the full path to the file in the container
+        let result = execute_buildah_command(&["unshare", "sh", "-c", &script])?;
-        let full_source_path = format!("{}{}", mount_point, source_path);
 
-        // Copy the file from the mounted container to the temporary file
+        if !result.success {
-        execute_buildah_command(&["copy", container_id, &full_source_path, &temp_path])?;
+            return Err(BuildahError::Other(format!(
-
+                "Failed to execute read_content script in unshare session: {}",
-        // Unmount the container
+                result.stderr
-        execute_buildah_command(&["umount", container_id])?;
+            )));
-
+        }
-        // Read the content from the temporary file
-        let mut file = File::open(temp_file.path())
-            .map_err(|e| BuildahError::Other(format!("Failed to open temporary file: {}", e)))?;
 
+        // The script has copied the file content to our temporary file.
+        // Now, we read it.
         let mut content = String::new();
-        file.read_to_string(&mut content).map_err(|e| {
+        File::open(&temp_path)
-            BuildahError::Other(format!("Failed to read from temporary file: {}", e))
+            .and_then(|mut f| f.read_to_string(&mut content))
-        })?;
+            .map_err(|e| BuildahError::Other(format!("Failed to read from temporary file: {}", e)))?;
 
         Ok(content)
     }

virt/src/rhai/buildah.rs

@@ -98,20 +98,42 @@ fn bah_error_to_rhai_error<T>(result: Result<T, BuildahError>) -> Result<T, Box<
     })
 }
 
-// Helper function to convert Rhai Map to Rust HashMap
+// Helper function to convert Rhai Array of pairs to a Vec of tuples
-fn convert_map_to_hashmap(options: Map) -> Result<HashMap<String, String>, Box<EvalAltResult>> {
+fn convert_array_to_vec(
-    let mut config_options = HashMap::<String, String>::new();
+    options: Array,
+) -> Result<Vec<(String, String)>, Box<EvalAltResult>> {
+    let mut config_options: Vec<(String, String)> = Vec::new();
 
-    for (key, value) in options.iter() {
+    for option_pair_dynamic in options {
-        if let Ok(value_str) = value.clone().into_string() {
+        let pair = option_pair_dynamic.into_array().map_err(|_e| {
-            // Convert SmartString to String
+            Box::new(EvalAltResult::ErrorRuntime(
-            config_options.insert(key.to_string(), value_str);
+                "Each option must be an array of [key, value]".into(),
-        } else {
+                rhai::Position::NONE,
+            ))
+        })?;
+
+        if pair.len() != 2 {
             return Err(Box::new(EvalAltResult::ErrorRuntime(
-                format!("Option '{}' must be a string", key).into(),
+                "Each option must be an array of [key, value] with 2 elements".into(),
                 rhai::Position::NONE,
             )));
         }
+
+        let key = pair[0].clone().into_string().map_err(|_e| {
+            Box::new(EvalAltResult::ErrorRuntime(
+                "Option key must be a string".into(),
+                rhai::Position::NONE,
+            ))
+        })?;
+
+        let value = pair[1].clone().into_string().map_err(|_e| {
+            Box::new(EvalAltResult::ErrorRuntime(
+                "Option value must be a string".into(),
+                rhai::Position::NONE,
+            ))
+        })?;
+
+        config_options.push((key, value));
     }
 
     Ok(config_options)
@@ -157,8 +179,10 @@ pub fn builder_add(
 pub fn builder_commit(
     builder: &mut Builder,
     image_name: &str,
+    options: Array,
 ) -> Result<CommandResult, Box<EvalAltResult>> {
-    bah_error_to_rhai_error(builder.commit(image_name))
+    let commit_options = convert_array_to_vec(options)?;
+    bah_error_to_rhai_error(builder.commit(image_name, Some(commit_options)))
 }
 
 pub fn builder_remove(builder: &mut Builder) -> Result<CommandResult, Box<EvalAltResult>> {
@@ -167,27 +191,34 @@ pub fn builder_remove(builder: &mut Builder) -> Result<CommandResult, Box<EvalAl
 
 pub fn builder_config(
     builder: &mut Builder,
-    options: Map,
+    options: Array,
 ) -> Result<CommandResult, Box<EvalAltResult>> {
-    // Convert Rhai Map to Rust HashMap
+    let config_options = convert_array_to_vec(options)?;
-    let config_options = convert_map_to_hashmap(options)?;
     bah_error_to_rhai_error(builder.config(config_options))
 }
 
 /// Set the entrypoint for the container
 pub fn builder_set_entrypoint(
     builder: &mut Builder,
-    entrypoint: &str,
+    entrypoint: Array,
 ) -> Result<CommandResult, Box<EvalAltResult>> {
-    bah_error_to_rhai_error(builder.set_entrypoint(entrypoint))
+    let entrypoint_vec: Vec<String> = entrypoint
+        .into_iter()
+        .map(|item| item.into_string().unwrap_or_default())
+        .collect();
+    bah_error_to_rhai_error(builder.set_entrypoint(entrypoint_vec))
 }
 
 /// Set the default command for the container
 pub fn builder_set_cmd(
     builder: &mut Builder,
-    cmd: &str,
+    cmd: Array,
 ) -> Result<CommandResult, Box<EvalAltResult>> {
-    bah_error_to_rhai_error(builder.set_cmd(cmd))
+    let cmd_vec: Vec<String> = cmd
+        .into_iter()
+        .map(|item| item.into_string().unwrap_or_default())
+        .collect();
+    bah_error_to_rhai_error(builder.set_cmd(cmd_vec))
 }
 
 /// Write content to a file in the container