From 2c171cdcac4f3384ce65ac867f1a1bb756c503d8 Mon Sep 17 00:00:00 2001
From: Vadim Rutkovsky <vrutkovs@redhat.com>
Date: Sat, 24 Oct 2020 11:26:53 +0200
Subject: [PATCH] pkg/daemon: avoid duplicate extension verification

When package aliases (or groups) were introduced recently,
this list is now being verified twice - during `validateExtensions` on
MCC and in `generateExtensionsArgs` on MCD. This breaks OKD installs,
as we'd like to avoid verifying a pre-approved list of RPMs on FCOS.

However, MCC can't know which systems it would manage, so it applies
RHCOS allowlist on all systems unknowingly.

This commit refactors extensions verification and application:

* available packages are listed in `constants.SupportedPackages`
* groups/aliases MCD knows about at listed in `constants.SupportedPackageAliases`
* `validateExtensions` ensures that `spec.extensions` items are in either list
* `generateExtensionsArgs` expands aliases into a list of RPMs

On OKD `validateExtensions` is not called, however `generateExtensionsArgs`
would still apply. This might bite us later, but at least it won't block
install, so its safe to merge now
---
 pkg/daemon/constants/constants.go |  9 +++++++
 pkg/daemon/update.go              | 42 ++++++++++++++++++-------------
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/pkg/daemon/constants/constants.go b/pkg/daemon/constants/constants.go
index 746051be92..cb4b8d3da6 100644
--- a/pkg/daemon/constants/constants.go
+++ b/pkg/daemon/constants/constants.go
@@ -56,3 +56,12 @@ const (
 	// to proceed and attempt to "reconcile" to the new "desiredConfig" state regardless.
 	MachineConfigDaemonForceFile = "/run/machine-config-daemon-force"
 )
+
+var (
+	// SupportedPackages lists RPMs available in machine-os-content for RHCOS
+	SupportedPackages = []string{"kernel-headers", "usbguard"}
+	// SupportedPackageAliases expands allowed aliases to a list of RPMs to install
+	SupportedPackageAliases = map[string][]string{
+		"kernel-devel": {"kernel-headers", "kernel-devel"},
+	}
+)
diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go
index cf1f89478d..a4eb1b681f 100644
--- a/pkg/daemon/update.go
+++ b/pkg/daemon/update.go
@@ -865,18 +865,15 @@ func generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineConfig) []string
 		}
 	}
 
-	// Supported extensions has package list info that is required
-	// to enable an extension
-	extensions := getSupportedExtensions()
-
+	// Extension may be an RPM name or an alias for a list of RPMs
 	extArgs := []string{"update"}
 	for _, ext := range added {
-		for _, pkg := range extensions[ext] {
+		for _, pkg := range extensionToRPMList(ext) {
 			extArgs = append(extArgs, "--install", pkg)
 		}
 	}
 	for _, ext := range removed {
-		for _, pkg := range extensions[ext] {
+		for _, pkg := range extensionToRPMList(ext) {
 			extArgs = append(extArgs, "--uninstall", pkg)
 		}
 	}
@@ -884,26 +881,35 @@ func generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineConfig) []string
 	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.
+// getAliasList returns a list of keys from SupportedPackageAliases
+func getAliasList() []string {
+	keys := make([]string, 0, len(constants.SupportedPackageAliases))
+	for k := range constants.SupportedPackageAliases {
+		keys = append(keys, k)
+	}
+	return keys
+}
 
-	// These are RHCOS supported extensions.
-	// Each extension keeps a list of packages required to get enabled on host.
-	return map[string][]string{
-		"usbguard":     {"usbguard"},
-		"kernel-devel": {"kernel-devel", "kernel-headers"},
+// return a list of RPMs to install for particular extension
+func extensionToRPMList(ext string) []string {
+	if ctrlcommon.InSlice(ext, getAliasList()) {
+		return constants.SupportedPackageAliases[ext]
 	}
+	return []string{ext}
 }
 
 func validateExtensions(exts []string) error {
-	supportedExtensions := getSupportedExtensions()
+	// Extension can either be an RPM or an alias
+	aliases := getAliasList()
 	invalidExts := []string{}
 	for _, ext := range exts {
-		if _, ok := supportedExtensions[ext]; !ok {
-			invalidExts = append(invalidExts, ext)
+		if ctrlcommon.InSlice(ext, constants.SupportedPackages) {
+			continue
+		}
+		if ctrlcommon.InSlice(ext, aliases) {
+			continue
 		}
+		invalidExts = append(invalidExts, ext)
 	}
 	if len(invalidExts) != 0 {
 		return fmt.Errorf("invalid extensions found: %v", invalidExts)