diff --git a/.test/deploy-arch-mismatch/in.jq b/.test/deploy-arch-mismatch/in.jq new file mode 100644 index 0000000..a9f7442 --- /dev/null +++ b/.test/deploy-arch-mismatch/in.jq @@ -0,0 +1,59 @@ +# fully synthetic input for "deploy.jq" arch filtering (so nothing here depends on real images, registries, or "builds.json" content) + +# obviously fake digests: "a" => sha256:aaaa... +def digest($c): "sha256:" + ($c * 64); + +# $annotation is the value of the bashbrew arch annotation (null means no annotation at all) +def manifest($c; $arch; $annotation): + { + mediaType: "application/vnd.oci.image.manifest.v1+json", + digest: digest($c), + size: 1, + annotations: ( + if $annotation == null then {} else { + "com.docker.official-images.bashbrew.arch": $annotation, + } end + ), + platform: { os: "linux", architecture: $arch }, + } +; + +# both builds share the "synthetic:latest" tag (like "library/" tags are shared by every architecture) and have their own arch-specific tag +def build($arch; $manifests): + { + buildId: "synthetic-\($arch)", + build: { + $arch, + resolved: { + schemaVersion: 2, + manifests: $manifests, + }, + }, + source: { + arches: { + ($arch): { + tags: [ "synthetic:latest" ], + archTags: [ "synthetic:\($arch)" ], + }, + }, + }, + } +; + +[ + build("amd64"; [ + manifest("a"; "amd64"; "amd64"), # correct arch: KEEP + manifest("b"; "arm64"; "arm64v8"), # annotated for a different arch (cross-architecture "bleeding"): DROP + manifest("c"; "amd64"; ""), # empty (but present) annotation, which is truthy in jq: DROP + manifest("d"; "amd64"; null), # no annotation at all: DROP + empty # trailing comma + ]), + build("arm64v8"; [ + manifest("e"; "arm64"; "arm64v8"), # correct arch: KEEP + manifest("f"; "amd64"; "amd64"), # the same bleeding in the other direction: DROP + empty # trailing comma + ]), + empty # trailing comma +] +| map({ (.buildId): . }) +| add diff --git a/.test/deploy-arch-mismatch/in.json b/.test/deploy-arch-mismatch/in.json new file mode 100644 index 0000000..a751900 --- /dev/null +++ b/.test/deploy-arch-mismatch/in.json @@ -0,0 +1,118 @@ +{ + "synthetic-amd64": { + "buildId": "synthetic-amd64", + "build": { + "arch": "amd64", + "resolved": { + "schemaVersion": 2, + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "amd64" + }, + "platform": { + "os": "linux", + "architecture": "amd64" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "arm64v8" + }, + "platform": { + "os": "linux", + "architecture": "arm64" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "" + }, + "platform": { + "os": "linux", + "architecture": "amd64" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", + "size": 1, + "annotations": {}, + "platform": { + "os": "linux", + "architecture": "amd64" + } + } + ] + } + }, + "source": { + "arches": { + "amd64": { + "tags": [ + "synthetic:latest" + ], + "archTags": [ + "synthetic:amd64" + ] + } + } + } + }, + "synthetic-arm64v8": { + "buildId": "synthetic-arm64v8", + "build": { + "arch": "arm64v8", + "resolved": { + "schemaVersion": 2, + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "arm64v8" + }, + "platform": { + "os": "linux", + "architecture": "arm64" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "amd64" + }, + "platform": { + "os": "linux", + "architecture": "amd64" + } + } + ] + } + }, + "source": { + "arches": { + "arm64v8": { + "tags": [ + "synthetic:latest" + ], + "archTags": [ + "synthetic:arm64v8" + ] + } + } + } + } +} diff --git a/.test/deploy-arch-mismatch/out.json b/.test/deploy-arch-mismatch/out.json new file mode 100644 index 0000000..e316350 --- /dev/null +++ b/.test/deploy-arch-mismatch/out.json @@ -0,0 +1,72 @@ +{ + "synthetic:latest": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "amd64" + }, + "platform": { + "os": "linux", + "architecture": "amd64" + } + }, + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "arm64v8" + }, + "platform": { + "os": "linux", + "architecture": "arm64" + } + } + ], + "synthetic:amd64": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "amd64" + }, + "platform": { + "os": "linux", + "architecture": "amd64" + } + } + ], + "synthetic:arm64v8": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "arm64v8" + }, + "platform": { + "os": "linux", + "architecture": "arm64" + } + } + ] +} +{ + "synthetic:amd64": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "size": 1, + "annotations": { + "com.docker.official-images.bashbrew.arch": "amd64" + }, + "platform": { + "os": "linux", + "architecture": "amd64" + } + } + ] +} diff --git a/.test/deploy-arch-mismatch/test.jq b/.test/deploy-arch-mismatch/test.jq new file mode 100644 index 0000000..7b7c5c7 --- /dev/null +++ b/.test/deploy-arch-mismatch/test.jq @@ -0,0 +1,7 @@ +include "deploy"; + +# every ref ("library/" and arch-specific) for every build: only "a" and "e" should survive (each under its own build's tags, and both under the shared tag) +tagged_manifests(true; .source.arches[.build.arch].tags, .source.arches[.build.arch].archTags), + +# just amd64: only "a" +arch_tagged_manifests("amd64") diff --git a/.test/test.sh b/.test/test.sh index 16f1e8f..45953a4 100755 --- a/.test/test.sh +++ b/.test/test.sh @@ -19,6 +19,8 @@ export BASHBREW_STAGING_TEMPLATE='oisupport/staging-ARCH:BUILD' dir="$(dirname "$BASH_SOURCE")" dir="$(readlink -ve "$dir")" export BASHBREW_LIBRARY="$dir/library" +BASHBREW_META_SCRIPTS="$(dirname "$dir")" +export BASHBREW_META_SCRIPTS doDeploy= if [ "${1:-}" = '--deploy' ]; then @@ -32,15 +34,15 @@ set -- docker:cli docker:dind docker:windowsservercore notary busybox:{latest,gl time bashbrew fetch "$@" # generate sources, but remove the first item so we can test cache with some missing -time "$dir/../sources.sh" "$@" | jq --tab 'del(first(.[]))' > "$dir/sources-cache.json" +time "$BASHBREW_META_SCRIPTS/sources.sh" "$@" | jq --tab 'del(first(.[]))' > "$dir/sources-cache.json" # again but with cache -time "$dir/../sources.sh" --cache-file "$dir/sources-cache.json" "$@" > "$dir/sources-doi.json" +time "$BASHBREW_META_SCRIPTS/sources.sh" --cache-file "$dir/sources-cache.json" "$@" > "$dir/sources-doi.json" # also fetch/include Tianon's more cursed "infosiftr/moby" example (a valid manifest with arch-specific non-archTags that end up mapping to the same sourceId) bashbrew fetch infosiftr-moby -( BASHBREW_ARCH_NAMESPACES= "$dir/../sources.sh" infosiftr-moby > "$dir/sources-cache.json" ) +( BASHBREW_ARCH_NAMESPACES= "$BASHBREW_META_SCRIPTS/sources.sh" infosiftr-moby > "$dir/sources-cache.json" ) # again but with cache -( BASHBREW_ARCH_NAMESPACES= "$dir/../sources.sh" --cache-file="$dir/sources-cache.json" infosiftr-moby > "$dir/sources-moby.json" ) +( BASHBREW_ARCH_NAMESPACES= "$BASHBREW_META_SCRIPTS/sources.sh" --cache-file="$dir/sources-cache.json" infosiftr-moby > "$dir/sources-moby.json" ) # technically, this *also* needs BASHBREW_STAGING_TEMPLATE='tianon/zz-staging:ARCH-BUILD', but that's a "builds.sh" flag and separating that would complicate including this even more, so Tianon has run the following one-liner to "inject" those builds as if they lived in 'oisupport/staging-ARCH:BUILD' instead: # jq --raw-output '[ .[] | select(any(.source.arches[].tags[]; startswith("infosiftr-moby:"))) | "tianon/zz-staging:\(.build.arch)-\(.buildId)" as $tianon | @sh "../bin/lookup \($tianon) | jq --arg img \(.build.img) \("{ indexes: { ($img): . } }")" ] | "{ " + join(" && ") + @sh " && cat cache-builds.json; } | jq --slurp --tab \("reduce .[] as $i ({ indexes: { } }; .indexes += $i.indexes)") > cache-builds.json.new && mv cache-builds.json.new cache-builds.json"' builds.json | bash -Eeuo pipefail -x # (and then re-run the tests to canonicalize the file ordering) @@ -77,7 +79,7 @@ time "$coverage/builds.sh" --cache "$dir/cache-builds.json" "$dir/sources.json" time "$coverage/builds.sh" --cache="$dir/cache-builds.json" "$dir/sources.json" > "$dir/builds.json" # test "lookup" code for more edge cases -"$dir/../.go-env.sh" go build -coverpkg=./... -trimpath -o "$coverage/bin/lookup" ./cmd/lookup +"$BASHBREW_META_SCRIPTS/.go-env.sh" go build -coverpkg=./... -trimpath -o "$coverage/bin/lookup" ./cmd/lookup lookup=( # force a config blob lookup for platform object creation (and top-level Docker media type!) 'tianon/test@sha256:2f19ce27632e6baf4ebb1b582960d68948e52902c8cfac10133da0058f1dab23' @@ -135,7 +137,7 @@ lookup=( if [ -n "$doDeploy" ]; then # also test "deploy" (optional, disabled by default, because it's a much heavier test) - "$dir/../.go-env.sh" go build -coverpkg=./... -trimpath -o "$coverage/bin/deploy" ./cmd/deploy + "$BASHBREW_META_SCRIPTS/.go-env.sh" go build -coverpkg=./... -trimpath -o "$coverage/bin/deploy" ./cmd/deploy docker rm -vf meta-scripts-test-registry &> /dev/null || : trap 'docker rm -vf meta-scripts-test-registry &> /dev/null || :' EXIT @@ -270,12 +272,12 @@ if [ -n "$doDeploy" ]; then fi # Go tests -"$dir/../.go-env.sh" go test -coverpkg=./... ./... -args -test.gocoverdir="$GOCOVERDIR" +"$BASHBREW_META_SCRIPTS/.go-env.sh" go test -coverpkg=./... ./... -args -test.gocoverdir="$GOCOVERDIR" # combine the coverage data into the "legacy" coverage format (understood by "go tool cover") and pre-generate HTML for easier digestion of the data -"$dir/../.go-env.sh" go tool covdata textfmt -i "$GOCOVERDIR" -o "$coverage/coverage.txt" -"$dir/../.go-env.sh" go tool cover -html "$coverage/coverage.txt" -o "$coverage/coverage.html" -"$dir/../.go-env.sh" go tool cover -func "$coverage/coverage.txt" +"$BASHBREW_META_SCRIPTS/.go-env.sh" go tool covdata textfmt -i "$GOCOVERDIR" -o "$coverage/coverage.txt" +"$BASHBREW_META_SCRIPTS/.go-env.sh" go tool cover -html "$coverage/coverage.txt" -o "$coverage/coverage.html" +"$BASHBREW_META_SCRIPTS/.go-env.sh" go tool cover -func "$coverage/coverage.txt" # also run our "jq" tests (like generating example commands from the "builds.json" we just generated) "$dir/jq.sh" diff --git a/Jenkinsfile.meta b/Jenkinsfile.meta index 04d1245..b2d6857 100644 --- a/Jenkinsfile.meta +++ b/Jenkinsfile.meta @@ -66,6 +66,7 @@ node('doi-meta') { // use the dedicated meta instance } env.BASHBREW_LIBRARY = workspace + '/meta/.doi/library' + env.BASHBREW_META_SCRIPTS = workspace + '/meta/.scripts' dir('meta') { withCredentials([ diff --git a/cmd/builds/main.go b/cmd/builds/main.go index 5fc24a6..39104dd 100644 --- a/cmd/builds/main.go +++ b/cmd/builds/main.go @@ -14,6 +14,7 @@ import ( "github.com/docker-library/meta-scripts/om" "github.com/docker-library/meta-scripts/registry" + "github.com/docker-library/meta-scripts/sm" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -50,7 +51,7 @@ type MetaBuild struct { var ( // keys are image/tag names, values are functions that return either *ocispec.Index or error - cacheResolve = sync.Map{} + cacheResolve = sm.Map[string, func() (*ocispec.Index, error)]{} cacheFile string ) @@ -71,7 +72,7 @@ func resolveIndex(ctx context.Context, img string, diskCacheForSure bool) (*ocis return registry.SynthesizeIndex(ctx, ref) })) - index, err := cacheFunc.(func() (*ocispec.Index, error))() + index, err := cacheFunc() if err != nil { return nil, err } @@ -174,7 +175,7 @@ func loadCacheFromFile() error { fun, _ := cacheResolve.LoadOrStore(img.String(), sync.OnceValues(func() (*ocispec.Index, error) { return index, nil })) - index2, err := fun.(func() (*ocispec.Index, error))() + index2, err := fun() if err != nil { // this should never happen (hence panic vs return) 🙈 panic(err) diff --git a/cmd/deploy/main.go b/cmd/deploy/main.go index 73161fe..2f79b8f 100644 --- a/cmd/deploy/main.go +++ b/cmd/deploy/main.go @@ -10,6 +10,7 @@ import ( "sync" "github.com/docker-library/meta-scripts/registry" + "github.com/docker-library/meta-scripts/sm" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -62,7 +63,7 @@ func main() { // a set of RWMutex objects for synchronizing the pushing of "child" objects before their parents later in the list of documents // for every RWMutex, it will be *write*-locked during push, and *read*-locked during reading (which means we won't limit the parallelization of multiple parents after a given child is pushed, but we will stop parents from being pushed before their children) - childMutexes := sync.Map{} + childMutexes := sm.Map[string, *sync.RWMutex]{} wg := sync.WaitGroup{} var dryRunOuts chan chan []byte @@ -135,8 +136,7 @@ func main() { necessaryReadLockRefs = append(necessaryReadLockRefs, lockRef) } else { seenRefs[lockRefStr] = true - lock, _ := childMutexes.LoadOrStore(lockRefStr, &sync.RWMutex{}) - mutex = lock.(*sync.RWMutex) + mutex, _ = childMutexes.LoadOrStore(lockRefStr, &sync.RWMutex{}) // if we have a "child" mutex, lock it immediately so we don't create a race between inputs mutex.Lock() // (this gets unlocked in the goroutine below) // this is sane to lock here because interdependent inputs are required to be in-order (children first), so if this hangs it's 100% a bug in the input order @@ -216,9 +216,9 @@ func main() { continue } seenChildren[lockRefStr] = true - lock, _ := childMutexes.LoadOrStore(lockRefStr, &sync.RWMutex{}) - lock.(*sync.RWMutex).RLock() - defer lock.(*sync.RWMutex).RUnlock() + childMutex, _ := childMutexes.LoadOrStore(lockRefStr, &sync.RWMutex{}) + childMutex.RLock() + defer childMutex.RUnlock() } logText := ref.StringWithKnownDigest(refsDigest) + logSuffix diff --git a/deploy.jq b/deploy.jq index d470b69..b8a6d47 100644 --- a/deploy.jq +++ b/deploy.jq @@ -12,7 +12,7 @@ def tagged_manifests(builds_selector; tags_extractor): # as an extra protection against cross-architecture "bleeding" ("riscv64" infra pushing "amd64" images, for example), filter the list of manifests to those whose architecture matches the architecture it is supposed to be for # to be explicitly clear, this filtering is *also* done as part of our "builds.json" generation, so this is an added layer of best-effort protection that will be especially important to preserve and/or replicate if/when we solve the "not built yet so include the previous contents of the tag" portion of the problem at this layer instead of in the currently-separate put-shared process $i.build.resolved.manifests[] - | select(.annotations["com.docker.official-images.bashbrew.arch"] // "" == $i.build.arch) # this assumes "registry.SynthesizeIndex" created this list of manifests (because it sets this annotation), but it would be reasonable for us to reimplement that conversion of "OCI platform object" to "bashbrew architecture" in pure jq if it was prudent or necessary to do so + | select((.annotations["com.docker.official-images.bashbrew.arch"] // "") == $i.build.arch) # this assumes "registry.SynthesizeIndex" created this list of manifests (because it sets this annotation), but it would be reasonable for us to reimplement that conversion of "OCI platform object" to "bashbrew architecture" in pure jq if it was prudent or necessary to do so ] ) ; diff --git a/oci.jq b/oci.jq index 7b9794e..3754fa8 100644 --- a/oci.jq +++ b/oci.jq @@ -165,11 +165,11 @@ def validate_oci_digest: | validate($dig; validate_IN(.algorithm; "sha256", "sha512", "blake3") | if .algorithm == "sha256" then - validate(.encoded; test("^[a-f0-9]{64}$"); "the encoded portion MUST match /[a-f0-9]{64}/") + validate(.encoded; test("^[a-f0-9]{64}$"); "sha256: the encoded portion MUST match /[a-f0-9]{64}/") elif .algorithm == "sha512" then - validate(.encoded; test("^[a-f0-9]{128}$"); "the encoded portion MUST match /[a-f0-9]{128}/") + validate(.encoded; test("^[a-f0-9]{128}$"); "sha512: the encoded portion MUST match /[a-f0-9]{128}/") elif .algorithm == "blake3" then # https://github.com/opencontainers/image-spec/pull/1240 - validate(.encoded; test("^[a-f0-9]{64}$"); "the encoded portion MUST match /[a-f0-9]{64}/") + validate(.encoded; test("^[a-f0-9]{64}$"); "blake3: the encoded portion MUST match /[a-f0-9]{64}/") else . end ) ; diff --git a/registry/cache.go b/registry/cache.go index 699d4b4..70da23c 100644 --- a/registry/cache.go +++ b/registry/cache.go @@ -5,6 +5,8 @@ import ( "io" "sync" + "github.com/docker-library/meta-scripts/sm" + "cuelabs.dev/go/oci/ociregistry" "cuelabs.dev/go/oci/ociregistry/ocimem" godigest "github.com/opencontainers/go-digest" @@ -32,7 +34,7 @@ type registryCache struct { registry ociregistry.Interface // a map of "repo@digest" or "repo:tag" to *sync.Mutex to ensure we don't double up on upstream lookups - refMutexes sync.Map + refMutexes sm.Map[string, *sync.Mutex] // (see "refMutex" function) // https://github.com/cue-labs/oci/issues/24 @@ -52,7 +54,7 @@ func cacheKeyTag(repo, tag string) string { func (rc *registryCache) refMutex(ref string) *sync.Mutex { refMu, _ := rc.refMutexes.LoadOrStore(ref, &sync.Mutex{}) - return refMu.(*sync.Mutex) + return refMu } // a helper that implements GetBlob and GetManifest generically (since they're the same function signature and it doesn't really help *us* to treat those object types differently here) diff --git a/registry/client.go b/registry/client.go index c279e31..19a26bb 100644 --- a/registry/client.go +++ b/registry/client.go @@ -9,6 +9,8 @@ import ( "strings" "sync" + "github.com/docker-library/meta-scripts/sm" + "cuelabs.dev/go/oci/ociregistry" "cuelabs.dev/go/oci/ociregistry/ociauth" "cuelabs.dev/go/oci/ociregistry/ociclient" @@ -130,7 +132,7 @@ func Client(host string, opts *ociclient.Options) (ociregistry.Interface, error) return client, nil })) - return f.(func() (ociregistry.Interface, error))() + return f() } type dockerAuthConfigWrapper struct { @@ -171,5 +173,5 @@ var ( } return dockerAuthConfigWrapper{config}, nil }) - clientCache = sync.Map{} // "(normalized) host" => OnceValues() => ociregistry.Interface, error + clientCache = sm.Map[string, func() (ociregistry.Interface, error)]{} // "(normalized) host" => OnceValues() => ociregistry.Interface, error ) diff --git a/registry/data-helper.go b/registry/data-helper.go new file mode 100644 index 0000000..5091936 --- /dev/null +++ b/registry/data-helper.go @@ -0,0 +1,23 @@ +package registry + +import ( + "cuelabs.dev/go/oci/ociregistry" + "cuelabs.dev/go/oci/ociregistry/ocimem" +) + +// returns true if the given descriptor's "data" field is non-nil, "digest" and "size" are valid, and if "data" matches them +func isDescriptorDataValid(desc ociregistry.Descriptor) bool { + return desc.Data != nil && + desc.Size == int64(len(desc.Data)) && + desc.Digest.Validate() == nil && + desc.Digest == desc.Digest.Algorithm().FromBytes(desc.Data) +} + +// if the given descriptor's "data" field is valid, returns a new [ociregistry.BlobReader], otherwise returns nil +func descriptorDataReader(desc ociregistry.Descriptor) ociregistry.BlobReader { + // if desc.Data == nil && desc.Size == 0, we could upgrade desc.Data to be []byte{} here, but that's not something any of our real actual images will have so it's not worth the added lines (and Tianon has added this comment for Future Tianon's sake - don't do it, friend!) + if isDescriptorDataValid(desc) { + return ocimem.NewBytesReader(desc.Data, desc) + } + return nil +} diff --git a/registry/data-helper_test.go b/registry/data-helper_test.go new file mode 100644 index 0000000..209b208 --- /dev/null +++ b/registry/data-helper_test.go @@ -0,0 +1,150 @@ +package registry + +import ( + "io" + "testing" + + "cuelabs.dev/go/oci/ociregistry" + godigest "github.com/opencontainers/go-digest" +) + +func TestIsDescriptorDataValid(t *testing.T) { + t.Parallel() + + data := []byte("buffy the vampire slayer\n") + validDesc := ociregistry.Descriptor{ + Digest: godigest.FromBytes(data), + Size: int64(len(data)), + Data: data, + } + + for _, o := range []struct { + name string + desc func() ociregistry.Descriptor + want bool + }{ + {"valid", func() ociregistry.Descriptor { return validDesc }, true}, + {"valid sha512", func() ociregistry.Descriptor { + desc := validDesc + desc.Digest = godigest.SHA512.FromBytes(data) + return desc + }, true}, + {"valid empty", func() ociregistry.Descriptor { + return ociregistry.Descriptor{ + Digest: godigest.FromBytes(nil), + Size: 0, + Data: []byte{}, + } + }, true}, + + {"nil data", func() ociregistry.Descriptor { + desc := validDesc + desc.Data = nil + return desc + }, false}, + {"nil data, empty digest", func() ociregistry.Descriptor { + return ociregistry.Descriptor{ + Digest: godigest.FromBytes(nil), + } + }, false}, + {"size too small", func() ociregistry.Descriptor { + desc := validDesc + desc.Size-- + return desc + }, false}, + {"size too big", func() ociregistry.Descriptor { + desc := validDesc + desc.Size++ + return desc + }, false}, + {"wrong digest", func() ociregistry.Descriptor { + desc := validDesc + desc.Digest = godigest.FromString("not the right data") + return desc + }, false}, + {"data is not the digest", func() ociregistry.Descriptor { + // the original bug: "Data" interpreted *as* a digest instead of hashing the contents of it + desc := validDesc + desc.Data = []byte(validDesc.Digest) + desc.Size = int64(len(desc.Data)) + return desc + }, false}, + {"empty digest", func() ociregistry.Descriptor { + desc := validDesc + desc.Digest = "" + return desc + }, false}, + {"invalid digest syntax", func() ociregistry.Descriptor { + desc := validDesc + desc.Digest = "not-a-digest" + return desc + }, false}, + {"invalid digest length", func() ociregistry.Descriptor { + desc := validDesc + desc.Digest = "sha256:deadbeef" + return desc + }, false}, + {"unavailable algorithm", func() ociregistry.Descriptor { + desc := validDesc + desc.Digest = "blake3:0000000000000000000000000000000000000000000000000000000000000000" + return desc + }, false}, + } { + o := o // https://github.com/golang/go/issues/60078 (go.mod is still "go 1.21") + + t.Run(o.name, func(t *testing.T) { + t.Parallel() + + if got := isDescriptorDataValid(o.desc()); got != o.want { + t.Errorf("isDescriptorDataValid: got %v, want %v", got, o.want) + } + }) + } +} + +func TestDescriptorDataReader(t *testing.T) { + t.Parallel() + + data := []byte("buffy the vampire slayer\n") + desc := ociregistry.Descriptor{ + MediaType: "application/vnd.oci.image.config.v1+json", + Digest: godigest.FromBytes(data), + Size: int64(len(data)), + Data: data, + } + + t.Run("valid", func(t *testing.T) { + t.Parallel() + + r := descriptorDataReader(desc) + if r == nil { + t.Fatal("unexpected nil reader") + } + defer r.Close() + + if got := r.Descriptor(); got.Digest != desc.Digest || got.Size != desc.Size || got.MediaType != desc.MediaType { + t.Errorf("unexpected descriptor: %+v", got) + } + + got, err := io.ReadAll(r) + if err != nil { + t.Fatal("unexpected error", err) + } + if string(got) != string(data) { + t.Errorf("unexpected data: %q", got) + } + }) + + t.Run("invalid", func(t *testing.T) { + t.Parallel() + + bad := desc + bad.Digest = godigest.FromString("not the right data") + + // explicitly comparing to a literal "nil" (not just "r != nil") to make sure we never hand back a non-nil interface wrapping a nil pointer + if r := descriptorDataReader(bad); r != nil { + r.Close() + t.Fatalf("expected nil reader, got %#v", r) + } + }) +} diff --git a/registry/read-helpers.go b/registry/read-helpers.go index 03d796f..bf154e6 100644 --- a/registry/read-helpers.go +++ b/registry/read-helpers.go @@ -34,12 +34,15 @@ func readJSONHelper(r ociregistry.BlobReader, v interface{}) error { // decode directly! (mostly avoids double memory hit for big objects) // (TODO protect against malicious objects somehow?) - if err := json.NewDecoder(tee).Decode(v); err != nil { + decoder := json.NewDecoder(tee) + if err := decoder.Decode(v); err != nil { return err } + // (json.Decoder uses a buffer to make reads more efficient but it means if we want to read the leftovers ourselves we have to join their buffer back in front of our reader) + leftovers := io.MultiReader(decoder.Buffered(), tee) // read anything leftover ... - bs, err := io.ReadAll(tee) + bs, err := io.ReadAll(leftovers) if err != nil { return err } diff --git a/registry/read-helpers_test.go b/registry/read-helpers_test.go new file mode 100644 index 0000000..c1b2427 --- /dev/null +++ b/registry/read-helpers_test.go @@ -0,0 +1,79 @@ +package registry + +import ( + "strings" + "testing" + + "cuelabs.dev/go/oci/ociregistry" + "cuelabs.dev/go/oci/ociregistry/ocimem" + godigest "github.com/opencontainers/go-digest" +) + +func TestReadJSONHelper(t *testing.T) { + t.Parallel() + + for _, o := range []struct { + name string + data string + + // optional tweaks to the (otherwise correct) descriptor of "data" + sizeDelta int64 + digest godigest.Digest + + wantErr string + }{ + {name: "exact", data: `{"a":1}`}, + {name: "trailing newline", data: "{\"a\":1}\n"}, + {name: "leading and trailing whitespace", data: " \n\t{\"a\":1}\r\n \n"}, + + // the decoder buffers ahead, so anything after the object is already in its buffer by the time we go looking for "leftovers" (and used to go completely unchecked) + {name: "trailing garbage", data: `{"a":1} x`, wantErr: "unexpected non-whitespace"}, + {name: "trailing second object", data: `{"a":1}{"a":2}`, wantErr: "unexpected non-whitespace"}, + {name: "trailing garbage after whitespace", data: "{\"a\":1}\n\n x\n", wantErr: "unexpected non-whitespace"}, + + {name: "invalid json", data: `{"a":`, wantErr: "unexpected EOF"}, + {name: "wrong type", data: `{"a":"b"}`, wantErr: "cannot unmarshal"}, + + // descriptor lies about the object + {name: "size too big", data: `{"a":1}`, sizeDelta: 1, wantErr: "smaller than it should be"}, + {name: "size too small", data: "{\"a\":1}\n\n\n", sizeDelta: -3, wantErr: "bigger than it should be"}, + {name: "wrong digest", data: `{"a":1}`, digest: godigest.FromString("not the right data"), wantErr: "not correct"}, + {name: "invalid digest", data: `{"a":1}`, digest: "not-a-digest", wantErr: "invalid checksum digest format"}, + } { + o := o // https://github.com/golang/go/issues/60078 (go.mod is still "go 1.21") + + t.Run(o.name, func(t *testing.T) { + t.Parallel() + + desc := ociregistry.Descriptor{ + Digest: godigest.FromString(o.data), + Size: int64(len(o.data)) + o.sizeDelta, + } + if o.digest != "" { + desc.Digest = o.digest + } + + var v struct { + A int `json:"a"` + } + err := readJSONHelper(ocimem.NewBytesReader([]byte(o.data), desc), &v) + + if o.wantErr == "" { + if err != nil { + t.Fatal("unexpected error", err) + } + if v.A != 1 { + t.Errorf("unexpected value: %+v", v) + } + return + } + + if err == nil { + t.Fatalf("expected error containing %q, got nil", o.wantErr) + } + if !strings.Contains(err.Error(), o.wantErr) { + t.Errorf("expected error containing %q, got %q", o.wantErr, err) + } + }) + } +} diff --git a/registry/synthesize-index.go b/registry/synthesize-index.go index ee2c8d9..e367342 100644 --- a/registry/synthesize-index.go +++ b/registry/synthesize-index.go @@ -7,7 +7,6 @@ import ( "github.com/docker-library/bashbrew/architecture" "cuelabs.dev/go/oci/ociregistry" - "cuelabs.dev/go/oci/ociregistry/ocimem" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -45,7 +44,7 @@ func SynthesizeIndex(ctx context.Context, ref Reference) (*ocispec.Index, error) } default: - return nil, fmt.Errorf("unsupported mediaType: %q", desc.MediaType) + return nil, fmt.Errorf("%s: unsupported mediaType: %q", ref, desc.MediaType) } switch index.SchemaVersion { @@ -54,7 +53,7 @@ func SynthesizeIndex(ctx context.Context, ref Reference) (*ocispec.Index, error) case 2: // all good, do nothing! default: - return nil, fmt.Errorf("unsupported index schemaVersion: %q", index.SchemaVersion) + return nil, fmt.Errorf("%s: unsupported index schemaVersion: %q", ref, index.SchemaVersion) } switch index.MediaType { @@ -69,7 +68,7 @@ func SynthesizeIndex(ctx context.Context, ref Reference) (*ocispec.Index, error) case ocispec.MediaTypeImageIndex, mediaTypeDockerManifestList: // all good, do nothing! default: - return nil, fmt.Errorf("unsupported index mediaType: %q", index.MediaType) + return nil, fmt.Errorf("%s: unsupported index mediaType: %q", ref, index.MediaType) } setRefAnnotation(&index.Annotations, ref, desc.Digest) @@ -145,9 +144,8 @@ func normalizeManifestPlatform(ctx context.Context, m *ocispec.Descriptor, r oci case ocispec.MediaTypeImageManifest, mediaTypeDockerImageManifest: var err error if r == nil { - if m.Data != nil && int64(len(m.Data)) == m.Size { - r = ocimem.NewBytesReader(m.Data, *m) - } else { + r = descriptorDataReader(*m) + if r == nil { r, err = client.GetManifest(ctx, ref.Repository, m.Digest) if err != nil { return err @@ -163,10 +161,8 @@ func normalizeManifestPlatform(ctx context.Context, m *ocispec.Descriptor, r oci switch manifest.Config.MediaType { case ocispec.MediaTypeImageConfig, mediaTypeDockerImageConfig: - var r ociregistry.BlobReader - if manifest.Config.Data != nil && int64(len(manifest.Config.Data)) == manifest.Config.Size { - r = ocimem.NewBytesReader(manifest.Config.Data, manifest.Config) - } else { + r := descriptorDataReader(manifest.Config) + if r == nil { r, err = client.GetBlob(ctx, ref.Repository, manifest.Config.Digest) if err != nil { return err diff --git a/sm/sync-map.go b/sm/sync-map.go new file mode 100644 index 0000000..35e72bb --- /dev/null +++ b/sm/sync-map.go @@ -0,0 +1,18 @@ +package sm + +import ( + "sync" +) + +// sync.Map, but with generics (and a limited set of methods) +// see also: +// - https://github.com/golang/go/issues/71076 +// - https://github.com/golang/go/issues/47657 +type Map[K comparable, V any] struct { + m sync.Map +} + +func (m *Map[K, V]) LoadOrStore(key K, value V) (V, bool) { + actual, loaded := m.m.LoadOrStore(key, value) + return actual.(V), loaded +} diff --git a/sm/sync-map_test.go b/sm/sync-map_test.go new file mode 100644 index 0000000..b8a7194 --- /dev/null +++ b/sm/sync-map_test.go @@ -0,0 +1,75 @@ +package sm_test + +import ( + "sync" + "sync/atomic" + "testing" + + "github.com/docker-library/meta-scripts/sm" +) + +func TestMapLoadOrStore(t *testing.T) { + t.Parallel() + + var m sm.Map[string, int] + + if v, loaded := m.LoadOrStore("a", 1); loaded || v != 1 { + t.Errorf("first store: got (%d, %v), want (1, false)", v, loaded) + } + if v, loaded := m.LoadOrStore("a", 2); !loaded || v != 1 { + t.Errorf("second store: got (%d, %v), want (1, true)", v, loaded) + } + if v, loaded := m.LoadOrStore("b", 3); loaded || v != 3 { + t.Errorf("different key: got (%d, %v), want (3, false)", v, loaded) + } +} + +func TestMapLoadOrStorePointer(t *testing.T) { + t.Parallel() + + var m sm.Map[string, *sync.Mutex] + + first := &sync.Mutex{} + if v, loaded := m.LoadOrStore("a", first); loaded || v != first { + t.Errorf("first store: got (%p, %v), want (%p, false)", v, loaded, first) + } + if v, loaded := m.LoadOrStore("a", &sync.Mutex{}); !loaded || v != first { + t.Errorf("second store: got (%p, %v), want (%p, true)", v, loaded, first) + } +} + +func TestMapLoadOrStoreConcurrent(t *testing.T) { + t.Parallel() + + const goroutines = 100 + + var ( + m sm.Map[string, int] + wg sync.WaitGroup + stored atomic.Int32 + values [goroutines]int + ) + for i := 0; i < goroutines; i++ { + i := i // https://github.com/golang/go/issues/60078 (go.mod is still "go 1.21") + wg.Add(1) + go func() { + defer wg.Done() + + v, loaded := m.LoadOrStore("a", i) + if !loaded { + stored.Add(1) + } + values[i] = v + }() + } + wg.Wait() + + if got := stored.Load(); got != 1 { + t.Errorf("expected exactly one goroutine to store, got %d", got) + } + for i, v := range values { + if v != values[0] { + t.Errorf("goroutine %d saw %d, but goroutine 0 saw %d", i, v, values[0]) + } + } +}