[PATCH 5/6] guestfs: replace ansible group permisison requirement on libvirt system uri

[Luis Chamberlain <mcgrof@kernel.org>]

The bringup process for libvirt system URI support (not session),
so all debian based distros, requieres us to be paranoid about the
permissions of our storage directory where we place our libvirt
storage pool, and guestfs images.

We used to be stupid and were hammering with a sledge hammer a crazy
sudo chown -R on a target storage path. That was removed by commit
c31459dc384c ("scripts/bringup_guestfs.sh: fix silly directory permission
fix"). I rushed that change in because it was affecting live systems
and we needed to get testing moving.

This replaces the old requirement with some more less aggressive
directory creation and directory permission requirements and adds some
sanity checks which don't do the crazy wild permission changes that
are possible with a recursive call.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 kconfigs/Kconfig.guestfs                      | 10 +++
 .../roles/bringup_guestfs/tasks/main.yml      | 76 +++++++++++++++++++
 scripts/bringup_guestfs.sh                    |  3 -
 scripts/guestfs.Makefile                      |  2 +-
 4 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/kconfigs/Kconfig.guestfs b/kconfigs/Kconfig.guestfs
index c6d2d1907dd5..d309436fa7c9 100644
--- a/kconfigs/Kconfig.guestfs
+++ b/kconfigs/Kconfig.guestfs
@@ -5,6 +5,16 @@ config STORAGE_POOL_PATH
 	output yaml
 	default LIBVIRT_STORAGE_POOL_PATH
 
+config GUESTFS_STORAGE_DIR
+	string
+	output yaml
+	default "{{ kdevops_storage_pool_path }}/kdevops/guestfs"
+
+config GUESTFS_BASE_IMAGE_DIR
+	string
+	output yaml
+	default "{{ guestfs_storage_dir }}/base_images"
+
 config GUESTFS_HAS_CUSTOM_RAW_IMAGE
 	bool
 
diff --git a/playbooks/roles/bringup_guestfs/tasks/main.yml b/playbooks/roles/bringup_guestfs/tasks/main.yml
index dcbbaef02522..0b193dad807f 100644
--- a/playbooks/roles/bringup_guestfs/tasks/main.yml
+++ b/playbooks/roles/bringup_guestfs/tasks/main.yml
@@ -42,6 +42,82 @@
   when: guestfs_subdirectories.matched == 0
   tags: [ 'config-check' ]
 
+- name: Create storage pool path directory if (libvirt session uri)
+  file:
+    path: "{{ libvirt_storage_pool_path }}"
+    state: directory
+  when: 'not libvirt_uri_system|bool'
+  tags: ['storage-pool-path']
+
+- name: Create storage pool path directory and set group if using (libvirt system uri)
+  file:
+    path: "{{ libvirt_storage_pool_path }}"
+    state: directory
+    owner: root
+    group: "{{ libvirt_qemu_group }}"
+    mode: "0775"
+  when: 'libvirt_uri_system|bool'
+  tags: ['storage-pool-path']
+
+- name: Create kdevops guestfs storage directory if missing (libvirt session uri)
+  file:
+    path: "{{ guestfs_base_image_dir }}"
+    state: directory
+    mode: '0755'
+  tags: ['storage-pool-path']
+  when:
+    - 'not libvirt_uri_system|bool'
+
+- name: Create kdevops guestfs storage directory if missing (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  file:
+    path: "{{ guestfs_base_image_dir }}"
+    state: directory
+    mode: '0775'
+    group: "{{ libvirt_qemu_group }}"
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+
+- name: Check if directory is owned by the correct group (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  command: stat -c '%G' "{{ libvirt_storage_pool_path }}"
+  register: dir_group
+  changed_when: false
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+
+- name: Check if directory has group write permissions (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  command: stat -c '%A' "{{ libvirt_storage_pool_path }}"
+  register: dir_perms
+  changed_when: false
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+
+- name: Verify storage pool path directory is group-writable (libvirt system uri)
+  become: yes
+  become_flags: 'su - -c'
+  become_method: sudo
+  fail:
+    msg: |
+      The permissions for {{ libvirt_storage_pool_path }} should be group
+      writeable by the group used by libvirt: {{ libvirt_qemu_group }}
+      Current group: {{ dir_group.stdout }}
+      Current permissions: {{ dir_perms.stdout }}
+  tags: ['storage-pool-path']
+  when:
+    - 'libvirt_uri_system|bool'
+    - (dir_group.stdout != libvirt_qemu_group) or (dir_perms.stdout[5] != 'w')
+
 - name: Check for dnsmasq configuration files
   stat:
     path: "{{ item }}"
diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
index 976d1e78ed6a..bc0176f8f5b4 100755
--- a/scripts/bringup_guestfs.sh
+++ b/scripts/bringup_guestfs.sh
@@ -271,9 +271,6 @@ if [[ "$CONFIG_LIBVIRT_URI_SYSTEM" == "y" ]]; then
 	USE_SUDO="sudo "
 fi
 
-$USE_SUDO mkdir -p $STORAGEDIR
-$USE_SUDO mkdir -p $BASE_IMAGE_DIR
-
 cmdfile=$(mktemp)
 
 if [ ! -f $BASE_IMAGE ]; then
diff --git a/scripts/guestfs.Makefile b/scripts/guestfs.Makefile
index d08e697f3cfb..e1cf25d62d04 100644
--- a/scripts/guestfs.Makefile
+++ b/scripts/guestfs.Makefile
@@ -83,7 +83,7 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS)
 		playbooks/bringup_guestfs.yml \
 		-e 'ansible_python_interpreter=/usr/bin/python3' \
 		--extra-vars=@./extra_vars.yaml \
-		--tags config-check,network
+		--tags config-check,network,storage-pool-path
 	$(Q)$(TOPDIR)/scripts/bringup_guestfs.sh
 	$(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \
 		--inventory localhost, \
-- 
2.47.2