distro: small followups for PR#1682

pkg/distro/defs/distros.yaml

@@ -43,7 +43,7 @@ distros:
   - &fedora_stable
     <<: *fedora_rawhide
     name: "fedora-{{.MajorVersion}}"
-    match: 'fedora-[0-9][0-9]+'
+    match: 'fedora-[1-9][0-9]+'
     preview: false
     os_version: "{{.MajorVersion}}"
     release_version: "{{.MajorVersion}}"
@@ -297,7 +297,7 @@ distros:
 
   - &rhel7
     name: "rhel-{{.MajorVersion}}.{{.MinorVersion}}"
-    match: 'rhel-7\.[0-9]{1,2}'
+    match: 'rhel-7\.[0-9]'
     distro_like: rhel-7
     product: "Red Hat Enterprise Linux"
     codename: "Maipo"

pkg/distro/defs/id.go

@@ -7,6 +7,15 @@ import (
 	"github.com/osbuild/images/pkg/distro"
 )
 
+func validateSubexpMatch(re *regexp.Regexp, names ...string) error {
+	for _, name := range names {
+		if re.SubexpIndex(name) == -1 {
+			return fmt.Errorf("cannot find submatch field %q", name)
+		}
+	}
+	return nil
+}
+
 // matchAndNormalize() matches and normalizes the given nameVer
 // based on the reStr. On match it returns the normalized version
 // of the given nameVer.
@@ -27,14 +36,19 @@ func matchAndNormalize(reStr, nameVer string) (string, error) {
 	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))
+	// handling case 2: is not needed, its an incomplete match, we need at least name,major and
+	// captured by the "default" below
 	case 3:
 		// distro only uses major ver and needs normalizing
+		if err := validateSubexpMatch(re, "name", "major"); err != nil {
+			return "", err
+		}
 		return fmt.Sprintf("%s-%s", l[re.SubexpIndex("name")], l[re.SubexpIndex("major")]), nil
 	case 4:
 		// common case, major/minor and normalizing
+		if err := validateSubexpMatch(re, "name", "major", "minor"); err != nil {
+			return "", err
+		}
 		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))

pkg/distro/defs/id_test.go

@@ -39,6 +39,10 @@ func TestMatchAndNormalizeSad(t *testing.T) {
 		{`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)`},
+		// capture groups have incorrect names
+		{`(?P<missingName>centos)-(?P<major>[0-9])stream`, "centos-9stream", `cannot find submatch field "name"`},
+		{`(?P<name>centos)-(?P<missingMajor>[0-9])stream`, "centos-9stream", `cannot find submatch field "major"`},
+		{`(?P<name>rhel)-(?P<major>8)\.?(?P<missingMinor>[0-9]{1,2})`, "rhel-8.10", `cannot find submatch field "minor"`},
 	} {
 		_, err := matchAndNormalize(tc.reStr, tc.nameVer)
 		assert.ErrorContains(t, err, tc.expectedErr)

pkg/distro/generic/fedora_test.go

@@ -965,11 +965,7 @@ func TestFedoraDistroFactory(t *testing.T) {
 			expected: nil,
 		},
 		{
-			strID:    "rhel-9",
-			expected: nil,
-		},
-		{
-			strID:    "rhel-8.4.1",
+			strID:    "fedora-043",
 			expected: nil,
 		},
 	}

pkg/distro/generic/rhel7_test.go

@@ -449,7 +449,13 @@ func TestRhel7DistroFactory(t *testing.T) {
 			expected: nil,
 		},
 		{
-			strID:    "rhel-79", // this is intentionally not supported for el7
+			// the latest RHEL-7 is 7.9 and there won't be any newer one
+			strID:    "rhel-7.10",
+			expected: nil,
+		},
+		{
+			// this is intentionally not supported for el7
+			strID:    "rhel-79",
 			expected: nil,
 		},
 		{

pkg/distro/generic/rhel8_test.go

@@ -983,3 +983,37 @@ func TestRH8_DiskCustomizationsCheckOptions(t *testing.T) {
 		}
 	}
 }
+
+func TestRhel8_DistroFactory(t *testing.T) {
+	type testCase struct {
+		strID    string
+		expected distro.Distro
+	}
+
+	testCases := []testCase{
+		{
+			strID:    "rhel-8.10",
+			expected: generic.DistroFactory("rhel-8.10"),
+		},
+		{
+			strID:    "rhel-8.4.1",
+			expected: nil,
+		},
+		{
+			strID:    "rhel-8",
+			expected: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.strID, func(t *testing.T) {
+			d := generic.DistroFactory(tc.strID)
+			if tc.expected == nil {
+				assert.Nil(t, d)
+			} else {
+				assert.NotNil(t, d)
+				assert.Equal(t, tc.expected.Name(), d.Name())
+			}
+		})
+	}
+}

pkg/distro/generic/rhel9_test.go

@@ -939,3 +939,37 @@ func TestRhel9_NoDiskCustomizationsNoError(t *testing.T) {
 		}
 	}
 }
+
+func TestRhel9_DistroFactory(t *testing.T) {
+	type testCase struct {
+		strID    string
+		expected distro.Distro
+	}
+
+	testCases := []testCase{
+		{
+			strID:    "rhel-9.6",
+			expected: generic.DistroFactory("rhel-9.6"),
+		},
+		{
+			strID:    "rhel-9.6.1",
+			expected: nil,
+		},
+		{
+			strID:    "rhel-9",
+			expected: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.strID, func(t *testing.T) {
+			d := generic.DistroFactory(tc.strID)
+			if tc.expected == nil {
+				assert.Nil(t, d)
+			} else {
+				assert.NotNil(t, d)
+				assert.Equal(t, tc.expected.Name(), d.Name())
+			}
+		})
+	}
+}