Merge pull request #4824 from isabella-janssen/ocpbugs-34586

[release-4.17] OCPBUGS-49363: Auto-recover from MC with invalid extension

Author: openshift-merge-bot[bot]

pkg/controller/common/helpers.go

@@ -609,9 +609,50 @@ func ValidateMachineConfig(cfg mcfgv1.MachineConfigSpec) error {
 			return err
 		}
 	}
+
+	// Validate MC extensions are in allowlist
+	if len(cfg.Extensions) > 0 {
+		if err := ValidateMachineConfigExtensions(cfg); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// Validates that a given MachineConfig's extensions are supported.
+func ValidateMachineConfigExtensions(cfg mcfgv1.MachineConfigSpec) error {
+	return validateExtensions(cfg.Extensions)
+}
+func validateExtensions(exts []string) error {
+	supportedExtensions := SupportedExtensions()
+	invalidExts := []string{}
+	for _, ext := range exts {
+		if _, ok := supportedExtensions[ext]; !ok {
+			invalidExts = append(invalidExts, ext)
+		}
+	}
+	if len(invalidExts) != 0 {
+		return fmt.Errorf("invalid extensions found: %v", invalidExts)
+	}
 	return nil
 }
 
+// Returns list of extensions possible to install on a CoreOS based system.
+func SupportedExtensions() map[string][]string {
+	// In future when list of extensions grow, it will make
+	// more sense to populate it in a dynamic way.
+	// These are RHCOS supported extensions.
+	// Each extension keeps a list of packages required to get enabled on host.
+	return map[string][]string{
+		"wasm":                 {"crun-wasm"},
+		"ipsec":                {"NetworkManager-libreswan", "libreswan"},
+		"usbguard":             {"usbguard"},
+		"kerberos":             {"krb5-workstation", "libkadm5"},
+		"kernel-devel":         {"kernel-devel", "kernel-headers"},
+		"sandboxed-containers": {"kata-containers"},
+	}
+}
+
 // IgnParseWrapper parses rawIgn for both V2 and V3 ignition configs and returns
 // a V2 or V3 Config or an error. This wrapper is necessary since V2 and V3 use different parsers.
 func IgnParseWrapper(rawIgn []byte) (interface{}, error) {

pkg/controller/common/helpers_test.go

@@ -793,3 +793,40 @@ func TestParseAndConvertGzippedConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateMachineConfigExtensions(t *testing.T) {
+	t.Parallel()
+	testCases := []struct {
+		name        string
+		extensions  []string
+		errExpected bool
+	}{
+		{
+			name:       "Supported",
+			extensions: []string{"kerberos", "sandboxed-containers"},
+		},
+		{
+			name:        "Unsupported",
+			extensions:  []string{"unsupported1", "unsupported2"},
+			errExpected: true,
+		},
+	}
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			mcfgSpec := mcfgv1.MachineConfigSpec{
+				Extensions: testCase.extensions,
+			}
+			err := ValidateMachineConfigExtensions(mcfgSpec)
+			if testCase.errExpected {
+				assert.Error(t, err)
+				for _, ext := range testCase.extensions {
+					assert.Contains(t, err.Error(), ext)
+				}
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

pkg/daemon/daemon.go

@@ -2862,7 +2862,7 @@ func (dn *Daemon) getUnsupportedPackages() {
 	}
 
 	supportedPackages := make(map[string]bool)
-	for _, packages := range getSupportedExtensions() {
+	for _, packages := range ctrlcommon.SupportedExtensions() {
 		for _, pkg := range packages {
 			supportedPackages[pkg] = true
 		}

pkg/daemon/update.go

@@ -1663,7 +1663,7 @@ func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineCon
 	extArgs := []string{"update"}
 
 	if dn.os.IsEL() {
-		extensions := getSupportedExtensions()
+		extensions := ctrlcommon.SupportedExtensions()
 		for _, ext := range added {
 			for _, pkg := range extensions[ext] {
 				extArgs = append(extArgs, "--install", pkg)
@@ -1693,38 +1693,6 @@ func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineCon
 	return extArgs
 }
 
-// Returns list of extensions possible to install on a CoreOS based system.
-func getSupportedExtensions() map[string][]string {
-	// In future when list of extensions grow, it will make
-	// more sense to populate it in a dynamic way.
-
-	// These are RHCOS supported extensions.
-	// Each extension keeps a list of packages required to get enabled on host.
-	return map[string][]string{
-		"wasm":                 {"crun-wasm"},
-		"ipsec":                {"NetworkManager-libreswan", "libreswan"},
-		"usbguard":             {"usbguard"},
-		"kerberos":             {"krb5-workstation", "libkadm5"},
-		"kernel-devel":         {"kernel-devel", "kernel-headers"},
-		"sandboxed-containers": {"kata-containers"},
-	}
-}
-
-func validateExtensions(exts []string) error {
-	supportedExtensions := getSupportedExtensions()
-	invalidExts := []string{}
-	for _, ext := range exts {
-		if _, ok := supportedExtensions[ext]; !ok {
-			invalidExts = append(invalidExts, ext)
-		}
-	}
-	if len(invalidExts) != 0 {
-		return fmt.Errorf("invalid extensions found: %v", invalidExts)
-	}
-	return nil
-
-}
-
 func (dn *CoreOSDaemon) applyExtensions(oldConfig, newConfig *mcfgv1.MachineConfig) error {
 	extensionsEmpty := len(oldConfig.Spec.Extensions) == 0 && len(newConfig.Spec.Extensions) == 0
 	if (extensionsEmpty) ||
@@ -1733,7 +1701,7 @@ func (dn *CoreOSDaemon) applyExtensions(oldConfig, newConfig *mcfgv1.MachineConf
 	}
 
 	// Validate extensions allowlist on RHCOS nodes
-	if err := validateExtensions(newConfig.Spec.Extensions); err != nil && dn.os.IsEL() {
+	if err := ctrlcommon.ValidateMachineConfigExtensions(newConfig.Spec); err != nil && dn.os.IsEL() {
 		return err
 	}