particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use RepoRegistry in composer and Weldr API

Browse source 
Modify composer to use RepoRegistry, instead of loading the host
repositories, when initializing WeldrAPI.

Modify WeldrAPI to use RepoRegistry, instead of a map of repository
definitions. Make sure that the RepoRegistry method specific to image
type is used in Welder where appropriate. Specifically when depsolving a
Blueprint, which is used to build a specific image type. Update Weldr
API unit tests to reflect the change.

Add a new method to RepoRegistry, allowing to get list of repositories,
which should be used for building an image for a given architecture,
without specifying the exact image type. Add relevant unit tests.

Signed-off-by: Tomas Hozza <thozza@redhat.com>
This commit is contained in:
Tomas Hozza 2021-05-05 18:09:08 +02:00 • committed by Ondřej Budai
parent fba9fe1072
commit aa6665ad01
6 changed files with 361 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

13
cmd/osbuild-composer/composer.go
View file

@ -17,6 +17,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/distroregistry"
"github.com/osbuild/osbuild-composer/internal/jobqueue/fsjobqueue"
"github.com/osbuild/osbuild-composer/internal/kojiapi"
"github.com/osbuild/osbuild-composer/internal/reporegistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/store"
"github.com/osbuild/osbuild-composer/internal/weldr"
@ -85,15 +86,21 @@ func (c *Composer) InitWeldr(repoPaths []string, weldrListener net.Listener) err
return fmt.Errorf("Host distro does not support host architecture: %v", err)
}
repos, err := rpmmd.LoadRepositories(repoPaths, hostDistro.Name())
rr, err := reporegistry.New(repoPaths)
if err != nil {
return fmt.Errorf("Error loading repositories for %s: %v", hostDistro.Name(), err)
return fmt.Errorf("error loading repository definitions: %v", err)
}
// Check if repositories for the host distro and arch were loaded
_, err = rr.ReposByArch(arch, false)
if err != nil {
return fmt.Errorf("loaded repository definitions don't contain any for the host distro/arch: %v", err)
}
store := store.New(&c.stateDir, arch, c.logger)
compatOutputDir := path.Join(c.stateDir, "outputs")
c.weldr = weldr.New(c.rpm, arch, hostDistro, repos[archName], c.logger, store, c.workers, compatOutputDir)
c.weldr = weldr.New(c.rpm, arch, hostDistro, rr, c.logger, store, c.workers, compatOutputDir)
c.weldrListener = weldrListener

13
internal/client/unit_test.go
View file

