diff --git a/cmd/build/main.go b/cmd/build/main.go index 67c039e501..d7e6f6443b 100644 --- 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 index bb76311ce5..ffcf43a72c 100644 --- 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 index 1ac7ae634e..359837711b 100644 --- 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 index 89573aeb03..f8fb91b484 100644 --- 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 index 2289e1ec15..164663647c 100644 --- 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 index a4cdc96ea3..6e5c79541c 100644 --- 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 index dd85d2cc08..382ec26ebe 100644 --- 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 index e06b499390..844da3cd9a 100644 --- 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 index c1190fbd0f..47367c5a26 100644 --- 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()) + }) + } + +}