diff --git a/app/helpers/foreman_leapp/template_helper.rb b/app/helpers/foreman_leapp/template_helper.rb index a6078127..91f0633b 100644 --- a/app/helpers/foreman_leapp/template_helper.rb +++ b/app/helpers/foreman_leapp/template_helper.rb @@ -4,11 +4,8 @@ module ForemanLeapp module TemplateHelper def build_remediation_plan(remediation_ids, host) PreupgradeReportEntry.remediation_details(remediation_ids, host) - .map { |e| e['remediations'] } - .flatten - .compact - .select { |r| r['type'] == 'command' } - .map { |r| "#{r['context'].join(' ')}\n" } + .group_by { |_detail, leapp_version| leapp_version } + .map { |leapp_version, pairs| RemediationPlan.build(pairs.map(&:first), leapp_version) } .join end end diff --git a/app/lib/actions/foreman_leapp/preupgrade_job.rb b/app/lib/actions/foreman_leapp/preupgrade_job.rb index a58791a2..a80446d8 100644 --- a/app/lib/actions/foreman_leapp/preupgrade_job.rb +++ b/app/lib/actions/foreman_leapp/preupgrade_job.rb @@ -3,6 +3,9 @@ module Actions module ForemanLeapp class PreupgradeJob < Actions::EntryAction + # The `rpm -q leapp` line of the job output, e.g. leapp-0.21.0-1.el8.noarch + LEAPP_NVR = /^leapp-(\d[\d.]*)/.freeze + def self.subscribe Actions::RemoteExecution::RunHostJob end @@ -16,9 +19,11 @@ def plan(job_invocation, host, *_args) def finalize(*_args) host = Host.find(input[:host_id]) - leapp_report = format_output(task.main_action.continuous_output.humanize) + job_output = task.main_action.continuous_output.humanize + leapp_report = format_output(job_output) - PreupgradeReport.create_report(host, leapp_report, input[:job_invocation_id]) + PreupgradeReport.create_report(host, leapp_report, input[:job_invocation_id], + leapp_version(job_output)) end private @@ -31,6 +36,12 @@ def format_output(job_output) .join('') JSON.parse(output) end + + # The version of the leapp package the report was generated with. It drives + # how the remediation commands from the report have to be quoted. + def leapp_version(job_output) + job_output[LEAPP_NVR, 1] + end end end end diff --git a/app/lib/foreman_leapp/remediation_plan.rb b/app/lib/foreman_leapp/remediation_plan.rb new file mode 100644 index 00000000..be24e48d --- /dev/null +++ b/app/lib/foreman_leapp/remediation_plan.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'shellwords' + +module ForemanLeapp + # Assembles the shell script out of the command remediations found in a leapp report. + # + # Leapp changed the way remediation commands are quoted. Up to leapp 0.21 the + # arguments in the "context" list were already quoted by leapp itself, so they + # were meant to be joined verbatim. Starting with leapp 0.22 the arguments are + # reported as they are and have to be escaped by the consumer of the report. + # See https://github.com/oamg/leapp-repository/pull/1520 + # + # The new quoting is shipped by leapp-upgrade-eltoel 0.25, which is released + # together with leapp 0.22, so the version of the leapp package tells the two + # formats apart. + module RemediationPlan + # The first leapp version that reports remediation command arguments unquoted. + UNQUOTED_ARGUMENTS_SINCE = Gem::Version.new('0.22') + + class << self + def build(detail, leapp_version) + quoted = quoted_by_leapp?(leapp_version) + Array.wrap(detail) + .flat_map { |d| d['remediations'] } + .compact + .select { |remediation| remediation['type'] == 'command' } + .map { |remediation| "#{command_line(remediation['context'], quoted)}\n" } + .join + end + + def command_line(context, quoted) + arguments = Array.wrap(context) + return arguments.join(' ') if quoted + + arguments.map { |argument| Shellwords.escape(argument) }.join(' ') + end + + # Unknown versions are treated as old leapp, so that reports collected + # before the version was recorded keep rendering the way they used to. + def quoted_by_leapp?(leapp_version) + version = Gem::Version.new(leapp_version.to_s[/\A[0-9][0-9.]*/].to_s.chomp('.')) + version < UNQUOTED_ARGUMENTS_SINCE + rescue ArgumentError + true + end + end + end +end diff --git a/app/models/preupgrade_report.rb b/app/models/preupgrade_report.rb index 379353b6..c9b2f914 100644 --- a/app/models/preupgrade_report.rb +++ b/app/models/preupgrade_report.rb @@ -6,7 +6,7 @@ class PreupgradeReport < ::Report scoped_search on: :job_invocation_id, only_explicit: true - def self.create_report(host, data, job_invocation_id) + def self.create_report(host, data, job_invocation_id, leapp_version = nil) # We don't have specific permissions for the Preupgrade leapp reports, # so we need to skip the permission check for non-admin users. # The user is still required to have permission to run the job and view the hosts. @@ -16,12 +16,12 @@ def self.create_report(host, data, job_invocation_id) reported_at: DateTime.now.utc data['entries']&.each do |entry| - PreupgradeReportEntry.create! entry_params(report, entry, host, data) + PreupgradeReportEntry.create! entry_params(report, entry, host, data, leapp_version) end end end - def self.entry_params(report, entry, host, data) + def self.entry_params(report, entry, host, data, leapp_version = nil) { preupgrade_report: report, host_id: host.id, hostname: host.name, @@ -30,6 +30,7 @@ def self.entry_params(report, entry, host, data) audience: entry['audience'], severity: entry['severity'], leapp_run_id: data['leapp_run_id'], + leapp_version: leapp_version, summary: entry['summary'], tags: entry['tags'], flags: entry['flags'], diff --git a/app/models/preupgrade_report_entry.rb b/app/models/preupgrade_report_entry.rb index b2b7b4f0..aebacecc 100644 --- a/app/models/preupgrade_report_entry.rb +++ b/app/models/preupgrade_report_entry.rb @@ -50,8 +50,10 @@ class PreupgradeReportEntry < ApplicationRecord )) } + # Returns [detail, leapp_version] pairs. The leapp version is needed because + # the quoting of the remediation commands differs between leapp versions. def self.remediation_details(remediation_ids, host) - where(id: remediation_ids, host: host).where.not(detail: nil).pluck(:detail) + where(id: remediation_ids, host: host).where.not(detail: nil).pluck(:detail, :leapp_version) end def self.search_yes_no_fields(key, operator, value) diff --git a/app/views/api/v2/preupgrade_report_entries/base.json.rabl b/app/views/api/v2/preupgrade_report_entries/base.json.rabl index 4a8e6959..cc708246 100644 --- a/app/views/api/v2/preupgrade_report_entries/base.json.rabl +++ b/app/views/api/v2/preupgrade_report_entries/base.json.rabl @@ -1,4 +1,5 @@ object @preupgrade_report_entry attributes :id, :detail, :preupgrade_report_id, :host_id, :hostname, :title, :actor, :audience, - :severity, :leapp_run_id, :summary, :tags, :flags, :created_at, :updated_at + :severity, :leapp_run_id, :leapp_version, :summary, :tags, :flags, :created_at, + :updated_at diff --git a/app/views/foreman_leapp/job_templates/leapp_preupgrade.erb b/app/views/foreman_leapp/job_templates/leapp_preupgrade.erb index d371a463..59f8a421 100644 --- a/app/views/foreman_leapp/job_templates/leapp_preupgrade.erb +++ b/app/views/foreman_leapp/job_templates/leapp_preupgrade.erb @@ -27,6 +27,8 @@ template_inputs: <%= render_template 'Check Leapp' %> +rpm -q leapp + rm -f /var/log/leapp/leapp-report.json leapp preupgrade <%= options_string %> diff --git a/db/migrate/20260825120000_add_leapp_version_to_report_entries.rb b/db/migrate/20260825120000_add_leapp_version_to_report_entries.rb new file mode 100644 index 00000000..60f64a9e --- /dev/null +++ b/db/migrate/20260825120000_add_leapp_version_to_report_entries.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddLeappVersionToReportEntries < ActiveRecord::Migration[7.0] + def change + add_column :preupgrade_report_entries, :leapp_version, :string + end +end diff --git a/docs/developer_guide.md b/docs/developer_guide.md index 0259d2c2..dbd8a4b7 100644 --- a/docs/developer_guide.md +++ b/docs/developer_guide.md @@ -76,7 +76,15 @@ leapp preupgrade leapp preupgrade --verbose # Enable verbose logging leapp preupgrade --debug # Enable debug mode ``` -For the report sample see the file in `report_samples` folder. +For the report sample see the files in the `report_samples` folder. `leapp-report.json` +comes from leapp 0.21, `leapp-report-0.22.json` from leapp 0.22. + +Starting with leapp 0.22 the arguments of the `command` remediations are reported +verbatim, while older leapp quoted them itself +([leapp-repository#1520](https://github.com/oamg/leapp-repository/pull/1520)). +The remediation plan is therefore assembled based on the version of the `leapp` +package the report was collected with, which is stored in +`preupgrade_report_entries.leapp_version`. **Upgrade** diff --git a/report_samples/leapp-report-0.22.json b/report_samples/leapp-report-0.22.json new file mode 100644 index 00000000..a6437a5a --- /dev/null +++ b/report_samples/leapp-report-0.22.json @@ -0,0 +1,441 @@ +{ + "entries": [ + { + "audience": "sysadmin", + "detail": { + "remediations": [ + { + "context": "If some of excluded repositories are still required to be used during the upgrade, execute leapp with the --enablerepo option with the repoid of the repository required to be enabled as an argument (the option can be used multiple times).", + "type": "hint" + } + ] + }, + "key": "1b9132cb2362ae7830e48eee7811be9527747de8", + "severity": "info", + "summary": "The following repositories are not supported by Red Hat and are excluded from the list of repositories used during the upgrade.\n- codeready-builder-for-rhel-9-x86_64-eus-rpms\n- codeready-builder-beta-for-rhel-9-ppc64le-rpms\n- codeready-builder-for-rhel-9-ppc64le-rpms\n- codeready-builder-beta-for-rhel-9-aarch64-rpms\n- codeready-builder-for-rhel-9-aarch64-rpms\n- codeready-builder-for-rhel-9-rhui-rpms\n- codeready-builder-for-rhel-9-ppc64le-eus-rpms\n- codeready-builder-for-rhel-9-aarch64-eus-rpms\n- codeready-builder-for-rhel-9-s390x-eus-rpms\n- rhui-codeready-builder-for-rhel-9-x86_64-rhui-rpms\n- codeready-builder-for-rhel-9-x86_64-rpms\n- rhui-codeready-builder-for-rhel-9-aarch64-rhui-rpms\n- codeready-builder-beta-for-rhel-9-x86_64-rpms\n- codeready-builder-for-rhel-9-s390x-rpms\n- crb\n- codeready-builder-beta-for-rhel-9-s390x-rpms\n- codeready-builder-for-rhel-9-x86_64-rhui-rpms", + "title": "Excluded target system repositories", + "timeStamp": "2026-05-21T13:47:48.521164Z", + "hostname": "localhost.localdomain", + "actor": "repositories_blacklist", + "id": "41d4243926b6b0632062e49fe513453edac79b27f43113312d2c5a3f2684aeb2", + "flags": [ + "failure" + ], + "tags": [ + "repository" + ] + }, + { + "audience": "sysadmin", + "key": "61eb181bbc56328fbe03b5229d25a8ea5ebdc7a2", + "severity": "high", + "summary": "The following processors are no longer maintained in RHEL 9:\n - ATOM_GOLDMONT_D\n", + "title": "Leapp detected a processor no longer maintained in the target OS", + "timeStamp": "2026-05-21T13:47:50.024681Z", + "hostname": "localhost.localdomain", + "actor": "check_detected_devices_and_drivers", + "id": "0bceaadc454e6056316a151b25ebdfc424a603dba6291fea88ad284af70701ca", + "flags": [], + "tags": [ + "kernel", + "boot" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "leapp upgrade stops with Inhibitor \"Upgrade requires links in root directory to be relative\"", + "url": "https://access.redhat.com/solutions/6989732" + } + ], + "remediations": [ + { + "context": [ + "bash", + "-c", + "ln -snf \"usr/lib\" \"/lib\" && ln -snf \"usr/lib64\" \"/lib64\" && ln -snf \"var/log/leapp\" \"/leapp-logs\" && ln -snf \"var/log/leapp\" \"/quote'quote\" && ln -snf \"var/log/leapp\" \"/quote'quote'quote\" && ln -snf \"var/log/leapp\" \"/mark!mark\" && ln -snf \"var/log/leapp\" \"/space space\" && ln -snf \"var/log/leapp\" \"/\\$USER\" && ln -snf \"var/log/leapp\" \"/\\`zsh\\`\" && ln -snf \"var/log/leapp\" \"/backslash\\\\backslash\" && ln -snf \"var/log/leapp\" \"/escapedmark\\\\!escapedmark\" && ln -snf \"var/log/leapp\" \"/newline\\\\nnewline\" && ln -snf \"var/log/leapp\" \"/tab\\\\ttab\" && ln -snf \"var/log/leapp\" \"/return\\\\rreturn\" && ln -snf \"var/log/leapp\" \"/hex\\\\xFFhex\"" + ], + "type": "command" + } + ] + }, + "flags": [ + "inhibitor" + ], + "key": "3d895ad37ceaf4157864d439edb6bd75562061fa", + "severity": "high", + "summary": "After rebooting, parts of the upgrade process can fail if symbolic links in / point to absolute paths.\nPlease change these links to relative ones.", + "title": "Upgrade requires links in root directory to be relative", + "timeStamp": "2026-05-21T13:47:51.042544Z", + "hostname": "localhost.localdomain", + "actor": "check_root_symlinks", + "id": "428be11bb86765a8d1a80fd5d6d2e9e58d9ff05a12b75c122bc8310586de4a2b", + "tags": [] + }, + { + "audience": "sysadmin", + "key": "ac7030e05d2ee248d34f08a9fa040b352bc410a3", + "severity": "high", + "summary": "On legacy (BIOS) systems, GRUB2 core (located in the gap between the MBR and the first partition) cannot be updated during the rpm transaction and Leapp has to initiate the update running \"grub2-install\" after the transaction. No action is needed before the upgrade. After the upgrade, it is recommended to check the GRUB configuration.", + "title": "GRUB2 core will be automatically updated during the upgrade", + "timeStamp": "2026-05-21T13:47:51.128654Z", + "hostname": "localhost.localdomain", + "actor": "check_grub_core", + "id": "03d872939cc6e61ef0282339fd01b26ebd5e8fc416fb26ac00c186041e6835de", + "flags": [], + "tags": [ + "boot" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "Importing existing VDO volumes to LVM", + "url": "https://red.ht/import-existing-vdo-volumes-to-lvm" + } + ], + "remediations": [ + { + "context": "Install the 'vdo' package and re-run upgrade to check for VDO devices requiring conversion or confirm that all VDO devices, if any, are managed by LVM.", + "type": "hint" + } + ] + }, + "flags": [ + "inhibitor" + ], + "key": "429a99e13b19a7eebadbb8cb35233d8119bcf255", + "severity": "high", + "summary": "The check of block devices could not be performed as the 'vdo' package is not installed. All VDO devices must be converted to LVM management prior to the upgrade to prevent the loss of data.", + "title": "Cannot perform the VDO check of block devices", + "timeStamp": "2026-05-21T13:47:51.887612Z", + "hostname": "localhost.localdomain", + "actor": "check_vdo", + "id": "21699162f34e5bd6675fb2d05a42a750463a60dc50d34e1cff4460becea604c9", + "tags": [ + "services", + "drivers" + ] + }, + { + "audience": "sysadmin", + "key": "8fb81863f8413bd617c2a55b69b8e10ff03d7c72", + "severity": "info", + "summary": "SElinux relabeling will be scheduled as the status is permissive/enforcing.", + "title": "SElinux relabeling will be scheduled", + "timeStamp": "2026-05-21T13:47:53.630045Z", + "hostname": "localhost.localdomain", + "actor": "check_se_linux", + "id": "d51c96844380e938500f8e4ec4790d6d22e9b4b6f3b47990892357fe2c15c80a", + "flags": [], + "tags": [ + "selinux", + "security" + ] + }, + { + "audience": "sysadmin", + "detail": { + "remediations": [ + { + "context": "Make sure there are no SElinux related warnings after the upgrade and enable SElinux manually afterwards. Notice: You can ignore the \"/root/tmp_leapp_py3\" SElinux warnings.", + "type": "hint" + } + ] + }, + "key": "39d7183dafba798aa4bbb1e70b0ef2bbe5b1772f", + "severity": "low", + "summary": "SElinux will be set to permissive mode. Current mode: enforcing. This action is required by the upgrade process to make sure the upgraded system can boot without beinig blocked by SElinux rules.", + "title": "SElinux will be set to permissive mode", + "timeStamp": "2026-05-21T13:47:53.631756Z", + "hostname": "localhost.localdomain", + "actor": "check_se_linux", + "id": "dce92f130a6f8032c7e5506879c4a6e2d3a3ae406d8651fe7c1735aae8fb0283", + "flags": [], + "tags": [ + "selinux", + "security" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "Why Leapp Preupgrade for RHEL 8 to 9 getting \"Possible problems with remote login using root account\" ?", + "url": "https://access.redhat.com/solutions/7003083" + } + ], + "related_resources": [ + { + "scheme": "package", + "title": "openssh-server" + }, + { + "scheme": "file", + "title": "/etc/ssh/sshd_config" + } + ], + "remediations": [ + { + "context": "If you depend on remote root logins using passwords, consider setting up a different user for remote administration or adding a comment into the sshd_config next to the \"PermitRootLogin yes\" directive to prevent rpm replacing it during the upgrade.", + "type": "hint" + } + ] + }, + "flags": [ + "inhibitor" + ], + "key": "3d21e8cc9e1c09dc60429de7716165787e99515f", + "severity": "high", + "summary": "OpenSSH configuration file will get updated to RHEL 9 version, no longer allowing root login with password. It is a good practice to use non-root administrative user and non-password authentications, but if you rely on the remote root login, this change can lock you out of this system.", + "title": "Possible problems with remote login using root account", + "timeStamp": "2026-05-21T13:47:54.257416Z", + "hostname": "localhost.localdomain", + "actor": "openssh_permit_root_login", + "id": "53c3cd09510713d2e1e5b0d7e183dce1e72cfc99e636923809456eefd896bc23", + "tags": [ + "authentication", + "security", + "network", + "services" + ] + }, + { + "audience": "sysadmin", + "detail": { + "related_resources": [ + { + "scheme": "package", + "title": "openssh-server" + }, + { + "scheme": "file", + "title": "/etc/ssh/sshd_config" + } + ], + "remediations": [ + { + "context": "If you depend on remote root logins using passwords, consider setting up a different user for remote administration. Otherwise you can ignore this message.", + "type": "hint" + } + ] + }, + "key": "e738f78bc8f3a84411a4210e3b609057139d1855", + "severity": "high", + "summary": "RHEL 9 no longer allows remote root logins, but the server configuration explicitly overrides this default. The configuration file will not be updated and root is stillgoing to be allowed to login with password. This is not recommended and considered as a security risk. ", + "title": "Remote root logins globally allowed using password", + "timeStamp": "2026-05-21T13:47:54.258658Z", + "hostname": "localhost.localdomain", + "actor": "openssh_permit_root_login", + "id": "d7c092639272ee149e216d168178e4fdf5c076ddbac1d0efe79eac92d23250f0", + "flags": [], + "tags": [ + "authentication", + "security", + "network", + "services" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "Handling the migration of your custom and third-party applications", + "url": "https://red.ht/customize-rhel-upgrade-actors" + } + ], + "remediations": [ + { + "context": "The most simple solution that does not require additional knowledge about the upgrade process is the uninstallation of such packages before the upgrade and installing these (or their newer versions compatible with the target system) back after the upgrade. Also you can just try to upgrade the system on a testing machine (or after the full system backup) to see the result.\nHowever, it is common use case to migrate or upgrade installed third party packages together with the system during the in-place upgrade process. To examine how to customize the process to deal with such packages, follow the documentation in the attached link for more details.", + "type": "hint" + } + ] + }, + "key": "13f0791ae5f19f50e7d0d606fb6501f91b1efb2c", + "severity": "high", + "summary": "The official solution for in-place upgrades contains instructions for the migration of packages signed by the vendor of the system distribution. Third-party content is not known to the upgrade process and it might need to be handled extra. Third-party packages may be removed automatically during the upgrade if they depend on official distribution content which is not present on the target system - therefore RPM dependencies of such packages cannot be satisfied and hence such packages cannot be installed on the target system.\n\nThe following packages have not been signed by the vendor of the distribution:\n - leapp\n - leapp-deps\n - leapp-upgrade-el8toel9\n - leapp-upgrade-el8toel9-deps\n - python3-leapp", + "title": "Packages not signed by the distribution vendor found on the system", + "timeStamp": "2026-05-21T13:47:54.480745Z", + "hostname": "localhost.localdomain", + "actor": "distribution_signed_rpm_check", + "id": "025eb827a37f53920974c5f428272310661e2368553df808b3f7731d00d0ac0c", + "flags": [], + "tags": [ + "sanity" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "Changes in firewalld related to Zone Drifting", + "url": "https://access.redhat.com/articles/4855631" + }, + { + "title": "Leapp Preupgrade check fails with error - \"Inhibitor: Firewalld Configuration AllowZoneDrifting Is Unsupported\".", + "url": "https://access.redhat.com/solutions/6969130" + } + ], + "remediations": [ + { + "context": "Set AllowZoneDrifting=no in /etc/firewalld/firewalld.conf", + "type": "hint" + }, + { + "context": [ + "sed", + "-i", + "s/^AllowZoneDrifting=.*/AllowZoneDrifting=no/", + "/etc/firewalld/firewalld.conf" + ], + "type": "command" + } + ] + }, + "flags": [ + "inhibitor" + ], + "key": "5b1cf050e1a877b0358b6e8c612277c591d40c13", + "severity": "high", + "summary": "Firewalld has enabled configuration option \"AllowZoneDrifting\" which has been removed in RHEL 9. New behavior is as if \"AllowZoneDrifting\" was set to \"no\".", + "title": "Firewalld Configuration AllowZoneDrifting Is Unsupported", + "timeStamp": "2026-05-21T13:47:55.009835Z", + "hostname": "localhost.localdomain", + "actor": "firewalld_check_allow_zone_drifting", + "id": "09ab1f5678b34ee01db586e7857bba13eafc9cbd86b2ed03bc8d720dbab282b1", + "tags": [ + "sanity", + "firewall" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "Limiting LVM device visibility and usage", + "url": "https://red.ht/limiting-lvm-devices-visibility-and-usage" + } + ], + "remediations": [ + { + "context": "While not required, switching to the LVM devices file from the LVM filter is possible using the following command. The command uses the existing LVM filter to create the system.devices file which is then used instead of the LVM filter. Before running the command, make sure that 'use_devicesfile=1' is set in /etc/lvm/lvm.conf.", + "type": "hint" + }, + { + "context": [ + "vgimportdevices" + ], + "type": "command" + } + ] + }, + "key": "419ad408c11ef7e8e89a0fb83106988453f362d6", + "severity": "info", + "summary": "Beginning with RHEL 9, LVM devices file is used by default to select devices used by LVM. Since leapp detected the use of LVM filter in the /etc/lvm/lvm.conf configuration file, the configuration won't be modified to use devices file during the upgrade and the LVM filter will remain in use after the upgrade.", + "title": "LVM filter definition detected.", + "timeStamp": "2026-05-21T13:47:55.272739Z", + "hostname": "localhost.localdomain", + "actor": "check_lvm", + "id": "b29b2b08051d3dfe37ee15b72b33c5c15ea110abd68723cdb063a833d227a616", + "flags": [], + "tags": [] + }, + { + "audience": "sysadmin", + "key": "9e5088e3c1f371e020ec777c3d86578f4be143cf", + "severity": "high", + "summary": "Environment variable LEAPP_UNSUPPORTED has been detected. A successful and safe upgrade process cannot be guaranteed. From now on you are continuing at your own risk.\n", + "title": "Upgrade is unsupported", + "timeStamp": "2026-05-21T13:47:55.684594Z", + "hostname": "localhost.localdomain", + "actor": "unsupported_upgrade_check", + "id": "8e87a3203d2cee4ec00f8b63158762102d0ffdb6782705a2fd3e1a37fd16e1ad", + "flags": [], + "tags": [ + "upgrade process", + "sanity" + ] + }, + { + "audience": "sysadmin", + "key": "693963253195f418526f045b6d630a1f4c7a193d", + "severity": "info", + "summary": "After the upgrade, this system will be automatically registered into Red Hat Lightspeed. To skip the automatic registration, use the '--no-insights-register' command line option or set the LEAPP_NO_INSIGHTS_REGISTER environment variable.", + "title": "Automatic registration into Red Hat Lightspeed", + "timeStamp": "2026-05-21T13:47:55.820569Z", + "hostname": "localhost.localdomain", + "actor": "check_insights_auto_register", + "id": "8d18a3d007017c171b980a1aeb214465e9389ee126551d8ab1ffbf2a13c518f5", + "flags": [], + "tags": [ + "services" + ] + }, + { + "audience": "sysadmin", + "detail": { + "external": [ + { + "title": "Leapp upgrade fail with error \"Inhibitor: Missing required answers in the answer file.\"", + "url": "https://access.redhat.com/solutions/7035321" + } + ], + "related_resources": [ + { + "scheme": "dialog", + "title": "check_vdo.confirm" + } + ], + "remediations": [ + { + "context": "Please register user choices with leapp answer cli command or by manually editing the answerfile.", + "type": "hint" + }, + { + "context": [ + "leapp", + "answer", + "--section", + "check_vdo.confirm=True" + ], + "type": "command" + } + ] + }, + "flags": [ + "inhibitor" + ], + "key": "2f9802dc91315806c7cdc3c18d7b74f2a2383285", + "severity": "high", + "summary": "One or more sections in answerfile are missing user choices: check_vdo.confirm\nFor more information consult https://red.ht/leapp-dialogs.", + "title": "Missing required answers in the answer file", + "timeStamp": "2026-05-21T13:47:56.358174Z", + "hostname": "localhost.localdomain", + "actor": "verify_check_results", + "id": "cee979d7a92fbbf57255851b83d42333f23cb5d8a1a8be51605cb1120940d3b4", + "tags": [] + }, + { + "audience": "sysadmin", + "flags": [], + "key": "ceb4a05b348658e87d9a7a7b1270dedc24fb0014", + "severity": "high", + "summary": "{\"details\": \"details about the issue if we have any\", \"hint\": \"possible hint here, if we know about any\", \"whatever\": \"else fields\", \"note\": \"all these details in \\\"details\\\" object are free form key:value\"}", + "title": "Here is an error title", + "timeStamp": "2026-05-21T13:47:56.359831Z", + "hostname": "localhost.localdomain", + "actor": "verify_check_results", + "id": "a0ede1c25aeaa52384f438d7092bb0a4ebba3a31b193092c9ee91b89a2f63af3", + "tags": [] + } + ], + "leapp_run_id": "960e4752-6358-4d03-bc08-0d3792a57004" +} diff --git a/test/helpers/template_helper_test.rb b/test/helpers/template_helper_test.rb index 92feef70..dd21d545 100644 --- a/test/helpers/template_helper_test.rb +++ b/test/helpers/template_helper_test.rb @@ -8,6 +8,17 @@ class TemplateHelperTest < ActionView::TestCase let(:report) { FactoryBot.create(:preupgrade_report, host: host) } let(:entry) { FactoryBot.create(:preupgrade_report_entry, host: host, preupgrade_report: report) } + let(:quoting_context) do + { + remediations: [ + { + type: 'command', + context: ['bash', '-c', 'ln -snf "var/log/leapp" "/space space"'], + }, + ], + } + end + describe 'build_remediation_plan' do test 'with remediation commands' do template = build_remediation_plan([entry.id], host) @@ -26,6 +37,40 @@ class TemplateHelperTest < ActionView::TestCase template = build_remediation_plan([entry.id], FactoryBot.create(:host)) assert_equal template, '' end + + test 'joins the arguments verbatim for reports from leapp older than 0.22' do + old_entry = FactoryBot.create(:preupgrade_report_entry, host: host, preupgrade_report: report, + leapp_version: '0.21.0', detail: quoting_context) + + template = build_remediation_plan([old_entry.id], host) + + assert_equal "bash -c ln -snf \"var/log/leapp\" \"/space space\"\n", template + end + + test 'escapes the arguments for reports from leapp 0.22 and newer' do + new_entry = FactoryBot.create(:preupgrade_report_entry, host: host, preupgrade_report: report, + leapp_version: '0.22.0', detail: quoting_context) + + template = build_remediation_plan([new_entry.id], host) + + assert_equal ['bash', '-c', 'ln -snf "var/log/leapp" "/space space"'], + Shellwords.split(template) + end + + test 'quotes every entry according to the leapp version it was reported by' do + old_entry = FactoryBot.create(:preupgrade_report_entry, host: host, preupgrade_report: report, + leapp_version: '0.21.0', detail: quoting_context) + new_entry = FactoryBot.create(:preupgrade_report_entry, host: host, preupgrade_report: report, + leapp_version: '0.22.0', detail: quoting_context) + + lines = build_remediation_plan([old_entry.id, new_entry.id], host).lines + verbatim_line = "bash -c ln -snf \"var/log/leapp\" \"/space space\"\n" + + assert_equal 2, lines.size + assert_includes lines, verbatim_line + assert_equal ['bash', '-c', 'ln -snf "var/log/leapp" "/space space"'], + Shellwords.split(lines.find { |line| line != verbatim_line }) + end end end end diff --git a/test/models/preupgrade_report_entry_test.rb b/test/models/preupgrade_report_entry_test.rb index e46baf8f..b7d855ab 100644 --- a/test/models/preupgrade_report_entry_test.rb +++ b/test/models/preupgrade_report_entry_test.rb @@ -20,14 +20,15 @@ class PreupgradeReportEntryTest < ActiveSupport::TestCase @host = FactoryBot.create(:host) report = FactoryBot.create(:preupgrade_report) - @entry1 = FactoryBot.create(:preupgrade_report_entry, preupgrade_report: report, host: @host) + @entry1 = FactoryBot.create(:preupgrade_report_entry, preupgrade_report: report, host: @host, + leapp_version: '0.22.0') @entry2 = FactoryBot.create(:preupgrade_report_entry, preupgrade_report: report, host: @host, detail: nil) end it do details = PreupgradeReportEntry.remediation_details([@entry1.id, @entry2.id], @host.id) assert_equal details.size, 1 - assert_equal details[0], @entry1.detail + assert_equal details[0], [@entry1.detail, '0.22.0'] details = PreupgradeReportEntry.remediation_details([@entry1.id, @entry2.id], FactoryBot.create(:host)) assert_equal details.size, 0 diff --git a/test/models/preupgrade_report_test.rb b/test/models/preupgrade_report_test.rb index 0fd4d129..7b07fa09 100644 --- a/test/models/preupgrade_report_test.rb +++ b/test/models/preupgrade_report_test.rb @@ -19,6 +19,22 @@ class PreupgradeReportTest < ActiveSupport::TestCase end end + it 'stores the leapp version the report was generated with' do + data = { 'leapp_run_id' => 'leapp_c13e471c', 'entries' => [sample_entry_1] } + + PreupgradeReport.create_report(host, data, job_invocation, '0.22.0') + + assert_equal ['0.22.0'], PreupgradeReportEntry.where(host: host).pluck(:leapp_version) + end + + it 'leaves the leapp version empty when it is not known' do + data = { 'leapp_run_id' => 'leapp_c13e471c', 'entries' => [sample_entry_1] } + + PreupgradeReport.create_report(host, data, job_invocation) + + assert_nil PreupgradeReportEntry.where(host: host).first.leapp_version + end + it 'no entries' do data = { 'leapp_run_id' => 'leapp_c13e471c' } diff --git a/test/unit/actions/preupgrade_job_test.rb b/test/unit/actions/preupgrade_job_test.rb index b6c94706..dc60c367 100644 --- a/test/unit/actions/preupgrade_job_test.rb +++ b/test/unit/actions/preupgrade_job_test.rb @@ -40,4 +40,31 @@ class PreupgradeJobTest < ActiveSupport::TestCase it { assert_finalize_phase planned_action } end end + + # Parsing the job output does not need any of the remote execution setup. + class PreupgradeJobOutputTest < ActiveSupport::TestCase + include Dynflow::Testing + + let(:action) { create_action(Actions::ForemanLeapp::PreupgradeJob) } + + describe '#leapp_version' do + test 'picks the leapp version up from the rpm output' do + output = "leapp-0.22.0-1.el9.noarch\n===leap_upgrade_report_start===\n{}\nExit status: 0\n" + + assert_equal '0.22.0', action.send(:leapp_version, output) + end + + test 'ignores other leapp packages' do + output = "leapp-upgrade-el8toel9-0.25.0-1.el8.noarch\nleapp-0.22.0-1.el8.noarch\n" + + assert_equal '0.22.0', action.send(:leapp_version, output) + end + + test 'returns nil when the leapp package is not reported' do + output = "package leapp is not installed\n===leap_upgrade_report_start===\n{}\n" + + assert_nil action.send(:leapp_version, output) + end + end + end end diff --git a/test/unit/remediation_plan_test.rb b/test/unit/remediation_plan_test.rb new file mode 100644 index 00000000..cb293932 --- /dev/null +++ b/test/unit/remediation_plan_test.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'test_plugin_helper' + +module ForemanLeapp + class RemediationPlanTest < ActiveSupport::TestCase + # Taken from a leapp <= 0.21 report - leapp quoted the arguments itself. + OLD_SYMLINK_CONTEXT = [ + 'sh', + '-c', + '"ln -snf usr/lib /lib && ln -snf var/log/leapp /space space && ' \ + 'ln -snf var/log/leapp /quote\'quote"', + ].freeze + + # The very same remediation as reported by leapp >= 0.22 - the arguments + # are reported verbatim. https://github.com/oamg/leapp-repository/pull/1520 + NEW_SYMLINK_CONTEXT = [ + 'bash', + '-c', + 'ln -snf "usr/lib" "/lib" && ln -snf "var/log/leapp" "/space space" && ' \ + 'ln -snf "var/log/leapp" "/quote\'quote"', + ].freeze + + describe '#command_line' do + test 'joins the arguments verbatim when leapp did the quoting' do + assert_equal OLD_SYMLINK_CONTEXT.join(' '), + RemediationPlan.command_line(OLD_SYMLINK_CONTEXT, true) + end + + test 'escapes the arguments when leapp reports them verbatim' do + line = RemediationPlan.command_line(NEW_SYMLINK_CONTEXT, false) + + assert_equal NEW_SYMLINK_CONTEXT, Shellwords.split(line) + end + + test 'escapes shell metacharacters when leapp reports them verbatim' do + context = ['sed', '-i', 's/^AllowZoneDrifting=.*/AllowZoneDrifting=no/', + '/etc/firewalld/firewalld.conf'] + + line = RemediationPlan.command_line(context, false) + + assert_equal context, Shellwords.split(line) + assert_not_includes line, '.*' + end + end + + describe '#quoted_by_leapp?' do + test 'true for leapp older than 0.22' do + assert RemediationPlan.quoted_by_leapp?('0.21.0') + end + + test 'false for leapp 0.22 and newer' do + assert_not RemediationPlan.quoted_by_leapp?('0.22.0') + assert_not RemediationPlan.quoted_by_leapp?('0.22') + end + + test 'keeps the old quoting when the leapp version is unknown' do + [nil, ''].each do |leapp_version| + assert RemediationPlan.quoted_by_leapp?(leapp_version) + end + end + + test 'keeps the old quoting when the leapp version cannot be parsed' do + assert RemediationPlan.quoted_by_leapp?('not-a-version') + end + + test 'handles a leapp version with a release suffix' do + assert_not RemediationPlan.quoted_by_leapp?('0.22.0-1.el9') + end + end + + describe '#build' do + let(:detail) do + { 'remediations' => [ + { 'type' => 'hint', 'context' => 'Set AllowZoneDrifting=no' }, + { 'type' => 'command', 'context' => %w[vgimportdevices] }, + { 'type' => 'command', 'context' => NEW_SYMLINK_CONTEXT }, + ] } + end + + test 'renders one line per command remediation and skips the hints' do + lines = RemediationPlan.build(detail, '0.22.0').lines + + assert_equal 2, lines.size + assert_equal ['vgimportdevices'], Shellwords.split(lines.first) + assert_equal NEW_SYMLINK_CONTEXT, Shellwords.split(lines.second) + end + + test 'returns an empty plan when there is nothing to run' do + assert_equal '', RemediationPlan.build(nil, '0.22.0') + assert_equal '', + RemediationPlan.build({ 'remediations' => [{ 'type' => 'hint', 'context' => 'meh.' }] }, + '0.22.0') + end + end + end +end