Skip to content

Commit

Permalink
main: add --force-repo flag
Browse files Browse the repository at this point in the history
This commit adds an `--force-repo` flag that can be used
to replace all the base repositories with a base url to
a repository. This is useful for testing but also dangerous
as it will not do any checks and happily use a fedora-42 repository
for centos-8 depsolving.

This will make the use-case of the koji builder easier and is
also something that the `build` tool in `images` supports.
  • Loading branch information
mvo5 committed Feb 13, 2025
1 parent e2aeece commit 34409d0
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 42 deletions.
8 changes: 7 additions & 1 deletion cmd/image-builder/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
8 changes: 8 additions & 0 deletions cmd/image-builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
37 changes: 37 additions & 0 deletions cmd/image-builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
9 changes: 9 additions & 0 deletions cmd/image-builder/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
71 changes: 35 additions & 36 deletions cmd/image-builder/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,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

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:`, 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
return []rpmmd.RepoConfig{
{
Id: baseURL.String(),
Name: baseURL.String(),
// 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) {
Expand All @@ -68,20 +72,15 @@ 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
}
for _, repoArchConfigs := range conf {
for arch := range repoArchConfigs {
archCfg := repoArchConfigs[arch]
archCfg = append(archCfg, repoConf...)
repoArchConfigs[arch] = archCfg
}
}

Expand Down
23 changes: 18 additions & 5 deletions cmd/image-builder/repos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
_, err := parseRepoURLs([]string{"/just/a/path"}, "forced")
assert.EqualError(t, err, `scheme missing in "/just/a/path", please prefix with e.g. file:`)

_, 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:`)
}

0 comments on commit 34409d0

Please sign in to comment.