diff --git a/.github/workflows/utility-actions.yml b/.github/workflows/utility-actions.yml index 7dcfc51..ad22556 100644 --- a/.github/workflows/utility-actions.yml +++ b/.github/workflows/utility-actions.yml @@ -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 diff --git a/.golangci.yaml b/.golangci.yaml index 710ea5f..4a6ff8e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -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$ \ No newline at end of file diff --git a/go.mod b/go.mod index 7c42609..5ba5ad4 100644 --- a/go.mod +++ b/go.mod @@ -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 ) diff --git a/go.sum b/go.sum index c9df313..bd6d523 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/pkg/yum/mocks/module.yaml.zst b/pkg/yum/mocks/module.yaml.zst index e5ac50c..476418d 100644 Binary files a/pkg/yum/mocks/module.yaml.zst and b/pkg/yum/mocks/module.yaml.zst differ diff --git a/pkg/yum/mocks/rhel8.modules.yaml.gz b/pkg/yum/mocks/rhel8.modules.yaml.gz index 92dd53c..3fb3fd8 100644 Binary files a/pkg/yum/mocks/rhel8.modules.yaml.gz and b/pkg/yum/mocks/rhel8.modules.yaml.gz differ diff --git a/pkg/yum/module_stream.go b/pkg/yum/module_stream.go index 60a7b4b..cd41fa6 100644 --- a/pkg/yum/module_stream.go +++ b/pkg/yum/module_stream.go @@ -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 @@ -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"` } @@ -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) @@ -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) } diff --git a/pkg/yum/module_stream_test.go b/pkg/yum/module_stream_test.go index 3c75a95..5f622f6 100644 --- a/pkg/yum/module_stream_test.go +++ b/pkg/yum/module_stream_test.go @@ -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) @@ -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) } diff --git a/pkg/yum/repository.go b/pkg/yum/repository.go index d4a1acf..3d55af4 100644 --- a/pkg/yum/repository.go +++ b/pkg/yum/repository.go @@ -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) @@ -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 @@ -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 { @@ -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 { @@ -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 } } @@ -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 } } @@ -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