manifest/os: split out OSCustomizations

This is meant to encapsulate the tweaks we do to the OS tree
orthogonally to anything else. For now it still contains some
configuration that only sometimes applies, but this should
continue being reworked until all the fields in this struct
always apply to any artefact that is using it.

At the same time, stop instantiating with default values, as the
empty values should work. This is not a functional change as the
caller always sets these now.

cmd/osbuild-playground/my-container.go

@@ -44,6 +44,10 @@ func (img *MyContainer) InstantiateManifest(m *manifest.Manifest, repos []rpmmd.
 
 	// create a minimal non-bootable OS tree
 	os := manifest.NewOS(m, build, &platform.X86{}, repos)
+	os.ExtraBasePackages = []string{"@core"}
+	os.OSCustomizations.Language = "en_US.UTF-8"
+	os.OSCustomizations.Hostname = "my-host"
+	os.OSCustomizations.Timezone = "UTC"
 
 	// create an OCI container containing the OS tree created above
 	manifest.NewOCIContainer(m, build, os)

internal/distro/fedora/manifests.go

@@ -255,12 +255,10 @@ func osPipeline(m *manifest.Manifest,
 	c *blueprint.Customizations,
 	options distro.ImageOptions,
 	rng *rand.Rand) (*manifest.OS, error) {
-
-	imageConfig := t.getDefaultImageConfig()
-
 	pl := manifest.NewOS(m, buildPipeline, t.platform, repos)
 	pl.Environment = t.environment
 	pl.Workload = workload
+	pl.OSCustomizations = osCustomizations(t, repos, osPackageSet, c, options)
 
 	if t.bootable {
 		var err error
@@ -271,19 +269,6 @@ func osPipeline(m *manifest.Manifest,
 		pl.PartitionTable = pt
 	}
 
-	if t.bootable || t.rpmOstree {
-		pl.KernelName = c.GetKernel().Name
-
-		var kernelOptions []string
-		if t.kernelOptions != "" {
-			kernelOptions = append(kernelOptions, t.kernelOptions)
-		}
-		if bpKernel := c.GetKernel(); bpKernel.Append != "" {
-			kernelOptions = append(kernelOptions, bpKernel.Append)
-		}
-		pl.KernelOptionsAppend = kernelOptions
-	}
-
 	if t.rpmOstree {
 		var parent *manifest.OSTreeParent
 		if options.OSTree.Parent != "" && options.OSTree.URL != "" {
@@ -297,78 +282,107 @@ func osPipeline(m *manifest.Manifest,
 		}
 	}
 
-	pl.ExtraBasePackages = osPackageSet.Include
+	return pl, nil
-	pl.ExcludeBasePackages = osPackageSet.Exclude
+}
-	pl.ExtraBaseRepos = osPackageSet.Repositories
 
-	pl.GPGKeyFiles = imageConfig.GPGKeyFiles
+func osCustomizations(
-	pl.ExcludeDocs = imageConfig.ExcludeDocs
+	t *imageType,
+	repos []rpmmd.RepoConfig,
+	osPackageSet rpmmd.PackageSet,
+	c *blueprint.Customizations,
+	options distro.ImageOptions) manifest.OSCustomizations {
+
+	imageConfig := t.getDefaultImageConfig()
+
+	osc := manifest.OSCustomizations{}
+
+	if t.bootable || t.rpmOstree {
+		osc.KernelName = c.GetKernel().Name
+
+		var kernelOptions []string
+		if t.kernelOptions != "" {
+			kernelOptions = append(kernelOptions, t.kernelOptions)
+		}
+		if bpKernel := c.GetKernel(); bpKernel.Append != "" {
+			kernelOptions = append(kernelOptions, bpKernel.Append)
+		}
+		osc.KernelOptionsAppend = kernelOptions
+	}
+
+	osc.ExtraBasePackages = osPackageSet.Include
+	osc.ExcludeBasePackages = osPackageSet.Exclude
+	osc.ExtraBaseRepos = osPackageSet.Repositories
+
+	osc.GPGKeyFiles = imageConfig.GPGKeyFiles
+	osc.ExcludeDocs = imageConfig.ExcludeDocs
 
 	if !t.bootISO {
 		// don't put users and groups in the payload of an installer
 		// add them via kickstart instead
-		pl.Groups = c.GetGroups()
+		osc.Groups = c.GetGroups()
-		pl.Users = c.GetUsers()
+		osc.Users = c.GetUsers()
 	}
 
-	pl.EnabledServices = imageConfig.EnabledServices
+	osc.EnabledServices = imageConfig.EnabledServices
-	pl.DisabledServices = imageConfig.DisabledServices
+	osc.DisabledServices = imageConfig.DisabledServices
-	pl.DefaultTarget = imageConfig.DefaultTarget
+	osc.DefaultTarget = imageConfig.DefaultTarget
 
-	pl.Firewall = c.GetFirewall()
+	osc.Firewall = c.GetFirewall()
 
 	language, keyboard := c.GetPrimaryLocale()
 	if language != nil {
-		pl.Language = *language
+		osc.Language = *language
 	} else {
-		pl.Language = imageConfig.Locale
+		osc.Language = imageConfig.Locale
 	}
 	if keyboard != nil {
-		pl.Keyboard = keyboard
+		osc.Keyboard = keyboard
 	} else if imageConfig.Keyboard != nil {
-		pl.Keyboard = &imageConfig.Keyboard.Keymap
+		osc.Keyboard = &imageConfig.Keyboard.Keymap
 	}
 
 	if hostname := c.GetHostname(); hostname != nil {
-		pl.Hostname = *hostname
+		osc.Hostname = *hostname
+	} else {
+		osc.Hostname = "localhost.localdomain"
 	}
 
 	timezone, ntpServers := c.GetTimezoneSettings()
 	if timezone != nil {
-		pl.Timezone = *timezone
+		osc.Timezone = *timezone
 	} else {
-		pl.Timezone = imageConfig.Timezone
+		osc.Timezone = imageConfig.Timezone
 	}
 
 	if len(ntpServers) > 0 {
-		pl.NTPServers = ntpServers
+		osc.NTPServers = ntpServers
 	} else if imageConfig.TimeSynchronization != nil {
-		pl.NTPServers = imageConfig.TimeSynchronization.Timeservers
+		osc.NTPServers = imageConfig.TimeSynchronization.Timeservers
 	}
 
-	if imageConfig.NoSElinux {
+	if !imageConfig.NoSElinux {
-		pl.SElinux = ""
+		osc.SElinux = "targeted"
 	}
 
-	pl.Grub2Config = imageConfig.Grub2Config
+	osc.Grub2Config = imageConfig.Grub2Config
-	pl.Sysconfig = imageConfig.Sysconfig
+	osc.Sysconfig = imageConfig.Sysconfig
-	pl.SystemdLogind = imageConfig.SystemdLogind
+	osc.SystemdLogind = imageConfig.SystemdLogind
-	pl.CloudInit = imageConfig.CloudInit
+	osc.CloudInit = imageConfig.CloudInit
-	pl.Modprobe = imageConfig.Modprobe
+	osc.Modprobe = imageConfig.Modprobe
-	pl.DracutConf = imageConfig.DracutConf
+	osc.DracutConf = imageConfig.DracutConf
-	pl.SystemdUnit = imageConfig.SystemdUnit
+	osc.SystemdUnit = imageConfig.SystemdUnit
-	pl.Authselect = imageConfig.Authselect
+	osc.Authselect = imageConfig.Authselect
-	pl.SELinuxConfig = imageConfig.SELinuxConfig
+	osc.SELinuxConfig = imageConfig.SELinuxConfig
-	pl.Tuned = imageConfig.Tuned
+	osc.Tuned = imageConfig.Tuned
-	pl.Tmpfilesd = imageConfig.Tmpfilesd
+	osc.Tmpfilesd = imageConfig.Tmpfilesd
-	pl.PamLimitsConf = imageConfig.PamLimitsConf
+	osc.PamLimitsConf = imageConfig.PamLimitsConf
-	pl.Sysctld = imageConfig.Sysctld
+	osc.Sysctld = imageConfig.Sysctld
-	pl.DNFConfig = imageConfig.DNFConfig
+	osc.DNFConfig = imageConfig.DNFConfig
-	pl.SshdConfig = imageConfig.SshdConfig
+	osc.SshdConfig = imageConfig.SshdConfig
-	pl.AuthConfig = imageConfig.Authconfig
+	osc.AuthConfig = imageConfig.Authconfig
-	pl.PwQuality = imageConfig.PwQuality
+	osc.PwQuality = imageConfig.PwQuality
 
-	return pl, nil
+	return osc
 }
 
 func ostreeCommitPipeline(m *manifest.Manifest,

internal/manifest/os.go

@@ -24,10 +24,13 @@ type OSTreeParent struct {
 	URL      string
 }
 
-// OS represents the filesystem tree of the target image. This roughly
+// OSCustomizations encapsulates all configuration applied to the base
-// correpsonds to the root filesystem once an instance of the image is running.
+// operating independently of where and how it is integrated and what
-type OS struct {
+// workload it is running.
-	Base
+// TODO: move out kernel/bootloader/cloud-init/... to other
+//       abstractions, this should ideally only contain things that
+//       can always be applied.
+type OSCustomizations struct {
 	// Packages to install in addition to the ones required by the
 	// pipeline.
 	ExtraBasePackages []string
@@ -38,14 +41,6 @@ type OS struct {
 	ExcludeBasePackages []string
 	// Additional repos to install the base packages from.
 	ExtraBaseRepos []rpmmd.RepoConfig
-	// Environment the system will run in
-	Environment environment.Environment
-	// Workload to install on top of the base system
-	Workload workload.Workload
-	// OSTree configuration, if nil the tree cannot be in an OSTree commit
-	OSTree *OSTree
-	// Partition table, if nil the tree cannot be put on a partioned disk
-	PartitionTable *disk.PartitionTable
 	// KernelName indicates that a kernel is installed, and names the kernel
 	// package.
 	KernelName string
@@ -92,6 +87,22 @@ type OS struct {
 	SshdConfig    *osbuild.SshdConfigStageOptions
 	AuthConfig    *osbuild.AuthconfigStageOptions
 	PwQuality     *osbuild.PwqualityConfStageOptions
+}
+
+// OS represents the filesystem tree of the target image. This roughly
+// corresponds to the root filesystem once an instance of the image is running.
+type OS struct {
+	Base
+	// Customizations to apply to the base OS
+	OSCustomizations
+	// Environment the system will run in
+	Environment environment.Environment
+	// Workload to install on top of the base system
+	Workload workload.Workload
+	// OSTree configuration, if nil the tree cannot be in an OSTree commit
+	OSTree *OSTree
+	// Partition table, if nil the tree cannot be put on a partioned disk
+	PartitionTable *disk.PartitionTable
 
 	repos        []rpmmd.RepoConfig
 	packageSpecs []rpmmd.PackageSpec
@@ -113,10 +124,6 @@ func NewOS(m *Manifest,
 		Base:     NewBase(m, "os", buildPipeline),
 		repos:    repos,
 		platform: platform,
-		Language: "C.UTF-8",
-		Hostname: "localhost.localdomain",
-		Timezone: "UTC",
-		SElinux:  "targeted",
 	}
 	buildPipeline.addDependent(p)
 	m.addPipeline(p)