Bug 2032565: makes config drift monitor aware of compressed files

pkg/daemon/config_drift_monitor_test.go

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -101,6 +100,40 @@ func TestConfigDriftMonitor(t *testing.T) {
 			expectedErr: fileErr,
 			mutateFile:  chmodFile,
 		},
+		// Compressed Ignition File
+		// These target the file called /etc/a-compressed-file defined by the test
+		// fixture.
+		// Targets a regression identified by:
+		// https://bugzilla.redhat.com/show_bug.cgi?id=2032565
+		{
+			name:                 "ign compressed file content drift",
+			expectedErr:          fileErr,
+			mutateCompressedFile: changeFileContent,
+		},
+		{
+			name:                 "ign compressed file touch",
+			mutateCompressedFile: touchFile,
+		},
+		{
+			name:                 "ign compressed file rename",
+			expectedErr:          fileErr,
+			mutateCompressedFile: renameFile,
+		},
+		{
+			name:                 "ign compressed file delete",
+			expectedErr:          fileErr,
+			mutateCompressedFile: os.Remove,
+		},
+		{
+			name:                 "ign compressed file overwrite",
+			expectedErr:          fileErr,
+			mutateCompressedFile: overwriteFile,
+		},
+		{
+			name:                 "ign compressed file chmod",
+			expectedErr:          fileErr,
+			mutateCompressedFile: chmodFile,
+		},
 		// Systemd Unit tests
 		// These target the systemd unit files in the test fixture:
 		// /etc/systemd/system/unittest.service
