From 94483fbafc99df0e46fc1350e2b863478050e59d Mon Sep 17 00:00:00 2001 From: ajayk Date: Sat, 4 Jan 2025 23:15:46 -0800 Subject: [PATCH 1/4] minor go cleanup --- pkg/build/pipeline.go | 51 ++++++++++---------- pkg/build/pipeline_test.go | 60 ++++++++++++++++++++++++ pkg/convert/relmon/release_monitoring.go | 2 +- pkg/renovate/cache/cache.go | 12 ++--- 4 files changed, 92 insertions(+), 33 deletions(-) diff --git a/pkg/build/pipeline.go b/pkg/build/pipeline.go index 07aa1ada4..158c0122e 100644 --- a/pkg/build/pipeline.go +++ b/pkg/build/pipeline.go @@ -27,7 +27,7 @@ import ( "strconv" "strings" - apko_types "chainguard.dev/apko/pkg/build/types" + apkoTypes "chainguard.dev/apko/pkg/build/types" "chainguard.dev/melange/pkg/cond" "chainguard.dev/melange/pkg/config" "chainguard.dev/melange/pkg/container" @@ -74,7 +74,7 @@ func (sm *SubstitutionMap) Subpackage(subpkg *config.Subpackage) *SubstitutionMa return &SubstitutionMap{nw} } -func NewSubstitutionMap(cfg *config.Configuration, arch apko_types.Architecture, flavor string, buildOpts []string) (*SubstitutionMap, error) { +func NewSubstitutionMap(cfg *config.Configuration, arch apkoTypes.Architecture, flavor string, buildOpts []string) (*SubstitutionMap, error) { pkg := cfg.Package nw := map[string]string{ @@ -139,39 +139,26 @@ func validateWith(data map[string]string, inputs map[string]config.Input) (map[s if data == nil { data = make(map[string]string) } - for k, v := range inputs { if data[k] == "" { data[k] = v.Default } - if k == "expected-sha256" && data[k] != "" { - if !matchValidShaChars(data[k]) { - return data, fmt.Errorf("checksum input %q for pipeline contains invalid characters", k) - } - if len(data[k]) != 64 { - return data, fmt.Errorf("checksum input %q for pipeline, invalid length", k) + if data[k] != "" { + switch k { + case "expected-sha256", "expected-sha512": + if !matchValidShaChars(data[k]) || len(data[k]) != expectedShaLength(k) { + return data, fmt.Errorf("checksum input %q for pipeline, invalid length", k) + } + case "expected-commit": + if !matchValidShaChars(data[k]) || len(data[k]) != 40 { + return data, fmt.Errorf("expected commit %q for pipeline contains invalid characters or invalid sha lengt", k) + } } } - if k == "expected-sha512" && data[k] != "" { - if !matchValidShaChars(data[k]) { - return data, fmt.Errorf("checksum input %q for pipeline contains invalid characters", k) - } - if len(data[k]) != 128 { - return data, fmt.Errorf("checksum input %q for pipeline, invalid length", k) - } - } - if k == "expected-commit" && data[k] != "" { - if !matchValidShaChars(data[k]) { - return data, fmt.Errorf("expectec commit %q for pipeline contains invalid characters", k) - } - if len(data[k]) != 40 { - return data, fmt.Errorf("expected commit %q for pipeline, invalid length", k) - } - } - if v.Required && data[k] == "" { return data, fmt.Errorf("required input %q for pipeline is missing", k) } + } return data, nil @@ -335,5 +322,17 @@ func shouldRun(ifs string) (bool, error) { return result, nil } +func expectedShaLength(shaType string) int { + switch shaType { + case "expected-sha256": + return 64 + case "expected-sha512": + return 128 + case "expected-commit": + return 40 + } + return 0 +} + //go:embed pipelines/* var f embed.FS diff --git a/pkg/build/pipeline_test.go b/pkg/build/pipeline_test.go index 6f87958fd..a28113c89 100644 --- a/pkg/build/pipeline_test.go +++ b/pkg/build/pipeline_test.go @@ -172,3 +172,63 @@ func TestAllPipelines(t *testing.T) { }) } } + +func Test_validateWith(t *testing.T) { + tests := []struct { + name string + data map[string]string + inputs map[string]config.Input + expected map[string]string + expectError bool + errorMsg string + }{ + { + name: "Valid SHA256 checksum", + data: map[string]string{ + "expected-sha256": "a3c2567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + }, + inputs: map[string]config.Input{ + "expected-sha256": {Default: "", Required: true}, + }, + expected: map[string]string{ + "expected-sha256": "a3c2567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + }, + expectError: false, + }, + { + name: "Invalid SHA256 length", + data: map[string]string{ + "expected-sha256": "abcdef", + }, + inputs: map[string]config.Input{ + "expected-sha256": {Default: "", Required: true}, + }, + expectError: true, + errorMsg: "checksum input \"expected-sha256\" for pipeline, invalid length", + }, + { + name: "Missing required input", + data: map[string]string{}, + inputs: map[string]config.Input{ + "expected-commit": {Default: "", Required: true}, + }, + expectError: true, + errorMsg: "required input \"expected-commit\" for pipeline is missing", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := validateWith(tt.data, tt.inputs) + + if tt.expectError { + require.Error(t, err) + require.EqualError(t, err, tt.errorMsg) + return // Skip further checks if error is expected + } + + require.NoError(t, err) + require.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/convert/relmon/release_monitoring.go b/pkg/convert/relmon/release_monitoring.go index ed3eba702..40a664931 100644 --- a/pkg/convert/relmon/release_monitoring.go +++ b/pkg/convert/relmon/release_monitoring.go @@ -41,7 +41,7 @@ type MonitorFinder struct { func (mf *MonitorFinder) FindMonitor(ctx context.Context, pkg string) (*Item, error) { var items *Items url := fmt.Sprintf(searchFmt, pkg) - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, err } diff --git a/pkg/renovate/cache/cache.go b/pkg/renovate/cache/cache.go index 11fcc0e91..c13f4ec60 100644 --- a/pkg/renovate/cache/cache.go +++ b/pkg/renovate/cache/cache.go @@ -222,7 +222,7 @@ func addFileToCache(ctx context.Context, cfg CacheConfig, downloadedFile string, func downloadFile(ctx context.Context, uri string) (string, error) { targetFile, err := os.CreateTemp("", "melange-update-*") if err != nil { - return "", err + return "", fmt.Errorf("failed to create temp file: %w", err) } defer targetFile.Close() @@ -234,9 +234,9 @@ func downloadFile(ctx context.Context, uri string) (string, error) { }, } - req, err := http.NewRequestWithContext(ctx, "GET", uri, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, uri, nil) if err != nil { - return "", err + return "", fmt.Errorf("failed to create HTTP request: %w", err) } // Set accept header to match the expected MIME types and avoid 403's for some servers like https://www.netfilter.org @@ -244,13 +244,13 @@ func downloadFile(ctx context.Context, uri string) (string, error) { resp, err := client.Do(req) if err != nil { - return "", err + return "", fmt.Errorf("failed to fetch URL %s: %w", uri, err) } defer resp.Body.Close() - if resp.StatusCode != 200 { - return "", fmt.Errorf("got %s when fetching %s", resp.Status, uri) + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("unexpected status code %d (%s) when fetching %s", resp.StatusCode, resp.Status, uri) } if _, err := io.Copy(targetFile, resp.Body); err != nil { From f227b1f10a12c5d9c3242f3855ff7507ef306f1a Mon Sep 17 00:00:00 2001 From: ajayk Date: Sat, 4 Jan 2025 23:34:40 -0800 Subject: [PATCH 2/4] minor go cleanup --- Makefile | 2 +- pkg/build/pipeline.go | 10 +++++-- pkg/http/http.go | 11 ++------ pkg/sbom/package.go | 26 ++++++++--------- pkg/sbom/package_test.go | 61 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 27 deletions(-) create mode 100644 pkg/sbom/package_test.go diff --git a/Makefile b/Makefile index eebfcc349..43571674e 100644 --- a/Makefile +++ b/Makefile @@ -117,7 +117,7 @@ GOLANGCI_LINT_BIN = $(GOLANGCI_LINT_DIR)/golangci-lint setup-golangci-lint: rm -f $(GOLANGCI_LINT_BIN) || : set -e ; - GOBIN=$(GOLANGCI_LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.59.0; + GOBIN=$(GOLANGCI_LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.63.4; .PHONY: fmt fmt: ## Format all go files diff --git a/pkg/build/pipeline.go b/pkg/build/pipeline.go index 158c0122e..66f398ced 100644 --- a/pkg/build/pipeline.go +++ b/pkg/build/pipeline.go @@ -23,7 +23,6 @@ import ( "os/signal" "path" "path/filepath" - "regexp" "strconv" "strings" @@ -165,8 +164,13 @@ func validateWith(data map[string]string, inputs map[string]config.Input) (map[s } func matchValidShaChars(s string) bool { - match, _ := regexp.MatchString("^[a-fA-F0-9]+$", s) - return match + for i := 0; i < len(s); i++ { + c := s[i] + if !(c >= '0' && c <= '9') && !(c >= 'a' && c <= 'f') && !(c >= 'A' && c <= 'F') { + return false + } + } + return true } // Build a script to run as part of evalRun diff --git a/pkg/http/http.go b/pkg/http/http.go index 7de77a44d..99e04fb8d 100644 --- a/pkg/http/http.go +++ b/pkg/http/http.go @@ -61,17 +61,12 @@ func (c *RLHTTPClient) GetArtifactSHA256(ctx context.Context, artifactURI string } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { return "", fmt.Errorf("%d when getting %s", resp.StatusCode, artifactURI) } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("reading body: %w", err) - } - h256 := sha256.New() - h256.Write(body) + if _, err := io.Copy(h256, resp.Body); err != nil { + return "", fmt.Errorf("hashing %s: %w", artifactURI, err) + } return fmt.Sprintf("%x", h256.Sum(nil)), nil } diff --git a/pkg/sbom/package.go b/pkg/sbom/package.go index c6a7dad57..949b6ecad 100644 --- a/pkg/sbom/package.go +++ b/pkg/sbom/package.go @@ -20,7 +20,6 @@ package sbom import ( "context" "fmt" - "regexp" "sort" "strconv" "strings" @@ -174,10 +173,6 @@ func (p Package) getExternalRefs() []spdx.ExternalRef { return result } -// invalidIDCharsRe is a regular expression that matches characters not -// considered valid in SPDX identifiers. -var invalidIDCharsRe = regexp.MustCompile(`[^a-zA-Z0-9-.]+`) - // stringToIdentifier converts a string to a valid SPDX identifier by replacing // invalid characters. Colons and slashes are replaced by dashes, and all other // invalid characters are replaced by their Unicode code point prefixed with @@ -189,20 +184,21 @@ var invalidIDCharsRe = regexp.MustCompile(`[^a-zA-Z0-9-.]+`) // "foo/bar" -> "foo-bar" // "foo bar" -> "fooC32bar" func stringToIdentifier(in string) string { - in = strings.ReplaceAll(in, ":", "-") - in = strings.ReplaceAll(in, "/", "-") - - invalidCharReplacer := func(s string) string { - sb := strings.Builder{} - for _, r := range s { + var sb strings.Builder + sb.Grow(len(in)) + + for _, r := range in { + switch { + case r == ':' || r == '/': + sb.WriteRune('-') + case r == '-' || r == '.' || (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9'): + sb.WriteRune(r) + default: sb.WriteString(encodeInvalidRune(r)) } - return sb.String() } - - return invalidIDCharsRe.ReplaceAllStringFunc(in, invalidCharReplacer) + return sb.String() } - func encodeInvalidRune(r rune) string { return "C" + strconv.Itoa(int(r)) } diff --git a/pkg/sbom/package_test.go b/pkg/sbom/package_test.go new file mode 100644 index 000000000..6c6154ea4 --- /dev/null +++ b/pkg/sbom/package_test.go @@ -0,0 +1,61 @@ +// Copyright 2024 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package sbom + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_stringToIdentifier(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "basic_colon", + input: "foo:bar", + expected: "foo-bar", // Colons replaced with dashes. + }, + { + name: "basic_slash", + input: "foo/bar", + expected: "foo-bar", // Slashes replaced with dashes. + }, + { + name: "space_replacement", + input: "foo bar", + expected: "fooC32bar", // Spaces encoded as Unicode prefix. + }, + { + name: "mixed_colon_and_slash", + input: "foo:bar/baz", + expected: "foo-bar-baz", // Mixed colons and slashes replaced with dashes. + }, + { + name: "valid_characters_unchanged", + input: "example-valid.123", + expected: "example-valid.123", // Valid characters remain unchanged. + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := stringToIdentifier(test.input) + require.Equal(t, test.expected, result, "unexpected result for input %q", test.input) + }) + } +} From 24f849586b940cb544e453d68b00275bc77efa6a Mon Sep 17 00:00:00 2001 From: Ajay Kemparaj Date: Mon, 6 Jan 2025 10:56:50 -0800 Subject: [PATCH 3/4] Update pkg/build/pipeline.go Co-authored-by: Jon Johnson Signed-off-by: Ajay Kemparaj --- pkg/build/pipeline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/build/pipeline.go b/pkg/build/pipeline.go index 66f398ced..5ef4cbe1a 100644 --- a/pkg/build/pipeline.go +++ b/pkg/build/pipeline.go @@ -150,7 +150,7 @@ func validateWith(data map[string]string, inputs map[string]config.Input) (map[s } case "expected-commit": if !matchValidShaChars(data[k]) || len(data[k]) != 40 { - return data, fmt.Errorf("expected commit %q for pipeline contains invalid characters or invalid sha lengt", k) + return data, fmt.Errorf("expected commit %q for pipeline contains invalid characters or invalid sha length", k) } } } From 255961a3ad5749ea2692e32584bfd9cc77cf3567 Mon Sep 17 00:00:00 2001 From: Ajay Kemparaj Date: Mon, 6 Jan 2025 10:57:00 -0800 Subject: [PATCH 4/4] Update pkg/build/pipeline.go Co-authored-by: Jon Johnson Signed-off-by: Ajay Kemparaj --- pkg/build/pipeline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/build/pipeline.go b/pkg/build/pipeline.go index 5ef4cbe1a..ed74f2fd9 100644 --- a/pkg/build/pipeline.go +++ b/pkg/build/pipeline.go @@ -149,7 +149,7 @@ func validateWith(data map[string]string, inputs map[string]config.Input) (map[s return data, fmt.Errorf("checksum input %q for pipeline, invalid length", k) } case "expected-commit": - if !matchValidShaChars(data[k]) || len(data[k]) != 40 { + if !matchValidShaChars(data[k]) || len(data[k]) != expectedShaLength(k) { return data, fmt.Errorf("expected commit %q for pipeline contains invalid characters or invalid sha length", k) } }