diff --git a/pkg/daemon/config_drift_monitor_test.go b/pkg/daemon/config_drift_monitor_test.go index 026893bfff..dbd9af345e 100644 --- a/pkg/daemon/config_drift_monitor_test.go +++ b/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))) } diff --git a/pkg/daemon/on_disk_validation.go b/pkg/daemon/on_disk_validation.go index 44c0c41e20..b4445c8b9b 100644 --- a/pkg/daemon/on_disk_validation.go +++ b/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 diff --git a/test/e2e-shared-tests/mcd_config_drift.go b/test/e2e-shared-tests/mcd_config_drift.go index dde473eab8..a1dad18312 100644 --- a/test/e2e-shared-tests/mcd_config_drift.go +++ b/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. { diff --git a/test/helpers/utils.go b/test/helpers/utils.go index cdebb02fc8..a8762de639 100644 --- a/test/helpers/utils.go +++ b/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{