Fix shadow-utils detection in imager as well as validator (#11914)

Author: dmcilvaney

toolkit/tools/imageconfigvalidator/imageconfigvalidator.go

@@ -15,7 +15,6 @@ import (
 	"github.com/microsoft/azurelinux/toolkit/tools/imagegen/installutils"
 	"github.com/microsoft/azurelinux/toolkit/tools/internal/exe"
 	"github.com/microsoft/azurelinux/toolkit/tools/internal/logger"
-	"github.com/microsoft/azurelinux/toolkit/tools/internal/pkgjson"
 	"github.com/microsoft/azurelinux/toolkit/tools/internal/timestamp"
 	"github.com/microsoft/azurelinux/toolkit/tools/pkg/profile"
 
@@ -135,28 +134,30 @@ func validatePackages(config configuration.Config) (err error) {
 			selinuxPkgName = configuration.SELinuxPolicyDefault
 		}
 
-		for _, pkg := range packageList {
-			// The installer tools have an undocumented feature which can support both "pkg-name" and "pkg-name=version" formats.
-			// This is in use, so we need to handle pinned versions in this check. Technically, 'tdnf' also supports "pkg-name-version" format,
-			// but it is not easily distinguishable from "long-package-name" format so it will not be supported here.
-			pkgVer, err := pkgjson.PackageStringToPackageVer(pkg)
-			if err != nil {
-				return fmt.Errorf("%s: %w", validateError, err)
-			}
+		foundKernelPackage, err := installutils.PackagelistContainsPackage(packageList, kernelPkgName)
+		if err != nil {
+			return fmt.Errorf("%s: %w", validateError, err)
+		}
 
-			if pkgVer.Name == kernelPkgName {
-				return fmt.Errorf("%s: kernel should not be included in a package list, add via config file's [KernelOptions] entry", validateError)
-			}
-			if pkgVer.Name == dracutFipsPkgName {
-				foundDracutFipsPackage = true
-			}
-			if pkgVer.Name == selinuxPkgName {
-				foundSELinuxPackage = true
-			}
-			if pkgVer.Name == userAddPkgName {
-				foundUserAddPackage = true
-			}
+		foundDracutFipsPackage, err = installutils.PackagelistContainsPackage(packageList, dracutFipsPkgName)
+		if err != nil {
+			return fmt.Errorf("%s: %w", validateError, err)
 		}
+
+		foundSELinuxPackage, err = installutils.PackagelistContainsPackage(packageList, selinuxPkgName)
+		if err != nil {
+			return fmt.Errorf("%s: %w", validateError, err)
+		}
+
+		foundUserAddPackage, err = installutils.PackagelistContainsPackage(packageList, userAddPkgName)
+		if err != nil {
+			return fmt.Errorf("%s: %w", validateError, err)
+		}
+
+		if foundKernelPackage {
+			return fmt.Errorf("%s: kernel should not be included in a package list, add via config file's [KernelOptions] entry", validateError)
+		}
+
 		if strings.Contains(kernelCmdLineString, fipsKernelCmdLine) || systemConfig.KernelCommandLine.EnableFIPS {
 			if !foundDracutFipsPackage {
 				return fmt.Errorf("%s: 'fips=1' provided on kernel cmdline, but '%s' package is not included in the package lists", validateError, dracutFipsPkgName)

toolkit/tools/imagegen/installutils/installutils.go

@@ -453,6 +453,29 @@ func orderPackageInstallList(packageList []string) []string {
 	return orderedPackageList
 }
 
+// PackagelistContainsPackage checks if the given package is in the list of packages to install. It will do
+// a fuzzy search for the package name and try to ignore version info.
+//
+// The installer tools have an undocumented feature which can support both "pkg-name" and "pkg-name=version" formats.
+// This is in use, so we need to handle pinned versions in this check. Technically, 'tdnf' also supports "pkg-name-version" format,
+// but it is not easily distinguishable from "long-package-name" format so it will not be supported here.
+func PackagelistContainsPackage(packageList []string, packageName string) (found bool, err error) {
+	if packageName == "" {
+		return false, fmt.Errorf("can't search for an empty package name")
+	}
+
+	for _, pkg := range packageList {
+		pkgVer, err := pkgjson.PackageStringToPackageVer(pkg)
+		if err != nil {
+			return false, err
+		}
+		if pkgVer.Name == packageName {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 // PopulateInstallRoot fills the installroot with packages and configures the image for boot
 // - installChroot is a pointer to the install Chroot object
 // - packagesToInstall is a slice of packages to install
@@ -532,7 +555,13 @@ func PopulateInstallRoot(installChroot *safechroot.Chroot, packagesToInstall []s
 
 	// imageconfigvalidator should have ensured that we intend to install shadow-utils, so we can go ahead and do that here.
 	if len(config.Users) > 0 || len(config.Groups) > 0 {
-		if !sliceutils.ContainsValue(packagesToInstall, "shadow-utils") {
+		var hasShadowUtils bool
+		hasShadowUtils, err = PackagelistContainsPackage(packagesToInstall, "shadow-utils")
+		if err != nil {
+			err = fmt.Errorf("failed to check for shadow-utils package in package list:\n%w", err)
+			return
+		}
+		if !hasShadowUtils {
 			err = fmt.Errorf("shadow-utils package must be added to the image's package lists when setting users or groups")
 			return
 		}

toolkit/tools/imagegen/installutils/installutils_test.go

@@ -197,3 +197,50 @@ func TestAddImageIDFileGuardClause(t *testing.T) {
 	err = AddImageIDFile(chroot.RootDir(), "")
 	assert.Error(t, err)
 }
+
+func TestPackagelistContainsPackage(t *testing.T) {
+	packageList := []string{
+		"package1",
+		"package2",
+		"package3",
+	}
+
+	found, err := PackagelistContainsPackage(packageList, "package2")
+	assert.NoError(t, err)
+	assert.True(t, found)
+
+	found, err = PackagelistContainsPackage(packageList, "package4")
+	assert.NoError(t, err)
+	assert.False(t, found)
+
+	found, err = PackagelistContainsPackage(nil, "package4")
+	assert.NoError(t, err)
+	assert.False(t, found)
+
+	found, err = PackagelistContainsPackage(packageList, "")
+	assert.Error(t, err)
+	assert.False(t, found)
+}
+
+func TestPackagelistContainsPackageBadVersion(t *testing.T) {
+	packageList := []string{
+		"bad package = bad < version",
+	}
+
+	found, err := PackagelistContainsPackage(packageList, "package2")
+	assert.Error(t, err)
+	assert.False(t, found)
+}
+
+func TestPackagelistContainsPackageVersions(t *testing.T) {
+	packageList := []string{
+		"package1",
+		"package2",
+		"package3",
+		"package4 = 1.2.3",
+	}
+
+	found, err := PackagelistContainsPackage(packageList, "package4")
+	assert.NoError(t, err)
+	assert.True(t, found)
+}