Merge pull request #1925 from LorbusChris/tune-kargs

Add kernel arg tuneable allowlist for FCOS

cmd/machine-config-daemon/pivot.go

@@ -34,12 +34,18 @@ const (
 	cmdLineFile      = "/proc/cmdline"
 )
 
-// TODO: fill out the allowlist
-// tuneableArgsAllowlist contains allowed keys for tunable arguments
-var tuneableArgsAllowlist = map[string]bool{
+// TODO: fill out the allowlists
+// tuneableRHCOSArgsAllowlist contains allowed keys for tunable kernel arguments on RHCOS
+var tuneableRHCOSArgsAllowlist = map[string]bool{
 	"nosmt": true,
 }
 
+// tuneableFCOSArgsAllowlist contains allowed keys for tunable kernel arguments on FCOS
+var tuneableFCOSArgsAllowlist = map[string]bool{
+	"systemd.unified_cgroup_hierarchy=0": true,
+	"mitigations=auto,nosmt":             true,
+}
+
 var pivotCmd = &cobra.Command{
 	Use:                   "pivot",
 	DisableFlagsInUseLine: true,
@@ -57,8 +63,20 @@ func init() {
 }
 
 // isArgTuneable returns if the argument provided is allowed to be modified
-func isArgTunable(arg string) bool {
-	return tuneableArgsAllowlist[arg]
+func isArgTunable(arg string) (bool, error) {
+	operatingSystem, err := daemon.GetHostRunningOS()
+	if err != nil {
+		return false, errors.Errorf("failed to get OS for determining whether kernel arg is tuneable: %v", err)
+	}
+
+	switch operatingSystem {
+	case daemon.MachineConfigDaemonOSRHCOS:
+		return tuneableRHCOSArgsAllowlist[arg], nil
+	case daemon.MachineConfigDaemonOSFCOS:
+		return tuneableFCOSArgsAllowlist[arg], nil
+	default:
+		return false, nil
+	}
 }
 
 // isArgInUse checks to see if the argument is already in use by the system currently
@@ -108,7 +126,11 @@ func parseTuningFile(tuningFilePath, cmdLinePath string) ([]types.TuneArgument,
 			// NOTE: Today only specific bare kernel arguments are allowed so
 			// there is not a need to split on =.
 			key := strings.TrimSpace(line[len("ADD "):])
-			if isArgTunable(key) {
+			tuneableKarg, err := isArgTunable(key)
+			if err != nil {
+				return addArguments, deleteArguments, err
+			}
+			if tuneableKarg {
 				// Find out if the argument is in use
 				inUse, err := isArgInUse(key, cmdLinePath)
 				if err != nil {
@@ -126,7 +148,11 @@ func parseTuningFile(tuningFilePath, cmdLinePath string) ([]types.TuneArgument,
 			// NOTE: Today only specific bare kernel arguments are allowed so
 			// there is not a need to split on =.
 			key := strings.TrimSpace(line[len("DELETE "):])
-			if isArgTunable(key) {
+			tuneableKarg, err := isArgTunable(key)
+			if err != nil {
+				return addArguments, deleteArguments, err
+			}
+			if tuneableKarg {
 				inUse, err := isArgInUse(key, cmdLinePath)
 				if err != nil {
 					return addArguments, deleteArguments, err

pkg/daemon/daemon.go

@@ -204,15 +204,15 @@ func New(
 
 	operatingSystem := "mock"
 	if !mock {
-		operatingSystem, err = getHostRunningOS()
+		operatingSystem, err = GetHostRunningOS()
 		if err != nil {
 			HostOS.WithLabelValues("unsupported", "").Set(1)
 			return nil, errors.Wrapf(err, "checking operating system")
 		}
 	}
 
 	// Only pull the osImageURL from OSTree when we are on RHCOS or FCOS
-	if operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS {
+	if operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS {
 		osImageURL, osVersion, err = nodeUpdaterClient.GetBootedOSImageURL()
 		if err != nil {
 			return nil, fmt.Errorf("error reading osImageURL from rpm-ostree: %v", err)
@@ -827,7 +827,7 @@ func (dn *Daemon) getStateAndConfigs(pendingConfigName string) (*stateAndConfigs
 // dynamically after a reboot.
 func (dn *Daemon) LogSystemData() {
 	// Print status if available
-	if dn.OperatingSystem == machineConfigDaemonOSRHCOS || dn.OperatingSystem == machineConfigDaemonOSFCOS {
+	if dn.OperatingSystem == MachineConfigDaemonOSRHCOS || dn.OperatingSystem == MachineConfigDaemonOSFCOS {
 		status, err := dn.NodeUpdaterClient.GetStatus()
 		if err != nil {
 			glog.Fatalf("unable to get rpm-ostree status: %s", err)
@@ -1369,7 +1369,7 @@ func compareOSImageURL(current, desired string) (bool, error) {
 // Otherwise if `false` is returned, then we need to perform an update.
 func (dn *Daemon) checkOS(osImageURL string) (bool, error) {
 	// Nothing to do if we're not on RHCOS or FCOS
-	if dn.OperatingSystem != machineConfigDaemonOSRHCOS && dn.OperatingSystem != machineConfigDaemonOSFCOS {
+	if dn.OperatingSystem != MachineConfigDaemonOSRHCOS && dn.OperatingSystem != MachineConfigDaemonOSFCOS {
 		glog.Infof(`Not booted into a CoreOS variant, ignoring target OSImageURL %s`, osImageURL)
 		return true, nil
 	}

pkg/daemon/osrelease.go

@@ -7,21 +7,21 @@ import (
 )
 
 const (
-	// machineConfigDaemonOSRHCOS denotes RHEL CoreOS
-	machineConfigDaemonOSRHCOS = "RHCOS"
+	// MachineConfigDaemonOSRHCOS denotes RHEL CoreOS
+	MachineConfigDaemonOSRHCOS = "RHCOS"
 	// machineConfigDaemonOSRHEL denotes RHEL
 	machineConfigDaemonOSRHEL = "RHEL"
 	// machineConfigDaemonOSCENTOS denotes CentOS
 	machineConfigDaemonOSCENTOS = "CENTOS"
-	// machineConfigDaemonOSFCOS denotes Fedora CoreOS
-	machineConfigDaemonOSFCOS = "FCOS"
+	// MachineConfigDaemonOSFCOS denotes Fedora CoreOS
+	MachineConfigDaemonOSFCOS = "FCOS"
 )
 
-// getHostRunningOS reads os-release from the rootFs prefix to return what
+// GetHostRunningOS reads os-release from the rootFs prefix to return what
 // OS variant the daemon is running on. If we are unable to read the
 // os-release file OR the information doesn't match MCD supported OS's
 // an error is returned.
-func getHostRunningOS() (string, error) {
+func GetHostRunningOS() (string, error) {
 	libPath := "/usr/lib/os-release"
 	etcPath := "/etc/os-release"
 
@@ -31,13 +31,13 @@ func getHostRunningOS() (string, error) {
 	}
 
 	if or.ID == "fedora" && or.VARIANT_ID == "coreos" {
-		return machineConfigDaemonOSFCOS, nil
+		return MachineConfigDaemonOSFCOS, nil
 	}
 
 	// See https://github.com/openshift/redhat-release-coreos/blob/master/redhat-release-coreos.spec
 	switch or.ID {
 	case "rhcos":
-		return machineConfigDaemonOSRHCOS, nil
+		return MachineConfigDaemonOSRHCOS, nil
 	case "rhel":
 		return machineConfigDaemonOSRHEL, nil
 	case "centos":

pkg/daemon/update.go

@@ -689,7 +689,7 @@ func (dn *Daemon) updateKernelArguments(oldConfig, newConfig *mcfgv1.MachineConf
 	if len(diff) == 0 {
 		return nil
 	}
-	if dn.OperatingSystem != machineConfigDaemonOSRHCOS && dn.OperatingSystem != machineConfigDaemonOSFCOS {
+	if dn.OperatingSystem != MachineConfigDaemonOSRHCOS && dn.OperatingSystem != MachineConfigDaemonOSFCOS {
 		return fmt.Errorf("Updating kargs on non-CoreOS nodes is not supported: %v", diff)
 	}
 
@@ -739,7 +739,7 @@ func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error
 		return nil
 	}
 	// We support Kernel update only on RHCOS nodes
-	if dn.OperatingSystem != machineConfigDaemonOSRHCOS {
+	if dn.OperatingSystem != MachineConfigDaemonOSRHCOS {
 		return fmt.Errorf("Updating kernel on non-RHCOS nodes is not supported")
 	}
 
@@ -906,7 +906,7 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ign3types.Config)
 		newFileSet[f.Path] = struct{}{}
 	}
 
-	operatingSystem, err := getHostRunningOS()
+	operatingSystem, err := GetHostRunningOS()
 	if err != nil {
 		return errors.Wrapf(err, "checking operating system")
 	}
@@ -926,7 +926,7 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ign3types.Config)
 				if _, err := exec.Command("rpm", "-qf", f.Path).CombinedOutput(); err == nil {
 					// File is owned by an rpm
 					restore = true
-				} else if strings.HasPrefix(f.Path, "/etc") && (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) {
+				} else if strings.HasPrefix(f.Path, "/etc") && (operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS) {
 					if _, err := os.Stat("/usr" + f.Path); err != nil {
 						if !os.IsNotExist(err) {
 							return err
@@ -1071,7 +1071,7 @@ func (dn *Daemon) disableUnit(unit ign3types.Unit) error {
 
 // writeUnits writes the systemd units to disk
 func (dn *Daemon) writeUnits(units []ign3types.Unit) error {
-	operatingSystem, err := getHostRunningOS()
+	operatingSystem, err := GetHostRunningOS()
 	if err != nil {
 		return errors.Wrapf(err, "checking operating system")
 	}
@@ -1081,7 +1081,7 @@ func (dn *Daemon) writeUnits(units []ign3types.Unit) error {
 			glog.Infof("Writing systemd unit dropin %q", u.Dropins[i].Name)
 			dpath := filepath.Join(pathSystemd, u.Name+".d", u.Dropins[i].Name)
 			if _, err := os.Stat("/usr" + dpath); err == nil &&
-				(operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) {
+				(operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS) {
 				if err := createOrigFile("/usr"+dpath, dpath); err != nil {
 					return err
 				}
@@ -1115,7 +1115,7 @@ func (dn *Daemon) writeUnits(units []ign3types.Unit) error {
 		if u.Contents != nil && *u.Contents != "" {
 			glog.Infof("Writing systemd unit %q", u.Name)
 			if _, err := os.Stat("/usr" + fpath); err == nil &&
-				(operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) {
+				(operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS) {
 				if err := createOrigFile("/usr"+fpath, fpath); err != nil {
 					return err
 				}
@@ -1300,7 +1300,7 @@ func (dn *Daemon) updateSSHKeys(newUsers []ign3types.PasswdUser) error {
 
 // updateOS updates the system OS to the one specified in newConfig
 func (dn *Daemon) updateOS(config *mcfgv1.MachineConfig) error {
-	if dn.OperatingSystem != machineConfigDaemonOSRHCOS && dn.OperatingSystem != machineConfigDaemonOSFCOS {
+	if dn.OperatingSystem != MachineConfigDaemonOSRHCOS && dn.OperatingSystem != MachineConfigDaemonOSFCOS {
 		glog.V(2).Info("Updating of non-CoreOS nodes are not supported")
 		return nil
 	}

pkg/daemon/update_test.go

@@ -36,7 +36,7 @@ func TestUpdateOS(t *testing.T) {
 	d := Daemon{
 		mock:              true,
 		name:              "nodeName",
-		OperatingSystem:   machineConfigDaemonOSRHCOS,
+		OperatingSystem:   MachineConfigDaemonOSRHCOS,
 		NodeUpdaterClient: testClient,
 		kubeClient:        k8sfake.NewSimpleClientset(),
 		bootedOSImageURL:  "test",
@@ -387,7 +387,7 @@ func TestUpdateSSHKeys(t *testing.T) {
 	d := Daemon{
 		mock:              true,
 		name:              "nodeName",
-		OperatingSystem:   machineConfigDaemonOSRHCOS,
+		OperatingSystem:   MachineConfigDaemonOSRHCOS,
 		NodeUpdaterClient: testClient,
 		kubeClient:        k8sfake.NewSimpleClientset(),
 		bootedOSImageURL:  "test",