@ -16,6 +16,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/distro/test_distro"
rpmmd_mock "github.com/osbuild/osbuild-composer/internal/mocks/rpmmd"
"github.com/osbuild/osbuild-composer/internal/reporegistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/weldr"
)
@ -49,9 +50,17 @@ func executeTests(m *testing.M) int {
if err != nil {
panic(err)
}
repos := []rpmmd.RepoConfig{{Name: "test-system-repo", BaseURL: "http://example.com/test/os/test_arch"}}
rr := reporegistry.NewFromDistrosRepoConfigs(rpmmd.DistrosRepoConfigs{
test_distro.TestDistroName: {
test_distro.TestArchName: {
{Name: "test-system-repo", BaseURL: "http://example.com/test/os/test_arch"},
},
},
})
logger := log.New(os.Stdout, "", 0)
api := weldr.New(rpm, arch, distro, repos, logger, fixture.Store, fixture.Workers, "")
api := weldr.New(rpm, arch, distro, rr, logger, fixture.Store, fixture.Workers, "")
server := http.Server{Handler: api}
defer server.Close()

55
internal/reporegistry/reporegistry.go
View file

@ -26,6 +26,10 @@ func New(repoConfigPaths []string) (*RepoRegistry, error) {
return &RepoRegistry{repositories}, nil
}
func NewFromDistrosRepoConfigs(distrosRepoConfigs rpmmd.DistrosRepoConfigs) *RepoRegistry {
return &RepoRegistry{distrosRepoConfigs}
}
// ReposByImageType returns a slice of rpmmd.RepoConfig instances, which should be used for building the specific
// image type. All repositories for the associated distribution and architecture, without any ImageTypeTags set,
// are always part of the returned slice. In addition, if there are repositories tagged with the specific image
@ -48,13 +52,9 @@ func (r *RepoRegistry) ReposByImageType(imageType distro.ImageType) ([]rpmmd.Rep
func (r *RepoRegistry) reposByImageTypeName(distro, arch, imageType string) ([]rpmmd.RepoConfig, error) {
repositories := []rpmmd.RepoConfig{}
distroRepos, found := r.repos[distro]
if !found {
return nil, fmt.Errorf("there are no repositories for distribution '%s'", distro)
}
archRepos, found := distroRepos[arch]
if !found {
return nil, fmt.Errorf("there are no repositories for distribution '%s' and architecture '%s'", distro, arch)
archRepos, err := r.reposByArchName(distro, arch, true)
if err != nil {
return nil, err
}
for _, repo := range archRepos {
@ -75,3 +75,44 @@ func (r *RepoRegistry) reposByImageTypeName(distro, arch, imageType string) ([]r
return repositories, nil
}
// ReposByArch returns a slice of rpmmd.RepoConfig instances, which should be used for building image types for the
// specific architecture. This includes by default all repositories without any image type tags specified.
// Depending on the `includeTagged` argument value, repositories with image type tags set will be added to the returned
// slice or not.
func (r *RepoRegistry) ReposByArch(arch distro.Arch, includeTagged bool) ([]rpmmd.RepoConfig, error) {
if arch.Distro() == nil || reflect.ValueOf(arch.Distro()).IsNil() {
return nil, fmt.Errorf("there is no distribution associated with the provided architecture")
}
return r.reposByArchName(arch.Distro().Name(), arch.Name(), includeTagged)
}
// reposByArchName returns a slice of rpmmd.RepoConfig instances, which should be used for building image types for the
// specific architecture and distribution. This includes by default all repositories without any image type tags specified.
// Depending on the `includeTagged` argument value, repositories with image type tags set will be added to the returned
// slice or not.
//
// The method does not verify if the given architecture name is actually part of the specific distribution definition.
func (r *RepoRegistry) reposByArchName(distro, arch string, includeTagged bool) ([]rpmmd.RepoConfig, error) {
repositories := []rpmmd.RepoConfig{}
distroRepos, found := r.repos[distro]
if !found {
return nil, fmt.Errorf("there are no repositories for distribution '%s'", distro)
}
archRepos, found := distroRepos[arch]
if !found {
return nil, fmt.Errorf("there are no repositories for distribution '%s' and architecture '%s'", distro, arch)
}
for _, repo := range archRepos {
// skip repos with image type tags if specified to do so
if !includeTagged && len(repo.ImageTypeTags) != 0 {
continue
}
repositories = append(repositories, repo)
}
return repositories, nil
}

177
internal/reporegistry/reporegistry_test.go
View file

@ -225,3 +225,180 @@ func TestInvalidReposByImageTypeName(t *testing.T) {
})
}
}
func TestReposByArch(t *testing.T) {
rr := getTestingRepoRegistry()
testDistro := test_distro.New()
ta, _ := testDistro.GetArch(test_distro.TestArchName)
ta2, _ := testDistro.GetArch(test_distro.TestArch2Name)
type args struct {
arch distro.Arch
taggedRepos bool
}
tests := []struct {
name string
args args
want []string
}{
{
name: "Test Arch 1, without tagged repos",
args: args{
arch: ta,
taggedRepos: false,
},
want: []string{"baseos", "appstream"},
},
{
name: "Test Arch 1, with tagged repos",
args: args{
arch: ta,
taggedRepos: true,
},
want: []string{"baseos", "appstream"},
},
{
name: "Test Arch 2, without tagged repos",
args: args{
arch: ta2,
taggedRepos: false,
},
want: []string{"baseos", "appstream"},
},
{
name: "Test Arch 2, with tagged repos",
args: args{
arch: ta2,
taggedRepos: true,
},
want: []string{"baseos", "appstream", "google-compute-engine", "google-cloud-sdk"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := rr.ReposByArch(tt.args.arch, tt.args.taggedRepos)
assert.Nil(t, err)
var gotNames []string
for _, r := range got {
gotNames = append(gotNames, r.Name)
}
if !reflect.DeepEqual(gotNames, tt.want) {
t.Errorf("ReposByArch() =\n got: %#v\n want: %#v", gotNames, tt.want)
}
got, err = rr.reposByArchName(tt.args.arch.Distro().Name(), tt.args.arch.Name(), tt.args.taggedRepos)
assert.Nil(t, err)
gotNames = []string{}
for _, r := range got {
gotNames = append(gotNames, r.Name)
}
if !reflect.DeepEqual(gotNames, tt.want) {
t.Errorf("reposByArchName() =\n got: %#v\n want: %#v", gotNames, tt.want)
}
})
}
}
// TestInvalidReposByArch tests return values from ReposByArch
// for invalid arch value
func TestInvalidReposByArch(t *testing.T) {
rr := getTestingRepoRegistry()
ta := test_distro.TestArch{}
repos, err := rr.ReposByArch(&ta, false)
assert.Nil(t, repos)
assert.NotNil(t, err)
repos, err = rr.ReposByArch(&ta, true)
assert.Nil(t, repos)
assert.NotNil(t, err)
}
// TestInvalidReposByArchName tests return values from reposByArchName
// for invalid distro name and arch
func TestInvalidReposByArchName(t *testing.T) {
rr := getTestingRepoRegistry()
type args struct {
distro string
arch string
taggedRepos bool
}
tests := []struct {
name string
args args
want func(repos []rpmmd.RepoConfig, err error) bool
}{
{
name: "invalid distro, valid arch, without tagged repos",
args: args{
distro: test_distro.TestDistroName + "-invalid",
arch: test_distro.TestArch2Name,
taggedRepos: false,
},
want: func(repos []rpmmd.RepoConfig, err error) bool {
// the list of repos should be nil and an error should be returned
if repos != nil || err == nil {
return false
}
return true
},
},
{
name: "invalid distro, valid arch, with tagged repos",
args: args{
distro: test_distro.TestDistroName + "-invalid",
arch: test_distro.TestArch2Name,
taggedRepos: true,
},
want: func(repos []rpmmd.RepoConfig, err error) bool {
// the list of repos should be nil and an error should be returned
if repos != nil || err == nil {
return false
}
return true
},
},
{
name: "invalid arch, valid distro, without tagged repos",
args: args{
distro: test_distro.TestDistroName,
arch: test_distro.TestArch2Name + "-invalid",
taggedRepos: false,
},
want: func(repos []rpmmd.RepoConfig, err error) bool {
// the list of repos should be nil and an error should be returned
if repos != nil || err == nil {
return false
}
return true
},
},
{
name: "invalid arch, valid distro, with tagged repos",
args: args{
distro: test_distro.TestDistroName,
arch: test_distro.TestArch2Name + "-invalid",
taggedRepos: true,
},
want: func(repos []rpmmd.RepoConfig, err error) bool {
// the list of repos should be nil and an error should be returned
if repos != nil || err == nil {
return false
}
return true
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := rr.reposByArchName(tt.args.distro, tt.args.arch, tt.args.taggedRepos)
assert.True(t, tt.want(got, err))
})
}
}

124
internal/weldr/api.go
View file

@ -33,6 +33,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/jobqueue"
osbuild "github.com/osbuild/osbuild-composer/internal/osbuild1"
"github.com/osbuild/osbuild-composer/internal/reporegistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/store"
"github.com/osbuild/osbuild-composer/internal/target"
@ -43,10 +44,10 @@ type API struct {
store *store.Store
workers *worker.Server
rpmmd rpmmd.RPMMD
arch distro.Arch
distro distro.Distro
repos []rpmmd.RepoConfig
rpmmd rpmmd.RPMMD
arch distro.Arch
distro distro.Distro
repoRegistry *reporegistry.RepoRegistry
logger *log.Logger
router *httprouter.Router
@ -82,8 +83,11 @@ func (cs ComposeState) ToString() string {
// systemRepoIDs returns a list of the system repos
// NOTE: The system repos have no concept of id vs. name so the id is returned
func (api *API) systemRepoNames() (names []string) {
for _, repo := range api.repos {
names = append(names, repo.Name)
repos, err := api.repoRegistry.ReposByArch(api.arch, false)
if err == nil {
for _, repo := range repos {
names = append(names, repo.Name)
}
}
return names
}
@ -91,14 +95,14 @@ func (api *API) systemRepoNames() (names []string) {
var ValidBlueprintName = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`)
var ValidOSTreeRef = regexp.MustCompile(`^(?:[\w\d][-._\w\d]*\/)*[\w\d][-._\w\d]*$`)
func New(rpmmd rpmmd.RPMMD, arch distro.Arch, distro distro.Distro, repos []rpmmd.RepoConfig, logger *log.Logger, store *store.Store, workers *worker.Server, compatOutputDir string) *API {
func New(rpmmd rpmmd.RPMMD, arch distro.Arch, distro distro.Distro, repoRegistry *reporegistry.RepoRegistry, logger *log.Logger, store *store.Store, workers *worker.Server, compatOutputDir string) *API {
api := &API{
store: store,
workers: workers,
rpmmd: rpmmd,
arch: arch,
distro: distro,
repos: repos,
repoRegistry: repoRegistry,
logger: logger,
compatOutputDir: compatOutputDir,
}
@ -465,17 +469,26 @@ func (api *API) getSourceConfigs(params httprouter.Params) (map[string]store.Sou
sources := map[string]store.SourceConfig{}
errors := []responseError{}
repos, err := api.repoRegistry.ReposByArch(api.arch, false)
if err != nil {
error := responseError{
ID: "InternalError",
Msg: fmt.Sprintf("error while getting system repos: %v", err),
}
errors = append(errors, error)
}
// if names is "*" we want all sources
if names == "*" {
sources = api.store.GetAllSourcesByID()
for _, repo := range api.repos {
for _, repo := range repos {
sources[repo.Name] = store.NewSourceConfig(repo, true)
}
} else {
for _, name := range strings.Split(names, ",") {
// check if the source is one of the base repos
found := false
for _, repo := range api.repos {
for _, repo := range repos {
if name == repo.Name {
sources[repo.Name] = store.NewSourceConfig(repo, true)
found = true
@ -1008,8 +1021,18 @@ func (api *API) modulesInfoHandler(writer http.ResponseWriter, request *http.Req
packageInfos := foundPackages.ToPackageInfos()
if modulesRequested {
repos, err := api.repoRegistry.ReposByArch(api.arch, false)
if err != nil {
errors := responseError{
ID: "InternalError",
Msg: fmt.Sprintf("error while getting system repos: %v", err),
}
statusResponseError(writer, http.StatusBadRequest, errors)
return
}
for i := range packageInfos {
err := packageInfos[i].FillDependencies(api.rpmmd, api.repos, api.distro.ModulePlatformID(), api.arch.Name())
err := packageInfos[i].FillDependencies(api.rpmmd, repos, api.distro.ModulePlatformID(), api.arch.Name())
if err != nil {
errors := responseError{
ID: errorId,
@ -1053,7 +1076,17 @@ func (api *API) projectsDepsolveHandler(writer http.ResponseWriter, request *htt
projects = projects[1:]
names := strings.Split(projects, ",")
packages, _, err := api.rpmmd.Depsolve(rpmmd.PackageSet{Include: names}, api.repos, api.distro.ModulePlatformID(), api.arch.Name())
repos, err := api.repoRegistry.ReposByArch(api.arch, false)
if err != nil {
errors := responseError{
ID: "InternalError",
Msg: fmt.Sprintf("error while getting system repos: %v", err),
}
statusResponseError(writer, http.StatusBadRequest, errors)
return
}
packages, _, err := api.rpmmd.Depsolve(rpmmd.PackageSet{Include: names}, repos, api.distro.ModulePlatformID(), api.arch.Name())
if err != nil {
errors := responseError{
@ -1843,8 +1876,14 @@ func ostreeResolveRef(location, ref string) (string, error) {
func (api *API) depsolveBlueprintForImageType(bp *blueprint.Blueprint, imageType distro.ImageType) (map[string][]rpmmd.PackageSpec, error) {
packageSets := imageType.PackageSets(*bp)
packageSpecSets := make(map[string][]rpmmd.PackageSpec)
imageTypeRepos, err := api.allRepositoriesByImageType(imageType)
if err != nil {
return nil, err
}
for name, packageSet := range packageSets {
packageSpecs, _, err := api.rpmmd.Depsolve(packageSet, api.allRepositories(), api.distro.ModulePlatformID(), api.arch.Name())
packageSpecs, _, err := api.rpmmd.Depsolve(packageSet, imageTypeRepos, api.distro.ModulePlatformID(), api.arch.Name())
if err != nil {
return nil, err
}
@ -2003,6 +2042,17 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
}
seed := bigSeed.Int64()
imageRepos, err := api.allRepositoriesByImageType(imageType)
// this shoudl not happen if the api.depsolveBlueprintForImageType() call above worked
if err != nil {
errors := responseError{
ID: "InternalError",
Msg: err.Error(),
}
statusResponseError(writer, http.StatusInternalServerError, errors)
return
}
manifest, err := imageType.Manifest(bp.Customizations,
distro.ImageOptions{
Size: size,
@ -2012,7 +2062,7 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
URL: cr.OSTree.URL,
},
},
api.allRepositories(),
imageRepos,
packageSets,
seed)
if err != nil {
@ -2755,21 +2805,55 @@ func (api *API) composeFailedHandler(writer http.ResponseWriter, request *http.R
}
func (api *API) fetchPackageList() (rpmmd.PackageList, error) {
packages, _, err := api.rpmmd.FetchMetadata(api.allRepositories(), api.distro.ModulePlatformID(), api.arch.Name())
repos, err := api.allRepositories()
if err != nil {
return nil, err
}
packages, _, err := api.rpmmd.FetchMetadata(repos, api.distro.ModulePlatformID(), api.arch.Name())
return packages, err
}
// Returns all configured repositories (base + sources) as rpmmd.RepoConfig
func (api *API) allRepositories() []rpmmd.RepoConfig {
repos := append([]rpmmd.RepoConfig{}, api.repos...)
// Returns all configured repositories (base, depending on the image type + sources) as rpmmd.RepoConfig
// The difference from allRepositories() is that this method may return additional repositories,
// which are needed to build the specific image type. The allRepositories() can't do this, because
// it is used in paces where image types are not considered.
func (api *API) allRepositoriesByImageType(imageType distro.ImageType) ([]rpmmd.RepoConfig, error) {
imageTypeRepos, err := api.repoRegistry.ReposByImageType(imageType)
if err != nil {
return nil, err
}
repos := append([]rpmmd.RepoConfig{}, imageTypeRepos...)
for id, source := range api.store.GetAllSourcesByID() {
repos = append(repos, source.RepoConfig(id))
}
return repos
return repos, nil
}
// Returns all configured repositories (base + sources) as rpmmd.RepoConfig
func (api *API) allRepositories() ([]rpmmd.RepoConfig, error) {
archRepos, err := api.repoRegistry.ReposByArch(api.arch, false)
if err != nil {
return nil, err
}
repos := append([]rpmmd.RepoConfig{}, archRepos...)
for id, source := range api.store.GetAllSourcesByID() {
repos = append(repos, source.RepoConfig(id))
}
return repos, nil
}
func (api *API) depsolveBlueprint(bp *blueprint.Blueprint) ([]rpmmd.PackageSpec, error) {
packages, _, err := api.rpmmd.Depsolve(rpmmd.PackageSet{Include: bp.GetPackages()}, api.allRepositories(), api.distro.ModulePlatformID(), api.arch.Name())
repos, err := api.allRepositories()
if err != nil {
return nil, err
}
packages, _, err := api.rpmmd.Depsolve(rpmmd.PackageSet{Include: bp.GetPackages()}, repos, api.distro.ModulePlatformID(), api.arch.Name())
if err != nil {
return nil, err
}

13
internal/weldr/api_test.go
View file

@ -20,6 +20,7 @@ import (
"github.com/osbuild/osbuild-composer/internal/distro"
"github.com/osbuild/osbuild-composer/internal/distro/test_distro"
rpmmd_mock "github.com/osbuild/osbuild-composer/internal/mocks/rpmmd"
"github.com/osbuild/osbuild-composer/internal/reporegistry"
"github.com/osbuild/osbuild-composer/internal/rpmmd"
"github.com/osbuild/osbuild-composer/internal/store"
"github.com/osbuild/osbuild-composer/internal/target"
@ -34,14 +35,22 @@ import (
func createWeldrAPI(tempdir string, fixtureGenerator rpmmd_mock.FixtureGenerator) (*API, *store.Store) {
fixture := fixtureGenerator(tempdir)
rpm := rpmmd_mock.NewRPMMDMock(fixture)
repos := []rpmmd.RepoConfig{{Name: "test-id", BaseURL: "http://example.com/test/os/x86_64", CheckGPG: true}}
rr := reporegistry.NewFromDistrosRepoConfigs(rpmmd.DistrosRepoConfigs{
test_distro.TestDistroName: {
test_distro.TestArchName: {
{Name: "test-id", BaseURL: "http://example.com/test/os/x86_64", CheckGPG: true},
},
},
})
d := test_distro.New()
arch, err := d.GetArch(test_distro.TestArchName)
if err != nil {
panic(err)
}
return New(rpm, arch, d, repos, nil, fixture.Store, fixture.Workers, ""), fixture.Store
return New(rpm, arch, d, rr, nil, fixture.Store, fixture.Workers, ""), fixture.Store
}
func TestBasic(t *testing.T) {