From ec12a46f4a01ec11139b9153897b2d2090da6361 Mon Sep 17 00:00:00 2001
From: Roberto Alfieri <ralfieri@redhat.com>
Date: Mon, 27 Nov 2023 16:49:55 +0100
Subject: [PATCH] Improve `security_opt` comparison between existing container

Since SElinux labels are basically annotations, they are merged in a single comma separated string in the list by podman, so we need to split them in a sorted list if we want to compare it to the list that we provide to the module.

Also, a proper test of this example has been added.

Signed-off-by: Roberto Alfieri <ralfieri@redhat.com>
---
 .../podman/podman_container_lib.py            | 15 ++++---
 .../targets/podman_container/tasks/main.yml   | 42 +++++++++++++++++++
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/plugins/module_utils/podman/podman_container_lib.py b/plugins/module_utils/podman/podman_container_lib.py
index 0433619a..f8a0f8a0 100644
--- a/plugins/module_utils/podman/podman_container_lib.py
+++ b/plugins/module_utils/podman/podman_container_lib.py
@@ -1220,11 +1220,16 @@ def diffparam_rm(self):
         return self._diff_update_and_compare('rm', before, after)
 
     def diffparam_security_opt(self):
-        before = self.info['hostconfig']['securityopt']
-        # In rootful containers with apparmor there is a default security opt
-        before = [o for o in before if 'apparmor=containers-default' not in o]
-        after = self.params['security_opt']
-        before, after = sorted(list(set(before))), sorted(list(set(after)))
+        unsorted_before = self.info['hostconfig']['securityopt']
+        unsorted_after = self.params['security_opt']
+        # In rootful containers with apparmor there is a profile, "container-default",
+        # which is already added by default
+        # Since SElinux labels are basically annotations, they are merged in a single list
+        # element by podman so we need to split them in a (sorted) list if we want to compare it
+        # to the list we provide to the module
+        before = sorted(item for element in unsorted_before for item in element.split(',')
+                        if 'apparmor=container-default' not in item)
+        after = sorted(list(set(unsorted_after)))
         return self._diff_update_and_compare('security_opt', before, after)
 
     def diffparam_stop_signal(self):
diff --git a/tests/integration/targets/podman_container/tasks/main.yml b/tests/integration/targets/podman_container/tasks/main.yml
index ef651d62..984dc946 100644
--- a/tests/integration/targets/podman_container/tasks/main.yml
+++ b/tests/integration/targets/podman_container/tasks/main.yml
@@ -363,6 +363,48 @@
         fail_msg: Deleting started container test failed!
         success_msg: Deleting started container test passed!
 
+    - name: Create container with security_opt
+      containers.podman.podman_container:
+        executable: "{{ test_executable | default('podman') }}"
+        name: container
+        image: docker.io/alpine:3.7
+        state: started
+        command: sleep 1d
+        security_opt:
+            - label=level:s0
+            - label=type:spc_t
+            - label=filetype:container_share_t
+            - seccomp=unconfined
+
+    - name: Recreate container with same security_opt flags
+      containers.podman.podman_container:
+        executable: "{{ test_executable | default('podman') }}"
+        name: container
+        image: docker.io/alpine:3.7
+        state: started
+        command: sleep 1d
+        security_opt:
+            - label=level:s0
+            - label=type:spc_t
+            - label=filetype:container_share_t
+            - seccomp=unconfined
+      register: recreate_security_opt
+
+    - name: Check if output is correct
+      assert:
+        that:
+          - recreate_security_opt is not changed
+          - recreate_security_opt.container is defined
+          - recreate_security_opt.container != {}
+          - recreate_security_opt.container['State']['Running']
+          - "'recreated container' not in recreate_security_opt.actions"
+
+    - name: Remove container
+      containers.podman.podman_container:
+        executable: "{{ test_executable | default('podman') }}"
+        name: container
+        state: absent
+
     - name: Recreate container with parameters
       containers.podman.podman_container:
         executable: "{{ test_executable | default('podman') }}"