From 63df312cfaf17854b7ce2ce13a625f701a411758 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 Jan 2025 07:44:58 +0100 Subject: [PATCH] osbuild: add rpmDownloader to `GenSources()` argument This commit enables to select what rpm downloader backend to use. The signature of GenSources() is just expanded to take an additonal parameter. It would be nice to re-use the `manifest.Inputs` struct but it is (subtly) different from what GenSource needs :/ I was also tempated to add a "Options" struct to GenSources instead of the enum but YAGNI - if we ever need to add another one this will become a options struct. --- pkg/manifest/manifest.go | 21 ++++++--------------- pkg/osbuild/source.go | 28 +++++++++++++++++++++------- pkg/osbuild/source_test.go | 8 ++++---- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go index 940aec1ffd..fa7b2cdec1 100644 --- a/pkg/manifest/manifest.go +++ b/pkg/manifest/manifest.go @@ -41,15 +41,6 @@ const ( DISTRO_FEDORA ) -// RpmDownloader specifies what backend to use for rpm downloads -// Note that the librepo backend requires a newer osbuild. -type RpmDownloader uint64 - -const ( - RpmDownloaderCurl = iota - RpmDownloaderLibrepo = iota -) - // Inputs specifies the inputs for manifest instantiating type Inputs struct { PackageSets map[string][]rpmmd.PackageSpec @@ -58,11 +49,6 @@ type Inputs struct { RpmRepos map[string][]rpmmd.RepoConfig } -// Options contains the (optional) configs for the manifest instantiating -type Options struct { - RpmDownloader RpmDownloader -} - // An OSBuildManifest is an opaque JSON object, which is a valid input to osbuild type OSBuildManifest []byte @@ -171,6 +157,11 @@ func (m Manifest) Serialize(packageSets map[string][]rpmmd.PackageSpec, containe return m.SerializeFull(inputs, nil) } +// Options contains the (optional) configs for the manifest instantiating +type Options struct { + RpmDownloader osbuild.RpmDownloader +} + // TODO: rename to Serialize() onces callers are fixed func (m Manifest) SerializeFull(inputs *Inputs, opts *Options) (OSBuildManifest, error) { if opts == nil { @@ -195,7 +186,7 @@ func (m Manifest) SerializeFull(inputs *Inputs, opts *Options) (OSBuildManifest, pipeline.serializeEnd() } - sources, err := osbuild.GenSources(packages, commits, inline, containers, inputs.RpmRepos) + sources, err := osbuild.GenSources(packages, commits, inline, containers, inputs.RpmRepos, opts.RpmDownloader) if err != nil { return nil, err } diff --git a/pkg/osbuild/source.go b/pkg/osbuild/source.go index 64ae896db5..209a26208d 100644 --- a/pkg/osbuild/source.go +++ b/pkg/osbuild/source.go @@ -3,7 +3,7 @@ package osbuild import ( "encoding/json" "errors" - "os" + "fmt" "github.com/osbuild/images/pkg/container" "github.com/osbuild/images/pkg/ostree" @@ -79,18 +79,32 @@ func addPackagesLibrepo(sources Sources, packages []rpmmd.PackageSpec, rpmRepos return nil } -func GenSources(packages []rpmmd.PackageSpec, ostreeCommits []ostree.CommitSpec, inlineData []string, containers []container.Spec, rpmRepos map[string][]rpmmd.RepoConfig) (Sources, error) { +// RpmDownloader specifies what backend to use for rpm downloads +// Note that the librepo backend requires a newer osbuild. +type RpmDownloader uint64 + +const ( + RpmDownloaderCurl = iota + RpmDownloaderLibrepo = iota +) + +func GenSources(packages []rpmmd.PackageSpec, ostreeCommits []ostree.CommitSpec, inlineData []string, containers []container.Spec, rpmRepos map[string][]rpmmd.RepoConfig, rpmDownloader RpmDownloader) (Sources, error) { + // The signature of this functionis already relatively long, + // if we need to add more options, refactor into "struct + // Inputs" (rpm,ostree,etc) and "struct Options" + // (rpmDownloader) sources := Sources{} // collect rpm package sources if len(packages) > 0 { - // XXX: hack, we have no good way to pass options to GenSource - // right now var err error - if s := os.Getenv("OSBUILD_USE_LIBREPO"); s == "1" { - err = addPackagesLibrepo(sources, packages, rpmRepos) - } else { + switch rpmDownloader { + case RpmDownloaderCurl: err = addPackagesCurl(sources, packages) + case RpmDownloaderLibrepo: + err = addPackagesLibrepo(sources, packages, rpmRepos) + default: + err = fmt.Errorf("unknown rpm downloader %v", rpmDownloader) } if err != nil { return nil, err diff --git a/pkg/osbuild/source_test.go b/pkg/osbuild/source_test.go index 8e16cb3428..aa4f612ee5 100644 --- a/pkg/osbuild/source_test.go +++ b/pkg/osbuild/source_test.go @@ -119,7 +119,7 @@ func TestSource_UnmarshalJSON(t *testing.T) { } func TestGenSourcesTrivial(t *testing.T) { - sources, err := GenSources(nil, nil, nil, nil, nil) + sources, err := GenSources(nil, nil, nil, nil, nil, 0) assert.NoError(t, err) jsonOutput, err := json.MarshalIndent(sources, "", " ") @@ -135,7 +135,7 @@ func TestGenSourcesContainerStorage(t *testing.T) { LocalStorage: true, }, } - sources, err := GenSources(nil, nil, nil, containers, nil) + sources, err := GenSources(nil, nil, nil, containers, nil, 0) assert.NoError(t, err) jsonOutput, err := json.MarshalIndent(sources, "", " ") @@ -159,7 +159,7 @@ func TestGenSourcesSkopeo(t *testing.T) { ImageID: imageID, }, } - sources, err := GenSources(nil, nil, nil, containers, nil) + sources, err := GenSources(nil, nil, nil, containers, nil, 0) assert.NoError(t, err) jsonOutput, err := json.MarshalIndent(sources, "", " ") @@ -190,7 +190,7 @@ func TestGenSourcesWithSkopeoIndex(t *testing.T) { ImageID: imageID, }, } - sources, err := GenSources(nil, nil, nil, containers, nil) + sources, err := GenSources(nil, nil, nil, containers, nil, 0) assert.NoError(t, err) jsonOutput, err := json.MarshalIndent(sources, "", " ")