Skip to content

Commit 0bf4883

Browse files
rverdilerverdile
authored
HMS-8948: fix parsing stream versions (#39)
* HMS-8948: fix parsing stream versions Stream versions that were floats with trailing zeros were not being parsed correctly. i.e. perl 5.30 -> perl 5.3. Adds custom unmarshal for streams so that the trailing zeros are preserved * Build: update linter to v2
1 parent 47f22f9 commit 0bf4883

9 files changed

Lines changed: 137 additions & 67 deletions

File tree

‎.github/workflows/utility-actions.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818
with:
1919
go-version: "1.24"
2020
- name: golangci-lint
21-
uses: golangci/golangci-lint-action@v2
21+
uses: golangci/golangci-lint-action@v7
2222
with:
2323
version: latest
2424
skip-go-installation: true

‎.golangci.yaml‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,36 @@
1-
# Configuration for golangci-lint. See https://golangci-lint.run/usage/configuration/.
1+
version: "2"
22
linters:
3-
disable-all: false # use default linters
43
enable:
5-
- gofmt
6-
- whitespace
7-
- govet
8-
- misspell
4+
- bodyclose
95
- forcetypeassert
6+
- gosec
7+
- misspell
8+
- whitespace
9+
settings:
10+
gosec:
11+
excludes:
12+
- G404
13+
exclusions:
14+
generated: lax
15+
presets:
16+
- comments
17+
- common-false-positives
18+
- legacy
19+
- std-error-handling
20+
rules:
21+
- path: (.+)\.go$
22+
text: composite
23+
paths:
24+
- third_party$
25+
- builtin$
26+
- examples$
27+
formatters:
28+
enable:
1029
- gci
11-
- bodyclose
12-
issues:
13-
exclude:
14-
- composite
30+
- gofmt
31+
exclusions:
32+
generated: lax
33+
paths:
34+
- third_party$
35+
- builtin$
36+
- examples$

‎go.mod‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ require (
77
github.com/goccy/go-yaml v1.18.0
88
github.com/h2non/filetype v1.1.3
99
github.com/klauspost/compress v1.18.0
10-
github.com/mitchellh/mapstructure v1.5.0
1110
github.com/stretchr/testify v1.11.1
1211
github.com/ulikunitz/xz v0.5.15
1312
)

‎go.sum‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ github.com/h2non/filetype v1.1.3 h1:FKkx9QbD7HR/zjK1Ia5XiBsq9zdLi5Kf3zGyFTAFkGg=
1010
github.com/h2non/filetype v1.1.3/go.mod h1:319b3zT68BvV+WRj7cwy856M2ehB3HqNOt6sy1HndBY=
1111
github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
1212
github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
13-
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
14-
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
1513
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
1614
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
1715
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=

‎pkg/yum/mocks/module.yaml.zst‎

321 Bytes
Binary file not shown.

‎pkg/yum/mocks/rhel8.modules.yaml.gz‎

69.5 KB
Binary file not shown.

‎pkg/yum/module_stream.go‎

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ import (
66
"fmt"
77
"io"
88
"net/http"
9+
"strings"
910

1011
"github.com/goccy/go-yaml"
11-
"github.com/mitchellh/mapstructure"
12+
"github.com/goccy/go-yaml/ast"
1213
)
1314

1415
// Better userfacing struct
@@ -18,28 +19,48 @@ type ModuleStream struct {
1819
}
1920

2021
type Stream struct {
21-
Name string `mapstructure:"name"`
22-
Stream string `mapstructure:"stream"`
23-
Version string `mapstructure:"version"`
24-
Context string `mapstructure:"context"`
25-
Arch string `mapstructure:"arch"`
26-
Summary string `mapstructure:"summary"`
27-
Description string `mapstructure:"description"`
28-
Artifacts Artifacts `mapstructure:"artifacts"`
29-
Profiles map[string]RpmProfiles `mapstructure:"profiles"`
22+
Name string `yaml:"name"`
23+
Stream StreamVersion `yaml:"stream"`
24+
Version string `yaml:"version"`
25+
Context string `yaml:"context"`
26+
Arch string `yaml:"arch"`
27+
Summary string `yaml:"summary"`
28+
Description string `yaml:"description"`
29+
Artifacts Artifacts `yaml:"artifacts"`
30+
Profiles map[string]RpmProfiles `yaml:"profiles"`
31+
}
32+
33+
type StreamVersion string
34+
35+
// unmarshalStreamVersion ensures trailing zeros is preserved
36+
// in cases are the stream value is a float like 5.30
37+
func unmarshalStreamVersion(s *StreamVersion, data []byte) error {
38+
str := strings.TrimSpace(string(data))
39+
40+
//Remove additional quotes when stream is represented as string
41+
if len(str) >= 2 && str[0] == '"' && str[len(str)-1] == '"' {
42+
str = str[1 : len(str)-1]
43+
}
44+
45+
*s = StreamVersion(str)
46+
return nil
47+
}
48+
49+
func (s StreamVersion) String() string {
50+
return string(s)
3051
}
3152

3253
type RpmProfiles struct {
33-
Rpms []string `mapstructure:"rpms"`
54+
Rpms []string `yaml:"rpms"`
3455
}
3556

3657
type Artifacts struct {
37-
Rpms []string `mapstructure:"rpms"`
58+
Rpms []string `yaml:"rpms"`
3859
}
3960

4061
type ModuleMD struct {
41-
Document string `mapstructure:"document"`
42-
Version int `mapstructure:"version"`
62+
Document string `yaml:"document"`
63+
Version int `yaml:"version"`
4364
Data Stream `yaml:"data"`
4465
}
4566

@@ -83,12 +104,10 @@ func (r *Repository) ModuleMDs(ctx context.Context) ([]ModuleMD, int, error) {
83104
return moduleMDs, 0, err
84105
}
85106

86-
// parses modulemd objects from a given io reader
87-
// modules yaml files include different types of documents which is hard to parse
88-
// this implements a two step process:
89-
//
90-
// Parse each document into a map, with the value of interface, and then
91-
// use mapstructure to parse the interface into a ModuleMD struct
107+
// parseModuleMDs moduleMDs contain multiple document types
108+
// this breaks parsing into two parts:
109+
// 1. use node to read the document type
110+
// 2. if the document type is modulemd, fully decode the value
92111
func parseModuleMDs(body io.ReadCloser) ([]ModuleMD, error) {
93112
moduleMDs := make([]ModuleMD, 0)
94113

@@ -97,32 +116,30 @@ func parseModuleMDs(body io.ReadCloser) ([]ModuleMD, error) {
97116
return moduleMDs, fmt.Errorf("error extracting compressed streams: %w", err)
98117
}
99118

119+
yaml.RegisterCustomUnmarshaler[StreamVersion](unmarshalStreamVersion)
120+
100121
decoder := yaml.NewDecoder(reader)
101122
for {
102-
var doc map[string]interface{}
103-
104-
// Decode the next document
105-
err := decoder.Decode(&doc)
123+
var node ast.Node
124+
err := decoder.Decode(&node)
106125
if err != nil {
107126
if errors.Is(err, io.EOF) {
108127
break
109128
}
110129
return nil, fmt.Errorf("error decoding streams: %w", err)
111130
}
112-
// Only care about modulemds right now
113-
if doc["document"] == "modulemd" {
131+
132+
var docType struct {
133+
Document string `yaml:"document"`
134+
}
135+
if err := yaml.NodeToValue(node, &docType); err != nil {
136+
return nil, fmt.Errorf("error decoding document type: %w", err)
137+
}
138+
139+
if docType.Document == "modulemd" {
114140
var module ModuleMD
115-
config := &mapstructure.DecoderConfig{
116-
WeaklyTypedInput: true,
117-
Result: &module,
118-
}
119-
mapDecode, err := mapstructure.NewDecoder(config)
120-
if err != nil {
121-
return moduleMDs, fmt.Errorf("error creating map decoder: %w", err)
122-
}
123-
err = mapDecode.Decode(doc)
124-
if err != nil {
125-
return nil, fmt.Errorf("error decoding map: %w", err)
141+
if err := yaml.NodeToValue(node, &module); err != nil {
142+
return nil, fmt.Errorf("error decoding modulemd: %w", err)
126143
}
127144
moduleMDs = append(moduleMDs, module)
128145
}

‎pkg/yum/module_stream_test.go‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,33 @@ func TestParseModuleMDs(t *testing.T) {
1515

1616
parsed, err := parseModuleMDs(f)
1717
assert.NoError(t, err)
18-
assert.Equal(t, 11, len(parsed))
18+
assert.Equal(t, 13, len(parsed))
1919
assert.NotEmpty(t, parsed[0].Data.Name)
2020
assert.NotEmpty(t, parsed[0].Data.Artifacts.Rpms)
2121
}
2222

23+
func TestStreamVersionPrecision(t *testing.T) {
24+
f, err := os.Open("mocks/module.yaml.zst")
25+
assert.NoError(t, err)
26+
27+
parsed, err := parseModuleMDs(f)
28+
assert.NoError(t, err)
29+
30+
handlesFloatFound, handlesStringFound := false, false
31+
for _, module := range parsed {
32+
if module.Data.Name == "testmodule" {
33+
handlesFloatFound = true
34+
assert.Equal(t, "5.30", module.Data.Stream.String())
35+
}
36+
if module.Data.Name == "testmodule-2" {
37+
handlesStringFound = true
38+
assert.Equal(t, "5.30", module.Data.Stream.String())
39+
}
40+
}
41+
assert.True(t, handlesFloatFound)
42+
assert.True(t, handlesStringFound)
43+
}
44+
2345
func TestParseRhel8Modules(t *testing.T) {
2446
f, err := os.Open("mocks/rhel8.modules.yaml.gz")
2547
assert.NoError(t, err)
@@ -29,19 +51,28 @@ func TestParseRhel8Modules(t *testing.T) {
2951
modules, err := parseModuleMDs(f)
3052
require.NoError(t, err)
3153

32-
assert.Len(t, modules, 862)
54+
assert.Len(t, modules, 961)
3355

3456
assert.NotEmpty(t, modules)
35-
found := false
57+
foundRuby, foundPerl := false, false
3658
for _, module := range modules {
3759
if module.Data.Name == "ruby" && module.Data.Stream == "2.5" {
38-
found = true
60+
foundRuby = true
3961
assert.NotEmpty(t, module.Data.Artifacts.Rpms)
4062
assert.NotEmpty(t, module.Data.Profiles)
4163
value, ok := module.Data.Profiles["common"]
4264
assert.True(t, ok)
4365
assert.Equal(t, []string{"ruby"}, value.Rpms)
4466
}
67+
if module.Data.Name == "perl" && module.Data.Stream == "5.30" {
68+
foundPerl = true
69+
assert.NotEmpty(t, module.Data.Artifacts.Rpms)
70+
assert.NotEmpty(t, module.Data.Profiles)
71+
value, ok := module.Data.Profiles["common"]
72+
assert.True(t, ok)
73+
assert.Equal(t, []string{"perl"}, value.Rpms)
74+
}
4575
}
46-
assert.True(t, found)
76+
assert.True(t, foundRuby)
77+
assert.True(t, foundPerl)
4778
}

‎pkg/yum/repository.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (r *Repository) Repomd(ctx context.Context) (*Repomd, int, error) {
158158
return r.repomd, 0, nil
159159
}
160160
if repomdURL, err = r.getRepomdURL(); err != nil {
161-
return nil, 0, fmt.Errorf("Error parsing Repomd URL: %w", err)
161+
return nil, 0, fmt.Errorf("error parsing Repomd URL: %w", err)
162162
}
163163

164164
req, err := http.NewRequestWithContext(ctx, http.MethodGet, repomdURL, nil)
@@ -172,10 +172,10 @@ func (r *Repository) Repomd(ctx context.Context) (*Repomd, int, error) {
172172
defer resp.Body.Close()
173173

174174
if resp.StatusCode != http.StatusOK {
175-
return nil, resp.StatusCode, fmt.Errorf("Cannot fetch %v: %v", repomdURL, resp.StatusCode)
175+
return nil, resp.StatusCode, fmt.Errorf("cannot fetch %v: %v", repomdURL, resp.StatusCode)
176176
}
177177
if result, err = ParseRepomdXML(resp.Body); err != nil {
178-
return nil, resp.StatusCode, fmt.Errorf("Error parsing repomd.xml: %w", err)
178+
return nil, resp.StatusCode, fmt.Errorf("error parsing repomd.xml: %w", err)
179179
}
180180

181181
r.repomd = &result
@@ -249,7 +249,7 @@ func (r *Repository) Packages(ctx context.Context) ([]Package, int, error) {
249249
}
250250

251251
if primaryURL, err = r.getPrimaryURL(ctx); err != nil {
252-
return nil, 0, fmt.Errorf("Error getting primary URL: %w", err)
252+
return nil, 0, fmt.Errorf("error getting primary URL: %w", err)
253253
}
254254

255255
if resp, err = r.settings.Client.Get(primaryURL); err != nil {
@@ -258,7 +258,7 @@ func (r *Repository) Packages(ctx context.Context) ([]Package, int, error) {
258258
defer resp.Body.Close()
259259

260260
if resp.StatusCode != http.StatusOK {
261-
return nil, resp.StatusCode, fmt.Errorf("Cannot fetch %v: %d", primaryURL, resp.StatusCode)
261+
return nil, resp.StatusCode, fmt.Errorf("cannot fetch %v: %d", primaryURL, resp.StatusCode)
262262
}
263263

264264
if packages, err = ParseCompressedXMLData(io.NopCloser(resp.Body), *r.settings.MaxXmlSize); err != nil {
@@ -356,9 +356,10 @@ func (r *Repository) getCompsURL() (*string, error) {
356356
var compsLocation string
357357

358358
for _, data := range r.repomd.Data {
359-
if data.Type == "group_gz" {
359+
switch data.Type {
360+
case "group_gz":
360361
compsLocation = data.Location.Href
361-
} else if data.Type == "group" {
362+
case "group":
362363
compsLocation = data.Location.Href
363364
}
364365
}
@@ -379,9 +380,10 @@ func (r *Repository) getModulesURL() (*string, error) {
379380
var compsLocation string
380381

381382
for _, data := range r.repomd.Data {
382-
if data.Type == "modules_gz" {
383+
switch data.Type {
384+
case "modules_gz":
383385
compsLocation = data.Location.Href
384-
} else if data.Type == "modules" {
386+
case "modules":
385387
compsLocation = data.Location.Href
386388
}
387389
}
@@ -487,13 +489,14 @@ func ParseCompsXML(body io.ReadCloser, url *string) (Comps, error) {
487489

488490
switch elType := t.(type) {
489491
case xml.StartElement:
490-
if elType.Name.Local == "group" {
492+
switch elType.Name.Local {
493+
case "group":
491494
var packageGroup PackageGroup
492495
if decodeElementError := decoder.DecodeElement(&packageGroup, &elType); decodeElementError != nil {
493496
return comps, decodeElementError
494497
}
495498
packageGroups = append(packageGroups, packageGroup)
496-
} else if elType.Name.Local == "environment" {
499+
case "environment":
497500
var environment Environment
498501
if decodeElementError := decoder.DecodeElement(&environment, &elType); decodeElementError != nil {
499502
return comps, decodeElementError

0 commit comments

Comments
 (0)