diff --git a/cmd/image-builder/filters.go b/cmd/image-builder/filters.go index 80d876c..a41e9ec 100644 --- a/cmd/image-builder/filters.go +++ b/cmd/image-builder/filters.go @@ -21,8 +21,14 @@ func newImageFilterDefault(dataDir string, extraRepos []string) (*imagefilter.Im } type repoOptions struct { - DataDir string + // DataDir contains the base dir for the repo definition search path + DataDir string + + // ExtraRepos contains extra baseURLs that get added to the depsolving ExtraRepos []string + + // ForceRepos contains baseURLs that replace *all* base repositories + ForceRepos []string } // should this be moved to images:imagefilter? diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index dc1335b..8efbde6 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -87,6 +87,10 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st if err != nil { return nil, err } + forceRepos, err := cmd.Flags().GetStringArray("force-repo") + if err != nil { + return nil, err + } archStr, err := cmd.Flags().GetString("arch") if err != nil { return nil, err @@ -140,6 +144,7 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st repoOpts := &repoOptions{ DataDir: dataDir, ExtraRepos: extraRepos, + ForceRepos: forceRepos, } img, err := getOneImage(distroStr, imgTypeStr, archStr, repoOpts) if err != nil { @@ -157,6 +162,8 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st Ostree: ostreeImgOpts, RpmDownloader: rpmDownloader, WithSBOM: withSBOM, + + ForceRepos: forceRepos, } err = generateManifest(dataDir, extraRepos, img, w, opts) return img, err @@ -317,6 +324,7 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support. } rootCmd.PersistentFlags().String("datadir", "", `Override the default data directory for e.g. custom repositories/*.json data`) rootCmd.PersistentFlags().StringArray("extra-repo", nil, `Add an extra repository during build (will *not* be gpg checked and not be part of the final image)`) + rootCmd.PersistentFlags().StringArray("force-repo", nil, `Override the base repositories during build (these will not be part of the final image)`) rootCmd.PersistentFlags().String("output-dir", "", `Put output into the specified directory`) rootCmd.PersistentFlags().BoolP("verbose", "v", false, `Switch to verbose mode`) rootCmd.SetOut(osStdout) diff --git a/cmd/image-builder/main_test.go b/cmd/image-builder/main_test.go index 95af02d..651b821 100644 --- a/cmd/image-builder/main_test.go +++ b/cmd/image-builder/main_test.go @@ -646,3 +646,40 @@ func TestManifestExtraRepo(t *testing.T) { 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) { + if testing.Short() { + t.Skip("manifest generation takes a while") + } + if !hasDepsolveDnf() { + t.Skip("no osbuild-depsolve-dnf binary found") + } + + var fakeStderr bytes.Buffer + restore := main.MockOsStderr(&fakeStderr) + defer restore() + + restore = main.MockOsArgs([]string{ + "manifest", + "qcow2", + "--distro=centos-9", + "--arch=x86_64", + "--force-repo=http://xxx.abcdefgh-no-such-host.com/repo", + }) + defer restore() + + // XXX: dnfjson is very chatty and puts a bunch of output on stderr + // we should probably silence this in images as its the job of the + // error to catpure this. Use CaptureStdio here to ensure we don't + // get noisy and confusing errors when this test runs. + var err error + testutil.CaptureStdio(t, func() { + err = main.Run() + }) + assert.ErrorContains(t, err, "forced repo#0 xxx.abcdefgh-no-such-host.com/repo: http://xxx.abcdefgh-no-such-host.com/repo]: Cannot download repomd.xml") + // XXX: we should probably look into "images" here, there is a bunch + // of redundancy in the full error message: + // + // `error depsolving: running osbuild-depsolve-dnf failed: + // DNF error occurred: RepoError: There was a problem reading a repository: Failed to download metadata for repo '9828718901ab404ac1b600157aec1a8b19f4b2139e7756f347fb0ecc06c92929' [forced repo#0 xxx.abcdefgh-no-such-host.com/repo: http://xxx.abcdefgh-no-such-host.com/repo]: Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried` +} diff --git a/cmd/image-builder/manifest.go b/cmd/image-builder/manifest.go index 18bf5e6..c254491 100644 --- a/cmd/image-builder/manifest.go +++ b/cmd/image-builder/manifest.go @@ -21,6 +21,8 @@ type manifestOptions struct { Ostree *ostree.ImageOptions RpmDownloader osbuild.RpmDownloader WithSBOM bool + + ForceRepos []string } func sbomWriter(outputDir, filename string, content io.Reader) error { @@ -58,6 +60,13 @@ func generateManifest(dataDir string, extraRepos []string, img *imagefilter.Resu return sbomWriter(outputDir, filename, content) } } + if len(opts.ForceRepos) > 0 { + forcedRepos, err := parseRepoURLs(opts.ForceRepos, "forced") + if err != nil { + return err + } + manifestGenOpts.OverrideRepos = forcedRepos + } mg, err := manifestgen.New(repos, manifestGenOpts) if err != nil { diff --git a/cmd/image-builder/repos.go b/cmd/image-builder/repos.go index bd28f0c..67b45a4 100644 --- a/cmd/image-builder/repos.go +++ b/cmd/image-builder/repos.go @@ -6,6 +6,7 @@ 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" ) @@ -24,33 +25,37 @@ type repoConfig struct { ExtraRepos []string } -func parseExtraRepo(extraRepo string) ([]rpmmd.RepoConfig, error) { - // 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 +func parseRepoURLs(repoURLs []string, what string) ([]rpmmd.RepoConfig, error) { + var repoConf []rpmmd.RepoConfig - baseURL, err := url.Parse(extraRepo) - if err != nil { - return nil, fmt.Errorf("cannot parse extra repo %w", err) - } - if baseURL.Scheme == "" { - return nil, fmt.Errorf(`scheme missing in %q, please prefix with e.g. file:`, extraRepo) - } + 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 - // 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 - return []rpmmd.RepoConfig{ - { - Id: baseURL.String(), - Name: baseURL.String(), + baseURL, err := url.Parse(repoURL) + if err != nil { + return nil, fmt.Errorf("cannot parse extra repo %w", err) + } + if baseURL.Scheme == "" { + 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, - }, - }, nil + }) + } + + return repoConf, nil } func newRepoRegistryImpl(dataDir string, extraRepos []string) (*reporegistry.RepoRegistry, error) { @@ -68,21 +73,19 @@ func newRepoRegistryImpl(dataDir string, extraRepos []string) (*reporegistry.Rep // XXX: this should probably go into manifestgen.Options as // a new Options.ExtraRepoConf eventually (just like OverrideRepos) - for _, repo := range extraRepos { - // XXX: this loads the extra repo unconditionally to all - // distro/arch versions. we do not know in advance where - // it belongs to - extraRepo, err := parseExtraRepo(repo) - if err != nil { - return nil, err - } - for _, repoArchConfigs := range conf { - for arch := range repoArchConfigs { - archCfg := repoArchConfigs[arch] - archCfg = append(archCfg, extraRepo...) - repoArchConfigs[arch] = archCfg - } - } + 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 } return reporegistry.NewFromDistrosRepoConfigs(conf), nil diff --git a/cmd/image-builder/repos_test.go b/cmd/image-builder/repos_test.go index 6a7ab49..a811b63 100644 --- a/cmd/image-builder/repos_test.go +++ b/cmd/image-builder/repos_test.go @@ -8,23 +8,36 @@ import ( "github.com/osbuild/images/pkg/rpmmd" ) -func TestParseExtraRepoHappy(t *testing.T) { +func TestParseRepoURLsHappy(t *testing.T) { checkGPG := false - cfg, err := parseExtraRepo("file:///path/to/repo") + cfg, err := parseRepoURLs([]string{ + "file:///path/to/repo", + "https://example.com/repo", + }, "forced") assert.NoError(t, err) assert.Equal(t, cfg, []rpmmd.RepoConfig{ { - Id: "file:///path/to/repo", - Name: "file:///path/to/repo", + Id: "forced-repo-0", + Name: "forced repo#0 /path/to/repo", BaseURLs: []string{"file:///path/to/repo"}, CheckGPG: &checkGPG, CheckRepoGPG: &checkGPG, }, + { + Id: "forced-repo-1", + Name: "forced repo#1 example.com/repo", + BaseURLs: []string{"https://example.com/repo"}, + CheckGPG: &checkGPG, + CheckRepoGPG: &checkGPG, + }, }) } func TestParseExtraRepoSad(t *testing.T) { - _, err := parseExtraRepo("/just/a/path") - assert.EqualError(t, err, `scheme missing in "/just/a/path", please prefix with e.g. file:`) + _, 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") + assert.EqualError(t, err, `scheme missing in "/just/a/path", please prefix with e.g. file:// or https://`) }