@@ -196,6 +229,8 @@ type configDriftMonitorTestCase struct {
 	// Only one of these may be used per testcase:
 	// The mutation to apply to the Ignition file
 	mutateFile func(string) error
+	// The mutation to apply to the compressed Ignition file
+	mutateCompressedFile func(string) error
 	// The mutation to apply to the systemd unit file
 	mutateUnit func(string) error
 	// The mutation to apply to the systemd dropin file
@@ -314,14 +349,18 @@ func (tc configDriftMonitorTestCase) run(t *testing.T) {
 }
 
 // Creates the Ignition Config test fixture
-func (tc configDriftMonitorTestCase) getIgnConfig() ign3types.Config {
+func (tc configDriftMonitorTestCase) getIgnConfig(t *testing.T) ign3types.Config {
+	compressedFile, err := helpers.CreateGzippedIgn3File("/etc/a-compressed-file", "thefilecontents", int(defaultFilePermissions))
+	require.Nil(t, err)
+
 	return ign3types.Config{
 		Ignition: ign3types.Ignition{
 			Version: ign3types.MaxVersion.String(),
 		},
 		Storage: ign3types.Storage{
 			Files: []ign3types.File{
-				helpers.CreateIgn3File("/etc/a-config-file", "data:,theafilecontents", int(defaultFilePermissions)),
+				helpers.CreateEncodedIgn3File("/etc/a-config-file", "thefilecontents", int(defaultFilePermissions)),
+				compressedFile,
 			},
 		},
 		Systemd: ign3types.Systemd{
@@ -350,6 +389,10 @@ func (tc configDriftMonitorTestCase) mutate(ignConfig ign3types.Config) error {
 		return tc.mutateFile(ignConfig.Storage.Files[0].Path)
 	}
 
+	if tc.mutateCompressedFile != nil {
+		return tc.mutateCompressedFile(ignConfig.Storage.Files[1].Path)
+	}
+
 	if tc.mutateDropin != nil {
 		dropinPath := getIgn3SystemdDropinPath(tc.systemdPath, ignConfig.Systemd.Units[0], ignConfig.Systemd.Units[0].Dropins[0])
 		return tc.mutateDropin(dropinPath)
@@ -368,13 +411,13 @@ func (tc configDriftMonitorTestCase) mutate(ignConfig ign3types.Config) error {
 func (tc configDriftMonitorTestCase) getFixtures(t *testing.T) (ign3types.Config, *mcfgv1.MachineConfig) {
 	t.Helper()
 
-	ignConfig := tc.getIgnConfig()
+	ignConfig := tc.getIgnConfig(t)
 
-	// Prefix all the ignition files with the temp directory and write there.
+	// Prefix all the ignition files with the temp directory and write to disk.
 	for i, file := range ignConfig.Storage.Files {
 		file.Path = filepath.Join(tc.tmpDir, file.Path)
-		tc.writeFileForTest(t, file.Path, file.Contents.Source)
 		ignConfig.Storage.Files[i] = file
+		tc.writeIgn3FileForTest(t, file)
 	}
 
 	// Prefix all the systemd files with the temp dir and write them.
@@ -418,13 +461,22 @@ func (tc configDriftMonitorTestCase) onDriftFunc(t *testing.T, err error) {
 	}
 }
 
+func (tc configDriftMonitorTestCase) writeIgn3FileForTest(t *testing.T, file ign3types.File) {
+	t.Helper()
+
+	decodedContents, err := decodeContents(file.Contents.Source, file.Contents.Compression)
+	require.Nil(t, err)
+
+	require.Nil(t, writeFileAtomicallyWithDefaults(file.Path, decodedContents))
+}
+
 func (tc configDriftMonitorTestCase) writeFileForTest(t *testing.T, path string, contents *string) {
 	t.Helper()
-	// writeFiles doesn't actually work like this. Writing files this way was
-	// done for the sake of simplicity.
+
 	out := ""
 	if contents != nil {
-		out = strings.TrimPrefix(*contents, "data:,")
+		out = *contents
 	}
+
 	require.Nil(t, writeFileAtomicallyWithDefaults(path, []byte(out)))
 }

pkg/daemon/on_disk_validation.go

@@ -15,7 +15,6 @@ import (
 	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/pkg/errors"
-	"github.com/vincent-petithory/dataurl"
 )
 
 // Validates that the on-disk state matches a given MachineConfig.
@@ -151,15 +150,11 @@ func checkV3Files(files []ign3types.File) error {
 		if f.Mode != nil {
 			mode = os.FileMode(*f.Mode)
 		}
-		contents := &dataurl.DataURL{}
-		if f.Contents.Source != nil {
-			var err error
-			contents, err = dataurl.DecodeString(*f.Contents.Source)
-			if err != nil {
-				return errors.Wrapf(err, "couldn't parse file %q", f.Path)
-			}
+		contents, err := decodeContents(f.Contents.Source, f.Contents.Compression)
+		if err != nil {
+			return fmt.Errorf("couldn't decode file %q: %w", f.Path, err)
 		}
-		if err := checkFileContentsAndMode(f.Path, contents.Data, mode); err != nil {
+		if err := checkFileContentsAndMode(f.Path, contents, mode); err != nil {
 			return err
 		}
 	}
@@ -182,11 +177,11 @@ func checkV2Files(files []ign2types.File) error {
 		if f.Mode != nil {
 			mode = os.FileMode(*f.Mode)
 		}
-		contents, err := dataurl.DecodeString(f.Contents.Source)
+		contents, err := decodeContents(&f.Contents.Source, &f.Contents.Compression)
 		if err != nil {
-			return errors.Wrapf(err, "couldn't parse file %q", f.Path)
+			return fmt.Errorf("couldn't decode file %q: %w", f.Path, err)
 		}
-		if err := checkFileContentsAndMode(f.Path, contents.Data, mode); err != nil {
+		if err := checkFileContentsAndMode(f.Path, contents, mode); err != nil {
 			return err
 		}
 		checkedFiles[f.Path] = true

test/e2e-shared-tests/mcd_config_drift.go

@@ -23,6 +23,7 @@ const (
 	configDriftSystemdUnitFileContents   string = "unittest-service-unit-contents"
 	configDriftSystemdDropinFilename     string = "/etc/systemd/system/unittest.service.d/10-unittest-service.conf"
 	configDriftSystemdDropinFileContents string = "unittest-service-dropin-contents"
+	configDriftCompressedFilename        string = "/etc/compressed-file"
 	configDriftFilename                  string = "/etc/etc-file"
 	configDriftFileContents              string = "expected-file-data"
 	configDriftMCPrefix                  string = "mcd-config-drift"
@@ -100,11 +101,36 @@ func (c *configDriftTest) Setup(t *testing.T) {
 	}
 
 	// This is the Machine Config that we drift from
-	c.mc = helpers.NewMachineConfigExtended(
+	c.mc = c.getMachineConfig(t)
+
+	// Delegate to the attached Setup Func for MachineConfig, MachineConfigPool
+	// creation.
+	c.SetupFunc(c.mc)
+
+	mcp, err := c.ClientSet.MachineConfigPools().Get(context.TODO(), c.MCPName, metav1.GetOptions{})
+	require.Nil(t, err)
+	c.mcp = *mcp
+
+	// Get the target node
+	c.node = helpers.GetSingleNodeByRole(t, c.ClientSet, c.MCPName)
+
+	// Get the MCD pod
+	pod, err := helpers.MCDForNode(c.ClientSet, &c.node)
+	require.Nil(t, err)
+
+	c.pod = *pod
+}
+
+func (c configDriftTest) getMachineConfig(t *testing.T) *mcfgv1.MachineConfig {
+	compressedFile, err := helpers.CreateGzippedIgn3File(configDriftCompressedFilename, configDriftFileContents, 420)
+	require.Nil(t, err)
+
+	return helpers.NewMachineConfigExtended(
 		fmt.Sprintf("%s-%s", configDriftMCPrefix, string(uuid.NewUUID())),
 		helpers.MCLabelForRole(c.MCPName),
 		[]ign3types.File{
-			helpers.CreateIgn3File(configDriftFilename, "data:,"+configDriftFileContents, 420),
+			helpers.CreateEncodedIgn3File(configDriftFilename, configDriftFileContents, 420),
+			compressedFile,
 		},
 		[]ign3types.Unit{
 			{
@@ -128,23 +154,6 @@ func (c *configDriftTest) Setup(t *testing.T) {
 		"",
 		"",
 	)
-
-	// Delegate to the attached Setup Func for MachineConfig, MachineConfigPool
-	// creation.
-	c.SetupFunc(c.mc)
-
-	mcp, err := c.ClientSet.MachineConfigPools().Get(context.TODO(), c.MCPName, metav1.GetOptions{})
-	require.Nil(t, err)
-	c.mcp = *mcp
-
-	// Get the target node
-	c.node = helpers.GetSingleNodeByRole(t, c.ClientSet, c.MCPName)
-
-	// Get the MCD pod
-	pod, err := helpers.MCDForNode(c.ClientSet, &c.node)
-	require.Nil(t, err)
-
-	c.pod = *pod
 }
 
 // Tears down the test objects by delegating to the attached TeardownFunc
@@ -185,6 +194,14 @@ func (c configDriftTest) Run(t *testing.T) {
 				c.runDegradeAndRecoverContentRevert(t, configDriftSystemdUnitFilename, configDriftSystemdUnitFileContents)
 			},
 		},
+		// Targets a regression identified by:
+		// https://bugzilla.redhat.com/show_bug.cgi?id=2032565
+		{
+			name: "revert file content recovery for compressed file",
+			testFunc: func(t *testing.T) {
+				c.runDegradeAndRecoverContentRevert(t, configDriftCompressedFilename, configDriftFileContents)
+			},
+		},
 		// 1. Mutates a file on the node.
 		// 2. Creates the forcefile to cause recovery.
 		{

test/helpers/utils.go

@@ -1,6 +1,8 @@
 package helpers
 
 import (
+	"bytes"
+	"compress/gzip"
 	"context"
 	"fmt"
 	"math/rand"
@@ -15,6 +17,7 @@ import (
 	"github.com/openshift/machine-config-operator/test/framework"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
+	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
@@ -383,6 +386,36 @@ func MCLabelForWorkers() map[string]string {
 // 	}
 // }
 
+// Creates an Ign3 file whose contents are gzipped and encoded according to
+// https://datatracker.ietf.org/doc/html/rfc2397
+func CreateGzippedIgn3File(path, content string, mode int) (ign3types.File, error) {
+	ign3File := ign3types.File{}
+
+	buf := bytes.NewBuffer([]byte{})
+
+	gzipWriter := gzip.NewWriter(buf)
+	if _, err := gzipWriter.Write([]byte(content)); err != nil {
+		return ign3File, err
+	}
+
+	if err := gzipWriter.Close(); err != nil {
+		return ign3File, err
+	}
+
+	ign3File = CreateEncodedIgn3File(path, buf.String(), mode)
+	ign3File.Contents.Compression = StrToPtr("gzip")
+
+	return ign3File, nil
+}
+
+// Creates an Ign3 file whose contents are encoded according to
+// https://datatracker.ietf.org/doc/html/rfc2397
+func CreateEncodedIgn3File(path, content string, mode int) ign3types.File {
+	encoded := dataurl.EncodeBytes([]byte(content))
+
+	return CreateIgn3File(path, encoded, mode)
+}
+
 func CreateIgn3File(path, content string, mode int) ign3types.File {
 	return ign3types.File{
 		FileEmbedded1: ign3types.FileEmbedded1{