From ef159be5f75efbb3e9332c36826e9afb2155c491 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 21 Nov 2023 17:23:57 -0700 Subject: [PATCH] refactor: improve support for ostree systems The dependency on `ansible.utils.update_fact` is causing issue with some users who now must install that collection in order to run the role, even if they do not care about ostree. The fix is to stop trying to set `ansible_facts.pkg_mgr`, and instead force the use of the ostree package manager with the `package:` module `use:` option. The strategy is - on ostree systems, set the flag `__$ROLENAME_is_ostree` if the system is an ostree system. The flag will either be undefined or `false` on non-ostree systems. Then, change every invocation of the `package:` module like this: ```yaml - name: Ensure required packages are present package: name: "{{ __$ROLENAME_packages }}" state: present use: "{{ (__$ROLENAME_is_ostree | d(false)) | ternary('ansible.posix.rhel_rpm_ostree', omit) }}" ``` This should ensure that the `use:` parameter is not used if the system is non-ostree. The goal is to make the ostree support as unobtrusive as possible for non-ostree systems. The user can also set `__$ROLENAME_is_ostree: true` in the inventory or play if the user knows that ostree is being used and wants to skip the check. Or, the user is concerned about the performance hit for ostree detection on non-ostree systems, and sets `__$ROLENAME_is_ostree: false` to skip the check. The flag `__$ROLENAME_is_ostree` can also be used in the role or tests to include or exclude tasks from being run on ostree systems. This fix also improves error reporting in the `get_ostree_data.sh` script when included roles cannot be found. Signed-off-by: Rich Megginson --- .ansible-lint | 1 - .ostree/get_ostree_data.sh | 41 +++++++++++++++++------- meta/collection-requirements.yml | 1 - tasks/ensure_selinux_packages.yml | 26 ++++++++------- tests/set_selinux_variables.yml | 4 +++ tests/tests_selinux_modules_checksum.yml | 3 +- 6 files changed, 49 insertions(+), 27 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index 8534703..a308df3 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -23,6 +23,5 @@ exclude_paths: mock_modules: - sefcontext - selogin - - ansible.utils.update_fact mock_roles: - linux-system-roles.selinux diff --git a/.ostree/get_ostree_data.sh b/.ostree/get_ostree_data.sh index d0a03a3..cec08b0 100755 --- a/.ostree/get_ostree_data.sh +++ b/.ostree/get_ostree_data.sh @@ -2,7 +2,6 @@ set -euo pipefail -role_collection_dir="${ROLE_COLLECTION_DIR:-fedora/linux_system_roles}" ostree_dir="${OSTREE_DIR:-"$(dirname "$(realpath "$0")")"}" if [ -z "${4:-}" ] || [ "${1:-}" = help ] || [ "${1:-}" = -h ]; then @@ -29,24 +28,40 @@ if [ "$pkgtype" = testing ]; then fi get_rolepath() { - local ostree_dir role rolesdir + local ostree_dir role rolesdir roles_parent_dir coll_path pth ostree_dir="$1" role="$2" - rolesdir="$(dirname "$(dirname "$ostree_dir")")/$role/.ostree" + roles_parent_dir="$(dirname "$(dirname "$ostree_dir")")" + rolesdir="$roles_parent_dir/$role/.ostree" + # assumes collection format if [ -d "$rolesdir" ]; then echo "$rolesdir" return 0 fi - if [ -n "${ANSIBLE_COLLECTIONS_PATHS:-}" ]; then - for pth in ${ANSIBLE_COLLECTIONS_PATHS//:/ }; do - rolesdir="$pth/ansible_collections/$role_collection_dir/roles/$role/.ostree" - if [ -d "$rolesdir" ]; then - echo "$rolesdir" - return 0 - fi + # assumes legacy role format like linux-system-roles.$role/ + for rolesdir in "$roles_parent_dir"/*-system-roles."$role"/.ostree; do + if [ -d "$rolesdir" ]; then + echo "$rolesdir" + return 0 + fi + done + # look elsewhere + coll_path="${ANSIBLE_COLLECTIONS_PATH:-}" + if [ -z "$coll_path" ]; then + coll_path="${ANSIBLE_COLLECTIONS_PATHS:-}" + fi + if [ -n "${coll_path}" ]; then + for pth in ${coll_path//:/ }; do + for rolesdir in "$pth"/ansible_collections/*/*_system_roles/roles/"$role"/.ostree; do + if [ -d "$rolesdir" ]; then + echo "$rolesdir" + return 0 + fi + done done fi - return 1 + 1>&2 echo ERROR - could not find role "$role" - please use ANSIBLE_COLLECTIONS_PATH + exit 2 } get_packages() { @@ -65,6 +80,10 @@ get_packages() { roles="$(cat "$rolefile")" for role in $roles; do rolepath="$(get_rolepath "$ostree_dir" "$role")" + if [ -z "$rolepath" ]; then + 1>&2 echo ERROR - could not find role "$role" - please use ANSIBLE_COLLECTIONS_PATH + exit 2 + fi get_packages "$rolepath" done fi diff --git a/meta/collection-requirements.yml b/meta/collection-requirements.yml index 3e9698f..92a0244 100644 --- a/meta/collection-requirements.yml +++ b/meta/collection-requirements.yml @@ -1,5 +1,4 @@ --- collections: - name: ansible.posix - - name: ansible.utils - name: community.general diff --git a/tasks/ensure_selinux_packages.yml b/tasks/ensure_selinux_packages.yml index 6860b6f..059ba9f 100644 --- a/tasks/ensure_selinux_packages.yml +++ b/tasks/ensure_selinux_packages.yml @@ -1,21 +1,15 @@ --- -- name: Ensure correct package manager for ostree systems - vars: - ostree_pkg_mgr: ansible.posix.rhel_rpm_ostree - ostree_booted_file: /run/ostree-booted - when: ansible_facts.pkg_mgr | d("") != ostree_pkg_mgr +- name: Determine if system is ostree and set flag + when: not __selinux_is_ostree is defined block: - name: Check if system is ostree stat: - path: "{{ ostree_booted_file }}" + path: /run/ostree-booted register: __ostree_booted_stat - - name: Set package manager to use for ostree - ansible.utils.update_fact: - updates: - - path: ansible_facts.pkg_mgr - value: "{{ ostree_pkg_mgr }}" - when: __ostree_booted_stat.stat.exists + - name: Set flag to indicate system is ostree + set_fact: + __selinux_is_ostree: "{{ __ostree_booted_stat.stat.exists }}" - name: Install SELinux python2 tools package: @@ -23,6 +17,8 @@ - libselinux-python - policycoreutils-python state: present + use: "{{ (__selinux_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: ansible_python_version is version('3', '<') - name: Install SELinux python3 tools @@ -31,6 +27,8 @@ - python3-libselinux - python3-policycoreutils state: present + use: "{{ (__selinux_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: - ansible_python_version is version('3', '>=') - ansible_os_family == "RedHat" @@ -41,6 +39,8 @@ - python3-selinux - python3-policycoreutils state: present + use: "{{ (__selinux_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: - ansible_python_version is version('3', '>=') - ansible_os_family == "Suse" @@ -50,6 +50,8 @@ name: - policycoreutils-python-utils state: present + use: "{{ (__selinux_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: ansible_distribution == "Fedora" or (ansible_distribution_major_version | int > 7 and ansible_distribution in ["CentOS", "RedHat", "Rocky"]) diff --git a/tests/set_selinux_variables.yml b/tests/set_selinux_variables.yml index da6d85d..8dd425a 100644 --- a/tests/set_selinux_variables.yml +++ b/tests/set_selinux_variables.yml @@ -25,12 +25,16 @@ package: name: selinux-policy-targeted state: present + use: "{{ (__selinux_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: __selinux_need_policy_targeted | d(false) - name: Ensure findmnt package: name: "{{ findmnt_pkg }}" state: present + use: "{{ (__selinux_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" when: __selinux_need_findmnt | d(false) vars: findmnt_pkg: "{{ 'util-linux-core' diff --git a/tests/tests_selinux_modules_checksum.yml b/tests/tests_selinux_modules_checksum.yml index 6ac1841..74fef4e 100644 --- a/tests/tests_selinux_modules_checksum.yml +++ b/tests/tests_selinux_modules_checksum.yml @@ -26,8 +26,7 @@ register: role_result - name: Get commit_num file set_fact: - commit_num_file: "{{ - (ansible_facts.pkg_mgr == 'ansible.posix.rhel_rpm_ostree') | + commit_num_file: "{{ __selinux_is_ostree | d(false) | ternary('/etc/selinux/targeted/active/commit_num', '/var/lib/selinux/targeted/active/commit_num') }}" - name: Get current commit_num