From b94540e65b10b1410718aabc9c3f376b23147d15 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 19 Dec 2023 12:15:41 +0100 Subject: [PATCH] dnfjson: return err in New{,Base}Solver The dnfjson.{,Base}Solver will not be very useful without an installed `osbuild-depsolve-dnf` so error early when none is found. --- cmd/build/main.go | 5 ++- cmd/gen-manifests/main.go | 5 ++- cmd/osbuild-dnf-json-tests/main_test.go | 3 +- cmd/osbuild-playground/playground.go | 5 ++- pkg/dnfjson/dnfjson.go | 32 ++++++++------ pkg/dnfjson/dnfjson_test.go | 55 +++++++++++++++++++++---- 6 files changed, 80 insertions(+), 25 deletions(-) diff --git a/cmd/build/main.go b/cmd/build/main.go index f1ae760b44..fe4e9a24ef 100644 --- a/cmd/build/main.go +++ b/cmd/build/main.go @@ -177,7 +177,10 @@ func resolvePipelineCommits(commitSources map[string][]ostree.SourceSpec) (map[s } func depsolve(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string][]rpmmd.PackageSpec, error) { - solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir) + solver, err := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir) + if err != nil { + return nil, err + } depsolvedSets := make(map[string][]rpmmd.PackageSpec) for name, pkgSet := range packageSets { res, err := solver.Depsolve(pkgSet) diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index a2f474b8ce..d5d5424825 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -412,7 +412,10 @@ func mockResolveCommits(commitSources map[string][]ostree.SourceSpec) map[string } func depsolve(cacheDir string, packageSets map[string][]rpmmd.PackageSet, d distro.Distro, arch string) (map[string][]rpmmd.PackageSpec, error) { - solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir) + solver, err := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch, d.Name(), cacheDir) + if err != nil { + return nil, err + } depsolvedSets := make(map[string][]rpmmd.PackageSpec) for name, pkgSet := range packageSets { res, err := solver.Depsolve(pkgSet) diff --git a/cmd/osbuild-dnf-json-tests/main_test.go b/cmd/osbuild-dnf-json-tests/main_test.go index 5e4d80dd7c..fc32196b20 100644 --- a/cmd/osbuild-dnf-json-tests/main_test.go +++ b/cmd/osbuild-dnf-json-tests/main_test.go @@ -31,7 +31,8 @@ func TestCrossArchDepsolve(t *testing.T) { // Set up temporary directory for rpm/dnf cache dir := t.TempDir() - baseSolver := dnfjson.NewBaseSolver(dir) + baseSolver, err := dnfjson.NewBaseSolver(dir) + require.Nil(t, err) repos, err := rpmmd.LoadRepositories([]string{repoDir}, cs9.Name()) require.NoErrorf(t, err, "Failed to LoadRepositories %v", cs9.Name()) diff --git a/cmd/osbuild-playground/playground.go b/cmd/osbuild-playground/playground.go index 44a6b28c39..dd3ee9f908 100644 --- a/cmd/osbuild-playground/playground.go +++ b/cmd/osbuild-playground/playground.go @@ -18,7 +18,10 @@ import ( func RunPlayground(img image.ImageKind, d distro.Distro, arch distro.Arch, repos map[string][]rpmmd.RepoConfig, state_dir string) { - solver := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch.Name(), d.Name(), path.Join(state_dir, "rpmmd")) + solver, err := dnfjson.NewSolver(d.ModulePlatformID(), d.Releasever(), arch.Name(), d.Name(), path.Join(state_dir, "rpmmd")) + if err != nil { + panic(err) + } // Set cache size to 1 GiB solver.SetMaxCacheSize(1 * common.GiB) diff --git a/pkg/dnfjson/dnfjson.go b/pkg/dnfjson/dnfjson.go index 887f3a0a11..9505e09b35 100644 --- a/pkg/dnfjson/dnfjson.go +++ b/pkg/dnfjson/dnfjson.go @@ -43,32 +43,37 @@ type BaseSolver struct { resultCache *dnfCache } +var depSolveDnfPaths = []string{"/usr/libexec/", "/usr/lib/osbuild/"} + // Find the osbuild-depsolve-dnf script. This checks the default location in // /usr/libexec but also /usr/lib in case it's used on a distribution that // doesn't use libexec. -func findDepsolveDnf() string { - locations := []string{"/usr/libexec/osbuild-depsolve-dnf", "/usr/lib/osbuild/osbuild-depsolve-dnf"} - for _, djPath := range locations { - _, err := os.Stat(djPath) +func findDepsolveDnf() (string, error) { + for _, djPath := range depSolveDnfPaths { + _, err := os.Stat(filepath.Join(djPath, "osbuild-depsolve-dnf")) if !os.IsNotExist(err) { - return djPath + return djPath, nil } } // if it's not found, return empty string; the run() function will fail if // it's used before setting. - return "" + return "", fmt.Errorf("cannot find required osbuild-depsolve-dnf in any of %q", depSolveDnfPaths) } // Create a new unconfigured BaseSolver (without platform information). It can // be used to create configured Solver instances with the NewWithConfig() // method. -func NewBaseSolver(cacheDir string) *BaseSolver { +func NewBaseSolver(cacheDir string) (*BaseSolver, error) { + depsolveDnf, err := findDepsolveDnf() + if err != nil { + return nil, err + } return &BaseSolver{ cache: newRPMCache(cacheDir, 1024*1024*1024), // 1 GiB - dnfJsonCmd: []string{findDepsolveDnf()}, + dnfJsonCmd: []string{depsolveDnf}, resultCache: NewDNFCache(60 * time.Second), - } + }, nil } // SetMaxCacheSize sets the maximum size for the global repository metadata @@ -129,9 +134,12 @@ type Solver struct { } // Create a new Solver with the given configuration. Initialising a Solver also loads system subscription information. -func NewSolver(modulePlatformID, releaseVer, arch, distro, cacheDir string) *Solver { - s := NewBaseSolver(cacheDir) - return s.NewWithConfig(modulePlatformID, releaseVer, arch, distro) +func NewSolver(modulePlatformID, releaseVer, arch, distro, cacheDir string) (*Solver, error) { + s, err := NewBaseSolver(cacheDir) + if err != nil { + return nil, err + } + return s.NewWithConfig(modulePlatformID, releaseVer, arch, distro), nil } // GetCacheDir returns a distro specific rpm cache directory diff --git a/pkg/dnfjson/dnfjson_test.go b/pkg/dnfjson/dnfjson_test.go index ff13d769b6..8f5218be61 100644 --- a/pkg/dnfjson/dnfjson_test.go +++ b/pkg/dnfjson/dnfjson_test.go @@ -11,6 +11,7 @@ import ( "github.com/osbuild/images/internal/mocks/rpmrepo" "github.com/osbuild/images/pkg/rpmmd" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var forceDNF = flag.Bool("force-dnf", false, "force dnf testing, making them fail instead of skip if dnf isn't installed") @@ -18,8 +19,8 @@ var forceDNF = flag.Bool("force-dnf", false, "force dnf testing, making them fai func TestDepsolver(t *testing.T) { if !*forceDNF { // dnf tests aren't forced: skip them if the dnf sniff check fails - if findDepsolveDnf() == "" { - t.Skip("Test needs an installed osbuild-depsolve-dnf") + if _, err := findDepsolveDnf(); err != nil { + t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err) } } @@ -29,7 +30,8 @@ func TestDepsolver(t *testing.T) { assert := assert.New(t) tmpdir := t.TempDir() - solver := NewSolver("platform:el9", "9", "x86_64", "rhel9.0", tmpdir) + solver, err := NewSolver("platform:el9", "9", "x86_64", "rhel9.0", tmpdir) + require.Nil(t, err) { // single depsolve pkgsets := []rpmmd.PackageSet{{Include: []string{"kernel", "vim-minimal", "tmux", "zsh"}, Repositories: []rpmmd.RepoConfig{s.RepoConfig}, InstallWeakDeps: true}} // everything you'll ever need @@ -57,6 +59,13 @@ func TestDepsolver(t *testing.T) { } func TestMakeDepsolveRequest(t *testing.T) { + if !*forceDNF { + // dnf tests aren't forced: skip them if the dnf sniff check fails + if _, err := findDepsolveDnf(); err != nil { + t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err) + } + } + baseOS := rpmmd.RepoConfig{ Name: "baseos", BaseURLs: []string{"https://example.org/baseos"}, @@ -402,7 +411,8 @@ func TestMakeDepsolveRequest(t *testing.T) { }, }, } - solver := NewSolver("", "", "", "", "") + solver, err := NewSolver("", "", "", "", "") + require.Nil(t, err) for idx, tt := range tests { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { req, _, err := solver.makeDepsolveRequest(tt.packageSets) @@ -543,8 +553,8 @@ func expectedResult(repo rpmmd.RepoConfig) []rpmmd.PackageSpec { func TestErrorRepoInfo(t *testing.T) { if !*forceDNF { // dnf tests aren't forced: skip them if the dnf sniff check fails - if findDepsolveDnf() == "" { - t.Skip("Test needs an installed osbuild-depsolve-dnf") + if _, err := findDepsolveDnf(); err != nil { + t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err) } } @@ -588,7 +598,8 @@ func TestErrorRepoInfo(t *testing.T) { }, } - solver := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache") + solver, err := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache") + require.Nil(t, err) for idx, tc := range testCases { t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { _, err := solver.Depsolve([]rpmmd.PackageSet{ @@ -605,6 +616,13 @@ func TestErrorRepoInfo(t *testing.T) { } func TestRepoConfigHash(t *testing.T) { + if !*forceDNF { + // dnf tests aren't forced: skip them if the dnf sniff check fails + if _, err := findDepsolveDnf(); err != nil { + t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err) + } + } + repos := []rpmmd.RepoConfig{ { Id: "repoid-1", @@ -617,7 +635,8 @@ func TestRepoConfigHash(t *testing.T) { }, } - solver := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache") + solver, err := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache") + require.Nil(t, err) rcs, err := solver.reposFromRPMMD(repos) assert.Nil(t, err) @@ -629,7 +648,15 @@ func TestRepoConfigHash(t *testing.T) { } func TestRequestHash(t *testing.T) { - solver := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache") + if !*forceDNF { + // dnf tests aren't forced: skip them if the dnf sniff check fails + if _, err := findDepsolveDnf(); err != nil { + t.Skipf("Test needs an installed osbuild-depsolve-dnf: %v", err) + } + } + + solver, err := NewSolver("f38", "38", "x86_64", "fedora-38", "/tmp/cache") + require.Nil(t, err) repos := []rpmmd.RepoConfig{ rpmmd.RepoConfig{ Name: "A test repository", @@ -655,3 +682,13 @@ func TestRepoConfigMarshalAlsmostEmpty(t *testing.T) { // double check here that anything that uses pointers has "omitempty" set assert.Equal(t, string(js), `{"id":"","gpgcheck":false,"check_repogpg":false,"ignoressl":false}`) } + +func TestFindDepsolveDnfError(t *testing.T) { + old := depSolveDnfPaths + depSolveDnfPaths = []string{"/rando1", "/rando2"} + defer func() { depSolveDnfPaths = old }() + + path, err := findDepsolveDnf() + assert.Equal(t, path, "") + assert.EqualError(t, err, `cannot find required osbuild-depsolve-dnf in any of ["/rando1" "/rando2"]`) +}