Skip to content

HMS-8948: fix parsing stream versions #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rverdile merged 2 commits into from
Nov 26, 2025
Merged

HMS-8948: fix parsing stream versions #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc73db1
HMS-8948: fix parsing stream versions
rverdile Nov 12, 2025
411d6c1
Build: update linter to v2
rverdile Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/utility-actions.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
with:
go-version: "1.24"
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
uses: golangci/golangci-lint-action@v7
with:
version: latest
skip-go-installation: true
Expand Down
42 changes: 32 additions & 10 deletions .golangci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,36 @@
# Configuration for golangci-lint. See https://golangci-lint.run/usage/configuration/.
version: "2"
linters:
disable-all: false # use default linters
enable:
- gofmt
- whitespace
- govet
- misspell
- bodyclose
- forcetypeassert
- gosec
- misspell
- whitespace
settings:
gosec:
excludes:
- G404
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
rules:
- path: (.+)\.go$
text: composite
paths:
- third_party$
- builtin$
- examples$
formatters:
enable:
- gci
- bodyclose
issues:
exclude:
- composite
- gofmt
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
1 change: 0 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ require (
github.com/goccy/go-yaml v1.18.0
github.com/h2non/filetype v1.1.3
github.com/klauspost/compress v1.18.0
github.com/mitchellh/mapstructure v1.5.0
github.com/stretchr/testify v1.11.1
github.com/ulikunitz/xz v0.5.15
)
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ github.com/h2non/filetype v1.1.3 h1:FKkx9QbD7HR/zjK1Ia5XiBsq9zdLi5Kf3zGyFTAFkGg=
github.com/h2non/filetype v1.1.3/go.mod h1:319b3zT68BvV+WRj7cwy856M2ehB3HqNOt6sy1HndBY=
github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
Expand Down
Binary file modified pkg/yum/mocks/module.yaml.zst
View file
Open in desktop
Binary file not shown.
Binary file modified pkg/yum/mocks/rhel8.modules.yaml.gz
View file
Open in desktop
Binary file not shown.
91 changes: 54 additions & 37 deletions pkg/yum/module_stream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (
"fmt"
"io"
"net/http"
"strings"

"github.com/goccy/go-yaml"
"github.com/mitchellh/mapstructure"
"github.com/goccy/go-yaml/ast"
)

// Better userfacing struct
Expand All @@ -18,28 +19,48 @@ type ModuleStream struct {
}

type Stream struct {
Name string `mapstructure:"name"`
Stream string `mapstructure:"stream"`
Version string `mapstructure:"version"`
Context string `mapstructure:"context"`
Arch string `mapstructure:"arch"`
Summary string `mapstructure:"summary"`
Description string `mapstructure:"description"`
Artifacts Artifacts `mapstructure:"artifacts"`
Profiles map[string]RpmProfiles `mapstructure:"profiles"`
Name string `yaml:"name"`
Stream StreamVersion `yaml:"stream"`
Version string `yaml:"version"`
Context string `yaml:"context"`
Arch string `yaml:"arch"`
Summary string `yaml:"summary"`
Description string `yaml:"description"`
Artifacts Artifacts `yaml:"artifacts"`
Profiles map[string]RpmProfiles `yaml:"profiles"`
}

type StreamVersion string

// unmarshalStreamVersion ensures trailing zeros is preserved
// in cases are the stream value is a float like 5.30
func unmarshalStreamVersion(s *StreamVersion, data []byte) error {
str := strings.TrimSpace(string(data))

//Remove additional quotes when stream is represented as string
if len(str) >= 2 && str[0] == '"' && str[len(str)-1] == '"' {
str = str[1 : len(str)-1]
}

*s = StreamVersion(str)
return nil
}

func (s StreamVersion) String() string {
return string(s)
}

type RpmProfiles struct {
Rpms []string `mapstructure:"rpms"`
Rpms []string `yaml:"rpms"`
}

type Artifacts struct {
Rpms []string `mapstructure:"rpms"`
Rpms []string `yaml:"rpms"`
}

type ModuleMD struct {
Document string `mapstructure:"document"`
Version int `mapstructure:"version"`
Document string `yaml:"document"`
Version int `yaml:"version"`
Data Stream `yaml:"data"`
}

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

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

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

yaml.RegisterCustomUnmarshaler[StreamVersion](unmarshalStreamVersion)

