From e1879b67a7e55d0c9220684e83942c68e58b9576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Thu, 12 Sep 2024 12:41:25 +0200 Subject: [PATCH 1/3] Fix loading of tested distro repos for unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NewTestedDefault() was returning an empty repositories list when used within unit tests. And unfortunately, this didn't trigger any error and no test was checking that the list is not empty, in addition to checking the error. Modify `Reporegistry.New()` to return error if no repos were loaded. Modify the `NewTestedDefault()` to consistently determine the path to load repositories from, regardless of the situation in which it is being called. Signed-off-by: Tomáš Hozza --- pkg/distro/distro_test_common/distro_test_common.go | 5 ++++- pkg/reporegistry/reporegistry.go | 13 ++++++++++++- pkg/reporegistry/reporegistry_test.go | 7 +++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/pkg/distro/distro_test_common/distro_test_common.go b/pkg/distro/distro_test_common/distro_test_common.go index 2afb35d2bf..035c5fcc00 100644 --- a/pkg/distro/distro_test_common/distro_test_common.go +++ b/pkg/distro/distro_test_common/distro_test_common.go @@ -465,5 +465,8 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) { func ListTestedDistros(t *testing.T) []string { testRepoRegistry, err := reporegistry.NewTestedDefault() require.Nil(t, err) - return testRepoRegistry.ListDistros() + require.NotEmpty(t, testRepoRegistry) + distros := testRepoRegistry.ListDistros() + require.NotEmpty(t, distros) + return distros } diff --git a/pkg/reporegistry/reporegistry.go b/pkg/reporegistry/reporegistry.go index aab3594817..8934fde245 100644 --- a/pkg/reporegistry/reporegistry.go +++ b/pkg/reporegistry/reporegistry.go @@ -2,6 +2,8 @@ package reporegistry import ( "fmt" + "path/filepath" + "runtime" "github.com/osbuild/images/pkg/distroidparser" "github.com/osbuild/images/pkg/rpmmd" @@ -21,6 +23,9 @@ func New(repoConfigPaths []string) (*RepoRegistry, error) { if err != nil { return nil, err } + if len(repositories) == 0 { + return nil, fmt.Errorf("no repositories found in the given paths: %v", repoConfigPaths) + } return &RepoRegistry{repositories}, nil } @@ -28,7 +33,13 @@ func New(repoConfigPaths []string) (*RepoRegistry, error) { // NewTestedDefault returns a new RepoRegistry instance with the data // loaded from the default test repositories func NewTestedDefault() (*RepoRegistry, error) { - testReposPath := []string{"./test/data"} + _, 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) } diff --git a/pkg/reporegistry/reporegistry_test.go b/pkg/reporegistry/reporegistry_test.go index 7124f42cad..55d1359a61 100644 --- a/pkg/reporegistry/reporegistry_test.go +++ b/pkg/reporegistry/reporegistry_test.go @@ -370,3 +370,10 @@ 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()) +} From ed9713755646598aa81d9e811516aa0aefbcf76f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Thu, 12 Sep 2024 13:04:54 +0200 Subject: [PATCH 2/3] Distro: fix TestDistro_ManifestFIPSWarning for RHEL-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the test case to correctly skip RHEL-7 distros, after the loading of test distros has been fixed. Signed-off-by: Tomáš Hozza --- pkg/distro/distro_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/distro/distro_test.go b/pkg/distro/distro_test.go index 56cc771c30..c93a72ec2d 100644 --- a/pkg/distro/distro_test.go +++ b/pkg/distro/distro_test.go @@ -583,7 +583,7 @@ func TestDistro_ManifestFIPSWarning(t *testing.T) { distros := distro_test_common.ListTestedDistros(t) for _, distroName := range distros { // FIPS blueprint customization is not supported for RHEL 7 images - if distroName == "rhel-7" { + if strings.HasPrefix(distroName, "rhel-7") { continue } d := distroFactory.GetDistro(distroName) From f034e96b39f6fbac0ece7c912813ee291b96ec62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Thu, 12 Sep 2024 13:34:55 +0200 Subject: [PATCH 3/3] Distro: remove PackageSetsChains() from ImageType interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the function from the ImageType interface, because it is not used anywhere and it just adds to the potential confusion. Signed-off-by: Tomáš Hozza --- pkg/distro/distro.go | 3 -- pkg/distro/distro_test.go | 55 -------------------------------- pkg/distro/fedora/imagetype.go | 4 --- pkg/distro/rhel/imagetype.go | 4 --- pkg/distro/test_distro/distro.go | 6 ---- 5 files changed, 72 deletions(-) diff --git a/pkg/distro/distro.go b/pkg/distro/distro.go index dfe0582171..a4f8bfd8fc 100644 --- a/pkg/distro/distro.go +++ b/pkg/distro/distro.go @@ -132,9 +132,6 @@ type ImageType interface { // Returns the package set names safe to install custom packages via custom repositories. PayloadPackageSets() []string - // Returns named arrays of package set names which should be depsolved in a chain. - PackageSetsChains() map[string][]string - // Returns the names of the stages that will produce the build output. Exports() []string diff --git a/pkg/distro/distro_test.go b/pkg/distro/distro_test.go index c93a72ec2d..5257f73eb3 100644 --- a/pkg/distro/distro_test.go +++ b/pkg/distro/distro_test.go @@ -21,61 +21,6 @@ import ( "github.com/stretchr/testify/require" ) -// Ensure that all package sets defined in the package set chains are defined for the image type -func TestImageType_PackageSetsChains(t *testing.T) { - distroFactory := distrofactory.NewDefault() - distros := distro_test_common.ListTestedDistros(t) - for _, distroName := range distros { - d := distroFactory.GetDistro(distroName) - for _, archName := range d.ListArches() { - arch, err := d.GetArch(archName) - require.Nil(t, err) - for _, imageTypeName := range arch.ListImageTypes() { - t.Run(fmt.Sprintf("%s/%s/%s", distroName, archName, imageTypeName), func(t *testing.T) { - imageType, err := arch.GetImageType(imageTypeName) - require.Nil(t, err) - - // set up bare minimum args for image type - var customizations *blueprint.Customizations - if imageType.Name() == "edge-simplified-installer" || imageType.Name() == "iot-simplified-installer" { - customizations = &blueprint.Customizations{ - InstallationDevice: "/dev/null", - } - } - bp := blueprint.Blueprint{ - Customizations: customizations, - } - options := distro.ImageOptions{ - OSTree: &ostree.ImageOptions{ - URL: "https://example.com", // required by some image types - }, - } - manifest, _, err := imageType.Manifest(&bp, options, nil, 0) - require.NoError(t, err) - imagePkgSets := manifest.GetPackageSetChains() - for packageSetName := range imageType.PackageSetsChains() { - _, ok := imagePkgSets[packageSetName] - if !ok { - // in the new pipeline generation logic the name of the package - // set chains are taken from the pipelines and do not match the - // package set names. - // TODO: redefine package set chains to make this unneccesary - switch packageSetName { - case "packages": - _, ok = imagePkgSets["os"] - if !ok { - _, ok = imagePkgSets["ostree-tree"] - } - } - } - assert.Truef(t, ok, "package set %q defined in a package set chain is not present in the image package sets", packageSetName) - } - }) - } - } - } -} - // Ensure all image types report the correct names for their pipelines. // Each image type contains a list of build and payload pipelines. They are // needed for knowing the names of pipelines from the static object without diff --git a/pkg/distro/fedora/imagetype.go b/pkg/distro/fedora/imagetype.go index 1300b5747f..daad76a55c 100644 --- a/pkg/distro/fedora/imagetype.go +++ b/pkg/distro/fedora/imagetype.go @@ -118,10 +118,6 @@ func (t *imageType) PayloadPackageSets() []string { return []string{blueprintPkgsKey} } -func (t *imageType) PackageSetsChains() map[string][]string { - return make(map[string][]string) -} - func (t *imageType) Exports() []string { if len(t.exports) > 0 { return t.exports diff --git a/pkg/distro/rhel/imagetype.go b/pkg/distro/rhel/imagetype.go index 0c055c96a6..5c0b9b45c6 100644 --- a/pkg/distro/rhel/imagetype.go +++ b/pkg/distro/rhel/imagetype.go @@ -161,10 +161,6 @@ func (t *ImageType) PayloadPackageSets() []string { return []string{BlueprintPkgsKey} } -func (t *ImageType) PackageSetsChains() map[string][]string { - return nil -} - func (t *ImageType) Exports() []string { if len(t.exports) > 0 { return t.exports diff --git a/pkg/distro/test_distro/distro.go b/pkg/distro/test_distro/distro.go index f8354ab806..90bcf7531b 100644 --- a/pkg/distro/test_distro/distro.go +++ b/pkg/distro/test_distro/distro.go @@ -226,12 +226,6 @@ func (t *TestImageType) PayloadPackageSets() []string { return []string{blueprintPkgsKey} } -func (t *TestImageType) PackageSetsChains() map[string][]string { - return map[string][]string{ - osPkgsKey: {osPkgsKey, blueprintPkgsKey}, - } -} - func (t *TestImageType) Exports() []string { return distro.ExportsFallback() }