Move Ignition file generation to controller/common, fix Compression

pkg/controller/common/helpers.go

@@ -45,6 +45,11 @@ import (
 	mcfgclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned"
 )
 
+// strToPtr converts the input string to a pointer to itself
+func strToPtr(s string) *string {
+	return &s
+}
+
 // MergeMachineConfigs combines multiple machineconfig objects into one object.
 // It sorts all the configs in increasing order of their name.
 // It uses the Ignition config from first object as base and appends all the rest.
@@ -765,6 +770,48 @@ func CalculateConfigFileDiffs(oldIgnConfig, newIgnConfig *ign3types.Config) []st
 	return diffFileSet
 }
 
+// NewIgnFile returns a simple ignition3 file from just path and file contents.
+// It also ensures the compression field is set to the empty string, which is
+// currently required for ensuring child configs that may be merged layer
+// know that the input is not compressed.
+//
+// Note the default Ignition file mode is 0644, owned by root/root.
+func NewIgnFile(path, contents string) ign3types.File {
+	return NewIgnFileBytes(path, []byte(contents))
+}
+
+// NewIgnFileBytes is like NewIgnFile, but accepts binary data
+func NewIgnFileBytes(path string, contents []byte) ign3types.File {
+	return ign3types.File{
+		Node: ign3types.Node{
+			Path: path,
+		},
+		FileEmbedded1: ign3types.FileEmbedded1{
+			Contents: ign3types.Resource{
+				Source:      strToPtr(dataurl.EncodeBytes(contents)),
+				Compression: strToPtr(""),
+			},
+		},
+	}
+}
+
+// NewIgnFileBytesOverwriting is like NewIgnFileBytes, but overwrites existing files by default
+func NewIgnFileBytesOverwriting(path string, contents []byte) ign3types.File {
+	overwrite := true
+	return ign3types.File{
+		Node: ign3types.Node{
+			Path:      path,
+			Overwrite: &overwrite,
+		},
+		FileEmbedded1: ign3types.FileEmbedded1{
+			Contents: ign3types.Resource{
+				Source:      strToPtr(dataurl.EncodeBytes(contents)),
+				Compression: strToPtr(""), // See https://github.com/coreos/butane/issues/332
+			},
+		},
+	}
+}
+
 // GetIgnitionFileDataByPath retrieves the file data for a specified path from a given ignition config
 func GetIgnitionFileDataByPath(config *ign3types.Config, path string) ([]byte, error) {
 	for _, f := range config.Storage.Files {

pkg/controller/common/helpers_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/clarketm/json"
 	ign2types "github.com/coreos/ignition/config/v2_2/types"
+	ign3 "github.com/coreos/ignition/v2/config/v3_2"
 	ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
 	validate3 "github.com/coreos/ignition/v2/config/validate"
 	"github.com/stretchr/testify/assert"
@@ -450,12 +451,35 @@ func TestRemoveIgnDuplicateFilesAndUnits(t *testing.T) {
 	assert.Equal(t, expectedIgn2Config, convertedIgn2Config)
 }
 
+// TestIgnitionMergeCompressed tests https://github.com/coreos/butane/issues/332
+func TestIgnitionMergeCompressed(t *testing.T) {
+	testIgn3Config := ign3types.Config{}
+	testIgn3Config.Ignition.Version = "3.2.0"
+	mode := 420
+	testfiledata := "data:;base64,H4sIAAAAAAAAA0vLz+cCAKhlMn4EAAAA"
+	compression := "gzip"
+	tempFile := ign3types.File{Node: ign3types.Node{Path: "/etc/testfileconfig"},
+		FileEmbedded1: ign3types.FileEmbedded1{Contents: ign3types.Resource{Source: &testfiledata, Compression: &compression}, Mode: &mode}}
+	testIgn3Config.Storage.Files = append(testIgn3Config.Storage.Files, tempFile)
+
+	testIgn3Config2 := ign3types.Config{}
+	testIgn3Config2.Ignition.Version = "3.2.0"
+	testIgn3Config2.Storage.Files = append(testIgn3Config2.Storage.Files, NewIgnFile("/etc/testfileconfig", "hello world"))
+
+	merged := ign3.Merge(testIgn3Config, testIgn3Config2)
+	assert.NotNil(t, merged)
+	mergedFile := merged.Storage.Files[0]
+	contents, err := DecodeIgnitionFileContents(mergedFile.Contents.Source, mergedFile.Contents.Compression)
+	require.NoError(t, err)
+	assert.Equal(t, string(contents), "hello world")
+}
+
 func TestCalculateConfigFileDiffs(t *testing.T) {
 	var testIgn3ConfigOld ign3types.Config
 	var testIgn3ConfigNew ign3types.Config
 
-	oldTempFile := helpers.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "oldcertificates")
-	newTempFile := helpers.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "newcertificates")
+	oldTempFile := NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "oldcertificates")
+	newTempFile := NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "newcertificates")
 
 	// Make an "old" config with the existing file in it
 	testIgn3ConfigOld.Ignition.Version = "3.2.0"

