distro: unify transform/match into a single concept

pkg/distro/defs/distros.yaml

@@ -43,7 +43,7 @@ distros:
   - &fedora_stable
     <<: *fedora_rawhide
     name: "fedora-{{.MajorVersion}}"
-    match: "fedora-[0-9][0-9]{,[0-9]}"
+    match: 'fedora-[0-9][0-9]+'
     preview: false
     os_version: "{{.MajorVersion}}"
     release_version: "{{.MajorVersion}}"
@@ -63,7 +63,7 @@ distros:
 
   - &rhel10
     name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: "rhel-10.[0-9]{,[0-9]}"
+    match: 'rhel-10\.[0-9]{1,2}'
     distro_like: rhel-10
     product: "Red Hat Enterprise Linux"
     os_version: "10.{{.MinorVersion}}"
@@ -115,7 +115,7 @@ distros:
 
   - <<: *rhel10
     name: "almalinux-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: "almalinux-10.[0-9]{,[0-9]}"
+    match: 'almalinux-10\.[0-9]{1,2}'
     product: "AlmaLinux"
     vendor: "almalinux"
     ostree_ref_tmpl: "almalinux/10/%%s/edge"
@@ -150,9 +150,8 @@ distros:
 
   - &rhel9
     name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: "rhel-9.[0-9]{,[0-9]}"
     # rhel9 support being named "rhel-91" for "rhel-9.1" or "rhel-910" for "rhel-9.10" etc
-    transform_re: "^(?P<name>rhel)-(?P<major>9)(?P<minor>[0-9]{1,2})$"
+    match: '(?P<name>rhel)-(?P<major>9)\.?(?P<minor>[0-9]{1,2})'
     distro_like: rhel-9
     product: "Red Hat Enterprise Linux"
     os_version: "9.{{.MinorVersion}}"
@@ -217,9 +216,8 @@ distros:
 
   - &rhel8
     name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: "rhel-8.[0-9]{,[0-9]}"
     # rhel8 support being named "rhel-81" for "rhel-8.1" or "rhel-810" for "rhel-8.10" etc
-    transform_re: "^(?P<name>rhel)-(?P<major>8)(?P<minor>[0-9]{1,2})$"
+    match: '(?P<name>rhel)-(?P<major>8)\.?(?P<minor>[0-9]{1,2})'
     distro_like: rhel-8
     product: "Red Hat Enterprise Linux"
     os_version: "8.{{.MinorVersion}}"
@@ -289,7 +287,7 @@ distros:
 
   - &rhel7
     name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: "rhel-7.[0-9]{,[0-9]}"
+    match: 'rhel-7\.[0-9]{1,2}'
     distro_like: rhel-7
     product: "Red Hat Enterprise Linux"
     codename: "Maipo"

pkg/distro/defs/id.go

@@ -7,25 +7,59 @@ import (
 	"github.com/osbuild/images/pkg/distro"
 )
 
+// matchAndNormalize() matches and normalizes the given nameVer
+// based on the reStr. On match it returns the normalized version
+// of the given nameVer.
+func matchAndNormalize(reStr, nameVer string) (string, error) {
+	if reStr == "" {
+		return "", nil
+	}
+
+	re, err := regexp.Compile(`^` + reStr + `$`)
+	if err != nil {
+		return "", fmt.Errorf("cannot use %q: %w", reStr, err)
+	}
+	l := re.FindStringSubmatch(nameVer)
+	switch len(l) {
+	case 0:
+		// no match
+		return "", nil
+	case 1:
+		// simple match, no named matching
+		return nameVer, nil
+	case 2:
+		// incomplete match, user did not provide <name>,<major>,<minor>
+		return "", fmt.Errorf("invalid number of submatches for %q %q (%v)", reStr, nameVer, len(l))
+	case 3:
+		// distro only uses major ver and needs normalizing
+		return fmt.Sprintf("%s-%s", l[re.SubexpIndex("name")], l[re.SubexpIndex("major")]), nil
+	case 4:
+		// common case, major/minor and normalizing
+		return fmt.Sprintf("%s-%s.%s", l[re.SubexpIndex("name")], l[re.SubexpIndex("major")], l[re.SubexpIndex("minor")]), nil
+	default:
+		return "", fmt.Errorf("invalid number of submatches for %q %q (%v)", reStr, nameVer, len(l))
+	}
+
+}
+
 // ParseID parse the given nameVer into a distro.ID. It will also
-// apply any matching `transform_re`. This is needed to support distro
-// names like "rhel-810" without dots.
+// apply normalizations from the distros `match` rule. This is needed
+// to support distro names like "rhel-810" without dots.
 //