decoder := yaml.NewDecoder(reader)
for {
var doc map[string]interface{}

// Decode the next document
err := decoder.Decode(&doc)
var node ast.Node
err := decoder.Decode(&node)
if err != nil {
if errors.Is(err, io.EOF) {
break
}
return nil, fmt.Errorf("error decoding streams: %w", err)
}
// Only care about modulemds right now
if doc["document"] == "modulemd" {

var docType struct {
Document string `yaml:"document"`
}
if err := yaml.NodeToValue(node, &docType); err != nil {
return nil, fmt.Errorf("error decoding document type: %w", err)
}

if docType.Document == "modulemd" {
var module ModuleMD
config := &mapstructure.DecoderConfig{
WeaklyTypedInput: true,
Result: &module,
}
mapDecode, err := mapstructure.NewDecoder(config)
if err != nil {
return moduleMDs, fmt.Errorf("error creating map decoder: %w", err)
}
err = mapDecode.Decode(doc)
if err != nil {
return nil, fmt.Errorf("error decoding map: %w", err)
if err := yaml.NodeToValue(node, &module); err != nil {
return nil, fmt.Errorf("error decoding modulemd: %w", err)
}
moduleMDs = append(moduleMDs, module)
}
Expand Down
41 changes: 36 additions & 5 deletions pkg/yum/module_stream_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,33 @@ func TestParseModuleMDs(t *testing.T) {

parsed, err := parseModuleMDs(f)
assert.NoError(t, err)
assert.Equal(t, 11, len(parsed))
assert.Equal(t, 13, len(parsed))
assert.NotEmpty(t, parsed[0].Data.Name)
assert.NotEmpty(t, parsed[0].Data.Artifacts.Rpms)
}

func TestStreamVersionPrecision(t *testing.T) {
f, err := os.Open("mocks/module.yaml.zst")
assert.NoError(t, err)

parsed, err := parseModuleMDs(f)
assert.NoError(t, err)

handlesFloatFound, handlesStringFound := false, false
for _, module := range parsed {
if module.Data.Name == "testmodule" {
handlesFloatFound = true
assert.Equal(t, "5.30", module.Data.Stream.String())
}
if module.Data.Name == "testmodule-2" {
handlesStringFound = true
assert.Equal(t, "5.30", module.Data.Stream.String())
}
}
assert.True(t, handlesFloatFound)
assert.True(t, handlesStringFound)
}

func TestParseRhel8Modules(t *testing.T) {
f, err := os.Open("mocks/rhel8.modules.yaml.gz")
assert.NoError(t, err)
Expand All @@ -29,19 +51,28 @@ func TestParseRhel8Modules(t *testing.T) {
modules, err := parseModuleMDs(f)
require.NoError(t, err)

assert.Len(t, modules, 862)
assert.Len(t, modules, 961)

assert.NotEmpty(t, modules)
found := false
foundRuby, foundPerl := false, false
for _, module := range modules {
if module.Data.Name == "ruby" && module.Data.Stream == "2.5" {
found = true
foundRuby = true
assert.NotEmpty(t, module.Data.Artifacts.Rpms)
assert.NotEmpty(t, module.Data.Profiles)
value, ok := module.Data.Profiles["common"]
assert.True(t, ok)
assert.Equal(t, []string{"ruby"}, value.Rpms)
}
if module.Data.Name == "perl" && module.Data.Stream == "5.30" {
foundPerl = true
assert.NotEmpty(t, module.Data.Artifacts.Rpms)
assert.NotEmpty(t, module.Data.Profiles)
value, ok := module.Data.Profiles["common"]
assert.True(t, ok)
assert.Equal(t, []string{"perl"}, value.Rpms)
}
}
assert.True(t, found)
assert.True(t, foundRuby)
assert.True(t, foundPerl)
}
25 changes: 14 additions & 11 deletions pkg/yum/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (r *Repository) Repomd(ctx context.Context) (*Repomd, int, error) {
return r.repomd, 0, nil
}
if repomdURL, err = r.getRepomdURL(); err != nil {
return nil, 0, fmt.Errorf("Error parsing Repomd URL: %w", err)
return nil, 0, fmt.Errorf("error parsing Repomd URL: %w", err)
}

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

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

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

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

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

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

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

for _, data := range r.repomd.Data {
if data.Type == "group_gz" {
switch data.Type {
case "group_gz":
compsLocation = data.Location.Href
} else if data.Type == "group" {
case "group":
compsLocation = data.Location.Href
}
}
Expand All @@ -379,9 +380,10 @@ func (r *Repository) getModulesURL() (*string, error) {
var compsLocation string

for _, data := range r.repomd.Data {
if data.Type == "modules_gz" {
switch data.Type {
case "modules_gz":
compsLocation = data.Location.Href
} else if data.Type == "modules" {
case "modules":
compsLocation = data.Location.Href
}
}
Expand Down Expand Up @@ -487,13 +489,14 @@ func ParseCompsXML(body io.ReadCloser, url *string) (Comps, error) {

switch elType := t.(type) {
case xml.StartElement:
if elType.Name.Local == "group" {
switch elType.Name.Local {
case "group":
var packageGroup PackageGroup
if decodeElementError := decoder.DecodeElement(&packageGroup, &elType); decodeElementError != nil {
return comps, decodeElementError
}
packageGroups = append(packageGroups, packageGroup)
} else if elType.Name.Local == "environment" {
case "environment":
var environment Environment
if decodeElementError := decoder.DecodeElement(&environment, &elType); decodeElementError != nil {
return comps, decodeElementError
Expand Down
Loading