pkg/controller/container-runtime-config/helpers.go

@@ -20,7 +20,6 @@ import (
 	apicfgv1 "github.com/openshift/api/config/v1"
 	apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1"
 	"github.com/openshift/runtime-utils/pkg/registries"
-	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -108,29 +107,13 @@ type updateConfigFunc func(data []byte, internal *mcfgv1.ContainerRuntimeConfigu
 // updated data.
 func createNewIgnition(configs []generatedConfigFile) ign3types.Config {
 	tempIgnConfig := ctrlcommon.NewIgnConfig()
-	mode := 0644
-	overwrite := true
 	// Create ignitions
 	for _, ignConf := range configs {
 		// If the file is not included, the data will be nil so skip over
 		if ignConf.data == nil {
 			continue
 		}
-		configdu := dataurl.New(ignConf.data, "text/plain")
-		configdu.Encoding = dataurl.EncodingASCII
-		configduStr := configdu.String()
-		configTempFile := ign3types.File{
-			Node: ign3types.Node{
-				Path:      ignConf.filePath,
-				Overwrite: &overwrite,
-			},
-			FileEmbedded1: ign3types.FileEmbedded1{
-				Mode: &mode,
-				Contents: ign3types.Resource{
-					Source: &(configduStr),
-				},
-			},
-		}
+		configTempFile := ctrlcommon.NewIgnFileBytesOverwriting(ignConf.filePath, ignConf.data)
 		tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, configTempFile)
 	}
 

pkg/controller/kubelet-config/helpers.go

@@ -11,7 +11,6 @@ import (
 	ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
 	"github.com/imdario/mergo"
 	osev1 "github.com/openshift/api/config/v1"
-	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -50,73 +49,23 @@ func createNewKubeletDynamicSystemReservedIgnition(autoSystemReserved *bool, use
 	}
 
 	config := fmt.Sprintf("NODE_SIZING_ENABLED=%s\nSYSTEM_RESERVED_MEMORY=%s\nSYSTEM_RESERVED_CPU=%s\n", autoNodeSizing, systemReservedMemory, systemReservedCPU)
-
-	mode := 0644
-	overwrite := true
-	du := dataurl.New([]byte(config), "text/plain")
-	du.Encoding = dataurl.EncodingASCII
-	duStr := du.String()
-
-	return &ign3types.File{
-		Node: ign3types.Node{
-			Path:      "/etc/node-sizing-enabled.env",
-			Overwrite: &overwrite,
-		},
-		FileEmbedded1: ign3types.FileEmbedded1{
-			Mode: &mode,
-			Contents: ign3types.Resource{
-				Source: &(duStr),
-			},
-		},
-	}
+	r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/node-sizing-enabled.env", []byte(config))
+	return &r
 }
 
 func createNewKubeletLogLevelIgnition(level int32) *ign3types.File {
 	config := fmt.Sprintf("[Service]\nEnvironment=\"KUBELET_LOG_LEVEL=%d\"\n", level)
-
-	mode := 0644
-	overwrite := true
-	du := dataurl.New([]byte(config), "text/plain")
-	du.Encoding = dataurl.EncodingASCII
-	duStr := du.String()
-
-	return &ign3types.File{
-		Node: ign3types.Node{
-			Path:      "/etc/systemd/system/kubelet.service.d/20-logging.conf",
-			Overwrite: &overwrite,
-		},
-		FileEmbedded1: ign3types.FileEmbedded1{
-			Mode: &mode,
-			Contents: ign3types.Resource{
-				Source: &(duStr),
-			},
-		},
-	}
+	r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/systemd/system/kubelet.service.d/20-logging.conf", []byte(config))
+	return &r
 }
 
 func createNewKubeletIgnition(jsonConfig []byte) *ign3types.File {
 	// Want the kubelet.conf file to have the pretty JSON formatting
 	buf := new(bytes.Buffer)
 	json.Indent(buf, jsonConfig, "", "  ")
 
-	mode := 0644
-	overwrite := true
-	du := dataurl.New(buf.Bytes(), "text/plain")
-	du.Encoding = dataurl.EncodingASCII
-	duStr := du.String()
-
-	return &ign3types.File{
-		Node: ign3types.Node{
-			Path:      "/etc/kubernetes/kubelet.conf",
-			Overwrite: &overwrite,
-		},
-		FileEmbedded1: ign3types.FileEmbedded1{
-			Mode: &mode,
-			Contents: ign3types.Resource{
-				Source: &(duStr),
-			},
-		},
-	}
+	r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/kubernetes/kubelet.conf", buf.Bytes())
+	return &r
 }
 
 func createNewDefaultFeatureGate() *osev1.FeatureGate {

pkg/controller/node/node_controller_test.go

@@ -959,8 +959,8 @@ func TestAlertOnPausedKubeletCA(t *testing.T) {
 	mcp := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v1")
 
 	mcfiles := []ign3types.File{
-		helpers.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", TestKubeletCABundle),
-		helpers.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "newcertificates"),
+		ctrlcommon.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", TestKubeletCABundle),
+		ctrlcommon.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "newcertificates"),
 	}
 
 	mcs := []*mcfgv1.MachineConfig{

pkg/daemon/update_test.go

@@ -571,18 +571,18 @@ func TestDropinCheck(t *testing.T) {
 // i.e. whether we need to reboot and what actions need to be taken if no reboot is needed
 func TestCalculatePostConfigChangeAction(t *testing.T) {
 	files := map[string]ign3types.File{
-		"pullsecret1":     helpers.NewIgnFile("/var/lib/kubelet/config.json", "kubelet conf 1\n"),
-		"pullsecret2":     helpers.NewIgnFile("/var/lib/kubelet/config.json", "kubelet conf 2\n"),
-		"registries1":     helpers.NewIgnFile("/etc/containers/registries.conf", "registries content 1\n"),
-		"registries2":     helpers.NewIgnFile("/etc/containers/registries.conf", "registries content 2\n"),
-		"randomfile1":     helpers.NewIgnFile("/etc/random-reboot-file", "test\n"),
-		"randomfile2":     helpers.NewIgnFile("/etc/random-reboot-file", "test 2\n"),
-		"kubeletCA1":      helpers.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "kubeletCA1\n"),
-		"kubeletCA2":      helpers.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "kubeletCA2\n"),
-		"policy1":         helpers.NewIgnFile("/etc/containers/policy.json", "policy1"),
-		"policy2":         helpers.NewIgnFile("/etc/containers/policy.json", "policy2"),
-		"containers-gpg1": helpers.NewIgnFile("/etc/machine-config-daemon/no-reboot/containers-gpg.pub", "containers-gpg1"),
-		"containers-gpg2": helpers.NewIgnFile("/etc/machine-config-daemon/no-reboot/containers-gpg.pub", "containers-gpg2"),
+		"pullsecret1":     ctrlcommon.NewIgnFile("/var/lib/kubelet/config.json", "kubelet conf 1\n"),
+		"pullsecret2":     ctrlcommon.NewIgnFile("/var/lib/kubelet/config.json", "kubelet conf 2\n"),
+		"registries1":     ctrlcommon.NewIgnFile("/etc/containers/registries.conf", "registries content 1\n"),
+		"registries2":     ctrlcommon.NewIgnFile("/etc/containers/registries.conf", "registries content 2\n"),
+		"randomfile1":     ctrlcommon.NewIgnFile("/etc/random-reboot-file", "test\n"),
+		"randomfile2":     ctrlcommon.NewIgnFile("/etc/random-reboot-file", "test 2\n"),
+		"kubeletCA1":      ctrlcommon.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "kubeletCA1\n"),
+		"kubeletCA2":      ctrlcommon.NewIgnFile("/etc/kubernetes/kubelet-ca.crt", "kubeletCA2\n"),
+		"policy1":         ctrlcommon.NewIgnFile("/etc/containers/policy.json", "policy1"),
+		"policy2":         ctrlcommon.NewIgnFile("/etc/containers/policy.json", "policy2"),
+		"containers-gpg1": ctrlcommon.NewIgnFile("/etc/machine-config-daemon/no-reboot/containers-gpg.pub", "containers-gpg1"),
+		"containers-gpg2": ctrlcommon.NewIgnFile("/etc/machine-config-daemon/no-reboot/containers-gpg.pub", "containers-gpg2"),
 	}
 
 	tests := []struct {

test/helpers/helpers.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/clarketm/json"
 	ign3types "github.com/coreos/ignition/v2/config/v3_2/types"
-	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -151,19 +150,6 @@ func NewMachineConfigPool(name string, mcSelector, nodeSelector *metav1.LabelSel
 	}
 }
 
-// NewIgnFile returns a simple ignition3 file from just path and file contents
-func NewIgnFile(path, contents string) ign3types.File {
-	return ign3types.File{
-		Node: ign3types.Node{
-			Path: path,
-		},
-		FileEmbedded1: ign3types.FileEmbedded1{
-			Contents: ign3types.Resource{
-				Source: StrToPtr(dataurl.EncodeBytes([]byte(contents)))},
-		},
-	}
-}
-
 // CreateMachineConfigFromIgnition returns a MachineConfig object from an Ignition config passed to it
 func CreateMachineConfigFromIgnition(ignCfg interface{}) *mcfgv1.MachineConfig {
 	return &mcfgv1.MachineConfig{