From a162e8d58917255c72662d04ad3b4aca40a74189 Mon Sep 17 00:00:00 2001 From: Simon de Vlieger Date: Wed, 24 Jun 2026 12:03:19 +0200 Subject: [PATCH 1/5] many: rename `overrideRepos` to `forceRepos` Let's be consistent and rename override to force in manifestgen and all callers of it. Signed-off-by: Simon de Vlieger --- cmd/image-builder/bib_main.go | 2 +- cmd/image-builder/manifest.go | 2 +- cmd/image-builder/repos.go | 2 +- pkg/distro/generic/bootc_test.go | 2 +- pkg/manifestgen/manifestgen.go | 12 ++++++------ pkg/manifestgen/manifestgen_test.go | 12 ++++++------ 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cmd/image-builder/bib_main.go b/cmd/image-builder/bib_main.go index c716d4afc8..d645fd663b 100644 --- a/cmd/image-builder/bib_main.go +++ b/cmd/image-builder/bib_main.go @@ -179,7 +179,7 @@ func bibManifestFromCobra(cmd *cobra.Command, args []string, pbar progress.Progr // XXX: hack to skip repo loading for the bootc image. // We need to add a SkipRepositories or similar to // manifestgen instead to make this clean - OverrideRepos: []rpmmd.RepoConfig{ + ForceRepos: []rpmmd.RepoConfig{ { BaseURLs: []string{"https://example.com/not-used"}, }, diff --git a/cmd/image-builder/manifest.go b/cmd/image-builder/manifest.go index 90b4c13215..a203c2281d 100644 --- a/cmd/image-builder/manifest.go +++ b/cmd/image-builder/manifest.go @@ -80,7 +80,7 @@ func generateManifest(repoDir string, extraRepos []string, img *imagefilter.Resu if err != nil { return err } - manifestGenOpts.OverrideRepos = forcedRepos + manifestGenOpts.ForceRepos = forcedRepos } if opts.IgnoreWarnings { manifestGenOpts.WarningsOutput = os.Stderr diff --git a/cmd/image-builder/repos.go b/cmd/image-builder/repos.go index a82f1f455d..7791bd7e61 100644 --- a/cmd/image-builder/repos.go +++ b/cmd/image-builder/repos.go @@ -83,7 +83,7 @@ func newRepoRegistryImpl(repoDir string, extraRepos []string) (*reporegistry.Rep // // XXX: this should probably go into manifestgen.Options as a // new Options.ExtraRepoConf eventually (just like - // OverrideRepos) + // ForceRepos) repoConf, err := parseRepoURLs(extraRepos, "extra") if err != nil { return nil, err diff --git a/pkg/distro/generic/bootc_test.go b/pkg/distro/generic/bootc_test.go index f0ff05147e..0faf52856f 100644 --- a/pkg/distro/generic/bootc_test.go +++ b/pkg/distro/generic/bootc_test.go @@ -1070,7 +1070,7 @@ func genManifest(t *testing.T, imgType distro.ImageType) string { var bp blueprint.Blueprint mg, err := manifestgen.New(nil, &manifestgen.Options{ - OverrideRepos: []rpmmd.RepoConfig{ + ForceRepos: []rpmmd.RepoConfig{ {Id: "not-used", BaseURLs: []string{"not-used"}}, }, }) diff --git a/pkg/manifestgen/manifestgen.go b/pkg/manifestgen/manifestgen.go index 02795ecbf8..a18bdaf2f4 100644 --- a/pkg/manifestgen/manifestgen.go +++ b/pkg/manifestgen/manifestgen.go @@ -69,9 +69,9 @@ type Options struct { // useful for testing CustomSeed *int64 - // OverrideRepos overrides the default repository selection. + // ForceRepos overrides the default repository selection. // This is mostly useful for testing - OverrideRepos []rpmmd.RepoConfig + ForceRepos []rpmmd.RepoConfig // Custom "solver" functions, if unset the defaults will be // used. Only needed for specialized use-cases. @@ -105,7 +105,7 @@ type Generator struct { rpmDownloader osbuild.RpmDownloader customSeed *int64 - overrideRepos []rpmmd.RepoConfig + forceRepos []rpmmd.RepoConfig useBootstrapContainer bool rpmlistWriter RPMListWriterFunc @@ -128,7 +128,7 @@ func New(reporegistry *reporegistry.RepoRegistry, opts *Options) (*Generator, er warningsOutput: opts.WarningsOutput, depsolveWarningsOutput: opts.DepsolveWarningsOutput, customSeed: opts.CustomSeed, - overrideRepos: opts.OverrideRepos, + forceRepos: opts.ForceRepos, useBootstrapContainer: opts.UseBootstrapContainer, rpmlistWriter: opts.RPMListWriter, } @@ -168,8 +168,8 @@ func (mg *Generator) Generate(bp *blueprint.Blueprint, imgType distro.ImageType, dist := a.Distro() var repos []rpmmd.RepoConfig - if mg.overrideRepos != nil { - repos = mg.overrideRepos + if mg.forceRepos != nil { + repos = mg.forceRepos } else { var err error repos, err = mg.reporegistry.ReposByImageTypeName(dist.Name(), a.Name(), imgType.Name()) diff --git a/pkg/manifestgen/manifestgen_test.go b/pkg/manifestgen/manifestgen_test.go index acd29b3975..2da0da8653 100644 --- a/pkg/manifestgen/manifestgen_test.go +++ b/pkg/manifestgen/manifestgen_test.go @@ -363,7 +363,7 @@ func TestManifestGeneratorDepsolveOutput(t *testing.T) { assert.Equal(t, []byte("fake depsolve output"), depsolveWarningsOutput.Bytes()) } -func TestManifestGeneratorOverrideRepos(t *testing.T) { +func TestManifestGeneratorForceRepos(t *testing.T) { repos, err := testrepos.New() assert.NoError(t, err) fac := distrofactory.NewDefault() @@ -374,13 +374,13 @@ func TestManifestGeneratorOverrideRepos(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(res)) - for _, withOverrideRepos := range []bool{false, true} { - t.Run(fmt.Sprintf("withOverrideRepos: %v", withOverrideRepos), func(t *testing.T) { + for _, withForceRepos := range []bool{false, true} { + t.Run(fmt.Sprintf("withForceRepos: %v", withForceRepos), func(t *testing.T) { opts := &manifestgen.Options{ Depsolve: fakeDepsolve, } - if withOverrideRepos { - opts.OverrideRepos = []rpmmd.RepoConfig{ + if withForceRepos { + opts.ForceRepos = []rpmmd.RepoConfig{ { Name: "overriden_repo", BaseURLs: []string{"http://example.com/overriden-repo"}, @@ -394,7 +394,7 @@ func TestManifestGeneratorOverrideRepos(t *testing.T) { var bp blueprint.Blueprint osbuildManifest, err := mg.Generate(&bp, res[0].ImgType, nil) assert.NoError(t, err) - if withOverrideRepos { + if withForceRepos { assert.Contains(t, string(osbuildManifest), "http://example.com/overriden-repo/") } else { assert.NotContains(t, string(osbuildManifest), "http://example.com/overriden-repo/") From 0367c45a3135d0080077de2d7e677e2e5e3e970b Mon Sep 17 00:00:00 2001 From: Simon de Vlieger Date: Wed, 24 Jun 2026 12:18:40 +0200 Subject: [PATCH 2/5] pkg/reporegistry: append repos Allow for the appending of repositories on a reporegistry. Also offer a useful ListArches function to get all the available arches for a distribution. Signed-off-by: Simon de Vlieger --- pkg/manifestgen/manifestgen.go | 2 +- pkg/reporegistry/reporegistry.go | 25 ++++++++++++++++ pkg/reporegistry/reporegistry_test.go | 43 +++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/pkg/manifestgen/manifestgen.go b/pkg/manifestgen/manifestgen.go index a18bdaf2f4..2d86309559 100644 --- a/pkg/manifestgen/manifestgen.go +++ b/pkg/manifestgen/manifestgen.go @@ -104,7 +104,7 @@ type Generator struct { rpmDownloader osbuild.RpmDownloader - customSeed *int64 + customSeed *int64 forceRepos []rpmmd.RepoConfig useBootstrapContainer bool diff --git a/pkg/reporegistry/reporegistry.go b/pkg/reporegistry/reporegistry.go index 5076a4505d..aeb41073e2 100644 --- a/pkg/reporegistry/reporegistry.go +++ b/pkg/reporegistry/reporegistry.go @@ -126,6 +126,31 @@ func (r *RepoRegistry) DistroHasRepos(distro, arch string) ([]rpmmd.RepoConfig, return r.reposByDistroArch(distro, arch) } +// AppendRepos appends the given repos to the entry for the specified +// distro and arch. If the distro/arch combination does not exist in +// the registry the repos are silently ignored. +func (r *RepoRegistry) AppendRepos(distro, arch string, repos ...rpmmd.RepoConfig) { + if archMap, ok := r.repos[distro]; ok { + if _, ok := archMap[arch]; ok { + archMap[arch] = append(archMap[arch], repos...) + } + } +} + +// ListArches returns a list of all architectures which have a +// repository defined for the given distro. +func (r *RepoRegistry) ListArches(distro string) []string { + archMap, ok := r.repos[distro] + if !ok { + return nil + } + arches := make([]string, 0, len(archMap)) + for arch := range archMap { + arches = append(arches, arch) + } + return arches +} + // ListDistros returns a list of all distros which have a repository defined // in the registry. func (r *RepoRegistry) ListDistros() []string { diff --git a/pkg/reporegistry/reporegistry_test.go b/pkg/reporegistry/reporegistry_test.go index f8670a27ef..b1c76904ec 100644 --- a/pkg/reporegistry/reporegistry_test.go +++ b/pkg/reporegistry/reporegistry_test.go @@ -353,3 +353,46 @@ func TestInvalidReposByArchName(t *testing.T) { }) } } + +func TestAppendRepos(t *testing.T) { + rr := getTestingRepoRegistry() + testDistro := test_distro.DistroFactory(test_distro.TestDistro1Name) + + repos, err := rr.ReposByArchName(testDistro.Name(), test_distro.TestArchName, false) + assert.NoError(t, err) + assert.Len(t, repos, 2) + + rr.AppendRepos(testDistro.Name(), test_distro.TestArchName, rpmmd.RepoConfig{ + Name: "extra", + BaseURLs: []string{"https://example.com/extra"}, + }) + + repos, err = rr.ReposByArchName(testDistro.Name(), test_distro.TestArchName, false) + assert.NoError(t, err) + assert.Len(t, repos, 3) + assert.Equal(t, "extra", repos[2].Name) +} + +func TestAppendReposIgnoresUnknown(t *testing.T) { + rr := getTestingRepoRegistry() + + rr.AppendRepos("no-such-distro", "no-such-arch", rpmmd.RepoConfig{ + Name: "extra", + }) +} + +func TestListArches(t *testing.T) { + rr := getTestingRepoRegistry() + testDistro := test_distro.DistroFactory(test_distro.TestDistro1Name) + + arches := rr.ListArches(testDistro.Name()) + assert.Len(t, arches, 2) + assert.ElementsMatch(t, []string{test_distro.TestArchName, test_distro.TestArch2Name}, arches) +} + +func TestListArchesUnknownDistro(t *testing.T) { + rr := getTestingRepoRegistry() + + arches := rr.ListArches("no-such-distro") + assert.Nil(t, arches) +} From 492df500353b90144fef65c7044320d3f406eba0 Mon Sep 17 00:00:00 2001 From: Simon de Vlieger Date: Wed, 24 Jun 2026 12:19:36 +0200 Subject: [PATCH 3/5] cmd/image-builder: use append repos We had a comment talking about moving this to the reporegistry. I've implemented it in a slightly different way in the previous commit but let's make use of it. The behavior is covered by tests already. Signed-off-by: Simon de Vlieger --- cmd/image-builder/repos.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cmd/image-builder/repos.go b/cmd/image-builder/repos.go index 7791bd7e61..026e20b26d 100644 --- a/cmd/image-builder/repos.go +++ b/cmd/image-builder/repos.go @@ -71,7 +71,7 @@ func newRepoRegistryImpl(repoDir string, extraRepos []string) (*reporegistry.Rep builtins = []fs.FS{repos.FS} } - conf, err := reporegistry.LoadAllRepositories(repoDirs, builtins) + reg, err := reporegistry.New(repoDirs, builtins) if err != nil { return nil, err } @@ -88,15 +88,13 @@ func newRepoRegistryImpl(repoDir string, extraRepos []string) (*reporegistry.Rep if err != nil { return nil, err } - for _, repoArchConfigs := range conf { - for arch := range repoArchConfigs { - archCfg := repoArchConfigs[arch] - archCfg = append(archCfg, repoConf...) - repoArchConfigs[arch] = archCfg + for _, distro := range reg.ListDistros() { + for _, arch := range reg.ListArches(distro) { + reg.AppendRepos(distro, arch, repoConf...) } } - return reporegistry.NewFromDistrosRepoConfigs(conf), nil + return reg, nil } // this is a variable to make it overridable in tests From 6d6be32c3e5c912472c08135ced81df88ca1e2f4 Mon Sep 17 00:00:00 2001 From: Simon de Vlieger Date: Wed, 24 Jun 2026 12:33:30 +0200 Subject: [PATCH 4/5] cmd/image-builder: distro/arch for repo parsing Include the distribution and architecture in calls to `parseRepoURLs` and pass them along. This means that `parseRepoURLs` gets called a bunch more but that's OK they currently contain single strings. This is necessary because in the future we want to support (for example) COPR lookups where we'll be using the COPR API to get the correct information to create a `RepoConfig`. For this we need access to the additional information to find the correct repository for a given image type. Signed-off-by: Simon de Vlieger --- cmd/image-builder/manifest.go | 3 ++- cmd/image-builder/repos.go | 10 +++++----- cmd/image-builder/repos_test.go | 6 +++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/image-builder/manifest.go b/cmd/image-builder/manifest.go index a203c2281d..c9d66945d1 100644 --- a/cmd/image-builder/manifest.go +++ b/cmd/image-builder/manifest.go @@ -76,7 +76,8 @@ func generateManifest(repoDir string, extraRepos []string, img *imagefilter.Resu } } if len(opts.ForceRepos) > 0 { - forcedRepos, err := parseRepoURLs(opts.ForceRepos, "forced") + forcedRepos, err := parseRepoURLs(opts.ForceRepos, "forced", + img.ImgType.Arch().Distro().Name(), img.ImgType.Arch().Name()) if err != nil { return err } diff --git a/cmd/image-builder/repos.go b/cmd/image-builder/repos.go index 026e20b26d..b9c5b28875 100644 --- a/cmd/image-builder/repos.go +++ b/cmd/image-builder/repos.go @@ -21,7 +21,7 @@ var defaultRepoDirs = []string{ "/usr/share/image-builder/repositories", } -func parseRepoURLs(repoURLs []string, what string) ([]rpmmd.RepoConfig, error) { +func parseRepoURLs(repoURLs []string, what, distroName, archName string) ([]rpmmd.RepoConfig, error) { var repoConf []rpmmd.RepoConfig for i, repoURL := range repoURLs { @@ -84,12 +84,12 @@ func newRepoRegistryImpl(repoDir string, extraRepos []string) (*reporegistry.Rep // XXX: this should probably go into manifestgen.Options as a // new Options.ExtraRepoConf eventually (just like // ForceRepos) - repoConf, err := parseRepoURLs(extraRepos, "extra") - if err != nil { - return nil, err - } for _, distro := range reg.ListDistros() { for _, arch := range reg.ListArches(distro) { + repoConf, err := parseRepoURLs(extraRepos, "extra", distro, arch) + if err != nil { + return nil, err + } reg.AppendRepos(distro, arch, repoConf...) } } diff --git a/cmd/image-builder/repos_test.go b/cmd/image-builder/repos_test.go index a31dd74e57..469041c837 100644 --- a/cmd/image-builder/repos_test.go +++ b/cmd/image-builder/repos_test.go @@ -17,7 +17,7 @@ func TestParseRepoURLsHappy(t *testing.T) { cfg, err := parseRepoURLs([]string{ "file:///path/to/repo", "https://example.com/repo", - }, "forced") + }, "forced", "", "") assert.NoError(t, err) assert.Equal(t, []rpmmd.RepoConfig{ { @@ -38,10 +38,10 @@ func TestParseRepoURLsHappy(t *testing.T) { } func TestParseExtraRepoSad(t *testing.T) { - _, err := parseRepoURLs([]string{"/just/a/path"}, "forced") + _, err := parseRepoURLs([]string{"/just/a/path"}, "forced", "", "") assert.EqualError(t, err, `scheme missing in "/just/a/path", please prefix with e.g. file:// or https://`) - _, err = parseRepoURLs([]string{"https://example.com", "/just/a/path"}, "forced") + _, err = parseRepoURLs([]string{"https://example.com", "/just/a/path"}, "forced", "", "") assert.EqualError(t, err, `scheme missing in "/just/a/path", please prefix with e.g. file:// or https://`) } From fb2ea1c707999979f8b04ff15e66fc272b8bfddb Mon Sep 17 00:00:00 2001 From: Simon de Vlieger Date: Wed, 24 Jun 2026 12:49:48 +0200 Subject: [PATCH 5/5] cmd/image-builder: `copr://` repo URLs Allow passing of `copr://@owner/project` URLs to `--(force|extra)-repo`. We look up the COPR chroot necessary through the distro/arch that we pass along. We rename some distro names as they use different chroot names from our internal names (`centos`) and for RHEL we go from specific-to-RHEL to EPEL. Signed-off-by: Simon de Vlieger --- cmd/image-builder/copr.go | 168 +++++++++++++++++++++++++ cmd/image-builder/copr_test.go | 211 ++++++++++++++++++++++++++++++++ cmd/image-builder/repos.go | 38 +++--- cmd/image-builder/repos_test.go | 62 ++++++++++ 4 files changed, 463 insertions(+), 16 deletions(-) create mode 100644 cmd/image-builder/copr.go create mode 100644 cmd/image-builder/copr_test.go diff --git a/cmd/image-builder/copr.go b/cmd/image-builder/copr.go new file mode 100644 index 0000000000..58d7874615 --- /dev/null +++ b/cmd/image-builder/copr.go @@ -0,0 +1,168 @@ +package main + +import ( + "encoding/json" + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/osbuild/image-builder/pkg/rpmmd" +) + +var coprBaseURL = "https://copr.fedorainfracloud.org" + +type coprProject struct { + FullName string + Owner string + Name string + ChrootRepos map[string]string + GPGKeyURL string +} + +type coprAPIResponse struct { + FullName string `json:"full_name"` + Ownername string `json:"ownername"` + Name string `json:"name"` + ChrootRepos map[string]string `json:"chroot_repos"` +} + +var coprHTTPClient = http.DefaultClient + +// parseCoprURL extracts the owner and project from a parsed +// copr:// URL. The expected form is copr://owner/project. +func parseCoprURL(u *url.URL) (owner, project string, err error) { + // TODO: do we want to require hostnames or something? This + // TODO: all seems to be a bit iffy as we're working around the + // TODO: url.URL kinda? + + // url.Parse("copr://@osbuild/osbuild") gives: + // Opaque: "//@osbuild/osbuild" + // url.Parse("copr://user/project") gives: + // Host: "user", Path: "/project" + + // Rebuild the path from the raw form to handle both uniformly. + raw := strings.TrimPrefix(u.String(), "copr://") + + parts := strings.SplitN(raw, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return "", "", fmt.Errorf("invalid copr URL %q, expected copr://owner/project (e.g. copr://@osbuild/osbuild)", u) + } + + return parts[0], parts[1], nil +} + +func fetchCoprProject(owner, project string) (*coprProject, error) { + apiURL := fmt.Sprintf("%s/api_3/project?ownername=%s&projectname=%s", + coprBaseURL, + url.QueryEscape(owner), + url.QueryEscape(project), + ) + + resp, err := coprHTTPClient.Get(apiURL) + if err != nil { + return nil, fmt.Errorf("cannot fetch copr project %s/%s: %w", owner, project, err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + return nil, fmt.Errorf("copr project %s/%s not found", owner, project) + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("copr API returned status %d for project %s/%s", resp.StatusCode, owner, project) + } + + var apiResp coprAPIResponse + if err := json.NewDecoder(resp.Body).Decode(&apiResp); err != nil { + return nil, fmt.Errorf("cannot decode copr API response for %s/%s: %w", owner, project, err) + } + + if len(apiResp.ChrootRepos) == 0 { + return nil, fmt.Errorf("copr project %s/%s has no chroot repos", owner, project) + } + + gpgKeyURL := fmt.Sprintf("https://download.copr.fedorainfracloud.org/results/%s/%s/pubkey.gpg", owner, project) + + return &coprProject{ + FullName: apiResp.FullName, + Owner: owner, + Name: project, + ChrootRepos: apiResp.ChrootRepos, + GPGKeyURL: gpgKeyURL, + }, nil +} + +func (cp *coprProject) repoConfig(chroot, what string, idx int) *rpmmd.RepoConfig { + baseURL, ok := cp.ChrootRepos[chroot] + if !ok { + return nil + } + + checkGPG := true + checkRepoGPG := false + return &rpmmd.RepoConfig{ + Id: fmt.Sprintf("%s-copr-%v", what, idx), + Name: fmt.Sprintf("Copr repo for %s owned by %s", cp.Name, cp.Owner), + BaseURLs: []string{baseURL}, + GPGKeys: []string{cp.GPGKeyURL}, + CheckGPG: &checkGPG, + CheckRepoGPG: &checkRepoGPG, + } +} + +// distroToCoprChroots returns candidate COPR chroot prefixes for a +// given distro name. The arch suffix is not included, callers append +// it themselves (see resolveCoprRepo). +func distroToCoprChroots(distroName string) []string { + // centos-10 → centos-stream-10 + if strings.HasPrefix(distroName, "centos-") { + ver := strings.TrimPrefix(distroName, "centos-") + return []string{ + "centos-stream-" + ver, + distroName, + } + } + + // rhel-10.2, rhel-10, then epel-10.2, epel-10 is our order + // of preference. in the future we might allow for explicit + // chroot selection through the URL + if strings.HasPrefix(distroName, "rhel-") { + ver := strings.TrimPrefix(distroName, "rhel-") + major, _, hasDot := strings.Cut(ver, ".") + candidates := []string{distroName} + if hasDot { + candidates = append(candidates, "rhel-"+major) + } + candidates = append(candidates, "epel-"+ver) + if hasDot { + candidates = append(candidates, "epel-"+major) + } + return candidates + } + + return []string{distroName} +} + +// resolveCoprRepo fetches the COPR project and returns a RepoConfig +// for the given distro/arch combination. nil if +// the project has no matching chroot. +func resolveCoprRepo(u *url.URL, what string, idx int, distroName, archName string) (*rpmmd.RepoConfig, error) { + owner, project, err := parseCoprURL(u) + if err != nil { + return nil, err + } + + cp, err := fetchCoprProject(owner, project) + if err != nil { + return nil, err + } + + for _, prefix := range distroToCoprChroots(distroName) { + chroot := prefix + "-" + archName + if rc := cp.repoConfig(chroot, what, idx); rc != nil { + return rc, nil + } + } + + return nil, nil +} diff --git a/cmd/image-builder/copr_test.go b/cmd/image-builder/copr_test.go new file mode 100644 index 0000000000..f817fc360e --- /dev/null +++ b/cmd/image-builder/copr_test.go @@ -0,0 +1,211 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func mustParseURL(t *testing.T, raw string) *url.URL { + t.Helper() + u, err := url.Parse(raw) + require.NoError(t, err) + return u +} + +func TestParseCoprURL(t *testing.T) { + tests := []struct { + spec string + owner string + project string + err string + }{ + {"copr://@osbuild/osbuild", "@osbuild", "osbuild", ""}, + {"copr://daan/myproject", "daan", "myproject", ""}, + {"copr://@group/my-project", "@group", "my-project", ""}, + {"copr://", "", "", "invalid copr URL"}, + {"copr://noslash", "", "", "invalid copr URL"}, + {"copr:///project", "", "", "invalid copr URL"}, + {"copr://owner/", "", "", "invalid copr URL"}, + } + + for _, tt := range tests { + t.Run(tt.spec, func(t *testing.T) { + u := mustParseURL(t, tt.spec) + owner, project, err := parseCoprURL(u) + if tt.err != "" { + assert.ErrorContains(t, err, tt.err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.owner, owner) + assert.Equal(t, tt.project, project) + } + }) + } +} + +func TestDistroToCoprChroots(t *testing.T) { + tests := []struct { + distro string + expected []string + }{ + {"fedora-44", []string{"fedora-44"}}, + {"fedora-43", []string{"fedora-43"}}, + {"centos-10", []string{"centos-stream-10", "centos-10"}}, + {"centos-9", []string{"centos-stream-9", "centos-9"}}, + {"rhel-10.2", []string{"rhel-10.2", "rhel-10", "epel-10.2", "epel-10"}}, + {"rhel-10", []string{"rhel-10", "epel-10"}}, + {"rhel-9.6", []string{"rhel-9.6", "rhel-9", "epel-9.6", "epel-9"}}, + {"almalinux-9.4", []string{"almalinux-9.4"}}, + } + + for _, tt := range tests { + t.Run(tt.distro, func(t *testing.T) { + assert.Equal(t, tt.expected, distroToCoprChroots(tt.distro)) + }) + } +} + +func mockCoprServer(t *testing.T, response coprAPIResponse, statusCode int) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(statusCode) + err := json.NewEncoder(w).Encode(response) + require.NoError(t, err) + })) +} + +func TestCoprProjectRepoConfig(t *testing.T) { + cp := &coprProject{ + FullName: "@osbuild/osbuild", + Owner: "@osbuild", + Name: "osbuild", + ChrootRepos: map[string]string{ + "fedora-44-x86_64": "https://download.copr.fedorainfracloud.org/results/@osbuild/osbuild/fedora-44-x86_64/", + "fedora-44-aarch64": "https://download.copr.fedorainfracloud.org/results/@osbuild/osbuild/fedora-44-aarch64/", + "centos-stream-10-x86_64": "https://download.copr.fedorainfracloud.org/results/@osbuild/osbuild/centos-stream-10-x86_64/", + }, + GPGKeyURL: "https://download.copr.fedorainfracloud.org/results/@osbuild/osbuild/pubkey.gpg", + } + + t.Run("matching chroot", func(t *testing.T) { + rc := cp.repoConfig("fedora-44-x86_64", "extra", 0) + require.NotNil(t, rc) + assert.Equal(t, "extra-copr-0", rc.Id) + assert.Equal(t, []string{"https://download.copr.fedorainfracloud.org/results/@osbuild/osbuild/fedora-44-x86_64/"}, rc.BaseURLs) + assert.Equal(t, []string{"https://download.copr.fedorainfracloud.org/results/@osbuild/osbuild/pubkey.gpg"}, rc.GPGKeys) + assert.True(t, *rc.CheckGPG) + assert.False(t, *rc.CheckRepoGPG) + }) + + t.Run("non-matching chroot", func(t *testing.T) { + rc := cp.repoConfig("fedora-99-x86_64", "extra", 0) + assert.Nil(t, rc) + }) +} + +func TestFetchCoprProjectNotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + origClient := coprHTTPClient + origURL := coprBaseURL + t.Cleanup(func() { + coprHTTPClient = origClient + coprBaseURL = origURL + }) + coprHTTPClient = srv.Client() + coprBaseURL = srv.URL + + _, err := fetchCoprProject("@noone", "noproject") + assert.ErrorContains(t, err, "not found") +} + +func TestFetchCoprProjectHappy(t *testing.T) { + response := coprAPIResponse{ + FullName: "@test/testproject", + Ownername: "@test", + Name: "testproject", + ChrootRepos: map[string]string{ + "fedora-44-x86_64": "https://download.copr.fedorainfracloud.org/results/@test/testproject/fedora-44-x86_64/", + }, + } + srv := mockCoprServer(t, response, http.StatusOK) + defer srv.Close() + + origClient := coprHTTPClient + origURL := coprBaseURL + t.Cleanup(func() { + coprHTTPClient = origClient + coprBaseURL = origURL + }) + coprHTTPClient = srv.Client() + coprBaseURL = srv.URL + + cp, err := fetchCoprProject("@test", "testproject") + require.NoError(t, err) + assert.Equal(t, "@test/testproject", cp.FullName) + assert.Equal(t, "@test", cp.Owner) + assert.Equal(t, "testproject", cp.Name) + assert.Contains(t, cp.ChrootRepos, "fedora-44-x86_64") + assert.Contains(t, cp.GPGKeyURL, "pubkey.gpg") +} + +func TestResolveCoprRepo(t *testing.T) { + response := coprAPIResponse{ + FullName: "@osbuild/osbuild", + Ownername: "@osbuild", + Name: "osbuild", + ChrootRepos: map[string]string{ + "fedora-44-x86_64": "https://example.com/fedora-44-x86_64/", + "centos-stream-10-x86_64": "https://example.com/centos-stream-10-x86_64/", + "epel-9-x86_64": "https://example.com/epel-9-x86_64/", + }, + } + srv := mockCoprServer(t, response, http.StatusOK) + defer srv.Close() + + origClient := coprHTTPClient + origURL := coprBaseURL + t.Cleanup(func() { + coprHTTPClient = origClient + coprBaseURL = origURL + }) + coprHTTPClient = srv.Client() + coprBaseURL = srv.URL + + t.Run("fedora direct match", func(t *testing.T) { + rc, err := resolveCoprRepo(mustParseURL(t, "copr://@osbuild/osbuild"), "extra", 0, "fedora-44", "x86_64") + require.NoError(t, err) + require.NotNil(t, rc) + assert.Equal(t, []string{"https://example.com/fedora-44-x86_64/"}, rc.BaseURLs) + }) + + t.Run("centos maps to centos-stream", func(t *testing.T) { + rc, err := resolveCoprRepo(mustParseURL(t, "copr://@osbuild/osbuild"), "extra", 0, "centos-10", "x86_64") + require.NoError(t, err) + require.NotNil(t, rc) + assert.Equal(t, []string{"https://example.com/centos-stream-10-x86_64/"}, rc.BaseURLs) + }) + + t.Run("rhel falls back to epel", func(t *testing.T) { + rc, err := resolveCoprRepo(mustParseURL(t, "copr://@osbuild/osbuild"), "extra", 0, "rhel-9.6", "x86_64") + require.NoError(t, err) + require.NotNil(t, rc) + assert.Equal(t, []string{"https://example.com/epel-9-x86_64/"}, rc.BaseURLs) + }) + + t.Run("no matching chroot returns nil", func(t *testing.T) { + rc, err := resolveCoprRepo(mustParseURL(t, "copr://@osbuild/osbuild"), "extra", 0, "fedora-99", "x86_64") + require.NoError(t, err) + assert.Nil(t, rc) + }) +} diff --git a/cmd/image-builder/repos.go b/cmd/image-builder/repos.go index b9c5b28875..15037b2ec4 100644 --- a/cmd/image-builder/repos.go +++ b/cmd/image-builder/repos.go @@ -25,11 +25,6 @@ func parseRepoURLs(repoURLs []string, what, distroName, archName string) ([]rpmm var repoConf []rpmmd.RepoConfig for i, repoURL := range repoURLs { - // We want to eventually support more URIs repos here: - // - config:/path/to/repo.json - // - copr:@osbuild/osbuild (with full gpg retrival via the copr API) - // But for now just default to base-urls - baseURL, err := url.Parse(repoURL) if err != nil { return nil, fmt.Errorf("cannot parse extra repo %w", err) @@ -38,17 +33,28 @@ func parseRepoURLs(repoURLs []string, what, distroName, archName string) ([]rpmm return nil, fmt.Errorf(`scheme missing in %q, please prefix with e.g. file:// or https://`, repoURL) } - // TODO: to support gpg checking we will need to add signing keys. - // We will eventually add support for our own "repo.json" format - // which is rich enough to contain gpg keys (and more). - checkGPG := false - repoConf = append(repoConf, rpmmd.RepoConfig{ - Id: fmt.Sprintf("%s-repo-%v", what, i), - Name: fmt.Sprintf("%s repo#%v %s%s", what, i, baseURL.Host, baseURL.Path), - BaseURLs: []string{baseURL.String()}, - CheckGPG: &checkGPG, - CheckRepoGPG: &checkGPG, - }) + switch baseURL.Scheme { + case "copr": + rc, err := resolveCoprRepo(baseURL, what, i, distroName, archName) + if err != nil { + return nil, err + } + if rc != nil { + repoConf = append(repoConf, *rc) + } + default: + // TODO: to support gpg checking we will need to add signing keys. + // We will eventually add support for our own "repo.json" format + // which is rich enough to contain gpg keys (and more). + checkGPG := false + repoConf = append(repoConf, rpmmd.RepoConfig{ + Id: fmt.Sprintf("%s-repo-%v", what, i), + Name: fmt.Sprintf("%s repo#%v %s%s", what, i, baseURL.Host, baseURL.Path), + BaseURLs: []string{baseURL.String()}, + CheckGPG: &checkGPG, + CheckRepoGPG: &checkGPG, + }) + } } return repoConf, nil diff --git a/cmd/image-builder/repos_test.go b/cmd/image-builder/repos_test.go index 469041c837..3072e87edf 100644 --- a/cmd/image-builder/repos_test.go +++ b/cmd/image-builder/repos_test.go @@ -1,6 +1,9 @@ package main import ( + "encoding/json" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -45,6 +48,65 @@ func TestParseExtraRepoSad(t *testing.T) { assert.EqualError(t, err, `scheme missing in "/just/a/path", please prefix with e.g. file:// or https://`) } +func TestParseRepoURLsCopr(t *testing.T) { + response := coprAPIResponse{ + FullName: "@osbuild/osbuild", + Ownername: "@osbuild", + Name: "osbuild", + ChrootRepos: map[string]string{ + "fedora-44-x86_64": "https://example.com/fedora-44-x86_64/", + "centos-stream-10-x86_64": "https://example.com/centos-stream-10-x86_64/", + }, + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + err := json.NewEncoder(w).Encode(response) + require.NoError(t, err) + })) + defer srv.Close() + + origClient := coprHTTPClient + origURL := coprBaseURL + t.Cleanup(func() { + coprHTTPClient = origClient + coprBaseURL = origURL + }) + coprHTTPClient = srv.Client() + coprBaseURL = srv.URL + + t.Run("copr resolves for matching chroot", func(t *testing.T) { + cfg, err := parseRepoURLs([]string{"copr://@osbuild/osbuild"}, "extra", "fedora-44", "x86_64") + require.NoError(t, err) + require.Len(t, cfg, 1) + assert.Equal(t, []string{"https://example.com/fedora-44-x86_64/"}, cfg[0].BaseURLs) + assert.True(t, *cfg[0].CheckGPG) + }) + + t.Run("copr skips non-matching chroot", func(t *testing.T) { + cfg, err := parseRepoURLs([]string{"copr://@osbuild/osbuild"}, "extra", "fedora-99", "x86_64") + require.NoError(t, err) + assert.Len(t, cfg, 0) + }) + + t.Run("copr mixed with regular URLs", func(t *testing.T) { + cfg, err := parseRepoURLs([]string{ + "https://example.com/regular", + "copr://@osbuild/osbuild", + }, "extra", "fedora-44", "x86_64") + require.NoError(t, err) + require.Len(t, cfg, 2) + assert.Equal(t, []string{"https://example.com/regular"}, cfg[0].BaseURLs) + assert.Equal(t, []string{"https://example.com/fedora-44-x86_64/"}, cfg[1].BaseURLs) + }) + + t.Run("centos maps to centos-stream chroot", func(t *testing.T) { + cfg, err := parseRepoURLs([]string{"copr://@osbuild/osbuild"}, "extra", "centos-10", "x86_64") + require.NoError(t, err) + require.Len(t, cfg, 1) + assert.Equal(t, []string{"https://example.com/centos-stream-10-x86_64/"}, cfg[0].BaseURLs) + }) +} + func TestNewRepoRegistryImplSmoke(t *testing.T) { registry, err := newRepoRegistryImpl("", nil) require.NoError(t, err)