distro: cleanups in the pkg/distro/generic area

pkg/distro/distro.go

@@ -102,10 +102,6 @@ type ImageType interface {
 	// has no partition table. Only support for RHEL 8.5+
 	PartitionType() disk.PartitionTableType
 
-	// Return the base partition tabe for the given image type, will
-	// return `nil` if there is none
-	BasePartitionTable() (*disk.PartitionTable, error)
-
 	// Returns the corresponding boot mode ("legacy", "uefi", "hybrid") or "none"
 	BootMode() platform.BootMode
 

pkg/distro/generic/distro.go

@@ -40,12 +40,6 @@ type distribution struct {
 	defs.DistroYAML
 
 	arches map[string]*architecture
-	// XXX: move into defs.DistroYAML? the downside of doing this
-	// is that we would have to duplicate the default image config
-	// accross the centos/alma/rhel distros.yaml, otherwise we
-	// just load it from the imagetypes file/dir and it is natually
-	// "in-sync"
-	defaultImageConfig *distro.ImageConfig
 }
 
 func (d *distribution) getISOLabelFunc(isoLabel string) isoLabelFunc {
@@ -85,9 +79,7 @@ func newDistro(nameVer string) (distro.Distro, error) {
 
 	rd := &distribution{
 		DistroYAML: *distroYAML,
-
-		defaultImageConfig: distroYAML.ImageConfig(),
-		arches:             make(map[string]*architecture),
+		arches:     make(map[string]*architecture),
 	}
 
 	for _, imgTypeYAML := range distroYAML.ImageTypes() {

pkg/distro/generic/imagetype.go

@@ -35,15 +35,47 @@ type imageType struct {
 	arch     *architecture
 	platform platform.Platform
 
-	// XXX: make definable via YAML
-	workload               workload.Workload
-	defaultImageConfig     *distro.ImageConfig
-	defaultInstallerConfig *distro.InstallerConfig
-
 	image    imageFunc
 	isoLabel isoLabelFunc
 }
 
+func newImageTypeFrom(d *distribution, ar *architecture, imgYAML defs.ImageTypeYAML) imageType {
+	it := imageType{
+		ImageTypeYAML: imgYAML,
+		isoLabel:      d.getISOLabelFunc(imgYAML.ISOLabel),
+	}
+
+	switch imgYAML.Image {
+	case "disk":
+		it.image = diskImage
+	case "container":
+		it.image = containerImage
+	case "image_installer":
+		it.image = imageInstallerImage
+	case "live_installer":
+		it.image = liveInstallerImage
+	case "bootable_container":
+		it.image = bootableContainerImage
+	case "iot":
+		it.image = iotImage
+	case "iot_commit":
+		it.image = iotCommitImage
+	case "iot_container":
+		it.image = iotContainerImage
+	case "iot_installer":
+		it.image = iotInstallerImage
+	case "iot_simplified_installer":
+		it.image = iotSimplifiedInstallerImage
+	case "tar":
+		it.image = tarImage
+	default:
+		err := fmt.Errorf("unknown image func: %v for %v", imgYAML.Image, imgYAML.Name())
+		panic(err)
+	}
+
+	return it
+}
+
 func (t *imageType) Name() string {
 	return t.ImageTypeYAML.Name()
 }
@@ -72,12 +104,11 @@ func (t *imageType) ISOLabel() (string, error) {
 	if !t.ImageTypeYAML.BootISO {
 		return "", fmt.Errorf("image type %q is not an ISO", t.Name())
 	}
-
-	if t.isoLabel != nil {
-		return t.isoLabel(t), nil
+	if t.isoLabel == nil {
+		return "", fmt.Errorf("no iso label function for %q", t.Name())
 	}
 
-	return "", nil
+	return t.isoLabel(t), nil
 }
 
 func (t *imageType) Size(size uint64) uint64 {
@@ -121,12 +152,8 @@ func (t *imageType) BootMode() platform.BootMode {
 	return platform.BOOT_NONE
 }
 
-func (t *imageType) BasePartitionTable() (*disk.PartitionTable, error) {
-	return t.ImageTypeYAML.PartitionTable(t.arch.distro.Name(), t.arch.name)
-}
-
 func (t *imageType) getPartitionTable(customizations *blueprint.Customizations, options distro.ImageOptions, rng *rand.Rand) (*disk.PartitionTable, error) {
-	basePartitionTable, err := t.BasePartitionTable()
+	basePartitionTable, err := t.ImageTypeYAML.PartitionTable(t.arch.distro.Name(), t.arch.name)
 	if err != nil {
 		return nil, err
 	}
@@ -160,25 +187,20 @@ func (t *imageType) getPartitionTable(customizations *blueprint.Customizations,
 }
 
 func (t *imageType) getDefaultImageConfig() *distro.ImageConfig {
-	// ensure that image always returns non-nil default config
-	imageConfig := t.defaultImageConfig
-	if imageConfig == nil {
-		imageConfig = &distro.ImageConfig{}
-	}
-	return imageConfig.InheritFrom(t.arch.distro.defaultImageConfig)
+	imageConfig := common.Must(t.ImageConfig(t.arch.distro.Name(), t.arch.name))
+	return imageConfig.InheritFrom(t.arch.distro.ImageConfig())
 
 }
 
 func (t *imageType) getDefaultInstallerConfig() (*distro.InstallerConfig, error) {
 	if !t.ImageTypeYAML.BootISO {
 		return nil, fmt.Errorf("image type %q is not an ISO", t.Name())
 	}
-
-	return t.defaultInstallerConfig, nil
+	return t.InstallerConfig(t.arch.distro.Name(), t.arch.name)
 }
 
 func (t *imageType) PartitionType() disk.PartitionTableType {
-	basePartitionTable, err := t.BasePartitionTable()
+	basePartitionTable, err := t.ImageTypeYAML.PartitionTable(t.arch.distro.Name(), t.arch.name)
 	if errors.Is(err, defs.ErrNoPartitionTableForImgType) {
 		return disk.PT_NONE
 	}
@@ -231,41 +253,31 @@ func (t *imageType) Manifest(bp *blueprint.Blueprint,
 		}
 	}
 
-	w := t.workload
-	if w == nil {
-		// XXX: this needs to get duplicaed in exactly the same
-		// way in rhel/imagetype.go
-		workloadRepos := payloadRepos
-		customRepos, err := bp.Customizations.GetRepositories()
-		if err != nil {
-			return nil, nil, err
-		}
-		installFromRepos := blueprint.RepoCustomizationsInstallFromOnly(customRepos)
-		workloadRepos = append(workloadRepos, installFromRepos...)
-
-		cw := &workload.Custom{
-			BaseWorkload: workload.BaseWorkload{
-				Repos: workloadRepos,
-			},
-			Packages:       bp.GetPackagesEx(false),
-			EnabledModules: bp.GetEnabledModules(),
-		}
-		if services := bp.Customizations.GetServices(); services != nil {
-			cw.Services = services.Enabled
-			cw.DisabledServices = services.Disabled
-			cw.MaskedServices = services.Masked
-		}
-		w = cw
+	customRepos, err := bp.Customizations.GetRepositories()
+	if err != nil {
+		return nil, nil, err
+	}
+	installFromRepos := blueprint.RepoCustomizationsInstallFromOnly(customRepos)
+	payloadRepos = append(payloadRepos, installFromRepos...)
+
+	cw := &workload.Custom{
+		BaseWorkload: workload.BaseWorkload{
+			Repos: payloadRepos,
+		},
+		Packages:       bp.GetPackagesEx(false),
+		EnabledModules: bp.GetEnabledModules(),
+	}
+	if services := bp.Customizations.GetServices(); services != nil {
+		cw.Services = services.Enabled
+		cw.DisabledServices = services.Disabled
+		cw.MaskedServices = services.Masked
 	}
 
 	if experimentalflags.Bool("no-fstab") {
-		if t.defaultImageConfig == nil {
-			t.defaultImageConfig = &distro.ImageConfig{
-				MountUnits: common.ToPtr(true),
-			}
-		} else {
-			t.defaultImageConfig.MountUnits = common.ToPtr(true)
+		if t.ImageConfigYAML.ImageConfig != nil {
+			t.ImageConfigYAML.ImageConfig = &distro.ImageConfig{}
 		}
+		t.ImageConfigYAML.ImageConfig.MountUnits = common.ToPtr(true)
 	}
 
 	containerSources := make([]container.SourceSpec, len(bp.Containers))
@@ -283,7 +295,7 @@ func (t *imageType) Manifest(bp *blueprint.Blueprint,
 	/* #nosec G404 */
 	rng := rand.New(source)
 
-	img, err := t.image(w, t, bp, options, staticPackageSets, containerSources, rng)
+	img, err := t.image(cw, t, bp, options, staticPackageSets, containerSources, rng)
 	if err != nil {
 		return nil, nil, err
 	}

pkg/distro/image_config.go

@@ -186,6 +186,9 @@ type DNFConfig struct {
 // InheritFrom inherits unset values from the provided parent configuration and
 // returns a new structure instance, which is a result of the inheritance.
 func (c *ImageConfig) InheritFrom(parentConfig *ImageConfig) *ImageConfig {
+	if c == nil {
+		c = &ImageConfig{}
+	}
 	return shallowMerge(c, parentConfig)
 }
 

pkg/distro/image_config_test.go

@@ -158,6 +158,20 @@ func TestImageConfigInheritFrom(t *testing.T) {
 				DisabledServices: []string{"named"},
 				DefaultTarget:    common.ToPtr("multi-user.target"),
 			},
+		}, {
+			name: "inheritance with nil imageConfig",
+			distroConfig: &ImageConfig{
+				Timezone: common.ToPtr("America/New_York"),
+			},
+			imageConfig: nil,
+			expectedConfig: &ImageConfig{
+				Timezone: common.ToPtr("America/New_York"),
+			},
+		}, {
+			name:           "inheritance with nil imageConfig and distroConfig",
+			distroConfig:   nil,
+			imageConfig:    nil,
+			expectedConfig: &ImageConfig{},
 		},
 	}
 	for idx, tt := range tests {