distrofactory: support specifying distro name aliases

Some users of the `distrofactory.Factory` have a need to support getting distro instances using distro ID string, which does not correspond to a valid distro. For example, osbuild-composer has the need to allow users to specify RHEL distro names without the minor version (e.g. `rhel-9`), while maintaining the mapping on their side. There are a few limitations: - An alias can't mask explicitly supported distro. - The target of an alias must map to a valid distro. Signed-off-by: Tomáš Hozza <[email protected]>
osbuild · Jan 9, 2024 · 399d74c · 399d74c
1 parent e3a4622
commit 399d74c
Show file tree

Hide file tree

Showing 9 changed files with 151 additions and 16 deletions.
diff --git a/cmd/build/main.go b/cmd/build/main.go
@@ -228,7 +228,10 @@ func main() {
 	if err != nil {
 		panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err))
 	}
-	distroFac := distrofactory.NewDefault()
+	distroFac, err := distrofactory.NewDefault(nil)
+	if err != nil {
+		panic(fmt.Sprintf("could not create distro factory: %v", err))
+	}
 
 	config := loadConfig(configFile)
 

diff --git a/cmd/gen-manifests/main.go b/cmd/gen-manifests/main.go
@@ -532,7 +532,10 @@ func main() {
 		panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err))
 	}
 
