Restrict orig preservation to only /usr/etc directory and files that …

…are claimed by the rpm; delete all incorrectly preserved orig files and create noorigs for them

pkg/daemon/file_writers.go

@@ -8,6 +8,7 @@ import (
 	"os/user"
 	"path/filepath"
 	"strconv"
+	"strings"
 
 	ign3types "github.com/coreos/ignition/v2/config/v3_4/types"
 	"github.com/google/renameio"
@@ -45,12 +46,81 @@ func noOrigFileStampName(fpath string) string {
 	return filepath.Join(noOrigParentDir(), fpath+".mcdnoorig")
 }
 
+func isFileOwnedByRPMPkg(fpath string) (bool, bool, error) {
+	// The first bool returns false if rpm exists, and true otherwise (indicating other Linux distros or Mac).
+	// We would like to skip orig file creation and preservation for [first bool = true] cases.
+	// The second bool returns true if the given path is owned by the rpm pkg.
+
+	// Check whether the underlying OS is Fedora/RHEL, skip the rest and return if not (e.g. Mac -> no rpm)
+	// If we cannot find the rpm binary, return an error.
+	path, err := exec.LookPath("rpm")
+	if err != nil {
+		return true, false, err
+	}
+
+	// Check whether the given path is owned by an rpm pkg
+	cmd := exec.Command(path, "-qf", fpath)
+	out, err := cmd.CombinedOutput()
+
+	if err == nil && cmd.ProcessState.ExitCode() == 0 {
+		return false, true, nil
+	}
+	fileNotOwnedMsg := fmt.Sprintf("file %s is not owned by any package", fpath)
+	if strings.Contains(string(out), fileNotOwnedMsg) {
+		return false, false, nil
+	}
+	return false, false, fmt.Errorf("command %q returned with unexpected error: %s: %w", cmd, string(out), err)
+}
+
 func createOrigFile(fromPath, fpath string) error {
-	if _, err := os.Stat(noOrigFileStampName(fpath)); err == nil {
-		// we already created the no orig file for this default file
-		return nil
+	orig := false
+
+	// https://issues.redhat.com/browse/OCPBUGS-11437
+	// MCO keeps the pull secret to .orig file once it replaced
+	// Adapt a check used in function deleteStaleData for backwards compatibility: basically if the file doesn't
+	// exist in /usr/etc (on FCOS/RHCOS) and no rpm is claiming it, we assume the orig file doesn't need to be created.
+	// Only do the check for existing files because files that wasn't present on disk before MCD took over will never
+	// be files that were shipped _with_ the underlying OS (e.g. a default chrony config).
+	if _, err := os.Stat(fpath); err == nil {
+		rpmNotFound, isOwned, err := isFileOwnedByRPMPkg(fpath)
+		if isOwned {
+			// File is owned by an rpm
+			orig = true
+		} else if !isOwned && (err == nil) {
+			// Run on Fedora/RHEL - check whether the file exist in /usr/etc (on FCOS/RHCOS)
+			if strings.HasPrefix(fpath, "/etc") {
+				if _, err := os.Stat(withUsrPath(fpath)); err != nil {
+					if !os.IsNotExist(err) {
+						return err
+					}
+				} else {
+					orig = true
+				}
+			}
+		} else if rpmNotFound {
+			// Run on non-Fedora/RHEL machine
+			klog.Infof("Running on non-Fedora/RHEL,skip orig file preservation.")
+		} else {
+			return err
+		}
+	} else if !os.IsNotExist(err) {
+		return err
 	}
-	if _, err := os.Stat(fpath); os.IsNotExist(err) {
+
+	if !orig {
+		if _, err := os.Stat(noOrigFileStampName(fpath)); err == nil {
+			// we already created the no orig file for this default file
+			return nil
+		}
+
+		// check whether there is already an orig preservation for the path, and remove upon existence (wrongly preserved)
+		if _, err := os.Stat(origFileName(fpath)); err == nil {
+			if delErr := os.Remove(origFileName(fpath)); delErr != nil {
+				return fmt.Errorf("deleting orig file for %q: %w", origFileName(fpath), delErr)
+			}
+			klog.Infof("Removing files %q completely for incorrect preservation", origFileName(fpath))
+		}
+
 		// create a noorig file that tells the MCD that the file wasn't present on disk before MCD
 		// took over so it can just remove it when deleting stale data, as opposed as restoring a file
 		// that was shipped _with_ the underlying OS (e.g. a default chrony config).

pkg/daemon/update.go

@@ -1245,6 +1245,7 @@ func (dn *Daemon) isPathInDropins(path string, systemd *ign3types.Systemd) bool
 //nolint:gocyclo
 func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig ign3types.Config) error {
 	klog.Info("Deleting stale data")
+
 	newFileSet := make(map[string]struct{})
 	for _, f := range newIgnConfig.Storage.Files {
 		newFileSet[f.Path] = struct{}{}
@@ -1282,20 +1283,27 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig ign3types.Config) e
 			// Add a check for backwards compatibility: basically if the file doesn't exist in /usr/etc (on FCOS/RHCOS)
 			// and no rpm is claiming it, we assume that the orig file came from a wrongful backup of a MachineConfig
 			// file instead of a file originally on disk. See https://bugzilla.redhat.com/show_bug.cgi?id=1814397
-			var restore bool
-			if _, err := exec.Command("rpm", "-qf", f.Path).CombinedOutput(); err == nil {
+			restore := false
+			rpmNotFound, isOwned, err := isFileOwnedByRPMPkg(f.Path)
+			if isOwned {
 				// File is owned by an rpm
 				restore = true
-			} else if strings.HasPrefix(f.Path, "/etc") && dn.os.IsCoreOSVariant() {
-				if _, err := os.Stat(withUsrPath(f.Path)); err != nil {
-					if !os.IsNotExist(err) {
-						return err
+			} else if !isOwned && (err == nil) {
+				// Run on Fedora/RHEL - check whether the file exist in /usr/etc (on FCOS/RHCOS)
+				if strings.HasPrefix(f.Path, "/etc") {
+					if _, err := os.Stat(withUsrPath(f.Path)); err != nil {
+						if !os.IsNotExist(err) {
+							return err
+						}
+					} else {
+						restore = true
 					}
-
-					// If the error is ErrNotExist then we don't restore the file
-				} else {
-					restore = true
 				}
+			} else if rpmNotFound {
+				// Run on non-Fedora/RHEL machine
+				klog.Infof("Running on non-Fedora/RHEL machine, skip file restoration.")
+			} else {
+				return err
 			}
 
 			if restore {

pkg/daemon/update_test.go

@@ -817,51 +817,20 @@ func TestOriginalFileBackupRestore(t *testing.T) {
 	testDir, cleanup := setupTempDirWithEtc(t)
 	defer cleanup()
 
-	// Write a normal file as a control to make sure normal case works
+	// Write a file in the /tmp dir to test whether orig files are selectively preserved
 	controlFile := filepath.Join(testDir, "control-file")
 	err := os.WriteFile(controlFile, []byte("control file contents"), 0755)
 	assert.Nil(t, err)
 
-	// Back up that normal file
+	// Back up the tmp file
 	err = createOrigFile(controlFile, controlFile)
 	assert.Nil(t, err)
 
-	// Now try again and make sure it knows it's already backed up
+	// Now try again and make sure it knows it's already backed up if it should be
 	err = createOrigFile(controlFile, controlFile)
 	assert.Nil(t, err)
 
-	// Restore the normal file
-	err = restorePath(controlFile)
-	assert.Nil(t, err)
-
-	// The normal file worked, try it with a symlink
-	// Write a file we can point a symlink at
-	err = os.WriteFile(filepath.Join(testDir, "target-file"), []byte("target file contents"), 0755)
-	assert.Nil(t, err)
-
-	// Make a relative symlink
-	relativeSymlink := filepath.Join(testDir, "etc", "relative-symlink-to-target-file")
-	relativeSymlinkTarget := filepath.Join("..", "target-file")
-	err = os.Symlink(relativeSymlinkTarget, relativeSymlink)
-	assert.Nil(t, err)
-
-	// Back up the relative symlink
-	err = createOrigFile(relativeSymlink, relativeSymlink)
-	assert.Nil(t, err)
-
-	// Remove the symlink and write a file over it
-	fileOverSymlink := filepath.Join(testDir, "etc", "relative-symlink-to-target-file")
-	err = os.Remove(fileOverSymlink)
-	assert.Nil(t, err)
-	err = os.WriteFile(fileOverSymlink, []byte("replacement contents"), 0755)
-	assert.Nil(t, err)
-
-	// Try to back it up again make sure it knows it's already backed up
-	err = createOrigFile(relativeSymlink, relativeSymlink)
-	assert.Nil(t, err)
-
-	// Finally, make sure we can restore the relative symlink if we rollback
-	err = restorePath(relativeSymlink)
-	assert.Nil(t, err)
-
+	// Check whether there is an orig preservation for the path - OK: there is no back up for the tmp file
+	_, err = os.Stat(origFileName(controlFile))
+	assert.True(t, os.IsNotExist(err))
 }