Simplify `pkg/dockerfile` interface by ditching pointer

This means slightly more typing in "zero-value" cases (`nil` vs `dockerfile.Metadata{}`), but the tradeoff is that it's simpler to use and reason about (and all the struct members are pointer-type map/slice values anyhow, so copying the struct is still pretty cheap).

This also swaps the scanner error handling to return the partially parsed Metadata object alongside the scanner error -- the error already tells us the object isn't fully complete data, so it's fair/fine to return and will likely just be ignored by the caller instead.  This also allows us to get to 100% code coverage. 👀

This also updates our "treat `oci-import` just like `FROM scratch`" code to *actually* parse `FROM scratch` so we can't accidentally cause "missing data" bugs there in the future, and I implemented that using `sync.OnceValues` which requires upgrading to Go 1.21, but IMO that's a worthwhile tradeoff (because `sync.OnceValues` makes that code so clean/simple).
This commit is contained in:
Tianon Gravi 2025-01-09 16:20:49 -08:00
parent 7ddf2bef73
commit 60ee93caf8
9 changed files with 26 additions and 34 deletions

View File

@ -1,4 +1,4 @@
FROM golang:1.20-bullseye AS build
FROM golang:1.21-bookworm AS build
SHELL ["bash", "-Eeuo", "pipefail", "-xc"]

View File

@ -1,4 +1,4 @@
FROM golang:1.20-bullseye
FROM golang:1.21-bookworm
SHELL ["bash", "-Eeuo", "pipefail", "-xc"]

View File

@ -1,4 +1,4 @@
FROM golang:1.20-bullseye
FROM golang:1.21-bookworm
SHELL ["bash", "-Eeuo", "pipefail", "-xc"]

View File

@ -6,11 +6,11 @@ set -Eeuo pipefail
dir="$(readlink -f "$BASH_SOURCE")"
dir="$(dirname "$dir")"
: "${CGO_ENABLED:=0}"
export GO111MODULE=on CGO_ENABLED
: "${CGO_ENABLED=0}" "${GOTOOLCHAIN=local}"
export CGO_ENABLED GOTOOLCHAIN
(
cd "$dir"
go build -o bin/bashbrew ./cmd/bashbrew > /dev/null
go build -trimpath -o bin/bashbrew ./cmd/bashbrew > /dev/null
)
exec "$dir/bin/bashbrew" "$@"

View File

@ -10,6 +10,7 @@ import (
"os/exec"
"path"
"strings"
"sync"
"github.com/docker-library/bashbrew/manifest"
"github.com/docker-library/bashbrew/pkg/dockerfile"
@ -37,27 +38,25 @@ func (r Repo) ArchDockerFroms(arch string, entry *manifest.Manifest2822Entry) ([
return dockerfileMeta.Froms, nil
}
func (r Repo) dockerfileMetadata(entry *manifest.Manifest2822Entry) (*dockerfile.Metadata, error) {
func (r Repo) dockerfileMetadata(entry *manifest.Manifest2822Entry) (dockerfile.Metadata, error) {
return r.archDockerfileMetadata(arch, entry)
}
var dockerfileMetadataCache = map[string]*dockerfile.Metadata{}
var (
dockerfileMetadataCache = map[string]dockerfile.Metadata{}
scratchDockerfileMetadata = sync.OnceValues(func() (dockerfile.Metadata, error) {
return dockerfile.Parse(`FROM scratch`)
})
)
func (r Repo) archDockerfileMetadata(arch string, entry *manifest.Manifest2822Entry) (*dockerfile.Metadata, error) {
func (r Repo) archDockerfileMetadata(arch string, entry *manifest.Manifest2822Entry) (dockerfile.Metadata, error) {
if builder := entry.ArchBuilder(arch); builder == "oci-import" {
return &dockerfile.Metadata{
StageFroms: []string{
"scratch",
},
Froms: []string{
"scratch",
},
}, nil
return scratchDockerfileMetadata()
}
commit, err := r.fetchGitRepo(arch, entry)
if err != nil {
return nil, cli.NewMultiError(fmt.Errorf("failed fetching Git repo for arch %q from entry %q", arch, entry.String()), err)
return dockerfile.Metadata{}, cli.NewMultiError(fmt.Errorf("failed fetching Git repo for arch %q from entry %q", arch, entry.String()), err)
}
dockerfileFile := path.Join(entry.ArchDirectory(arch), entry.ArchFile(arch))
@ -72,12 +71,12 @@ func (r Repo) archDockerfileMetadata(arch string, entry *manifest.Manifest2822En
df, err := gitShow(commit, dockerfileFile)
if err != nil {
return nil, cli.NewMultiError(fmt.Errorf(`failed "git show" for %q from commit %q`, dockerfileFile, commit), err)
return dockerfile.Metadata{}, cli.NewMultiError(fmt.Errorf(`failed "git show" for %q from commit %q`, dockerfileFile, commit), err)
}
meta, err := dockerfile.Parse(df)
if err != nil {
return nil, cli.NewMultiError(fmt.Errorf(`failed parsing Dockerfile metadata for %q from commit %q`, dockerfileFile, commit), err)
return dockerfile.Metadata{}, cli.NewMultiError(fmt.Errorf(`failed parsing Dockerfile metadata for %q from commit %q`, dockerfileFile, commit), err)
}
dockerfileMetadataCache[cacheKey] = meta

2
go.mod
View File

@ -1,6 +1,6 @@
module github.com/docker-library/bashbrew
go 1.20
go 1.21
require (
github.com/containerd/containerd v1.6.19

1
go.sum
View File

@ -617,6 +617,7 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O
github.com/prometheus/procfs v0.2.0/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU=
github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0VU=
github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=

View File

@ -16,12 +16,12 @@ type Metadata struct {
Froms []string // every "FROM" or "COPY --from=xxx" value (minus named and/or numbered stages in the case of "--from=")
}
func Parse(dockerfile string) (*Metadata, error) {
func Parse(dockerfile string) (Metadata, error) {
return ParseReader(strings.NewReader(dockerfile))
}
func ParseReader(dockerfile io.Reader) (*Metadata, error) {
meta := &Metadata{
func ParseReader(dockerfile io.Reader) (Metadata, error) {
meta := Metadata{
// panic: assignment to entry in nil map
StageNameFroms: map[string]string{},
// (nil slices work fine)
@ -171,10 +171,7 @@ func ParseReader(dockerfile io.Reader) (*Metadata, error) {
}
}
}
if err := scanner.Err(); err != nil {
return nil, err
}
return meta, nil
return meta, scanner.Err()
}
func latestizeRepoTag(repoTag string) string {

View File

@ -175,14 +175,9 @@ func TestParse(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if parsed == nil {
if !reflect.DeepEqual(parsed, td.metadata) {
t.Fatalf("expected:\n%#v\ngot:\n%#v", td.metadata, parsed)
}
if !reflect.DeepEqual(*parsed, td.metadata) {
t.Fatalf("expected:\n%#v\ngot:\n%#v", td.metadata, *parsed)
}
})
}
}