From 75f24bbd2ab4ef12d5ce2353bd4a15646f73eab2 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Tue, 19 Aug 2025 15:29:56 +0200
Subject: [PATCH] main: fix --extra-repos support when cross building

This commit fixes the issue that --extra-repo are not added
correctly to the repo registry when doing cross arch building.
The issue is that they get added early at the reporegistry
level and because we don't know yet what arch will be used
it was initially only added to the host arch. This was
fine before we supported cross-arch building.

But now that we do support cross-arch building this is
of course an issue and produces subtle bugs. This commit
is the "easy" fix, the extra repos are just added to for
all architectures. This may not be right but because they
are optional its the reponsibility of the user to provide
the right ones. Eventually we should probably be smarter
here and move extra repo support into manifestgen but
that requires changes in images and an images release so
this is good enough for now.

Thanks to Simon de Vlieger for reporting this.
---
 cmd/image-builder/main_test.go | 47 ++++++++++++++++++++++------------
 cmd/image-builder/repos.go     | 24 +++++++++--------
 2 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/cmd/image-builder/main_test.go b/cmd/image-builder/main_test.go
index 90cb4c7..fd0b714 100644
--- a/cmd/image-builder/main_test.go
+++ b/cmd/image-builder/main_test.go
@@ -753,7 +753,7 @@ func TestProgressFromCmd(t *testing.T) {
 	}
 }
 
-func TestManifestExtraRepo(t *testing.T) {
+func TestManifestExtraRepos(t *testing.T) {
 	if testing.Short() {
 		t.Skip("manifest generation takes a while")
 	}
@@ -776,25 +776,38 @@ func TestManifestExtraRepo(t *testing.T) {
         [[packages]]
         name = "dummy"
 `
-	restore := main.MockOsArgs([]string{
-		"manifest",
-		"qcow2",
-		"--distro=centos-9",
-		fmt.Sprintf("--extra-repo=file://%s", localRepoDir),
-		"--blueprint", makeTestBlueprint(t, pkgHelloBlueprint),
-	})
-	defer restore()
+	nativeArch := arch.Current()
+	// setup test for cross-arch, if we happen to run on the
+	// cross arch already just pick a different arch
+	crossArch := arch.ARCH_AARCH64
+	if arch.Current() == arch.ARCH_AARCH64 {
+		crossArch = arch.ARCH_X86_64
+	}
 
-	var fakeStdout bytes.Buffer
-	restore = main.MockOsStdout(&fakeStdout)
-	defer restore()
+	for _, arch := range []arch.Arch{nativeArch, crossArch} {
+		t.Run(arch.String(), func(t *testing.T) {
+			restore := main.MockOsArgs([]string{
+				"manifest",
+				"qcow2",
+				"--distro=centos-9",
+				fmt.Sprintf("--arch=%s", arch),
+				fmt.Sprintf("--extra-repo=file://%s", localRepoDir),
+				"--blueprint", makeTestBlueprint(t, pkgHelloBlueprint),
+			})
+			defer restore()
 
-	err = main.Run()
-	require.NoError(t, err)
+			var fakeStdout bytes.Buffer
+			restore = main.MockOsStdout(&fakeStdout)
+			defer restore()
 
-	// our local repo got added
-	assert.Contains(t, fakeStdout.String(), `"path":"dummy-1.0.0-0.noarch.rpm"`)
-	assert.Contains(t, fakeStdout.String(), fmt.Sprintf(`"url":"file://%s"`, localRepoDir))
+			err = main.Run()
+			require.NoError(t, err)
+
+			// our local repo got added
+			assert.Contains(t, fakeStdout.String(), `"path":"dummy-1.0.0-0.noarch.rpm"`)
+			assert.Contains(t, fakeStdout.String(), fmt.Sprintf(`"url":"file://%s"`, localRepoDir))
+		})
+	}
 }
 
 func TestManifestOverrideRepo(t *testing.T) {
diff --git a/cmd/image-builder/repos.go b/cmd/image-builder/repos.go
index 67b45a4..9a35731 100644
--- a/cmd/image-builder/repos.go
+++ b/cmd/image-builder/repos.go
@@ -6,7 +6,6 @@ import (
 	"net/url"
 
 	"github.com/osbuild/images/data/repositories"
-	"github.com/osbuild/images/pkg/arch"
 	"github.com/osbuild/images/pkg/reporegistry"
 	"github.com/osbuild/images/pkg/rpmmd"
 )
@@ -71,21 +70,24 @@ func newRepoRegistryImpl(dataDir string, extraRepos []string) (*reporegistry.Rep
 		return nil, err
 	}
 
-	// XXX: this should probably go into manifestgen.Options as
-	// a new Options.ExtraRepoConf eventually (just like OverrideRepos)
+	// Add extra repos to all architecture. We support
+	// cross-building but at this level here we don't know yet
+	// what manifests will be generated so we must (for now)
+	// rely on the user to DTRT with extraRepos.
+	//
+	// XXX: this should probably go into manifestgen.Options as a
+	// new Options.ExtraRepoConf eventually (just like
+	// OverrideRepos)
 	repoConf, err := parseRepoURLs(extraRepos, "extra")
 	if err != nil {
 		return nil, err
 	}
-	// Only add extra repos for the host architecture. We do not support
-	// cross-building (yet) so this is fine. Once we support cross-building
-	// this needs to move (probably into manifestgen) because at this
-	// level we we do not know (yet) what manifest we will generate.
-	myArch := arch.Current().String()
 	for _, repoArchConfigs := range conf {
-		archCfg := repoArchConfigs[myArch]
-		archCfg = append(archCfg, repoConf...)
-		repoArchConfigs[myArch] = archCfg
+		for arch := range repoArchConfigs {
+			archCfg := repoArchConfigs[arch]
+			archCfg = append(archCfg, repoConf...)
+			repoArchConfigs[arch] = archCfg
+		}
 	}
 
 	return reporegistry.NewFromDistrosRepoConfigs(conf), nil