Fixes #39717 - Prepare for the upcoming remediation changes

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

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

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-el<X>toel<Y> 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

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'],

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)

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

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 %>
 

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

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**