feat: unify cmdline handling GRUB/systemd-boot

Use cmdline from the UKI in Talos 1.12+ by default for new installs.

This brings GRUB in line with systemd-boot vs. cmdline behavior.

Fixes #12019

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>

Author: smira

cmd/installer/cmd/installer/install.go

@@ -32,6 +32,7 @@ func init() {
 	rootCmd.AddCommand(installCmd)
 }
 
+//nolint:gocyclo
 func runInstallCmd(ctx context.Context) (err error) {
 	log.Printf("running Talos installer %s", version.NewVersion().Tag)
 
@@ -50,6 +51,9 @@ func runInstallCmd(ctx context.Context) (err error) {
 	if err != nil {
 		if errors.Is(err, configloader.ErrNoConfig) {
 			log.Printf("machine configuration missing, skipping validation")
+
+			// machine configuration can be only missing while running an upgrade in maintenance mode, assume that we should follow GrubUseUKICmdline
+			options.GrubUseUKICmdline = true
 		} else {
 			return fmt.Errorf("error loading machine configuration: %w", err)
 		}
@@ -72,6 +76,10 @@ func runInstallCmd(ctx context.Context) (err error) {
 		if config.Machine() != nil && config.Machine().Install().LegacyBIOSSupport() {
 			options.LegacyBIOSSupport = true
 		}
+
+		if config.Machine() != nil && config.Machine().Install().GrubUseUKICmdline() {
+			options.GrubUseUKICmdline = true
+		}
 	}
 
 	return install.Install(ctx, p, mode, options)

cmd/installer/pkg/install/install.go

@@ -53,6 +53,7 @@ type Options struct {
 	Force               bool
 	Zero                bool
 	LegacyBIOSSupport   bool
+	GrubUseUKICmdline   bool
 	MetaValues          MetaValues
 	OverlayInstaller    overlay.Installer[overlay.ExtraOptions]
 	OverlayName         string
@@ -118,6 +119,7 @@ func Install(ctx context.Context, p runtime.Platform, mode Mode, opts *Options)
 		opts.ExtraOptions = extraOptions
 	}
 
+	// NOTE: this is legacy code which is only used when running in GRUB mode with GrubUseUKICmdline set to false.
 	cmdline := procfs.NewCmdline("")
 	cmdline.Append(constants.KernelParamPlatform, p.Name())
 
@@ -347,15 +349,16 @@ func (i *Installer) Install(ctx context.Context, mode Mode) (err error) {
 
 	// Install the bootloader.
 	bootInstallResult, err := bootlder.Install(bootloaderoptions.InstallOptions{
-		BootDisk:    i.options.Disk,
-		Arch:        i.options.Arch,
-		Cmdline:     i.cmdline.String(),
-		Version:     i.options.Version,
-		ImageMode:   mode.IsImage(),
-		MountPrefix: i.options.MountPrefix,
-		BootAssets:  i.options.BootAssets,
-		Printf:      i.options.Printf,
-		BlkidInfo:   info,
+		BootDisk:          i.options.Disk,
+		Arch:              i.options.Arch,
+		Cmdline:           i.cmdline.String(),
+		GrubUseUKICmdline: i.options.GrubUseUKICmdline,
+		Version:           i.options.Version,
+		ImageMode:         mode.IsImage(),
+		MountPrefix:       i.options.MountPrefix,
+		BootAssets:        i.options.BootAssets,
+		Printf:            i.options.Printf,
+		BlkidInfo:         info,
 
 		ExtraInstallStep: func() error {
 			if i.options.Board != constants.BoardNone {

cmd/talosctl/cmd/mgmt/cluster/create/clusterops/configmaker/internal/makers/common.go

@@ -211,7 +211,7 @@ func (m *Maker[T]) initVersionContract() error {
 // logic has been run.
 func (m *Maker[T]) GetClusterConfigs() (clusterops.ClusterConfigs, error) {
 	// These options needs to be generated after the implementing maker has made changes to the cluster request.
-	m.GenOps = slices.Concat(m.GenOps, m.Provisioner.GenOptions(m.ClusterRequest.Network))
+	m.GenOps = slices.Concat(m.GenOps, m.Provisioner.GenOptions(m.ClusterRequest.Network, m.VersionContract))
 	m.GenOps = slices.Concat(m.GenOps, []generate.Option{generate.WithEndpointList(m.Endpoints)})
 
 	m.ConfigBundleOps = append(m.ConfigBundleOps,

cmd/talosctl/cmd/mgmt/cluster/create/clusterops/configmaker/internal/makers/common_test.go

@@ -24,7 +24,7 @@ type testProvisioner struct {
 	provision.Provisioner
 }
 
-func (p testProvisioner) GenOptions(r provision.NetworkRequest) []generate.Option {
+func (p testProvisioner) GenOptions(r provision.NetworkRequest, _ *config.VersionContract) []generate.Option {
 	return []generate.Option{func(o *generate.Options) error { return nil }}
 }
 

hack/release.toml

@@ -125,6 +125,16 @@ Additionally `talosctl image cache-create` has some changes:
 Talos now supports optionally disabling kernel module signature verification by setting `module.sig_enforce=0` kernel parameter.
 By default module signature verification is enabled (`module.sig_enforce=1`).
 When using Factory or Imager supply as `-module.sig_enfore module.sig_enforce=0` kernel parameters to disable module signature enforcement.
+"""
+
+    [notes.grub]
+        title = "GRUB"
+        description = """\
+Talos Linux introduces new machine configuration option `.machine.install.grubUseUKICmdline` to control whether GRUB should use the kernel command line 
+provided by the boot assets (UKI) or to use the command line constructed by Talos itself (legacy behavior).
+
+This option defaults to `true` for new installations, which means that GRUB will use the command line from the UKI, making it easier to customize kernel parameters via boot asset generation.
+For existing installations upgrading to v1.12, this option will default to `false` to preserve the legacy behavior.
 """
 
 [make_deps]

hack/test/e2e-qemu.sh

@@ -207,17 +207,10 @@ case "${WITH_APPARMOR_LSM_ENABLED:-false}" in
   false)
     ;;
   *)
-    cat <<EOF > "${TMP}/kernel-security.patch"
-machine:
-  install:
-    extraKernelArgs:
-      - -selinux
-      - lsm=lockdown,capability,yama,apparmor,bpf
-      - apparmor=1
-EOF
-
-    QEMU_FLAGS+=("--config-patch=@${TMP}/kernel-security.patch")
-    QEMU_FLAGS+=("--extra-boot-kernel-args=-selinux")
+    # build disk image with specific kernel args to enable AppArmor LSM
+    make image-metal PLATFORM=linux/amd64 IMAGER_ARGS="--extra-kernel-arg -selinux --extra-kernel-arg lsm=lockdown,capability,yama,apparmor,bpf --extra-kernel-arg apparmor=1"
+
+    QEMU_FLAGS+=("--disk-image-path=_out/metal-amd64.raw.zst" "--skip-injecting-config" "--with-apply-config")
     ;;
 esac
 

internal/app/machined/pkg/runtime/v1alpha1/bootloader/dual/dual.go

@@ -7,6 +7,7 @@ package dual
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -135,7 +136,18 @@ func (c *Config) installGrub(opts options.InstallOptions) error {
 		return err
 	}
 
-	if err := grubConfig.Put(grubConfig.Default, opts.Cmdline, opts.Version); err != nil {
+	cmdline := opts.Cmdline
+
+	if opts.GrubUseUKICmdline {
+		cmdlineBytes, err := io.ReadAll(assetInfo.Cmdline)
+		if err != nil {
+			return fmt.Errorf("failed to read cmdline from UKI: %w", err)
+		}
+
+		cmdline = string(cmdlineBytes)
+	}
+
+	if err := grubConfig.Put(grubConfig.Default, cmdline, opts.Version); err != nil {
 		return err
 	}
 

internal/app/machined/pkg/runtime/v1alpha1/bootloader/grub/install.go

@@ -6,6 +6,7 @@ package grub
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -17,6 +18,7 @@ import (
 	"github.com/siderolabs/talos/internal/app/machined/pkg/runtime/v1alpha1/bootloader/mount"
 	"github.com/siderolabs/talos/internal/app/machined/pkg/runtime/v1alpha1/bootloader/options"
 	"github.com/siderolabs/talos/internal/pkg/partition"
+	"github.com/siderolabs/talos/internal/pkg/smbios"
 	"github.com/siderolabs/talos/internal/pkg/uki"
 	"github.com/siderolabs/talos/pkg/imager/utils"
 	"github.com/siderolabs/talos/pkg/machinery/constants"
@@ -94,6 +96,8 @@ func (c *Config) install(opts options.InstallOptions) (*options.InstallResult, e
 		return nil, err
 	}
 
+	cmdline := opts.Cmdline
+
 	// if we have a kernel path, assume that the kernel and initramfs are available
 	if _, err := os.Stat(opts.BootAssets.KernelPath); err == nil {
 		if err := utils.CopyFiles(
@@ -109,6 +113,10 @@ func (c *Config) install(opts options.InstallOptions) (*options.InstallResult, e
 		); err != nil {
 			return nil, err
 		}
+
+		if opts.GrubUseUKICmdline {
+			return nil, fmt.Errorf("cannot use UKI cmdline when boot assets are not UKI")
+		}
 	} else {
 		// if the kernel path does not exist, assume that the kernel and initramfs are in the UKI
 		assetInfo, err := uki.Extract(opts.BootAssets.UKIPath)
@@ -135,9 +143,24 @@ func (c *Config) install(opts options.InstallOptions) (*options.InstallResult, e
 		); err != nil {
 			return nil, err
 		}
+
+		if opts.GrubUseUKICmdline {
+			cmdlineBytes, err := io.ReadAll(assetInfo.Cmdline)
+			if err != nil {
+				return nil, fmt.Errorf("failed to read cmdline from UKI: %w", err)
+			}
+
+			cmdline = string(cmdlineBytes)
+
+			if extraCmdline, err := smbios.ReadOEMVariable(constants.SDStubCmdlineExtraOEMVar); err == nil {
+				for _, extra := range extraCmdline {
+					cmdline += " " + extra
+				}
+			}
+		}
 	}
 
-	if err := c.Put(c.Default, opts.Cmdline, opts.Version); err != nil {
+	if err := c.Put(c.Default, cmdline, opts.Version); err != nil {
 		return nil, err
 	}
 

internal/app/machined/pkg/runtime/v1alpha1/bootloader/options/options.go

@@ -19,8 +19,10 @@ type InstallOptions struct {
 	BootDisk string
 	// Target architecture.
 	Arch string
-	// Kernel command line (grub only).
+	// Kernel command line (grub only, and only if GrubUseUKICmdline is false).
 	Cmdline string
+	// Whether to use the UKI cmdline instead of building it on the host (grub only).
+	GrubUseUKICmdline bool
 	// Talos version.
 	Version string
 

internal/app/machined/pkg/runtime/v1alpha1/bootloader/sdboot/sdboot.go

@@ -30,7 +30,7 @@ import (
 	"github.com/siderolabs/talos/internal/pkg/efivarfs"
 	mountv3 "github.com/siderolabs/talos/internal/pkg/mount/v3"
 	"github.com/siderolabs/talos/internal/pkg/partition"
-	smbiosinternal "github.com/siderolabs/talos/internal/pkg/smbios"
+	"github.com/siderolabs/talos/internal/pkg/smbios"
 	"github.com/siderolabs/talos/internal/pkg/uki"
 	"github.com/siderolabs/talos/pkg/imager/utils"
 	"github.com/siderolabs/talos/pkg/machinery/constants"
@@ -252,20 +252,9 @@ func (c *Config) KexecLoad(r runtime.Runtime, disk string) error {
 		}
 
 		if !efi.GetSecureBoot() {
-			smbiosInfo, err := smbiosinternal.GetSMBIOSInfo()
-			if err == nil {
-				for _, structure := range smbiosInfo.Structures {
-					if structure.Header.Type != 11 {
-						continue
-					}
-
-					const kernelCmdlineExtra = "io.systemd.stub.kernel-cmdline-extra="
-
-					for _, s := range structure.Strings {
-						if strings.HasPrefix(s, kernelCmdlineExtra) {
-							cmdline.WriteString(" " + s[len(kernelCmdlineExtra):])
-						}
-					}
+			if extraCmdline, err := smbios.ReadOEMVariable(constants.SDStubCmdlineExtraOEMVar); err == nil {
+				for _, s := range extraCmdline {
+					cmdline.WriteString(" " + s)
 				}
 			}
 		}