Skip to content

Commit

Permalink
Add kernel arg tuneable allowlist for FCOS
Browse files Browse the repository at this point in the history 
This is required for Fedora CoreOS installs, which set
`mitigations=auto,nosmt` and may enable cgroups v2
in the future so it has to be allowed to disable it explicitly.
  • Loading branch information
@LorbusChris
LorbusChris committed Jul 14, 2020
1 parent 8814f92 commit faaf21d
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 29 deletions.
40 changes: 33 additions & 7 deletions cmd/machine-config-daemon/pivot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/daemon/daemon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/daemon/osrelease.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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":
Expand Down
16 changes: 8 additions & 8 deletions pkg/daemon/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -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")
}
Expand All @@ -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
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/daemon/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit faaf21d

Please sign in to comment.