-// If no transformations are needed it will return "nil"
+// If no match is found it will "nil" and no error (
 func ParseID(nameVer string) (*distro.ID, error) {
 	distros, err := loadDistros()
 	if err != nil {
 		return nil, err
 	}
 
 	for _, d := range distros.Distros {
-		re, err := regexp.Compile(d.TransformRE)
+		found, err := matchAndNormalize(d.Match, nameVer)
 		if err != nil {
 			return nil, err
 		}
-		if l := re.FindStringSubmatch(nameVer); len(l) == 4 {
-			transformed := fmt.Sprintf("%s-%s.%s", l[re.SubexpIndex("name")], l[re.SubexpIndex("major")], l[re.SubexpIndex("minor")])
-			return distro.ParseID(transformed)
+		if found != "" {
+			return distro.ParseID(found)
 		}
 	}
 	return nil, nil

pkg/distro/defs/id_test.go

@@ -0,0 +1,46 @@
+package defs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMatchAndNormalizeHappy(t *testing.T) {
+	for _, tc := range []struct {
+		reStr, nameVer string
+		expected       string
+	}{
+		// simple cases, no capture groups
+		{`rhel-10\.[0-9]{1,2}`, "rhel-100", ""},
+		{`rhel-10\.[0-9]{1,2}`, "rhel-10.0", "rhel-10.0"},
+		// capture groups for major/minor
+		{`(?P<name>rhel)-(?P<major>8)\.?(?P<minor>[0-9]{1,2})`, "rhel-8.10", "rhel-8.10"},
+		{`(?P<name>rhel)-(?P<major>8)\.?(?P<minor>[0-9]{1,2})`, "rhel-810", "rhel-8.10"},
+		// capture groups for just major
+		{`(?P<name>centos)-(?P<major>[0-9])stream`, "centos-9stream", "centos-9"},
+		// normalizing strange things works
+		{`(?P<major>[0-9])-(?P<name>foo)`, "8-foo", "foo-8"},
+	} {
+		found, err := matchAndNormalize(tc.reStr, tc.nameVer)
+		assert.NoError(t, err)
+		assert.Equal(t, found, tc.expected)
+	}
+}
+
+func TestMatchAndNormalizeSad(t *testing.T) {
+	for _, tc := range []struct {
+		reStr, nameVer string
+		expectedErr    string
+	}{
+		// simple cases, bad regex
+		{`rhel-10[`, "rhel-100", `cannot use "rhel-10[": error parsing regexp: missing closing ]`},
+		// incomplete capture groups
+		{`rhel-([0-9]+)`, "rhel-100", `invalid number of submatches for "rhel-([0-9]+)" "rhel-100" (2)`},
+		// too many capture groups
+		{`(rhel)-([0-9])([0-9])([0-9])`, "rhel-100", `invalid number of submatches for "(rhel)-([0-9])([0-9])([0-9])" "rhel-100" (5)`},
+	} {
+		_, err := matchAndNormalize(tc.reStr, tc.nameVer)
+		assert.ErrorContains(t, err, tc.expectedErr)
+	}
+}

pkg/distro/defs/loader.go

@@ -13,7 +13,6 @@ import (
 	"sort"
 	"text/template"
 
-	"github.com/gobwas/glob"
 	"gopkg.in/yaml.v3"
 
 	"github.com/osbuild/images/internal/common"
@@ -59,16 +58,12 @@ type distrosYAML struct {
 }
 
 type DistroYAML struct {
-	// Match can be used to match multiple versions via a
-	// fnmatch/glob style expression.
-	Match string `yaml:"match"`
-
-	// TransformRE can be used to transform a given name
+	// Match can be used to match/transform a given name
 	// into the canonical <distro>-<major>{,.<minor>} form.
 	// E.g.
-	//   (?P<distro>rhel)-(?P<major>8)(?P<minor>[0-9]+)
-	// will support a format like e.g. rhel-810
-	TransformRE string `yaml:"transform_re"`
+	//   (?P<distro>rhel)-(?P<major>8)\.?(?P<minor>[0-9]+)
+	// will support a format like e.g. rhel-810 and rhel-8.10
+	Match string `yaml:"match"`
 
 	// The distro metadata, can contain go text template strings
 	// for {{.Major}}, {{.Minor}} which will be expanded by the
@@ -211,13 +206,12 @@ func NewDistroYAML(nameVer string) (*DistroYAML, error) {
 			break
 		}
 
-		pat, err := glob.Compile(distro.Match)
+		found, err := matchAndNormalize(distro.Match, nameVer)
 		if err != nil {
 			return nil, err
 		}
-
-		if pat.Match(nameVer) {
-			if err := distro.runTemplates(nameVer); err != nil {
+		if found != "" {
+			if err := distro.runTemplates(found); err != nil {
 				return nil, err
 			}
 

pkg/distro/defs/loader_test.go

@@ -1200,12 +1200,11 @@ distros:
 	assert.EqualError(t, err, `platform conditionals for image type "server-qcow2" should match only once but matched 2 times`)
 }
 
-func TestDistrosLoadingTransformRE(t *testing.T) {
+func TestDistrosLoadingMatchTransforms(t *testing.T) {
 	fakeDistrosYAML := `
 distros:
   - name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: "rhel-8.*"
-    transform_re: "(?P<name>rhel)-(?P<major>8)(?P<minor>[0-9]+)"
+    match: '(?P<name>rhel)-(?P<major>8)\.?(?P<minor>[0-9]+)'
     os_version: "{{.MajorVersion}}.{{.MinorVersion}}"
     release_version: "{{.MajorVersion}}"
     module_platform_id: "platform:el{{.MajorVersion}}"
@@ -1230,8 +1229,7 @@ distros:
 		require.NoError(t, err)
 		assert.Equal(t, &defs.DistroYAML{
 			Name:             tc.expectedDistroNameVer,
-			Match:            "rhel-8.*",
-			TransformRE:      "(?P<name>rhel)-(?P<major>8)(?P<minor>[0-9]+)",
+			Match:            `(?P<name>rhel)-(?P<major>8)\.?(?P<minor>[0-9]+)`,
 			OsVersion:        tc.expectedOsVersion,
 			ReleaseVersion:   "8",
 			ModulePlatformID: "platform:el8",

pkg/distro/generic/rhel10_internal_test.go

@@ -66,6 +66,7 @@ func TestRH10DistroFactory(t *testing.T) {
 func TestRhel10_NoBootPartition(t *testing.T) {
 	for _, distroName := range []string{"rhel-10.0", "centos-10"} {
 		dist := DistroFactory(distroName)
+		require.NotNil(t, dist, distroName)
 		for _, archName := range dist.ListArches() {
 			arch, err := dist.GetArch(archName)
 			assert.NoError(t, err)