diff --git a/pkg/distro/distro.go b/pkg/distro/distro.go index e5bedeec9c..c9669591be 100644 --- a/pkg/distro/distro.go +++ b/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 diff --git a/pkg/distro/generic/distro.go b/pkg/distro/generic/distro.go index 93f4c7d100..d5cba182de 100644 --- a/pkg/distro/generic/distro.go +++ b/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() { diff --git a/pkg/distro/generic/imagedefs.go b/pkg/distro/generic/imagedefs.go deleted file mode 100644 index 3a436ed584..0000000000 --- a/pkg/distro/generic/imagedefs.go +++ /dev/null @@ -1,47 +0,0 @@ -package generic - -import ( - "fmt" - - "github.com/osbuild/images/internal/common" - "github.com/osbuild/images/pkg/distro/defs" -) - -func newImageTypeFrom(d *distribution, ar *architecture, imgYAML defs.ImageTypeYAML) imageType { - it := imageType{ - ImageTypeYAML: imgYAML, - isoLabel: d.getISOLabelFunc(imgYAML.ISOLabel), - } - it.defaultImageConfig = common.Must(it.ImageConfig(d.Name(), ar.name)) - it.defaultInstallerConfig = common.Must(it.InstallerConfig(d.Name(), ar.name)) - - 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 -} diff --git a/pkg/distro/generic/imagetype.go b/pkg/distro/generic/imagetype.go index a3b1a2ca22..efa1d52bf0 100644 --- a/pkg/distro/generic/imagetype.go +++ b/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,12 +187,8 @@ 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()) } @@ -173,12 +196,11 @@ 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 } diff --git a/pkg/distro/image_config.go b/pkg/distro/image_config.go index 58b1c24ee8..6a646fc30a 100644 --- a/pkg/distro/image_config.go +++ b/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) } diff --git a/pkg/distro/image_config_test.go b/pkg/distro/image_config_test.go index 432b503165..129a675ba3 100644 --- a/pkg/distro/image_config_test.go +++ b/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 {