From d7ae209b9d626c9a1289961ee6b90dbb749d8cb6 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 12 Nov 2024 17:33:45 +0100 Subject: [PATCH] reporegistry: add new `LoadAllRepositoriesFromFS` and use in tests This commit is a (hopefully) less controversial version of: https://github.com/osbuild/images/pull/1037 Here only a new helper to load repository information from an `fs.FS` is added and only used for loading the test repositories. This will help to have a single test repository that can be embedded accross osbuild-composer, image-builder and images. It also removes the public "NewTestedDefault()" in favor of a new `testrepos.New()` helper so that the `reporegistry` only has non-test API left. --- cmd/gen-manifests/main.go | 4 +-- cmd/list-images/main.go | 4 +-- .../main.go | 4 +-- .../distro_test_common/distro_test_common.go | 4 +-- pkg/imagefilter/imagefilter_test.go | 6 ++-- pkg/reporegistry/reporegistry.go | 15 ---------- pkg/reporegistry/reporegistry_test.go | 10 ++----- pkg/reporegistry/repository.go | 30 +++++++++++++------ pkg/rpmmd/repository.go | 7 ++++- test/data/repositories/testrepos.go | 19 ++++++++++++ 10 files changed, 59 insertions(+), 44 deletions(-) create mode 100644 test/data/repositories/testrepos.go diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go index 80e5c9ebe3..626e1e3095 100644 --- a/cmd/gen-manifests/main.go +++ b/cmd/gen-manifests/main.go @@ -28,10 +28,10 @@ import ( "github.com/osbuild/images/pkg/dnfjson" "github.com/osbuild/images/pkg/manifest" "github.com/osbuild/images/pkg/ostree" - "github.com/osbuild/images/pkg/reporegistry" "github.com/osbuild/images/pkg/rhsm/facts" "github.com/osbuild/images/pkg/rpmmd" "github.com/osbuild/images/pkg/sbom" + testrepos "github.com/osbuild/images/test/data/repositories" ) type buildRequest struct { @@ -513,7 +513,7 @@ func main() { flag.Parse() - testedRepoRegistry, err := reporegistry.NewTestedDefault() + testedRepoRegistry, err := testrepos.New() if err != nil { panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err)) } diff --git a/cmd/list-images/main.go b/cmd/list-images/main.go index 4f257afa27..9cd2001147 100644 --- a/cmd/list-images/main.go +++ b/cmd/list-images/main.go @@ -11,7 +11,7 @@ import ( "github.com/gobwas/glob" "github.com/osbuild/images/pkg/distrofactory" - "github.com/osbuild/images/pkg/reporegistry" + testrepos "github.com/osbuild/images/test/data/repositories" ) type multiValue []string @@ -75,7 +75,7 @@ func main() { flag.BoolVar(&json, "json", false, "print configs as json") flag.Parse() - testedRepoRegistry, err := reporegistry.NewTestedDefault() + testedRepoRegistry, err := testrepos.New() if err != nil { panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err)) } diff --git a/cmd/osbuild-composer-image-definitions/main.go b/cmd/osbuild-composer-image-definitions/main.go index 89573aeb03..c145e19238 100644 --- a/cmd/osbuild-composer-image-definitions/main.go +++ b/cmd/osbuild-composer-image-definitions/main.go @@ -6,14 +6,14 @@ import ( "os" "github.com/osbuild/images/pkg/distrofactory" - "github.com/osbuild/images/pkg/reporegistry" + testrepos "github.com/osbuild/images/test/data/repositories" ) func main() { definitions := map[string]map[string][]string{} distroFac := distrofactory.NewDefault() - testedRepoRegistry, err := reporegistry.NewTestedDefault() + testedRepoRegistry, err := testrepos.New() if err != nil { panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err)) } diff --git a/pkg/distro/distro_test_common/distro_test_common.go b/pkg/distro/distro_test_common/distro_test_common.go index 035c5fcc00..4e4858ad0a 100644 --- a/pkg/distro/distro_test_common/distro_test_common.go +++ b/pkg/distro/distro_test_common/distro_test_common.go @@ -11,7 +11,7 @@ import ( "github.com/osbuild/images/pkg/blueprint" "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/ostree" - "github.com/osbuild/images/pkg/reporegistry" + testrepos "github.com/osbuild/images/test/data/repositories" ) const RandomTestSeed = 0 @@ -463,7 +463,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) { // ListTestedDistros returns a list of distro names that are explicitly tested func ListTestedDistros(t *testing.T) []string { - testRepoRegistry, err := reporegistry.NewTestedDefault() + testRepoRegistry, err := testrepos.New() require.Nil(t, err) require.NotEmpty(t, testRepoRegistry) distros := testRepoRegistry.ListDistros() diff --git a/pkg/imagefilter/imagefilter_test.go b/pkg/imagefilter/imagefilter_test.go index ab20ff56bf..6f1c342c88 100644 --- a/pkg/imagefilter/imagefilter_test.go +++ b/pkg/imagefilter/imagefilter_test.go @@ -10,14 +10,14 @@ import ( "github.com/osbuild/images/pkg/distrofactory" "github.com/osbuild/images/pkg/imagefilter" - "github.com/osbuild/images/pkg/reporegistry" + testrepos "github.com/osbuild/images/test/data/repositories" ) func TestImageFilterSmoke(t *testing.T) { logrus.SetLevel(logrus.WarnLevel) fac := distrofactory.NewDefault() - repos, err := reporegistry.NewTestedDefault() + repos, err := testrepos.New() require.NoError(t, err) imgFilter, err := imagefilter.New(fac, repos) @@ -29,7 +29,7 @@ func TestImageFilterSmoke(t *testing.T) { func TestImageFilterFilter(t *testing.T) { fac := distrofactory.NewDefault() - repos, err := reporegistry.NewTestedDefault() + repos, err := testrepos.New() require.NoError(t, err) imgFilter, err := imagefilter.New(fac, repos) diff --git a/pkg/reporegistry/reporegistry.go b/pkg/reporegistry/reporegistry.go index 24209c9b51..7a15e042ad 100644 --- a/pkg/reporegistry/reporegistry.go +++ b/pkg/reporegistry/reporegistry.go @@ -2,8 +2,6 @@ package reporegistry import ( "fmt" - "path/filepath" - "runtime" "github.com/osbuild/images/pkg/distroidparser" "github.com/osbuild/images/pkg/rpmmd" @@ -27,19 +25,6 @@ func New(repoConfigPaths []string) (*RepoRegistry, error) { return &RepoRegistry{repositories}, nil } -// NewTestedDefault returns a new RepoRegistry instance with the data -// loaded from the default test repositories -func NewTestedDefault() (*RepoRegistry, error) { - _, callerSrc, _, ok := runtime.Caller(0) - var testReposPath []string - if !ok { - testReposPath = append(testReposPath, "../../test/data") - } else { - testReposPath = append(testReposPath, filepath.Join(filepath.Dir(callerSrc), "../../test/data")) - } - return New(testReposPath) -} - func NewFromDistrosRepoConfigs(distrosRepoConfigs rpmmd.DistrosRepoConfigs) *RepoRegistry { return &RepoRegistry{distrosRepoConfigs} } diff --git a/pkg/reporegistry/reporegistry_test.go b/pkg/reporegistry/reporegistry_test.go index 55d1359a61..db57f33cef 100644 --- a/pkg/reporegistry/reporegistry_test.go +++ b/pkg/reporegistry/reporegistry_test.go @@ -4,10 +4,11 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/osbuild/images/pkg/distro" "github.com/osbuild/images/pkg/distro/test_distro" "github.com/osbuild/images/pkg/rpmmd" - "github.com/stretchr/testify/assert" ) func getTestingRepoRegistry() *RepoRegistry { @@ -370,10 +371,3 @@ func TestInvalidReposByArchName(t *testing.T) { }) } } - -func Test_NewTestedDefault(t *testing.T) { - rr, err := NewTestedDefault() - assert.Nil(t, err) - assert.NotNil(t, rr) - assert.NotEmpty(t, rr.ListDistros()) -} diff --git a/pkg/reporegistry/repository.go b/pkg/reporegistry/repository.go index c51528f723..ffe14574a8 100644 --- a/pkg/reporegistry/repository.go +++ b/pkg/reporegistry/repository.go @@ -1,6 +1,7 @@ package reporegistry import ( + "io/fs" "os" "path/filepath" "strings" @@ -14,12 +15,24 @@ import ( // LoadAllRepositories loads all repositories for given distros from the given list of paths. // Behavior is the same as with the LoadRepositories() method. func LoadAllRepositories(confPaths []string) (rpmmd.DistrosRepoConfigs, error) { - distrosRepoConfigs := rpmmd.DistrosRepoConfigs{} + var confFSes []fs.FS for _, confPath := range confPaths { - reposPath := filepath.Join(confPath, "repositories") + confFSes = append(confFSes, os.DirFS(filepath.Join(confPath, "repositories"))) + } + + distrosRepoConfigs, err := LoadAllRepositoriesFromFS(confFSes) + if len(distrosRepoConfigs) == 0 { + return nil, &NoReposLoadedError{confPaths} + } + return distrosRepoConfigs, err +} + +func LoadAllRepositoriesFromFS(confPaths []fs.FS) (rpmmd.DistrosRepoConfigs, error) { + distrosRepoConfigs := rpmmd.DistrosRepoConfigs{} - fileEntries, err := os.ReadDir(reposPath) + for _, confPath := range confPaths { + fileEntries, err := fs.ReadDir(confPath, ".") if os.IsNotExist(err) { continue } else if err != nil { @@ -53,8 +66,11 @@ func LoadAllRepositories(confPaths []string) (rpmmd.DistrosRepoConfigs, error) { continue } - configFile := filepath.Join(reposPath, fileEntry.Name()) - distroRepos, err := rpmmd.LoadRepositoriesFromFile(configFile) + configFile, err := confPath.Open(fileEntry.Name()) + if err != nil { + return nil, err + } + distroRepos, err := rpmmd.LoadRepositoriesFromReader(configFile) if err != nil { return nil, err } @@ -66,10 +82,6 @@ func LoadAllRepositories(confPaths []string) (rpmmd.DistrosRepoConfigs, error) { } } - if len(distrosRepoConfigs) == 0 { - return nil, &NoReposLoadedError{confPaths} - } - return distrosRepoConfigs, nil } diff --git a/pkg/rpmmd/repository.go b/pkg/rpmmd/repository.go index 6375345bcb..e897a0a56b 100644 --- a/pkg/rpmmd/repository.go +++ b/pkg/rpmmd/repository.go @@ -4,6 +4,7 @@ import ( "crypto/sha256" "encoding/json" "fmt" + "io" "os" "sort" "strings" @@ -232,10 +233,14 @@ func LoadRepositoriesFromFile(filename string) (map[string][]RepoConfig, error) } defer f.Close() + return LoadRepositoriesFromReader(f) +} + +func LoadRepositoriesFromReader(r io.Reader) (map[string][]RepoConfig, error) { var reposMap map[string][]repository repoConfigs := make(map[string][]RepoConfig) - err = json.NewDecoder(f).Decode(&reposMap) + err := json.NewDecoder(r).Decode(&reposMap) if err != nil { return nil, err } diff --git a/test/data/repositories/testrepos.go b/test/data/repositories/testrepos.go new file mode 100644 index 0000000000..f9c72a4a47 --- /dev/null +++ b/test/data/repositories/testrepos.go @@ -0,0 +1,19 @@ +package testrepos + +import ( + "embed" + "io/fs" + + "github.com/osbuild/images/pkg/reporegistry" +) + +//go:embed *.json +var FS embed.FS + +func New() (*reporegistry.RepoRegistry, error) { + repositories, err := reporegistry.LoadAllRepositoriesFromFS([]fs.FS{FS}) + if err != nil { + return nil, err + } + return reporegistry.NewFromDistrosRepoConfigs(repositories), nil +}