From 0063a6da0782853ea02410ce400463d757f2a6f6 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 2 Jul 2024 15:21:41 +0200 Subject: [PATCH 01/14] manifest: make distro field private Make the distro field of the manifest.Manifest struct private and make sure it's set using the manifest.New() constructor. The intention is to make manifest.Manifest an interface and getting rid of private fields is the first step towards this. --- cmd/osbuild-playground/playground.go | 2 +- pkg/distro/fedora/imagetype.go | 3 +-- pkg/distro/rhel/imagetype.go | 10 +++++----- pkg/image/installer_image_test.go | 2 +- pkg/image/ostree_disk_test.go | 2 +- pkg/manifest/build_test.go | 4 ++-- pkg/manifest/manifest.go | 10 +++++----- pkg/manifest/os_test.go | 2 +- pkg/manifest/raw_bootc_test.go | 8 ++++---- 9 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cmd/osbuild-playground/playground.go b/cmd/osbuild-playground/playground.go index ba6dab04ed..01d7afb943 100644 --- a/cmd/osbuild-playground/playground.go +++ b/cmd/osbuild-playground/playground.go @@ -23,7 +23,7 @@ func RunPlayground(img image.ImageKind, d distro.Distro, arch distro.Arch, repos // Set cache size to 1 GiB solver.SetMaxCacheSize(1 * common.GiB) - manifest := manifest.New() + manifest := manifest.New(manifest.DISTRO_FEDORA) /* #nosec G404 */ rnd := rand.New(rand.NewSource(0)) diff --git a/pkg/distro/fedora/imagetype.go b/pkg/distro/fedora/imagetype.go index 1300b5747f..32fdba6e1d 100644 --- a/pkg/distro/fedora/imagetype.go +++ b/pkg/distro/fedora/imagetype.go @@ -256,8 +256,7 @@ func (t *imageType) Manifest(bp *blueprint.Blueprint, if err != nil { return nil, nil, err } - mf := manifest.New() - mf.Distro = manifest.DISTRO_FEDORA + mf := manifest.New(manifest.DISTRO_FEDORA) _, err = img.InstantiateManifest(&mf, repos, t.arch.distro.runner, rng) if err != nil { return nil, nil, err diff --git a/pkg/distro/rhel/imagetype.go b/pkg/distro/rhel/imagetype.go index 0c055c96a6..fc63813104 100644 --- a/pkg/distro/rhel/imagetype.go +++ b/pkg/distro/rhel/imagetype.go @@ -307,17 +307,17 @@ func (t *ImageType) Manifest(bp *blueprint.Blueprint, if err != nil { return nil, nil, err } - mf := manifest.New() + var mf manifest.Manifest switch t.Arch().Distro().Releasever() { case "7": - mf.Distro = manifest.DISTRO_EL7 + mf = manifest.New(manifest.DISTRO_EL7) case "8": - mf.Distro = manifest.DISTRO_EL8 + mf = manifest.New(manifest.DISTRO_EL8) case "9": - mf.Distro = manifest.DISTRO_EL9 + mf = manifest.New(manifest.DISTRO_EL9) case "10": - mf.Distro = manifest.DISTRO_EL10 + mf = manifest.New(manifest.DISTRO_EL10) default: return nil, nil, fmt.Errorf("unsupported distro release version: %s", t.Arch().Distro().Releasever()) } diff --git a/pkg/image/installer_image_test.go b/pkg/image/installer_image_test.go index 7ebd17cecc..80e45944b7 100644 --- a/pkg/image/installer_image_test.go +++ b/pkg/image/installer_image_test.go @@ -226,7 +226,7 @@ func instantiateAndSerialize(t *testing.T, img image.ImageKind, packages map[str /* #nosec G404 */ rng := rand.New(source) - mf := manifest.New() + mf := manifest.New(manifest.DISTRO_FEDORA) _, err := img.InstantiateManifest(&mf, nil, &runner.CentOS{Version: 9}, rng) assert.NoError(t, err) diff --git a/pkg/image/ostree_disk_test.go b/pkg/image/ostree_disk_test.go index 1220f5dc46..9c6341d4fb 100644 --- a/pkg/image/ostree_disk_test.go +++ b/pkg/image/ostree_disk_test.go @@ -37,7 +37,7 @@ func TestOSTreeDiskImageManifestSetsContainerBuildable(t *testing.T) { for _, containerBuildable := range []bool{true, false} { buildOpts = nil - mf := manifest.New() + mf := manifest.New(manifest.DISTRO_FEDORA) img := image.NewOSTreeDiskImageFromContainer(containerSource, ref) require.NotNil(t, img) img.Platform = &platform.X86{ diff --git a/pkg/manifest/build_test.go b/pkg/manifest/build_test.go index eb1da5680a..4bb8cba783 100644 --- a/pkg/manifest/build_test.go +++ b/pkg/manifest/build_test.go @@ -14,7 +14,7 @@ import ( func TestBuildContainerBuildableNo(t *testing.T) { repos := []rpmmd.RepoConfig{} - mf := New() + mf := New(DISTRO_FEDORA) runner := &runner.Fedora{Version: 39} buildIf := NewBuild(&mf, runner, repos, nil) @@ -94,7 +94,7 @@ func TestNewBuildFromContainerSpecs(t *testing.T) { Source: "ghcr.io/ondrejbudai/booc:fedora", }, } - mf := New() + mf := New(DISTRO_FEDORA) runner := &runner.Fedora{Version: 39} buildIf := NewBuildFromContainer(&mf, runner, containers, nil) diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go index 1ae10c5a0f..cf094a1366 100644 --- a/pkg/manifest/manifest.go +++ b/pkg/manifest/manifest.go @@ -68,16 +68,16 @@ type Manifest struct { // pipelines describe the build process for an image. pipelines []Pipeline - // Distro defines the distribution of the image that this manifest will + // distro defines the distribution of the image that this manifest will // generate. It is used for determining package names that differ between // different distributions and version. - Distro Distro + distro Distro } -func New() Manifest { +func New(d Distro) Manifest { return Manifest{ pipelines: make([]Pipeline, 0), - Distro: DISTRO_NULL, + distro: d, } } @@ -104,7 +104,7 @@ func (m Manifest) GetPackageSetChains() map[string][]rpmmd.PackageSet { chains := make(map[string][]rpmmd.PackageSet) for _, pipeline := range m.pipelines { - if chain := pipeline.getPackageSetChain(m.Distro); chain != nil { + if chain := pipeline.getPackageSetChain(m.distro); chain != nil { chains[pipeline.Name()] = chain } } diff --git a/pkg/manifest/os_test.go b/pkg/manifest/os_test.go index be229ebc8e..e273acf3fe 100644 --- a/pkg/manifest/os_test.go +++ b/pkg/manifest/os_test.go @@ -17,7 +17,7 @@ import ( // NewTestOS returns a minimally populated OS struct for use in testing func NewTestOS() *OS { repos := []rpmmd.RepoConfig{} - manifest := New() + manifest := New(DISTRO_FEDORA) runner := &runner.Fedora{Version: 38} build := NewBuild(&manifest, runner, repos, nil) build.Checkpoint() diff --git a/pkg/manifest/raw_bootc_test.go b/pkg/manifest/raw_bootc_test.go index c2a1aca134..18fd8f9090 100644 --- a/pkg/manifest/raw_bootc_test.go +++ b/pkg/manifest/raw_bootc_test.go @@ -33,7 +33,7 @@ func hasPipeline(haystack []manifest.Pipeline, needle manifest.Pipeline) bool { } func TestNewRawBootcImage(t *testing.T) { - mani := manifest.New() + mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} buildIf := manifest.NewBuildFromContainer(&mani, runner, nil, nil) build := buildIf.(*manifest.BuildrootFromContainer) @@ -48,7 +48,7 @@ func TestNewRawBootcImage(t *testing.T) { } func TestRawBootcImageSerialize(t *testing.T) { - mani := manifest.New() + mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} build := manifest.NewBuildFromContainer(&mani, runner, nil, nil) pf := &platform.X86{ @@ -76,7 +76,7 @@ func TestRawBootcImageSerialize(t *testing.T) { } func TestRawBootcImageSerializeMountsValidated(t *testing.T) { - mani := manifest.New() + mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} build := manifest.NewBuildFromContainer(&mani, runner, nil, nil) pf := &platform.X86{ @@ -103,7 +103,7 @@ func findMountIdx(mounts []osbuild.Mount, mntType string) int { } func makeFakeRawBootcPipeline() *manifest.RawBootcImage { - mani := manifest.New() + mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} pf := &platform.X86{ BasePlatform: platform.BasePlatform{}, From 649a71a820e8985c31ae817bb2c4d0430277e996 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 2 Jul 2024 18:43:23 +0200 Subject: [PATCH 02/14] manifest: introduce the Manifest interface manifest.Manifest is now an interface with one implementation called IntManifest (Internal Manifest), which is the previous Manifest type. All functions that interact with pointers to the old concrete Manifest type now interact with the interface instead. --- cmd/osbuild-playground/my-container.go | 2 +- cmd/osbuild-playground/my-image.go | 2 +- cmd/osbuild-playground/playground.go | 2 +- pkg/distro/distro.go | 2 +- pkg/distro/fedora/imagetype.go | 6 ++-- pkg/distro/rhel/imagetype.go | 6 ++-- pkg/distro/test_distro/distro.go | 4 +-- pkg/image/anaconda_container_installer.go | 2 +- pkg/image/anaconda_live_installer.go | 2 +- pkg/image/anaconda_ostree_installer.go | 2 +- pkg/image/anaconda_tar_installer.go | 2 +- pkg/image/archive.go | 2 +- pkg/image/bootc_disk.go | 2 +- pkg/image/bootc_disk_legacy.go | 2 +- pkg/image/bootc_disk_test.go | 2 +- pkg/image/container.go | 2 +- pkg/image/disk.go | 2 +- pkg/image/export_test.go | 2 +- pkg/image/image.go | 2 +- pkg/image/installer_image_test.go | 2 +- pkg/image/ostree_archive.go | 2 +- pkg/image/ostree_container.go | 2 +- pkg/image/ostree_disk.go | 2 +- pkg/image/ostree_disk_test.go | 4 +-- pkg/image/ostree_simplified_installer.go | 2 +- .../anaconda_installer_iso_tree_test.go | 2 +- pkg/manifest/anaconda_installer_test.go | 2 +- pkg/manifest/build.go | 6 ++-- pkg/manifest/build_test.go | 4 +-- pkg/manifest/empty.go | 2 +- pkg/manifest/manifest.go | 30 ++++++++++++------- pkg/manifest/os_test.go | 2 +- pkg/manifest/pipeline.go | 12 ++++---- pkg/manifest/raw_bootc_test.go | 8 ++--- 34 files changed, 70 insertions(+), 60 deletions(-) diff --git a/cmd/osbuild-playground/my-container.go b/cmd/osbuild-playground/my-container.go index 9883f6c8f2..d7bc546004 100644 --- a/cmd/osbuild-playground/my-container.go +++ b/cmd/osbuild-playground/my-container.go @@ -39,7 +39,7 @@ func init() { // Return nil when you are done, or an error if something // went wrong. Your manifest will be streamed to osbuild // for building. -func (img *MyContainer) InstantiateManifest(m *manifest.Manifest, +func (img *MyContainer) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/cmd/osbuild-playground/my-image.go b/cmd/osbuild-playground/my-image.go index 54dbc7e01f..7f13e03c03 100644 --- a/cmd/osbuild-playground/my-image.go +++ b/cmd/osbuild-playground/my-image.go @@ -23,7 +23,7 @@ func init() { AddImageType(&MyImage{}) } -func (img *MyImage) InstantiateManifest(m *manifest.Manifest, +func (img *MyImage) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/cmd/osbuild-playground/playground.go b/cmd/osbuild-playground/playground.go index 01d7afb943..7dd51a04d4 100644 --- a/cmd/osbuild-playground/playground.go +++ b/cmd/osbuild-playground/playground.go @@ -29,7 +29,7 @@ func RunPlayground(img image.ImageKind, d distro.Distro, arch distro.Arch, repos rnd := rand.New(rand.NewSource(0)) // TODO: query distro for runner - artifact, err := img.InstantiateManifest(&manifest, repos[arch.Name()], &runner.Fedora{Version: 36}, rnd) + artifact, err := img.InstantiateManifest(manifest, repos[arch.Name()], &runner.Fedora{Version: 36}, rnd) if err != nil { panic("InstantiateManifest() failed: " + err.Error()) } diff --git a/pkg/distro/distro.go b/pkg/distro/distro.go index dfe0582171..d10756aa76 100644 --- a/pkg/distro/distro.go +++ b/pkg/distro/distro.go @@ -143,7 +143,7 @@ type ImageType interface { // specified in the given blueprint; it also returns any warnings (e.g. // deprecation notices) generated by the manifest. // The packageSpecSets must be labelled in the same way as the originating PackageSets. - Manifest(bp *blueprint.Blueprint, options ImageOptions, repos []rpmmd.RepoConfig, seed int64) (*manifest.Manifest, []string, error) + Manifest(bp *blueprint.Blueprint, options ImageOptions, repos []rpmmd.RepoConfig, seed int64) (manifest.Manifest, []string, error) } // The ImageOptions specify options for a specific image build diff --git a/pkg/distro/fedora/imagetype.go b/pkg/distro/fedora/imagetype.go index 32fdba6e1d..16b93cf351 100644 --- a/pkg/distro/fedora/imagetype.go +++ b/pkg/distro/fedora/imagetype.go @@ -188,7 +188,7 @@ func (t *imageType) PartitionType() string { func (t *imageType) Manifest(bp *blueprint.Blueprint, options distro.ImageOptions, repos []rpmmd.RepoConfig, - seed int64) (*manifest.Manifest, []string, error) { + seed int64) (manifest.Manifest, []string, error) { warnings, err := t.checkOptions(bp, options) if err != nil { @@ -257,12 +257,12 @@ func (t *imageType) Manifest(bp *blueprint.Blueprint, return nil, nil, err } mf := manifest.New(manifest.DISTRO_FEDORA) - _, err = img.InstantiateManifest(&mf, repos, t.arch.distro.runner, rng) + _, err = img.InstantiateManifest(mf, repos, t.arch.distro.runner, rng) if err != nil { return nil, nil, err } - return &mf, warnings, err + return mf, warnings, err } // checkOptions checks the validity and compatibility of options and customizations for the image type. diff --git a/pkg/distro/rhel/imagetype.go b/pkg/distro/rhel/imagetype.go index fc63813104..3a8bd11893 100644 --- a/pkg/distro/rhel/imagetype.go +++ b/pkg/distro/rhel/imagetype.go @@ -231,7 +231,7 @@ func (t *ImageType) PartitionType() string { func (t *ImageType) Manifest(bp *blueprint.Blueprint, options distro.ImageOptions, repos []rpmmd.RepoConfig, - seed int64) (*manifest.Manifest, []string, error) { + seed int64) (manifest.Manifest, []string, error) { if t.Workload != nil { // For now, if an image type defines its own workload, don't allow any @@ -322,12 +322,12 @@ func (t *ImageType) Manifest(bp *blueprint.Blueprint, return nil, nil, fmt.Errorf("unsupported distro release version: %s", t.Arch().Distro().Releasever()) } - _, err = img.InstantiateManifest(&mf, repos, t.arch.distro.runner, rng) + _, err = img.InstantiateManifest(mf, repos, t.arch.distro.runner, rng) if err != nil { return nil, nil, err } - return &mf, warnings, err + return mf, warnings, err } // checkOptions checks the validity and compatibility of options and customizations for the image type. diff --git a/pkg/distro/test_distro/distro.go b/pkg/distro/test_distro/distro.go index f8354ab806..6566c08c53 100644 --- a/pkg/distro/test_distro/distro.go +++ b/pkg/distro/test_distro/distro.go @@ -236,7 +236,7 @@ func (t *TestImageType) Exports() []string { return distro.ExportsFallback() } -func (t *TestImageType) Manifest(b *blueprint.Blueprint, options distro.ImageOptions, repos []rpmmd.RepoConfig, seed int64) (*manifest.Manifest, []string, error) { +func (t *TestImageType) Manifest(b *blueprint.Blueprint, options distro.ImageOptions, repos []rpmmd.RepoConfig, seed int64) (manifest.Manifest, []string, error) { var bpPkgs []string if b != nil { mountpoints := b.Customizations.GetFilesystems() @@ -297,7 +297,7 @@ func (t *TestImageType) Manifest(b *blueprint.Blueprint, options distro.ImageOpt }, } - m := &manifest.Manifest{} + m := manifest.New(manifest.DISTRO_NULL) manifest.NewContentTest(m, buildPkgsKey, buildPackages, nil, nil) manifest.NewContentTest(m, osPkgsKey, osPackages, nil, ostreeSources) diff --git a/pkg/image/anaconda_container_installer.go b/pkg/image/anaconda_container_installer.go index 94dd0774e6..50d98d4600 100644 --- a/pkg/image/anaconda_container_installer.go +++ b/pkg/image/anaconda_container_installer.go @@ -60,7 +60,7 @@ func NewAnacondaContainerInstaller(container container.SourceSpec, ref string) * } } -func (img *AnacondaContainerInstaller) InstantiateManifest(m *manifest.Manifest, +func (img *AnacondaContainerInstaller) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/anaconda_live_installer.go b/pkg/image/anaconda_live_installer.go index b1acc99b58..7c6e3fa404 100644 --- a/pkg/image/anaconda_live_installer.go +++ b/pkg/image/anaconda_live_installer.go @@ -41,7 +41,7 @@ func NewAnacondaLiveInstaller() *AnacondaLiveInstaller { } } -func (img *AnacondaLiveInstaller) InstantiateManifest(m *manifest.Manifest, +func (img *AnacondaLiveInstaller) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/anaconda_ostree_installer.go b/pkg/image/anaconda_ostree_installer.go index cba994e7e3..0b92f60c95 100644 --- a/pkg/image/anaconda_ostree_installer.go +++ b/pkg/image/anaconda_ostree_installer.go @@ -59,7 +59,7 @@ func NewAnacondaOSTreeInstaller(commit ostree.SourceSpec) *AnacondaOSTreeInstall } } -func (img *AnacondaOSTreeInstaller) InstantiateManifest(m *manifest.Manifest, +func (img *AnacondaOSTreeInstaller) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/anaconda_tar_installer.go b/pkg/image/anaconda_tar_installer.go index 27ced33394..383e53441a 100644 --- a/pkg/image/anaconda_tar_installer.go +++ b/pkg/image/anaconda_tar_installer.go @@ -84,7 +84,7 @@ func NewAnacondaTarInstaller() *AnacondaTarInstaller { } } -func (img *AnacondaTarInstaller) InstantiateManifest(m *manifest.Manifest, +func (img *AnacondaTarInstaller) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/archive.go b/pkg/image/archive.go index 2ca2be45e6..d47a728eb9 100644 --- a/pkg/image/archive.go +++ b/pkg/image/archive.go @@ -27,7 +27,7 @@ func NewArchive() *Archive { } } -func (img *Archive) InstantiateManifest(m *manifest.Manifest, +func (img *Archive) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/bootc_disk.go b/pkg/image/bootc_disk.go index a1037e6cc5..792faac2a8 100644 --- a/pkg/image/bootc_disk.go +++ b/pkg/image/bootc_disk.go @@ -43,7 +43,7 @@ func NewBootcDiskImage(container container.SourceSpec) *BootcDiskImage { } } -func (img *BootcDiskImage) InstantiateManifestFromContainers(m *manifest.Manifest, +func (img *BootcDiskImage) InstantiateManifestFromContainers(m *manifest.IntManifest, containers []container.SourceSpec, runner runner.Runner, rng *rand.Rand) error { diff --git a/pkg/image/bootc_disk_legacy.go b/pkg/image/bootc_disk_legacy.go index e48867f74c..c092b603ba 100644 --- a/pkg/image/bootc_disk_legacy.go +++ b/pkg/image/bootc_disk_legacy.go @@ -29,7 +29,7 @@ func NewBootcLegacyDiskImage(real *BootcDiskImage) *BootcLegacyDiskImage { } } -func (img *BootcLegacyDiskImage) InstantiateManifestFromContainers(m *manifest.Manifest, +func (img *BootcLegacyDiskImage) InstantiateManifestFromContainers(m manifest.Manifest, containers []container.SourceSpec, runner runner.Runner, rng *rand.Rand) error { diff --git a/pkg/image/bootc_disk_test.go b/pkg/image/bootc_disk_test.go index f0937625eb..ecd693e63d 100644 --- a/pkg/image/bootc_disk_test.go +++ b/pkg/image/bootc_disk_test.go @@ -78,7 +78,7 @@ func makeBootcDiskImageOsbuildManifest(t *testing.T, opts *bootcDiskImageTestOpt img.Groups = opts.Groups img.SELinux = opts.SELinux - m := &manifest.Manifest{} + m := &manifest.IntManifest{} runi := &runner.Fedora{} err := img.InstantiateManifestFromContainers(m, containers, runi, nil) require.Nil(t, err) diff --git a/pkg/image/container.go b/pkg/image/container.go index 1ab16377d8..34ac6b0aae 100644 --- a/pkg/image/container.go +++ b/pkg/image/container.go @@ -27,7 +27,7 @@ func NewBaseContainer() *BaseContainer { } } -func (img *BaseContainer) InstantiateManifest(m *manifest.Manifest, +func (img *BaseContainer) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/disk.go b/pkg/image/disk.go index 3d7d34c5c9..b173271049 100644 --- a/pkg/image/disk.go +++ b/pkg/image/disk.go @@ -49,7 +49,7 @@ func NewDiskImage() *DiskImage { } } -func (img *DiskImage) InstantiateManifest(m *manifest.Manifest, +func (img *DiskImage) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/export_test.go b/pkg/image/export_test.go index 877c20c306..64b1a392cb 100644 --- a/pkg/image/export_test.go +++ b/pkg/image/export_test.go @@ -6,7 +6,7 @@ import ( "github.com/osbuild/images/pkg/runner" ) -func MockManifestNewBuild(new func(m *manifest.Manifest, runner runner.Runner, repos []rpmmd.RepoConfig, opts *manifest.BuildOptions) manifest.Build) (restore func()) { +func MockManifestNewBuild(new func(m manifest.Manifest, runner runner.Runner, repos []rpmmd.RepoConfig, opts *manifest.BuildOptions) manifest.Build) (restore func()) { saved := manifestNewBuild manifestNewBuild = new return func() { diff --git a/pkg/image/image.go b/pkg/image/image.go index 7693a2400c..5a1444c892 100644 --- a/pkg/image/image.go +++ b/pkg/image/image.go @@ -11,7 +11,7 @@ import ( type ImageKind interface { Name() string - InstantiateManifest(m *manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) + InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) } type Base struct { diff --git a/pkg/image/installer_image_test.go b/pkg/image/installer_image_test.go index 80e45944b7..b58ac4c9d4 100644 --- a/pkg/image/installer_image_test.go +++ b/pkg/image/installer_image_test.go @@ -227,7 +227,7 @@ func instantiateAndSerialize(t *testing.T, img image.ImageKind, packages map[str rng := rand.New(source) mf := manifest.New(manifest.DISTRO_FEDORA) - _, err := img.InstantiateManifest(&mf, nil, &runner.CentOS{Version: 9}, rng) + _, err := img.InstantiateManifest(mf, nil, &runner.CentOS{Version: 9}, rng) assert.NoError(t, err) mfs, err := mf.Serialize(packages, containers, commits, nil) diff --git a/pkg/image/ostree_archive.go b/pkg/image/ostree_archive.go index 6aac3db1d8..d7bd59bc6d 100644 --- a/pkg/image/ostree_archive.go +++ b/pkg/image/ostree_archive.go @@ -49,7 +49,7 @@ func NewOSTreeArchive(ref string) *OSTreeArchive { } } -func (img *OSTreeArchive) InstantiateManifest(m *manifest.Manifest, +func (img *OSTreeArchive) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/ostree_container.go b/pkg/image/ostree_container.go index a39e25e623..84fff1799d 100644 --- a/pkg/image/ostree_container.go +++ b/pkg/image/ostree_container.go @@ -40,7 +40,7 @@ func NewOSTreeContainer(ref string) *OSTreeContainer { } } -func (img *OSTreeContainer) InstantiateManifest(m *manifest.Manifest, +func (img *OSTreeContainer) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/ostree_disk.go b/pkg/image/ostree_disk.go index 4b588e86f3..da1ea48529 100644 --- a/pkg/image/ostree_disk.go +++ b/pkg/image/ostree_disk.go @@ -90,7 +90,7 @@ func baseRawOstreeImage(img *OSTreeDiskImage, buildPipeline manifest.Build, opts // replaced in testing var manifestNewBuild = manifest.NewBuild -func (img *OSTreeDiskImage) InstantiateManifest(m *manifest.Manifest, +func (img *OSTreeDiskImage) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/image/ostree_disk_test.go b/pkg/image/ostree_disk_test.go index 9c6341d4fb..17d28e2e22 100644 --- a/pkg/image/ostree_disk_test.go +++ b/pkg/image/ostree_disk_test.go @@ -28,7 +28,7 @@ func TestOSTreeDiskImageManifestSetsContainerBuildable(t *testing.T) { } var buildOpts []*manifest.BuildOptions - restore := image.MockManifestNewBuild(func(m *manifest.Manifest, r runner.Runner, repos []rpmmd.RepoConfig, opts *manifest.BuildOptions) manifest.Build { + restore := image.MockManifestNewBuild(func(m manifest.Manifest, r runner.Runner, repos []rpmmd.RepoConfig, opts *manifest.BuildOptions) manifest.Build { buildOpts = append(buildOpts, opts) return manifest.NewBuild(m, r, repos, opts) }) @@ -51,7 +51,7 @@ func TestOSTreeDiskImageManifestSetsContainerBuildable(t *testing.T) { img.OSName = "osname" img.ContainerBuildable = containerBuildable - _, err := img.InstantiateManifest(&mf, repos, r, rng) + _, err := img.InstantiateManifest(mf, repos, r, rng) require.Nil(t, err) require.NotNil(t, img) diff --git a/pkg/image/ostree_simplified_installer.go b/pkg/image/ostree_simplified_installer.go index 253c8c1bf5..71a1f7ff40 100644 --- a/pkg/image/ostree_simplified_installer.go +++ b/pkg/image/ostree_simplified_installer.go @@ -69,7 +69,7 @@ func NewOSTreeSimplifiedInstaller(rawImage *OSTreeDiskImage, installDevice strin } } -func (img *OSTreeSimplifiedInstaller) InstantiateManifest(m *manifest.Manifest, +func (img *OSTreeSimplifiedInstaller) InstantiateManifest(m manifest.Manifest, repos []rpmmd.RepoConfig, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error) { diff --git a/pkg/manifest/anaconda_installer_iso_tree_test.go b/pkg/manifest/anaconda_installer_iso_tree_test.go index fefe8cc53d..ffa41ecc26 100644 --- a/pkg/manifest/anaconda_installer_iso_tree_test.go +++ b/pkg/manifest/anaconda_installer_iso_tree_test.go @@ -24,7 +24,7 @@ const ( // newTestAnacondaISOTree returns a base AnacondaInstallerISOTree pipeline. func newTestAnacondaISOTree() *AnacondaInstallerISOTree { - m := &Manifest{} + m := &IntManifest{} runner := &runner.Linux{} build := NewBuild(m, runner, nil, nil) diff --git a/pkg/manifest/anaconda_installer_test.go b/pkg/manifest/anaconda_installer_test.go index aaa3ebbb7f..0bc6ac8308 100644 --- a/pkg/manifest/anaconda_installer_test.go +++ b/pkg/manifest/anaconda_installer_test.go @@ -12,7 +12,7 @@ import ( ) func newAnacondaInstaller() *AnacondaInstaller { - m := &Manifest{} + m := &IntManifest{} runner := &runner.Linux{} build := NewBuild(m, runner, nil, nil) diff --git a/pkg/manifest/build.go b/pkg/manifest/build.go index eca633d44a..65a278abc4 100644 --- a/pkg/manifest/build.go +++ b/pkg/manifest/build.go @@ -21,7 +21,7 @@ import ( type Build interface { Name() string Checkpoint() - Manifest() *Manifest + Manifest() Manifest addDependent(dep Pipeline) } @@ -45,7 +45,7 @@ type BuildOptions struct { // NewBuild creates a new build pipeline from the repositories in repos // and the specified packages. -func NewBuild(m *Manifest, runner runner.Runner, repos []rpmmd.RepoConfig, opts *BuildOptions) Build { +func NewBuild(m Manifest, runner runner.Runner, repos []rpmmd.RepoConfig, opts *BuildOptions) Build { if opts == nil { opts = &BuildOptions{} } @@ -164,7 +164,7 @@ type BuildrootFromContainer struct { // NewBuildFromContainer creates a new build pipeline from the given // containers specs -func NewBuildFromContainer(m *Manifest, runner runner.Runner, containerSources []container.SourceSpec, opts *BuildOptions) Build { +func NewBuildFromContainer(m Manifest, runner runner.Runner, containerSources []container.SourceSpec, opts *BuildOptions) Build { if opts == nil { opts = &BuildOptions{} } diff --git a/pkg/manifest/build_test.go b/pkg/manifest/build_test.go index 4bb8cba783..7c5e80df04 100644 --- a/pkg/manifest/build_test.go +++ b/pkg/manifest/build_test.go @@ -17,7 +17,7 @@ func TestBuildContainerBuildableNo(t *testing.T) { mf := New(DISTRO_FEDORA) runner := &runner.Fedora{Version: 39} - buildIf := NewBuild(&mf, runner, repos, nil) + buildIf := NewBuild(mf, runner, repos, nil) build := buildIf.(*BuildrootFromPackages) require.NotNil(t, build) @@ -97,7 +97,7 @@ func TestNewBuildFromContainerSpecs(t *testing.T) { mf := New(DISTRO_FEDORA) runner := &runner.Fedora{Version: 39} - buildIf := NewBuildFromContainer(&mf, runner, containers, nil) + buildIf := NewBuildFromContainer(mf, runner, containers, nil) require.NotNil(t, buildIf) build := buildIf.(*BuildrootFromContainer) diff --git a/pkg/manifest/empty.go b/pkg/manifest/empty.go index 05c60cb4e1..92d544a7da 100644 --- a/pkg/manifest/empty.go +++ b/pkg/manifest/empty.go @@ -30,7 +30,7 @@ type ContentTest struct { // NewContentTest creates a new ContentTest pipeline with a given name and // content sources. -func NewContentTest(m *Manifest, name string, packageSets []rpmmd.PackageSet, containers []container.SourceSpec, commits []ostree.SourceSpec) *ContentTest { +func NewContentTest(m Manifest, name string, packageSets []rpmmd.PackageSet, containers []container.SourceSpec, commits []ostree.SourceSpec) *ContentTest { pipeline := &ContentTest{ Base: NewBase(name, nil), packageSets: packageSets, diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go index cf094a1366..87953afdb7 100644 --- a/pkg/manifest/manifest.go +++ b/pkg/manifest/manifest.go @@ -63,7 +63,17 @@ func (m *OSBuildManifest) UnmarshalJSON(payload []byte) error { // (PackageSetChains, ContainerSourceSpecs, OSTreeSourceSpecs) must be // retrieved through their corresponding Getters and resolved before // serializing. -type Manifest struct { +type Manifest interface { + addPipeline(p Pipeline) + GetPackageSetChains() map[string][]rpmmd.PackageSet + GetContainerSourceSpecs() map[string][]container.SourceSpec + GetOSTreeSourceSpecs() map[string][]ostree.SourceSpec + Serialize(packageSets map[string][]rpmmd.PackageSpec, containerSpecs map[string][]container.Spec, ostreeCommits map[string][]ostree.CommitSpec, rpmRepos map[string][]rpmmd.RepoConfig) (OSBuildManifest, error) + GetCheckpoints() []string + GetExports() []string +} + +type IntManifest struct { // pipelines describe the build process for an image. pipelines []Pipeline @@ -74,14 +84,14 @@ type Manifest struct { distro Distro } -func New(d Distro) Manifest { - return Manifest{ +func New(d Distro) *IntManifest { + return &IntManifest{ pipelines: make([]Pipeline, 0), distro: d, } } -func (m *Manifest) addPipeline(p Pipeline) { +func (m *IntManifest) addPipeline(p Pipeline) { for _, pipeline := range m.pipelines { if pipeline.Name() == p.Name() { panic("duplicate pipeline name in manifest") @@ -100,7 +110,7 @@ func (m *Manifest) addPipeline(p Pipeline) { type PackageSelector func([]rpmmd.PackageSet) []rpmmd.PackageSet -func (m Manifest) GetPackageSetChains() map[string][]rpmmd.PackageSet { +func (m IntManifest) GetPackageSetChains() map[string][]rpmmd.PackageSet { chains := make(map[string][]rpmmd.PackageSet) for _, pipeline := range m.pipelines { @@ -112,7 +122,7 @@ func (m Manifest) GetPackageSetChains() map[string][]rpmmd.PackageSet { return chains } -func (m Manifest) GetContainerSourceSpecs() map[string][]container.SourceSpec { +func (m IntManifest) GetContainerSourceSpecs() map[string][]container.SourceSpec { // Containers should only appear in the payload pipeline. // Let's iterate over all pipelines to avoid assuming pipeline names, but // return all the specs as a single slice. @@ -125,7 +135,7 @@ func (m Manifest) GetContainerSourceSpecs() map[string][]container.SourceSpec { return containerSpecs } -func (m Manifest) GetOSTreeSourceSpecs() map[string][]ostree.SourceSpec { +func (m IntManifest) GetOSTreeSourceSpecs() map[string][]ostree.SourceSpec { // OSTree commits should only appear in one pipeline. // Let's iterate over all pipelines to avoid assuming pipeline names, but // return all the specs as a single slice if there are multiple. @@ -138,7 +148,7 @@ func (m Manifest) GetOSTreeSourceSpecs() map[string][]ostree.SourceSpec { return ostreeSpecs } -func (m Manifest) Serialize(packageSets map[string][]rpmmd.PackageSpec, containerSpecs map[string][]container.Spec, ostreeCommits map[string][]ostree.CommitSpec, rpmRepos map[string][]rpmmd.RepoConfig) (OSBuildManifest, error) { +func (m IntManifest) Serialize(packageSets map[string][]rpmmd.PackageSpec, containerSpecs map[string][]container.Spec, ostreeCommits map[string][]ostree.CommitSpec, rpmRepos map[string][]rpmmd.RepoConfig) (OSBuildManifest, error) { pipelines := make([]osbuild.Pipeline, 0) packages := make([]rpmmd.PackageSpec, 0) commits := make([]ostree.CommitSpec, 0) @@ -172,7 +182,7 @@ func (m Manifest) Serialize(packageSets map[string][]rpmmd.PackageSpec, containe ) } -func (m Manifest) GetCheckpoints() []string { +func (m IntManifest) GetCheckpoints() []string { checkpoints := []string{} for _, p := range m.pipelines { if p.getCheckpoint() { @@ -182,7 +192,7 @@ func (m Manifest) GetCheckpoints() []string { return checkpoints } -func (m Manifest) GetExports() []string { +func (m IntManifest) GetExports() []string { exports := []string{} for _, p := range m.pipelines { if p.getExport() { diff --git a/pkg/manifest/os_test.go b/pkg/manifest/os_test.go index e273acf3fe..29989f19e8 100644 --- a/pkg/manifest/os_test.go +++ b/pkg/manifest/os_test.go @@ -19,7 +19,7 @@ func NewTestOS() *OS { repos := []rpmmd.RepoConfig{} manifest := New(DISTRO_FEDORA) runner := &runner.Fedora{Version: 38} - build := NewBuild(&manifest, runner, repos, nil) + build := NewBuild(manifest, runner, repos, nil) build.Checkpoint() // create an x86_64 platform with bios boot diff --git a/pkg/manifest/pipeline.go b/pkg/manifest/pipeline.go index a9325693e6..da039bdada 100644 --- a/pkg/manifest/pipeline.go +++ b/pkg/manifest/pipeline.go @@ -28,9 +28,9 @@ type Pipeline interface { BuildPipeline() Build // Manifest returns a reference to the Manifest which this Pipeline belongs to. - Manifest() *Manifest + Manifest() Manifest - setManifest(*Manifest) + setManifest(Manifest) getCheckpoint() bool @@ -74,7 +74,7 @@ type Pipeline interface { // A Base represents the core functionality shared between each of the pipeline // implementations, and the Base struct must be embedded in each of them. type Base struct { - manifest *Manifest + manifest Manifest name string build Build checkpoint bool @@ -108,11 +108,11 @@ func (p Base) BuildPipeline() Build { return p.build } -func (p Base) Manifest() *Manifest { +func (p Base) Manifest() Manifest { return p.manifest } -func (p *Base) setManifest(m *Manifest) { +func (p *Base) setManifest(m Manifest) { p.manifest = m } @@ -190,7 +190,7 @@ func (p Base) serialize() osbuild.Pipeline { // TreePipeline is any pipeline that produces a directory tree. type TreePipeline interface { Name() string - Manifest() *Manifest + Manifest() Manifest BuildPipeline() Build Platform() platform.Platform } diff --git a/pkg/manifest/raw_bootc_test.go b/pkg/manifest/raw_bootc_test.go index 18fd8f9090..d9b017553a 100644 --- a/pkg/manifest/raw_bootc_test.go +++ b/pkg/manifest/raw_bootc_test.go @@ -35,7 +35,7 @@ func hasPipeline(haystack []manifest.Pipeline, needle manifest.Pipeline) bool { func TestNewRawBootcImage(t *testing.T) { mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} - buildIf := manifest.NewBuildFromContainer(&mani, runner, nil, nil) + buildIf := manifest.NewBuildFromContainer(mani, runner, nil, nil) build := buildIf.(*manifest.BuildrootFromContainer) rawBootcPipeline := manifest.NewRawBootcImage(build, containers, nil) @@ -50,7 +50,7 @@ func TestNewRawBootcImage(t *testing.T) { func TestRawBootcImageSerialize(t *testing.T) { mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} - build := manifest.NewBuildFromContainer(&mani, runner, nil, nil) + build := manifest.NewBuildFromContainer(mani, runner, nil, nil) pf := &platform.X86{ BasePlatform: platform.BasePlatform{}, UEFIVendor: "test", @@ -78,7 +78,7 @@ func TestRawBootcImageSerialize(t *testing.T) { func TestRawBootcImageSerializeMountsValidated(t *testing.T) { mani := manifest.New(manifest.DISTRO_NULL) runner := &runner.Linux{} - build := manifest.NewBuildFromContainer(&mani, runner, nil, nil) + build := manifest.NewBuildFromContainer(mani, runner, nil, nil) pf := &platform.X86{ BasePlatform: platform.BasePlatform{}, UEFIVendor: "test", @@ -109,7 +109,7 @@ func makeFakeRawBootcPipeline() *manifest.RawBootcImage { BasePlatform: platform.BasePlatform{}, UEFIVendor: "test", } - build := manifest.NewBuildFromContainer(&mani, runner, nil, nil) + build := manifest.NewBuildFromContainer(mani, runner, nil, nil) rawBootcPipeline := manifest.NewRawBootcImage(build, nil, pf) rawBootcPipeline.PartitionTable = testdisk.MakeFakePartitionTable("/", "/boot", "/boot/efi") rawBootcPipeline.SerializeStart(nil, []container.Spec{{Source: "foo"}}, nil, nil) From 8a96b5c5a1a019c9e7cb2fd0c84a238cd8a64008 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 2 Jul 2024 18:56:50 +0200 Subject: [PATCH 03/14] manifest: add OTKManifest type The OTKManifest is a second implementation of the manifest.Manifest interface that will be used to generate manifests from otk yaml files. The type only implements the Serialize() method. None of the other methods are important (at least for now). The content spec methods in particular are not needed since otk will handle content resolution (depsolving, container and ostree commit resolves) itself. --- pkg/manifest/otk.go | 56 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 pkg/manifest/otk.go diff --git a/pkg/manifest/otk.go b/pkg/manifest/otk.go new file mode 100644 index 0000000000..0d5ff8b9eb --- /dev/null +++ b/pkg/manifest/otk.go @@ -0,0 +1,56 @@ +package manifest + +import ( + "bytes" + "fmt" + "os/exec" + + "github.com/osbuild/images/pkg/container" + "github.com/osbuild/images/pkg/ostree" + "github.com/osbuild/images/pkg/rpmmd" +) + +type OTKManifest struct { + // entrypoint yaml file for this otk manifest + entrypoint string +} + +func NewOTK(path string) *OTKManifest { + return &OTKManifest{entrypoint: path} +} + +func (m *OTKManifest) addPipeline(p Pipeline) {} + +func (m *OTKManifest) GetPackageSetChains() map[string][]rpmmd.PackageSet { + return nil +} + +func (m *OTKManifest) GetContainerSourceSpecs() map[string][]container.SourceSpec { + return nil +} + +func (m *OTKManifest) GetOSTreeSourceSpecs() map[string][]ostree.SourceSpec { + return nil +} + +func (m *OTKManifest) Serialize(_ map[string][]rpmmd.PackageSpec, _ map[string][]container.Spec, _ map[string][]ostree.CommitSpec, _ map[string][]rpmmd.RepoConfig) (OSBuildManifest, error) { + cmd := exec.Command("otk", "compile", "--target=osbuild", m.entrypoint) + var stdoutBuffer bytes.Buffer + var stderrBuffer bytes.Buffer + cmd.Stdout = &stdoutBuffer + cmd.Stderr = &stderrBuffer + err := cmd.Run() + if err != nil { + return nil, fmt.Errorf("compiling omnifest: %v %s", err, stderrBuffer.String()) + + } + return OSBuildManifest(stdoutBuffer.Bytes()), nil +} + +func (m *OTKManifest) GetCheckpoints() []string { + return nil +} + +func (m *OTKManifest) GetExports() []string { + return nil +} From cdf3b4720f8cdbe4791b71117533ac1ce7abf06b Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 2 Jul 2024 20:43:47 +0200 Subject: [PATCH 04/14] test: add some minimal otk yaml files for testing The directory structure follows the assumptions of the test_otk_distro. --- test/data/otk/fakedistro/aarch64/qcow2.yml | 13 +++++++++++++ test/data/otk/fakedistro/fakearch/qcow2.yml | 13 +++++++++++++ test/data/otk/fakedistro/properties.yaml | 5 +++++ test/data/otk/fakedistro/x86_64/qcow2.yml | 13 +++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 test/data/otk/fakedistro/aarch64/qcow2.yml create mode 100644 test/data/otk/fakedistro/fakearch/qcow2.yml create mode 100644 test/data/otk/fakedistro/properties.yaml create mode 100644 test/data/otk/fakedistro/x86_64/qcow2.yml diff --git a/test/data/otk/fakedistro/aarch64/qcow2.yml b/test/data/otk/fakedistro/aarch64/qcow2.yml new file mode 100644 index 0000000000..37b17b67f7 --- /dev/null +++ b/test/data/otk/fakedistro/aarch64/qcow2.yml @@ -0,0 +1,13 @@ +--- +otk.version: "1" + +otk.target.osbuild.name: + pipelines: + - name: "build" + stages: + - type: "org.osbuild.rpm" + options: + - name: "os" + stages: + - type: "org.osbuild.rpm" + options: diff --git a/test/data/otk/fakedistro/fakearch/qcow2.yml b/test/data/otk/fakedistro/fakearch/qcow2.yml new file mode 100644 index 0000000000..37b17b67f7 --- /dev/null +++ b/test/data/otk/fakedistro/fakearch/qcow2.yml @@ -0,0 +1,13 @@ +--- +otk.version: "1" + +otk.target.osbuild.name: + pipelines: + - name: "build" + stages: + - type: "org.osbuild.rpm" + options: + - name: "os" + stages: + - type: "org.osbuild.rpm" + options: diff --git a/test/data/otk/fakedistro/properties.yaml b/test/data/otk/fakedistro/properties.yaml new file mode 100644 index 0000000000..872b734109 --- /dev/null +++ b/test/data/otk/fakedistro/properties.yaml @@ -0,0 +1,5 @@ +--- +name: "FakeDistro" +os_version: "42.0" +release_version: "42" +runner: "org.osbuild.fakedistro42" diff --git a/test/data/otk/fakedistro/x86_64/qcow2.yml b/test/data/otk/fakedistro/x86_64/qcow2.yml new file mode 100644 index 0000000000..37b17b67f7 --- /dev/null +++ b/test/data/otk/fakedistro/x86_64/qcow2.yml @@ -0,0 +1,13 @@ +--- +otk.version: "1" + +otk.target.osbuild.name: + pipelines: + - name: "build" + stages: + - type: "org.osbuild.rpm" + options: + - name: "os" + stages: + - type: "org.osbuild.rpm" + options: From 50fd7ae989017bc4c6a3460e66774407445ecff2 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 9 Jul 2024 20:03:21 +0200 Subject: [PATCH 05/14] pkg: add otkdistro New package that loads a distribution from otk yaml files. Distribution-level properties, like name, codename, versions, etc, are loaded from a file called 'properties.yaml' in the root of the distribution's directory. An error is returned if this file is missing or if any of the required properties are undefined/empty. The distro creates a distro->arch->imagetype hierarchy based on the contents of the filesystem tree. It assumes a directory structure `//.yaml`. For example: fedora-40/ properties.yaml x86_64/ qcow2.yaml ami.yaml aarch64/ qcow2.yaml The top-level directory is used as the unique distro ID, whereas the name is defined in the properties.yaml. The distinction is useful for separating the on-disk, unique ID of a distro from the display name. --- pkg/otkdistro/otkdistro.go | 293 +++++++++++++++++++++++++++++++++++++ 1 file changed, 293 insertions(+) create mode 100644 pkg/otkdistro/otkdistro.go diff --git a/pkg/otkdistro/otkdistro.go b/pkg/otkdistro/otkdistro.go new file mode 100644 index 0000000000..b80408ba47 --- /dev/null +++ b/pkg/otkdistro/otkdistro.go @@ -0,0 +1,293 @@ +package otkdistro + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/distro" + "github.com/osbuild/images/pkg/manifest" + "github.com/osbuild/images/pkg/rpmmd" + "gopkg.in/yaml.v3" +) + +type Distro struct { + id string + arches map[string]distro.Arch + otkRoot string + + props DistroProperties +} + +func New(otkRoot string) (distro.Distro, error) { + // use path basename as distro id + id := filepath.Base(otkRoot) + + d := &Distro{ + id: id, + otkRoot: otkRoot, + } + + if err := d.loadProperties(); err != nil { + return nil, fmt.Errorf("error loading otk distro at %q: %w", otkRoot, err) + } + if err := d.readArches(); err != nil { + return nil, fmt.Errorf("error loading otk distro at %q: %w", otkRoot, err) + } + return d, nil +} + +type DistroProperties struct { + Name string `yaml:"name"` + Codename string `yaml:"codename"` + Product string `yaml:"product"` + OSVersion string `yaml:"os_version"` + ReleaseVersion string `yaml:"release_version"` + ModulePlatformID string `yaml:"module_platform_id"` + UEFIVendor string `yaml:"uefi_vendor"` + OSTreeRefTmpl string `yaml:"ostree_ref_template"` // TODO: figure out templating + Runner string `yaml:"runner"` // TODO: expose through a method on the interface +} + +// loadProperties reads the properties.yaml file in the distro's otkRoot and +// loads the information into the distro fields. Errors are returned if the +// file is missing or one of the required fields are not set. +func (d *Distro) loadProperties() error { + propsPath := filepath.Join(d.otkRoot, "properties.yaml") + propsFile, err := os.Open(propsPath) + if err != nil { + return fmt.Errorf("error opening props file %q: %w", propsPath, err) + } + + var props DistroProperties + decoder := yaml.NewDecoder(propsFile) + decoder.KnownFields(true) // produce error if there are fields in the file that don't exist in the target struct + if err := decoder.Decode(&props); err != nil { + return fmt.Errorf("error reading props file %q: %w", propsPath, err) + } + + // check that required properties aren't empty + // TODO: use reflect? + missingPropError := func(propName string) error { + return fmt.Errorf("incomplete props file %q: %q is required", propsPath, propName) + } + if props.Name == "" { + return missingPropError("name") + } + if props.OSVersion == "" { + return missingPropError("os_version") + } + if props.ReleaseVersion == "" { + return missingPropError("release_version") + } + if props.Runner == "" { + return missingPropError("runner") + } + + d.props = props + + return nil +} + +func (d *Distro) readArches() error { + // read otkRoot and discover arches based on expected directory structure + entries, err := os.ReadDir(d.otkRoot) + if err != nil { + return fmt.Errorf("failed to read otk root for distro %q: %s", d.Name(), err) + } + + d.arches = make(map[string]distro.Arch) + for _, entry := range entries { + if !entry.IsDir() { + // ignore files + continue + } + // assume each subdir is an architecture + name := entry.Name() + a := Arch{ + name: name, + otkRoot: filepath.Join(d.otkRoot, entry.Name()), + } + if err := a.readImageTypes(); err != nil { + return fmt.Errorf("error reading architecture at %q for otk distro %q: %w", a.otkRoot, d.Name(), err) + } + d.arches[a.Name()] = a + } + return nil +} + +func (d Distro) Name() string { + return d.props.Name +} + +func (d Distro) SymbolicName() string { + return d.id +} + +func (d Distro) Codename() string { + return d.props.Codename +} + +func (d Distro) Releasever() string { + return d.props.ReleaseVersion +} + +func (d Distro) OsVersion() string { + return d.props.OSVersion +} + +func (d Distro) ModulePlatformID() string { + return d.props.ModulePlatformID +} + +func (d Distro) Product() string { + return d.props.Product +} + +func (d Distro) OSTreeRef() string { + return d.props.OSTreeRefTmpl +} + +func (d Distro) ListArches() []string { + arches := make([]string, 0, len(d.arches)) + for arch := range d.arches { + arches = append(arches, arch) + } + return arches +} + +func (d Distro) GetArch(name string) (distro.Arch, error) { + return d.arches[name], nil +} + +type Arch struct { + distribution *Distro + name string + imageTypes map[string]distro.ImageType + otkRoot string +} + +func (a *Arch) readImageTypes() error { + entries, err := os.ReadDir(a.otkRoot) + if err != nil { + return fmt.Errorf("error reading architecture directory %q for otk test distro: %s", a.otkRoot, err) + } + + a.imageTypes = make(map[string]distro.ImageType) + for _, entry := range entries { + if entry.IsDir() { + // ignore subdirectories + continue + } + + switch filepath.Ext(entry.Name()) { + case ".yml", ".yaml": + imageType := newOtkImageType(filepath.Join(a.otkRoot, entry.Name())) + a.imageTypes[imageType.Name()] = &imageType + } + } + return nil +} + +func (a Arch) Name() string { + return a.name +} + +func (a Arch) Distro() distro.Distro { + return a.distribution +} + +func (a Arch) ListImageTypes() []string { + names := make([]string, 0, len(a.imageTypes)) + for name := range a.imageTypes { + names = append(names, name) + } + return names +} + +func (a Arch) GetImageType(name string) (distro.ImageType, error) { + return a.imageTypes[name], nil +} + +type otkImageType struct { + architecture *Arch + name string + otkPath string +} + +func newOtkImageType(path string) otkImageType { + baseFilename := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) + it := otkImageType{ + name: baseFilename, + otkPath: path, + } + return it +} + +func (t *otkImageType) Name() string { + return t.name +} + +func (t *otkImageType) Arch() distro.Arch { + return t.architecture +} + +func (t *otkImageType) Filename() string { + return "" +} + +func (t *otkImageType) MIMEType() string { + return "" +} + +func (t *otkImageType) OSTreeRef() string { + d := t.Arch().Distro() + return fmt.Sprintf(d.OSTreeRef(), t.architecture.Name()) +} + +func (t *otkImageType) ISOLabel() (string, error) { + return "", nil +} + +func (t *otkImageType) Size(size uint64) uint64 { + return size +} + +func (t *otkImageType) BuildPipelines() []string { + return nil +} + +func (t *otkImageType) PayloadPipelines() []string { + return nil +} + +func (t *otkImageType) PayloadPackageSets() []string { + return nil +} + +func (t *otkImageType) PackageSetsChains() map[string][]string { + return nil +} + +func (t *otkImageType) Exports() []string { + return nil +} + +func (t *otkImageType) BootMode() distro.BootMode { + return distro.BOOT_NONE +} + +func (t *otkImageType) PartitionType() string { + return "" +} + +func (t *otkImageType) Manifest(bp *blueprint.Blueprint, + options distro.ImageOptions, + repos []rpmmd.RepoConfig, + seed int64) (manifest.Manifest, []string, error) { + + mf := manifest.NewOTK(t.otkPath) + return mf, nil, nil +} From 988f80a08890455dcd72d7403a15e6919a6ae59e Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Tue, 9 Jul 2024 20:31:53 +0200 Subject: [PATCH 06/14] otkdistro: add a basic distro load test Test that the distro is loaded with the expected properties, arches, and image types. --- pkg/otkdistro/otkdistro_test.go | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 pkg/otkdistro/otkdistro_test.go diff --git a/pkg/otkdistro/otkdistro_test.go b/pkg/otkdistro/otkdistro_test.go new file mode 100644 index 0000000000..a36d527a6f --- /dev/null +++ b/pkg/otkdistro/otkdistro_test.go @@ -0,0 +1,40 @@ +package otkdistro_test + +import ( + "fmt" + "testing" + + "github.com/osbuild/images/pkg/otkdistro" + "github.com/stretchr/testify/require" +) + +func TestDistroLoad(t *testing.T) { + require := require.New(t) + + // TODO: we can write the fragments during the test setup and make the + // whole test self-contained + distro, err := otkdistro.New("../../test/data/otk/fakedistro") + require.NoError(err) + require.Equal("FakeDistro", distro.Name()) + require.Equal("42.0", distro.OsVersion()) + require.Equal("42", distro.Releasever()) + // TODO: check runner when it's added + + archImageTypes := make([]string, 0) + for _, archName := range distro.ListArches() { + arch, err := distro.GetArch(archName) + require.NoError(err) + + for _, imageTypeName := range arch.ListImageTypes() { + archImageTypes = append(archImageTypes, fmt.Sprintf("%s/%s", archName, imageTypeName)) + } + } + + expected := []string{ + "aarch64/qcow2", + "fakearch/qcow2", + "x86_64/qcow2", + } + + require.ElementsMatch(expected, archImageTypes) +} From 00b80509ce950f72b919c552f0394d3303d6457d Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Wed, 10 Jul 2024 15:36:48 +0200 Subject: [PATCH 07/14] otkdistro: add DistroFactory() Add the DistroFactory() function to otkdistro, which will enable loading distros defined using otk files alongside the internally defined ones using the same mechanism. The root of the otk path is hard-coded to "./otk", but we will have to make this configurable. --- pkg/otkdistro/otkdistro.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/otkdistro/otkdistro.go b/pkg/otkdistro/otkdistro.go index b80408ba47..ec8e9f62f9 100644 --- a/pkg/otkdistro/otkdistro.go +++ b/pkg/otkdistro/otkdistro.go @@ -6,6 +6,8 @@ import ( "path/filepath" "strings" + "github.com/sirupsen/logrus" + "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/manifest" @@ -291,3 +293,19 @@ func (t *otkImageType) Manifest(bp *blueprint.Blueprint, mf := manifest.NewOTK(t.otkPath) return mf, nil, nil } + +func DistroFactory(idStr string) distro.Distro { + otkRoot := "otk" // TODO: configurable path + otkDistroRoot := filepath.Join(otkRoot, idStr) + d, err := New(otkDistroRoot) + if err != nil { + if strings.HasPrefix(idStr, "otk") { + // NOTE: printing error only if the distro name is otk otherwise we + // get an error for every distro that's checked. When we move to + // having only otk-based distros, this will be a fatal error. + logrus.Errorf("failed to load otk distro at %q: %s", otkDistroRoot, err) + } + return nil + } + return d +} From 04fc22f9cd8274577e8100ed7fa46720ccc9bb6d Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Wed, 10 Jul 2024 15:39:24 +0200 Subject: [PATCH 08/14] distrofactory: add the otkdistro DistroFactory With this addition, for every file found in the repository configuration path (for this project, "test/data/repositories/" by default), the distrofactory will try to load an otk distro at "otk//". --- pkg/distrofactory/distrofactory.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/distrofactory/distrofactory.go b/pkg/distrofactory/distrofactory.go index 4e264b8a07..3a8a0c91d0 100644 --- a/pkg/distrofactory/distrofactory.go +++ b/pkg/distrofactory/distrofactory.go @@ -11,6 +11,7 @@ import ( "github.com/osbuild/images/pkg/distro/rhel/rhel8" "github.com/osbuild/images/pkg/distro/rhel/rhel9" "github.com/osbuild/images/pkg/distro/test_distro" + "github.com/osbuild/images/pkg/otkdistro" ) // FactoryFunc is a function that returns a distro.Distro for a given distro @@ -114,6 +115,7 @@ func NewDefault() *Factory { rhel8.DistroFactory, rhel9.DistroFactory, rhel10.DistroFactory, + otkdistro.DistroFactory, ) } From 81ffc41b57cc9ff27f853d6405cdabb66a44e5e0 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Thu, 11 Jul 2024 21:58:28 +0200 Subject: [PATCH 09/14] manifest: add Blueprint to OTKManifest When initialising an OTKManifest, add the Blueprint to it as well, so we can create variables (defines, modifications) based on it before compiling. --- pkg/manifest/otk.go | 7 +++++-- pkg/otkdistro/otkdistro.go | 6 +++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/manifest/otk.go b/pkg/manifest/otk.go index 0d5ff8b9eb..c6a9c1bff7 100644 --- a/pkg/manifest/otk.go +++ b/pkg/manifest/otk.go @@ -5,6 +5,7 @@ import ( "fmt" "os/exec" + "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/container" "github.com/osbuild/images/pkg/ostree" "github.com/osbuild/images/pkg/rpmmd" @@ -13,10 +14,12 @@ import ( type OTKManifest struct { // entrypoint yaml file for this otk manifest entrypoint string + + blueprint blueprint.Blueprint } -func NewOTK(path string) *OTKManifest { - return &OTKManifest{entrypoint: path} +func NewOTK(path string, bp blueprint.Blueprint) *OTKManifest { + return &OTKManifest{entrypoint: path, blueprint: bp} } func (m *OTKManifest) addPipeline(p Pipeline) {} diff --git a/pkg/otkdistro/otkdistro.go b/pkg/otkdistro/otkdistro.go index ec8e9f62f9..0d995506b3 100644 --- a/pkg/otkdistro/otkdistro.go +++ b/pkg/otkdistro/otkdistro.go @@ -290,7 +290,11 @@ func (t *otkImageType) Manifest(bp *blueprint.Blueprint, repos []rpmmd.RepoConfig, seed int64) (manifest.Manifest, []string, error) { - mf := manifest.NewOTK(t.otkPath) + if bp == nil { + bp = &blueprint.Blueprint{} + } + + mf := manifest.NewOTK(t.otkPath, *bp) return mf, nil, nil } From 8195be057b89ec75875fd553015772151ac825ef Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Thu, 11 Jul 2024 22:01:16 +0200 Subject: [PATCH 10/14] pkg: add otk for handling otk-related things New otk package for managing otk-related things. The otkCustomizationsFile type handles creating an otk file with defines for customizations and a single otk.include. The purpose is to use this to create a 'customizations.yaml' file that 'otk.define's all the variables represented by a blueprint then 'otk.include' the file we want to compile, effectively setting variables defined by the user in the compilation of the manifest. Currently it only writes a few simple customizations as a proof of concept, but it will grow to cover all potential customizations. --- pkg/otk/otk.go | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 pkg/otk/otk.go diff --git a/pkg/otk/otk.go b/pkg/otk/otk.go new file mode 100644 index 0000000000..a6a9ea77b9 --- /dev/null +++ b/pkg/otk/otk.go @@ -0,0 +1,90 @@ +package otk + +import ( + "fmt" + "os" + "path/filepath" + + "gopkg.in/yaml.v3" + + "github.com/osbuild/images/pkg/blueprint" +) + +type otkCustomizationsFile struct { + file *os.File +} + +type defines struct { + Hostname string `yaml:"hostname"` + KernelAppend string `yaml:"kernel_append"` + Languages []string `yaml:"languages"` + Keyboard string `yaml:"keyboard"` +} + +func bpToDefines(bp blueprint.Blueprint) defines { + d := defines{} + + customizations := bp.Customizations + if customizations == nil { + return d + } + + if customizations.Hostname != nil { + d.Hostname = *customizations.Hostname + } + + if customizations.Kernel != nil { + d.KernelAppend = customizations.Kernel.Append + } + + if customizations.Locale != nil { + d.Languages = customizations.Locale.Languages + if customizations.Locale.Keyboard != nil { + d.Keyboard = *customizations.Locale.Keyboard + } + } + + return d + +} + +func NewCustomizationsFile(bp blueprint.Blueprint, entrypoint string) (*otkCustomizationsFile, error) { + tmpfile, err := os.CreateTemp("", "otk-customizations") + if err != nil { + return nil, fmt.Errorf("failed to create temporary customizations file: %w", err) + } + defer tmpfile.Close() + + type cust struct { + Defines defines `yaml:"otk.define.customizations"` + Include string `yaml:"otk.include"` + } + absPath, err := filepath.Abs(entrypoint) + if err != nil { + return nil, fmt.Errorf("failed to get absolute path for %q: %w", entrypoint, err) + } + + c := cust{ + Defines: bpToDefines(bp), + Include: absPath, + } + + if err := yaml.NewEncoder(tmpfile).Encode(c); err != nil { + return nil, fmt.Errorf("failed to write temporary customizations file: %w", err) + } + + return &otkCustomizationsFile{ + file: tmpfile, + }, nil +} + +func (f *otkCustomizationsFile) Path() string { + return f.file.Name() +} + +func (f *otkCustomizationsFile) Cleanup() error { + if err := os.Remove(f.file.Name()); err != nil { + return fmt.Errorf("failed to remove temporary customizations file: %w", err) + } + return nil +} From a740db5480c5139b47cf4d59e9aee37032829ec2 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Thu, 11 Jul 2024 22:19:58 +0200 Subject: [PATCH 11/14] manifest: create customizations file when serializing When serializing an OTKManifest, create a customizations file that includes 'otk.define's to represent the options of a blueprint, then 'otk.include' the original entrypoint to generate a manifest with the user customizations included. --- pkg/manifest/otk.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/manifest/otk.go b/pkg/manifest/otk.go index c6a9c1bff7..fb40b21a1f 100644 --- a/pkg/manifest/otk.go +++ b/pkg/manifest/otk.go @@ -8,6 +8,7 @@ import ( "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/container" "github.com/osbuild/images/pkg/ostree" + "github.com/osbuild/images/pkg/otk" "github.com/osbuild/images/pkg/rpmmd" ) @@ -37,13 +38,18 @@ func (m *OTKManifest) GetOSTreeSourceSpecs() map[string][]ostree.SourceSpec { } func (m *OTKManifest) Serialize(_ map[string][]rpmmd.PackageSpec, _ map[string][]container.Spec, _ map[string][]ostree.CommitSpec, _ map[string][]rpmmd.RepoConfig) (OSBuildManifest, error) { - cmd := exec.Command("otk", "compile", "--target=osbuild", m.entrypoint) + customizations, err := otk.NewCustomizationsFile(m.blueprint, m.entrypoint) + if err != nil { + return nil, err + } + defer customizations.Cleanup() + + cmd := exec.Command("otk", "compile", "--target=osbuild", customizations.Path()) var stdoutBuffer bytes.Buffer var stderrBuffer bytes.Buffer cmd.Stdout = &stdoutBuffer cmd.Stderr = &stderrBuffer - err := cmd.Run() - if err != nil { + if err := cmd.Run(); err != nil { return nil, fmt.Errorf("compiling omnifest: %v %s", err, stderrBuffer.String()) } From dd3d40e4139bb9183f69ed4bb16678726a35fa9f Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 12 Jul 2024 14:00:07 +0200 Subject: [PATCH 12/14] otk: add test for customizations file creation Define the expected file contents as a map[string]interface{} and deserialize the file contents before comparing for convenience and nicer error output (compared to strings). --- pkg/otk/otk_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 pkg/otk/otk_test.go diff --git a/pkg/otk/otk_test.go b/pkg/otk/otk_test.go new file mode 100644 index 0000000000..4a5579b454 --- /dev/null +++ b/pkg/otk/otk_test.go @@ -0,0 +1,122 @@ +package otk_test + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/osbuild/images/internal/common" + "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/otk" + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" +) + +func readFile(path string) map[string]interface{} { + file, err := os.Open(path) + if err != nil { + panic(fmt.Sprintf("failed to open file %q: %s", path, err)) + } + defer file.Close() + + ds := map[string]interface{}{} + if err := yaml.NewDecoder(file).Decode(&ds); err != nil { + panic(fmt.Sprintf("error deserializing customizations file for test: %s", err)) + } + + return ds + +} + +func TestNewCustomizationsFile(t *testing.T) { + type testCase struct { + bp blueprint.Blueprint + entrypoint string // entrypoint filename + expected map[string]interface{} // expected file contents (deserialized from yaml) + err error // expected error (nil if happy) + } + + cwd, err := os.Getwd() + if err != nil { + panic(fmt.Sprintf("error getting working directory for test: %s", err)) + } + + testCases := map[string]testCase{ + "empty-and-happy-abspath": { + bp: blueprint.Blueprint{}, + entrypoint: "/tmp/test.yaml", + expected: map[string]interface{}{ + "otk.define.customizations": map[string]interface{}{ + "hostname": "", + "kernel_append": "", + "languages": []interface{}{}, + "keyboard": "", + }, + "otk.include": "/tmp/test.yaml", + }, + }, + "empty-and-happy-relpath": { + bp: blueprint.Blueprint{}, + entrypoint: "./rel/test.yaml", + expected: map[string]interface{}{ + "otk.define.customizations": map[string]interface{}{ + "hostname": "", + "kernel_append": "", + "languages": []interface{}{}, + "keyboard": "", + }, + "otk.include": filepath.Join(cwd, "rel/test.yaml"), + }, + }, + "full-and-happy": { + bp: blueprint.Blueprint{ + Customizations: &blueprint.Customizations{ + Hostname: common.ToPtr("test-host"), + Kernel: &blueprint.KernelCustomization{ + Append: "ro", + }, + Locale: &blueprint.LocaleCustomization{ + Languages: []string{"de_DE.UTF-8", "el_CY.UTF-8"}, + Keyboard: common.ToPtr("uk"), + }, + }, + Distro: "", + Minimal: false, + }, + entrypoint: "/etc/otk/test.yaml", + expected: map[string]interface{}{ + "otk.define.customizations": map[string]interface{}{ + "hostname": "test-host", + "kernel_append": "ro", + "languages": []interface{}{"de_DE.UTF-8", "el_CY.UTF-8"}, + "keyboard": "uk", + }, + "otk.include": "/etc/otk/test.yaml", + }, + }, + } + + for name := range testCases { + tc := testCases[name] + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + cfile, err := otk.NewCustomizationsFile(tc.bp, tc.entrypoint) + defer func() { + err := cfile.Cleanup() + if err != nil { + panic(fmt.Sprintf("error cleaning up customization file %q: %s", cfile.Path(), err)) + } + }() + if tc.err == nil { + // happy + assert.NoError(err) + assert.Equal(tc.expected, readFile(cfile.Path())) + } else { + // error + assert.Error(err) + assert.Equal(tc.err, err) + } + }) + } +} From efd019abeb7cc5427809f70829ae7843e5f0d10d Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 12 Jul 2024 14:48:47 +0200 Subject: [PATCH 13/14] manifest: test otk serialization Add tests for otk serialization with some simple blueprint customizations. The entrypoint contents and expected serialized manifests are defined as map[string]interface for convenience and nicer error output (compared to strings). --- pkg/manifest/otk_test.go | 143 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 pkg/manifest/otk_test.go diff --git a/pkg/manifest/otk_test.go b/pkg/manifest/otk_test.go new file mode 100644 index 0000000000..8826d951b0 --- /dev/null +++ b/pkg/manifest/otk_test.go @@ -0,0 +1,143 @@ +package manifest_test + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" + + "gopkg.in/yaml.v3" + + "github.com/osbuild/images/pkg/blueprint" + "github.com/osbuild/images/pkg/manifest" + "github.com/stretchr/testify/assert" +) + +func writeEntrypoint(data map[string]interface{}, path string) error { + file, err := os.Create(path) + if err != nil { + return fmt.Errorf("failed to create entrypoint file %q: %w", path, err) + } + defer func() { + if err := file.Close(); err != nil { + panic(fmt.Sprintf("error closing entrypoint file %q: %s", path, err)) + } + }() + + if err := yaml.NewEncoder(file).Encode(data); err != nil { + return fmt.Errorf("failed to write entrypoint file %q: %w", path, err) + } + return nil +} + +func deserializeManifest(data []byte) map[string]interface{} { + ds := map[string]interface{}{} + + if err := json.NewDecoder(bytes.NewReader(data)).Decode(&ds); err != nil { + panic(fmt.Sprintf("error deserializing manifest for test: %s", err)) + } + + return ds +} + +func TestOTKSerialize(t *testing.T) { + type testCase struct { + otkEntrypoint map[string]interface{} // entrypoint contents + bp blueprint.Blueprint + manifest map[string]interface{} // expected manifest (deserialized into map) + err error // expected error (nil if happy) + } + + testCases := map[string]testCase{ + "happy-empty": { + otkEntrypoint: map[string]interface{}{ + "otk.version": "1", + "otk.target.osbuild": map[string]interface{}{ + "pipelines": nil, + }, + }, + manifest: map[string]interface{}{ + "version": "2", + "pipelines": nil, + }, + }, + + "happy-customized": { + otkEntrypoint: map[string]interface{}{ + "otk.version": "1", + "otk.target.osbuild": map[string]interface{}{ + "pipelines": []interface{}{ + map[string]interface{}{ + // using the kernel append customization to test + // that variables are resolved; the location of the + // variable is not important + "name": "${kernel_append}", + }, + }, + }, + }, + bp: blueprint.Blueprint{ + Customizations: &blueprint.Customizations{ + Kernel: &blueprint.KernelCustomization{ + Append: "customization-value", + }, + }, + }, + manifest: map[string]interface{}{ + "version": "2", + "pipelines": []interface{}{ + map[string]interface{}{ + "name": "customization-value", + }, + }, + }, + }, + + "happy-empty-var": { + otkEntrypoint: map[string]interface{}{ + "otk.version": "1", + "otk.target.osbuild": map[string]interface{}{ + "pipelines": []interface{}{ + map[string]interface{}{ + "name": "${kernel_append}", + }, + }, + }, + }, + manifest: map[string]interface{}{ + "version": "2", + "pipelines": []interface{}{ + map[string]interface{}{ + "name": "", + }, + }, + }, + }, + } + + tmpRoot := t.TempDir() + + for name := range testCases { + tc := testCases[name] + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + entrypointPath := filepath.Join(tmpRoot, name+".yaml") + assert.NoError(writeEntrypoint(tc.otkEntrypoint, entrypointPath)) + + otkManifest := manifest.NewOTK(entrypointPath, tc.bp) + serialized, err := otkManifest.Serialize(nil, nil, nil, nil) + if tc.err == nil { + // happy + assert.NoError(err) + assert.Equal(tc.manifest, deserializeManifest(serialized)) + } else { + // error + assert.Error(err) + assert.Equal(tc.err, err) + } + }) + } +} From 4eab381703b1d050acc74056017bd866aed99fcd Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Mon, 15 Jul 2024 20:15:21 +0200 Subject: [PATCH 14/14] otkdistro: create test distro dynamically Remove test/otk/fakedistro and instead create it when running the unit test. This will make it easier to evolve the test to cover more features. --- pkg/otkdistro/otkdistro_test.go | 95 ++++++++++++++++++--- test/data/otk/fakedistro/aarch64/qcow2.yml | 13 --- test/data/otk/fakedistro/fakearch/qcow2.yml | 13 --- test/data/otk/fakedistro/properties.yaml | 5 -- test/data/otk/fakedistro/x86_64/qcow2.yml | 13 --- 5 files changed, 85 insertions(+), 54 deletions(-) delete mode 100644 test/data/otk/fakedistro/aarch64/qcow2.yml delete mode 100644 test/data/otk/fakedistro/fakearch/qcow2.yml delete mode 100644 test/data/otk/fakedistro/properties.yaml delete mode 100644 test/data/otk/fakedistro/x86_64/qcow2.yml diff --git a/pkg/otkdistro/otkdistro_test.go b/pkg/otkdistro/otkdistro_test.go index a36d527a6f..b1b12fc98c 100644 --- a/pkg/otkdistro/otkdistro_test.go +++ b/pkg/otkdistro/otkdistro_test.go @@ -2,23 +2,104 @@ package otkdistro_test import ( "fmt" + "os" + "path/filepath" "testing" + "gopkg.in/yaml.v3" + "github.com/osbuild/images/pkg/otkdistro" "github.com/stretchr/testify/require" ) +func makeFakeDistro(root string) ([]string, error) { + arches := []string{ + "aarch64", + "fakearch", + "x86_64", + } + + imageTypes := []string{ + "qcow2", + "container", + } + + props := map[string]interface{}{ + "name": "FakeDistro", + "os_version": "42.0", + "release_version": "42", + "runner": "org.osbuild.fakedistro42", + } + propsfile, err := os.Create(filepath.Join(root, "properties.yaml")) + if err != nil { + return nil, fmt.Errorf("error creating properties file for test distro: %w", err) + } + if err := yaml.NewEncoder(propsfile).Encode(props); err != nil { + return nil, fmt.Errorf("error writing properties file for test distro: %w", err) + } + + // make all otk files have the same content for now + otkcontent := map[string]interface{}{ + "otk.version": "1", + "otk.target.osbuild.name": map[string]interface{}{ + "pipelines": []map[string]interface{}{ + { + "name": "build", + "stages": []map[string]interface{}{ + { + "type": "org.osbuild.rpm", + "options": nil, + }, + }, + }, + { + "name": "os", + "stages": []map[string]interface{}{ + { + "type": "org.osbuild.rpm", + "options": nil, + }, + }, + }, + }, + }, + } + + expected := make([]string, 0, len(arches)*len(imageTypes)) + for _, arch := range arches { + archpath := filepath.Join(root, arch) + if err := os.Mkdir(archpath, 0o777); err != nil { + return nil, fmt.Errorf("error creating architecture directory for test distro %q: %w", archpath, err) + } + for _, imageType := range imageTypes { + itpath := filepath.Join(archpath, imageType) + ".yaml" + itfile, err := os.Create(itpath) + if err != nil { + return nil, fmt.Errorf("error creating image type file for test distro %q: %w", itpath, err) + } + if err := yaml.NewEncoder(itfile).Encode(otkcontent); err != nil { + return nil, fmt.Errorf("error writing image type file for test distro %q: %w", itpath, err) + } + expected = append(expected, fmt.Sprintf("%s/%s", arch, imageType)) + } + } + + return expected, nil +} + func TestDistroLoad(t *testing.T) { require := require.New(t) - // TODO: we can write the fragments during the test setup and make the - // whole test self-contained - distro, err := otkdistro.New("../../test/data/otk/fakedistro") + distroRoot := t.TempDir() + expected, err := makeFakeDistro(distroRoot) + require.NoError(err) + + distro, err := otkdistro.New(distroRoot) require.NoError(err) require.Equal("FakeDistro", distro.Name()) require.Equal("42.0", distro.OsVersion()) require.Equal("42", distro.Releasever()) - // TODO: check runner when it's added + // TODO: check runner when it's added to the distro interface archImageTypes := make([]string, 0) for _, archName := range distro.ListArches() { @@ -30,11 +111,5 @@ func TestDistroLoad(t *testing.T) { } } - expected := []string{ - "aarch64/qcow2", - "fakearch/qcow2", - "x86_64/qcow2", - } - require.ElementsMatch(expected, archImageTypes) } diff --git a/test/data/otk/fakedistro/aarch64/qcow2.yml b/test/data/otk/fakedistro/aarch64/qcow2.yml deleted file mode 100644 index 37b17b67f7..0000000000 --- a/test/data/otk/fakedistro/aarch64/qcow2.yml +++ /dev/null @@ -1,13 +0,0 @@ ---- -otk.version: "1" - -otk.target.osbuild.name: - pipelines: - - name: "build" - stages: - - type: "org.osbuild.rpm" - options: - - name: "os" - stages: - - type: "org.osbuild.rpm" - options: diff --git a/test/data/otk/fakedistro/fakearch/qcow2.yml b/test/data/otk/fakedistro/fakearch/qcow2.yml deleted file mode 100644 index 37b17b67f7..0000000000 --- a/test/data/otk/fakedistro/fakearch/qcow2.yml +++ /dev/null @@ -1,13 +0,0 @@ ---- -otk.version: "1" - -otk.target.osbuild.name: - pipelines: - - name: "build" - stages: - - type: "org.osbuild.rpm" - options: - - name: "os" - stages: - - type: "org.osbuild.rpm" - options: diff --git a/test/data/otk/fakedistro/properties.yaml b/test/data/otk/fakedistro/properties.yaml deleted file mode 100644 index 872b734109..0000000000 --- a/test/data/otk/fakedistro/properties.yaml +++ /dev/null @@ -1,5 +0,0 @@ ---- -name: "FakeDistro" -os_version: "42.0" -release_version: "42" -runner: "org.osbuild.fakedistro42" diff --git a/test/data/otk/fakedistro/x86_64/qcow2.yml b/test/data/otk/fakedistro/x86_64/qcow2.yml deleted file mode 100644 index 37b17b67f7..0000000000 --- a/test/data/otk/fakedistro/x86_64/qcow2.yml +++ /dev/null @@ -1,13 +0,0 @@ ---- -otk.version: "1" - -otk.target.osbuild.name: - pipelines: - - name: "build" - stages: - - type: "org.osbuild.rpm" - options: - - name: "os" - stages: - - type: "org.osbuild.rpm" - options: