Bug 1754881

tl;dr Sat-clone has changed to use foreman-maintain to restore data and there seems to be a problem reading the "var/lib/foreman/tasks-backup/20190524/actions.csv" file from the backup's config_files.tar.gz with the ruby library f-maintain uses.

Some background: There was a change in satellite-clone to use foreman-maintain to restore backups rather than sat-clone's own implementation. Foreman-maintain uses the `rubygems` package to read the tar file and get a list of files. https://github.com/theforeman/foreman_maintain/blob/187dfd11235438fc40eb2ea192cf91f37a8828a1/lib/foreman_maintain/utils/backup.rb#L220

This reading of the files is the part that is erroring. From what I can tell, rubygems takes a stream of the binary data and then reads the headers of each entry in the tar file, and skips to the next entry based on the size. To debug, I printed out some of the headers rubygems is reading here: https://github.com/rubygems/rubygems/blob/064cfaf7f5357c16a4fae04d53587fcf69b6df3f/lib/rubygems/package/tar_header.rb#L103-L126 (I added a "puts fields" in /usr/share/rubygems/rubygems/package/tar_header.rb after fields is declared.)

This is what I got for results, here is the portion near the error, I'm adding some comments for context:


# This is a file that was successfully read
var/lib/foreman/tasks-backup/20180913/steps.csv
0000644
0001742
0001740 # this fourth column is the size of the file
05124460462
13346443057
022416
0

ustar
00
foreman
foreman
0000000
0000000

# This is the problematic file
var/lib/foreman/tasks-backup/20190524/actions.csv
0000644
0001742
0001740
00000000000 # this column is the size, notice it is zero!
13471726172
022656
0

ustar
00
foreman
foreman
0000000
0000000


# This is what rubygems thinks the next headers are, this is definitely not binary data in the correct header format
execution_plan_uuid,id,data,caller_execution_plan_id,caller_action_id,class,input,output,plan_step_i
d,run_st
ep_id,fi
nalize_s
tep_id
0006f
143-e9a7-43e
4-a952-8
9
78f6fd05b5,1,"{""class"":""Actions::Katello::Repository::Sync"",""execution_plan_id"":""0006f143-e9a
7-43e4
-a
952-8978f6fd05b5"",""id"":1,""pl
an_step_id"":1,""run_step_id"":1
8,""fina
lize_ste


The third entry shown there is actually the second line from var/lib/foreman/tasks-backup/20190524/actions.csv, which leads me to believe the rubygems package thinks the actions.csv file size is zero (as shown in the headers), then increments only by the header length, and then tries to read the next file's headers, which is actually still "actions.csv".


Why is this happening? I'm really not sure, using "tar -tf", which is what we have been doing in satellite-clone is showing the correct output for the list of files. This leads me to believe there is a bug in rubygems, or the tar.gz file is actually corrupted and tar is more forgiving.


I'm looking into potential workarounds now, but I imagine removing the problematic file(s) from the backup would solve this

The following script is the isolated part of foreman-maintain that is erroring and can be used to check if a .tar.gz file will error during a restore

```
#!/usr/bin/env ruby

require 'zlib'
require 'rubygems/package'

# UPDATE ME
FILE_NAME = "/path/to/my/backup/config_files.tar.gz"

def tarball_file_list(tarball)
  # accepts tar.gz files only
  file_list = []
  File.open(tarball, 'rb') do |file|
    ::Zlib::GzipReader.wrap(file) do |gz|
      ::Gem::Package::TarReader.new(gz) do |tar|
        tar.each { |entry| file_list << entry.full_name }
      end
    end
  end
  file_list
end

puts tarball_file_list(FILE_NAME)

```

It looks like removing the files that are causing issues will fix this issue. You can debug by adding a "puts fields" in /usr/share/rubygems/rubygems/package/tar_header.rb after fields is declared around line 106 (be sure to backup the original file). This will display output like I show above and you can isolate the problematic file. Then you can either remove the file on the Satellite and take another backup OR extract config_files.tar.gz, remove the file(s), and re-tar the files. If you do re-tar the files, just be sure its saved in the original format of the tar file, using the relative directory from root (var/lib/www/ vs /tmp/extractedbackup/var/lib/www)

I don't think any changes are needed for foreman-maintain at this time. I'm thinking this is an isolated case and I've posted steps to debug and workaround the issue. If there are any more known cases or someone else comes across this, perhaps we can dig further but I have yet to see this issue in a foreman-maintain restore. I'm open to other suggestions though.

Created redmine issue https://projects.theforeman.org/issues/27969 from this bug

Update:
- I was able to open list and extract the archive with system GNU tar
- rubygems internal tar implementation may not fully support all the edge cases e.g. huge archives
- foreman-maintain is using older system Ruby (no collections) so it is possible [1] or [2] are causing the problem
- planned fix is to replace usages of internal tar with system GNU tar in restore procedures
- according to my testing performance won't be affected (little speed-up is expected)

As a temporary workaround the following patch worked for me:

diff --git a/lib/foreman_maintain/utils/backup.rb b/lib/foreman_maintain/utils/backup.rb 
index 28c37e7..55793c5 100644 
--- a/lib/foreman_maintain/utils/backup.rb 
+++ b/lib/foreman_maintain/utils/backup.rb 
@@ -213,16 +213,7 @@ module ForemanMaintain 
      private 
 
      def tarball_file_list(tarball) 
-        # accepts tar.gz files only 
-        file_list = [] 
-        File.open(tarball, 'rb') do |file| 
-          ::Zlib::GzipReader.wrap(file) do |gz| 
-            ::Gem::Package::TarReader.new(gz) do |tar| 
-              tar.each { |entry| file_list << entry.full_name } 
-            end 
-          end 
-        end 
-        file_list 
+       `tar -tvf #{tarball}`.split("\n").map { |line| line.gsub(/(\S+\s+){5}(.*)/, '\2') } 
      end 
    end 
  end

[1] https://github.com/rubygems/rubygems/issues/2213
[2] https://github.com/rubygems/rubygems/pull/2780

Moving this bug to POST for triage into Satellite 6 since the upstream issue https://projects.theforeman.org/issues/27969 has been resolved.

This bug has been successfully verified on 6.7.

foreman-maintain package: rubygem-foreman_maintain-0.5.0-1.el7sat.noarch

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:1442