diff --git a/pkg/distro/defs/distros.yaml b/pkg/distro/defs/distros.yaml index a6106f754f..5fec7dc1ff 100644 --- a/pkg/distro/defs/distros.yaml +++ b/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: "^(?Prhel)-(?P9)(?P[0-9]{1,2})$" + match: '(?Prhel)-(?P9)\.?(?P[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: "^(?Prhel)-(?P8)(?P[0-9]{1,2})$" + match: '(?Prhel)-(?P8)\.?(?P[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" diff --git a/pkg/distro/defs/id.go b/pkg/distro/defs/id.go index 4a56b4f28c..434a02f0a0 100644 --- a/pkg/distro/defs/id.go +++ b/pkg/distro/defs/id.go @@ -7,11 +7,46 @@ 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 ,, + 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 { @@ -19,13 +54,12 @@ func ParseID(nameVer string) (*distro.ID, error) { } 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 diff --git a/pkg/distro/defs/id_test.go b/pkg/distro/defs/id_test.go new file mode 100644 index 0000000000..d3cf10675a --- /dev/null +++ b/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 + {`(?Prhel)-(?P8)\.?(?P[0-9]{1,2})`, "rhel-8.10", "rhel-8.10"}, + {`(?Prhel)-(?P8)\.?(?P[0-9]{1,2})`, "rhel-810", "rhel-8.10"}, + // capture groups for just major + {`(?Pcentos)-(?P[0-9])stream`, "centos-9stream", "centos-9"}, + // normalizing strange things works + {`(?P[0-9])-(?Pfoo)`, "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) + } +} diff --git a/pkg/distro/defs/loader.go b/pkg/distro/defs/loader.go index 5db32fe468..6f63850420 100644 --- a/pkg/distro/defs/loader.go +++ b/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 -{,.} form. // E.g. - // (?Prhel)-(?P8)(?P[0-9]+) - // will support a format like e.g. rhel-810 - TransformRE string `yaml:"transform_re"` + // (?Prhel)-(?P8)\.?(?P[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 } diff --git a/pkg/distro/defs/loader_test.go b/pkg/distro/defs/loader_test.go index d11127f8d0..23935b2dd4 100644 --- a/pkg/distro/defs/loader_test.go +++ b/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: "(?Prhel)-(?P8)(?P[0-9]+)" + match: '(?Prhel)-(?P8)\.?(?P[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: "(?Prhel)-(?P8)(?P[0-9]+)", + Match: `(?Prhel)-(?P8)\.?(?P[0-9]+)`, OsVersion: tc.expectedOsVersion, ReleaseVersion: "8", ModulePlatformID: "platform:el8", diff --git a/pkg/distro/generic/rhel10_internal_test.go b/pkg/distro/generic/rhel10_internal_test.go index 2d22ee7498..de263bfb96 100644 --- a/pkg/distro/generic/rhel10_internal_test.go +++ b/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)