-	distroFac := distrofactory.NewDefault()
+	distroFac, err := distrofactory.NewDefault(nil)
+	if err != nil {
+		panic(fmt.Sprintf("could not create distro factory: %v", err))
+	}
 	jobs := make([]manifestJob, 0)
 
 	contentResolve := map[string]bool{

diff --git a/cmd/list-images/main.go b/cmd/list-images/main.go
@@ -79,7 +79,10 @@ func main() {
 	if err != nil {
 		panic(fmt.Sprintf("failed to create repo registry with tested distros: %v", err))
 	}
-	distroFac := distrofactory.NewDefault()
+	distroFac, err := distrofactory.NewDefault(nil)
+	if err != nil {
+		panic(fmt.Sprintf("could not create distro factory: %v", err))
+	}
 	distros, invalidDistros := resolveArgValues(distros, testedRepoRegistry.ListDistros())
 	if len(invalidDistros) > 0 {
 		fmt.Fprintf(os.Stderr, "WARNING: invalid distro names: [%s]\n", strings.Join(invalidDistros, ","))

diff --git a/cmd/osbuild-composer-image-definitions/main.go b/cmd/osbuild-composer-image-definitions/main.go
@@ -11,7 +11,10 @@ import (
 
 func main() {
 	definitions := map[string]map[string][]string{}
-	distroFac := distrofactory.NewDefault()
+	distroFac, err := distrofactory.NewDefault(nil)
+	if err != nil {
+		panic(fmt.Sprintf("could not create distro factory: %v", err))
+	}
 
 	testedRepoRegistry, err := reporegistry.NewTestedDefault()
 	if err != nil {

diff --git a/cmd/osbuild-package-sets/main.go b/cmd/osbuild-package-sets/main.go
@@ -29,7 +29,10 @@ func main() {
 		os.Exit(1)
 	}
 
-	df := distrofactory.NewDefault()
+	df, err := distrofactory.NewDefault(nil)
+	if err != nil {
+		panic(fmt.Sprintf("could not create distro factory: %v", err))
+	}
 
 	d := df.GetDistro(distroName)
 	if d == nil {

diff --git a/cmd/osbuild-playground/main.go b/cmd/osbuild-playground/main.go
@@ -55,7 +55,10 @@ func main() {
 		}
 	}
 
-	distroFac := distrofactory.NewDefault()
+	distroFac, err := distrofactory.NewDefault(nil)
+	if err != nil {
+		panic(fmt.Sprintf("could not create distro factory: %v", err))
+	}
 	var d distro.Distro
 	if distroArg == "host" {
 		d = distroFac.FromHost()

diff --git a/pkg/distro/distro_test.go b/pkg/distro/distro_test.go
@@ -21,7 +21,8 @@ import (
 
 // 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()
+	distroFactory, err := distrofactory.NewDefault(nil)
+	require.NoError(t, err)
 	distros := distro_test_common.ListTestedDistros(t)
 	for _, distroName := range distros {
 		d := distroFactory.GetDistro(distroName)
@@ -97,7 +98,8 @@ func TestImageTypePipelineNames(t *testing.T) {
 	}
 
 	assert := assert.New(t)
-	distroFactory := distrofactory.NewDefault()
+	distroFactory, err := distrofactory.NewDefault(nil)
+	require.NoError(t, err)
 	distros := distro_test_common.ListTestedDistros(t)
 	for _, distroName := range distros {
 		d := distroFactory.GetDistro(distroName)
@@ -409,7 +411,8 @@ func TestPipelineRepositories(t *testing.T) {
 		},
 	}
 
-	distroFactory := distrofactory.NewDefault()
+	distroFactory, err := distrofactory.NewDefault(nil)
+	require.NoError(err)
 	distros := distro_test_common.ListTestedDistros(t)
 	for tName, tCase := range testCases {
 		t.Run(tName, func(t *testing.T) {

diff --git a/pkg/distrofactory/distrofactory.go b/pkg/distrofactory/distrofactory.go
@@ -19,12 +19,15 @@ type FactoryFunc func(idStr string) distro.Distro
 // Factory is a list of distro.Distro factories.
 type Factory struct {
 	factories []FactoryFunc
+
+	// distro ID string aliases
+	aliases map[string]string
 }
 
-// GetDistro returns the distro.Distro that matches the given distro ID. If no
+// getDistro returns the distro.Distro that matches the given distro ID. If no
 // distro.Distro matches the given distro ID, it returns nil. If multiple distro
 // factories match the given distro ID, it panics.
-func (f *Factory) GetDistro(name string) distro.Distro {
+func (f *Factory) getDistro(name string) distro.Distro {
 	var match distro.Distro
 	for _, f := range f.factories {
 		if d := f(name); d != nil {
@@ -38,6 +41,21 @@ func (f *Factory) GetDistro(name string) distro.Distro {
 	return match
 }
 
+// GetDistro returns the distro.Distro that matches the given distro ID. If no
+// distro.Distro matches the given distro ID, it tries to translate the given
+// distro ID using the aliases map and tries again. If no distro.Distro matches
+// the given distro ID, it returns nil. If multiple distro factories match the
+// given distro ID, it panics.
+func (f *Factory) GetDistro(name string) distro.Distro {
+	match := f.getDistro(name)
+
+	if alias, ok := f.aliases[name]; match == nil && ok {
+		match = f.getDistro(alias)
+	}
+
+	return match
+}
+
 // FromHost returns a distro.Distro instance, that is specific to the host.
 // If the host distro is not supported, nil is returned.
 func (f *Factory) FromHost() distro.Distro {
@@ -46,14 +64,44 @@ func (f *Factory) FromHost() distro.Distro {
 }
 
 // New returns a Factory of distro.Distro factories for the given distros.
-func New(factories ...FactoryFunc) *Factory {
-	return &Factory{factories: factories}
+// The provided aliases map has the following constraints:
+// - An alias must not mask an existing distro.
+// - An alias target must map to an existing distro.
+func New(aliases map[string]string, factories ...FactoryFunc) (*Factory, error) {
+	var errors []error
+	for alias, target := range aliases {
+		var targetExists bool
+		for _, factory := range factories {
+			if factory(alias) != nil {
+				errors = append(errors, fmt.Errorf("alias '%s' masks an existing distro", alias))
+			}
+			if factory(target) != nil {
+				targetExists = true
+			}
+		}
+		if !targetExists {
+			errors = append(errors, fmt.Errorf("alias '%s' targets a non-existing distro '%s'", alias, target))
+		}
+	}
+
+	if len(errors) > 0 {
+		return nil, fmt.Errorf("invalid aliases: %q", errors)
+	}
+
+	return &Factory{
+		factories: factories,
+		aliases:   aliases,
+	}, nil
 }
 
 // NewDefault returns a Factory of distro.Distro factories for all supported
 // distros.
-func NewDefault() *Factory {
+// The provided aliases map has the following constraints:
+// - An alias must not mask an existing distro.
+// - An alias target must map to an existing distro.
+func NewDefault(aliases map[string]string) (*Factory, error) {
 	return New(
+		aliases,
 		fedora.DistroFactory,
 		rhel7.DistroFactory,
 		rhel8.DistroFactory,
@@ -62,8 +110,12 @@ func NewDefault() *Factory {
 }
 
 // NewTestDefault returns a Factory of distro.Distro factory for the test_distro.
-func NewTestDefault() *Factory {
+// The provided aliases map has the following constraints:
+// - An alias must not mask an existing distro.
+// - An alias target must map to an existing distro.
+func NewTestDefault(aliases map[string]string) (*Factory, error) {
 	return New(
+		aliases,
 		test_distro.DistroFactory,
 	)
 }
diff --git a/pkg/distrofactory/distrofactory_test.go b/pkg/distrofactory/distrofactory_test.go
@@ -47,7 +47,8 @@ func TestGetDistroDefaultList(t *testing.T) {
 		},
 	}
 
-	dl := NewDefault()
+	dl, err := NewDefault(nil)
+	assert.NoError(t, err)
 
 	for _, tc := range testCases {
 		t.Run(tc.strID, func(t *testing.T) {
@@ -58,3 +59,64 @@ func TestGetDistroDefaultList(t *testing.T) {
 	}
 
 }
+
+func TestGetDistroDefaultListWithAliases(t *testing.T) {
+	type testCase struct {
+		aliases            map[string]string
+		strID              string
+		expectedDistroName string
+		fail               bool
+		errorMsg           string
+	}
+
+	testCases := []testCase{
+		{
+			aliases: map[string]string{
+				"rhel-9": "rhel-9.1",
+			},
+			strID:              "rhel-9",
+			expectedDistroName: "rhel-9.1",
+		},
+		{
+			aliases: map[string]string{
+				"best_distro-123": "rhel-9.1",
+			},
+			strID:              "best_distro-123",
+			expectedDistroName: "rhel-9.1",
+		},
+		{
+			aliases: map[string]string{
+				"rhel-9.3": "rhel-9.1",
+				"rhel-9.2": "rhel-9.1",
+			},
+			fail:     true,
+			errorMsg: `invalid aliases: ["alias 'rhel-9.3' masks an existing distro" "alias 'rhel-9.2' masks an existing distro"]`,
+		},
+		{
+			aliases: map[string]string{
+				"rhel-12": "rhel-12.12",
+				"rhel-13": "rhel-13.13",
+			},
+			fail:     true,
+			errorMsg: `invalid aliases: ["alias 'rhel-12' targets a non-existing distro 'rhel-12.12'" "alias 'rhel-13' targets a non-existing distro 'rhel-13.13'"]`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.strID, func(t *testing.T) {
+			dl, err := NewDefault(tc.aliases)
+
+			if tc.fail {
+				assert.Error(t, err)
+				assert.Equal(t, tc.errorMsg, err.Error())
+				return
+			}
+
+			assert.NoError(t, err)
+			d := dl.GetDistro(tc.strID)
+			assert.NotNil(t, d)
+			assert.Equal(t, tc.expectedDistroName, d.Name())
+		})
+	}
+
+}