Skip to content

Commit f7adb2b

Browse files
committed
Refactor registry code to be more correct
Previously, this couldn't fetch from Docker without `DOCKERHUB_PUBLIC_PROXY` (see `registry-1.docker.io` change) and was ignoring content digests. Now it works correctly with or without `DOCKERHUB_PUBLIC_PROXY`, verifies the size of every object it pulls, verifies the digest, _and_ should continue working with the in-progress Moby containerd-integration (where the local image ID becomes the digest of the manifest or index instead of the digest of the config blob as it is today).
1 parent 634e28f commit f7adb2b

File tree

4 files changed

+213
-97
lines changed

4 files changed

+213
-97
lines changed

.dockerignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@
66
!go.sum
77
!manifest/
88
!pkg/
9+
!registry/
910
!scripts/

cmd/bashbrew/cmd-push.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ func cmdPush(c *cli.Context) error {
3939
}
4040

4141
// we can't use "r.Tags()" here because it will include SharedTags, which we never want to push directly (see "cmd-put-shared.go")
42+
TagsLoop:
4243
for i, tag := range entry.Tags {
4344
if uniq && i > 0 {
4445
break
@@ -47,10 +48,12 @@ func cmdPush(c *cli.Context) error {
4748

4849
if !force {
4950
localImageId, _ := dockerInspect("{{.Id}}", tag)
50-
registryImageId := fetchRegistryImageId(tag)
51-
if registryImageId != "" && localImageId == registryImageId {
52-
fmt.Fprintf(os.Stderr, "skipping %s (remote image matches local)\n", tag)
53-
continue
51+
registryImageIds := fetchRegistryImageIds(tag)
52+
for _, registryImageId := range registryImageIds {
53+
if localImageId == registryImageId {
54+
fmt.Fprintf(os.Stderr, "skipping %s (remote image matches local)\n", tag)
55+
continue TagsLoop
56+
}
5457
}
5558
}
5659
fmt.Printf("Pushing %s\n", tag)

cmd/bashbrew/registry.go

Lines changed: 27 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -2,63 +2,48 @@ package main
22

33
import (
44
"context"
5-
"encoding/json"
6-
"net/url"
7-
"os"
85

9-
"github.com/containerd/containerd/images"
10-
"github.com/containerd/containerd/reference/docker"
11-
"github.com/containerd/containerd/remotes"
12-
dockerremote "github.com/containerd/containerd/remotes/docker"
13-
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
6+
"github.com/docker-library/bashbrew/registry"
147
)
158

16-
var registryImageIdCache = map[string]string{}
9+
var registryImageIdsCache = map[string][]string{}
1710

1811
// assumes the provided image name is NOT a manifest list (used for testing whether we need to "bashbrew push" or whether the remote image is already up-to-date)
1912
// this does NOT handle authentication, and will return the empty string for repositories which require it (causing "bashbrew push" to simply shell out to "docker push" which will handle authentication appropriately)
20-
func fetchRegistryImageId(image string) string {
13+
func fetchRegistryImageIds(image string) []string {
2114
ctx := context.Background()
2215

23-
ref, resolver, err := fetchRegistryResolveHelper(image)
16+
img, err := registry.Resolve(ctx, image)
2417
if err != nil {
25-
return ""
26-
}
27-
28-
name, desc, err := resolver.Resolve(ctx, ref)
29-
if err != nil {
30-
return ""
31-
}
32-
33-
if desc.MediaType != images.MediaTypeDockerSchema2Manifest && desc.MediaType != ocispec.MediaTypeImageManifest {
34-
return ""
18+
return nil
3519
}
3620

37-
digest := desc.Digest.String()
38-
if id, ok := registryImageIdCache[digest]; ok {
39-
return id
21+
digest := img.Desc.Digest.String()
22+
if ids, ok := registryImageIdsCache[digest]; ok {
23+
return ids
4024
}
4125

42-
fetcher, err := resolver.Fetcher(ctx, name)
26+
manifests, err := img.Manifests(ctx)
4327
if err != nil {
44-
return ""
28+
return nil
4529
}
4630

47-
r, err := fetcher.Fetch(ctx, desc)
48-
if err != nil {
49-
return ""
31+
ids := []string{}
32+
if img.IsImageIndex() {
33+
ids = append(ids, digest)
5034
}
51-
defer r.Close()
52-
53-
var manifest ocispec.Manifest
54-
if err := json.NewDecoder(r).Decode(&manifest); err != nil {
55-
return ""
35+
for _, manifestDesc := range manifests {
36+
ids = append(ids, manifestDesc.Digest.String())
37+
manifest, err := img.Manifest(ctx, manifestDesc)
38+
if err != nil {
39+
continue
40+
}
41+
ids = append(ids, manifest.Config.Digest.String())
5642
}
57-
id := manifest.Config.Digest.String()
58-
if id != "" {
59-
registryImageIdCache[digest] = id
43+
if len(ids) > 0 {
44+
registryImageIdsCache[digest] = ids
6045
}
61-
return id
46+
return ids
6247
}
6348

6449
var registryManifestListCache = map[string][]string{}
@@ -67,46 +52,22 @@ var registryManifestListCache = map[string][]string{}
6752
func fetchRegistryManiestListDigests(image string) []string {
6853
ctx := context.Background()
6954

70-
ref, resolver, err := fetchRegistryResolveHelper(image)
71-
if err != nil {
72-
return nil
73-
}
74-
75-
name, desc, err := resolver.Resolve(ctx, ref)
55+
img, err := registry.Resolve(ctx, image)
7656
if err != nil {
7757
return nil
7858
}
7959

80-
digest := desc.Digest.String()
81-
if desc.MediaType == images.MediaTypeDockerSchema2Manifest || desc.MediaType == ocispec.MediaTypeImageManifest {
82-
return []string{digest}
83-
}
84-
85-
if desc.MediaType != images.MediaTypeDockerSchema2ManifestList && desc.MediaType != ocispec.MediaTypeImageIndex {
86-
return nil
87-
}
88-
60+
digest := img.Desc.Digest.String()
8961
if digests, ok := registryManifestListCache[digest]; ok {
9062
return digests
9163
}
9264

93-
fetcher, err := resolver.Fetcher(ctx, name)
94-
if err != nil {
95-
return nil
96-
}
97-
98-
r, err := fetcher.Fetch(ctx, desc)
65+
manifests, err := img.Manifests(ctx)
9966
if err != nil {
10067
return nil
10168
}
102-
defer r.Close()
103-
104-
var manifestList ocispec.Index
105-
if err := json.NewDecoder(r).Decode(&manifestList); err != nil {
106-
return nil
107-
}
10869
digests := []string{}
109-
for _, manifest := range manifestList.Manifests {
70+
for _, manifest := range manifests {
11071
if manifest.Digest != "" {
11172
digests = append(digests, manifest.Digest.String())
11273
}
@@ -116,30 +77,3 @@ func fetchRegistryManiestListDigests(image string) []string {
11677
}
11778
return digests
11879
}
119-
120-
func fetchRegistryResolveHelper(image string) (string, remotes.Resolver, error) {
121-
ref, err := docker.ParseAnyReference(image)
122-
if err != nil {
123-
return "", nil, err
124-
}
125-
if namedRef, ok := ref.(docker.Named); ok {
126-
// add ":latest" if necessary
127-
namedRef = docker.TagNameOnly(namedRef)
128-
ref = namedRef
129-
}
130-
return ref.String(), dockerremote.NewResolver(dockerremote.ResolverOptions{
131-
Host: func(host string) (string, error) {
132-
if host == "docker.io" {
133-
if publicProxy := os.Getenv("DOCKERHUB_PUBLIC_PROXY"); publicProxy != "" {
134-
if publicProxyURL, err := url.Parse(publicProxy); err == nil {
135-
// TODO Scheme (also not sure if "host:port" will be satisfactory to containerd here, but 🤷)
136-
return publicProxyURL.Host, nil
137-
} else {
138-
return "", err
139-
}
140-
}
141-
}
142-
return host, nil
143-
},
144-
}), nil
145-
}

registry/registry.go

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package registry
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"io"
8+
"net/url"
9+
"os"
10+
"unicode"
11+
12+
// thanks, go-digest...
13+
_ "crypto/sha256"
14+
_ "crypto/sha512"
15+
16+
"github.com/containerd/containerd/images"
17+
"github.com/containerd/containerd/reference/docker"
18+
"github.com/containerd/containerd/remotes"
19+
dockerremote "github.com/containerd/containerd/remotes/docker"
20+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
21+
)
22+
23+
type ResolvedObject struct {
24+
Ref string
25+
Desc ocispec.Descriptor
26+
27+
resolver remotes.Resolver
28+
fetcher remotes.Fetcher
29+
}
30+
31+
func (obj ResolvedObject) FetchJSON(ctx context.Context, v interface{}) error {
32+
// prevent go-digest panics later
33+
if err := obj.Desc.Digest.Validate(); err != nil {
34+
return err
35+
}
36+
37+
r, err := obj.fetcher.Fetch(ctx, obj.Desc)
38+
if err != nil {
39+
return err
40+
}
41+
defer r.Close()
42+
43+
// make sure we can't possibly read (much) more than we're supposed to
44+
limited := &io.LimitedReader{
45+
R: r,
46+
N: obj.Desc.Size + 1, // +1 to allow us to detect if we read too much (see verification below)
47+
}
48+
49+
// copy all read data into the digest verifier so we can validate afterwards
50+
verifier := obj.Desc.Digest.Verifier()
51+
tee := io.TeeReader(limited, verifier)
52+
53+
// decode directly! (mostly avoids double memory hit for big objects)
54+
// (TODO protect against malicious objects somehow?)
55+
if err := json.NewDecoder(tee).Decode(v); err != nil {
56+
return err
57+
}
58+
59+
// read anything leftover ...
60+
bs, err := io.ReadAll(tee)
61+
if err != nil {
62+
return err
63+
}
64+
// ... and make sure it was just whitespace, if anything
65+
for _, b := range bs {
66+
if !unicode.IsSpace(rune(b)) {
67+
return fmt.Errorf("unexpected non-whitespace at the end of %q: %+v\n", obj.Desc.Digest.String(), rune(b))
68+
}
69+
}
70+
71+
// after reading *everything*, we should have exactly one byte left in our LimitedReader (anything else is an error)
72+
if limited.N < 1 {
73+
return fmt.Errorf("size of %q is bigger than it should be (%d)", obj.Desc.Digest.String(), obj.Desc.Size)
74+
} else if limited.N > 1 {
75+
return fmt.Errorf("size of %q is %d bytes smaller than it should be (%d)", obj.Desc.Digest.String(), limited.N-1, obj.Desc.Size)
76+
}
77+
78+
// and finally, let's verify our checksum
79+
if !verifier.Verified() {
80+
return fmt.Errorf("digest of %q not correct", obj.Desc.Digest.String())
81+
}
82+
83+
return nil
84+
}
85+
86+
func (obj ResolvedObject) Manifests(ctx context.Context) ([]ocispec.Descriptor, error) {
87+
if obj.IsImageManifest() {
88+
return []ocispec.Descriptor{obj.Desc}, nil
89+
}
90+
91+
if !obj.IsImageIndex() {
92+
return nil, fmt.Errorf("unknown media type: %q", obj.Desc.MediaType)
93+
}
94+
95+
// (perhaps use a containerd content store??)
96+
var index ocispec.Index
97+
if err := obj.FetchJSON(ctx, &index); err != nil {
98+
return nil, err
99+
}
100+
return index.Manifests, nil
101+
}
102+
103+
func (obj ResolvedObject) Manifest(ctx context.Context, desc ocispec.Descriptor) (*ocispec.Manifest, error) {
104+
obj.Desc = desc // swap our object to point at this manifest
105+
if !obj.IsImageManifest() {
106+
return nil, fmt.Errorf("unknown media type: %q", obj.Desc.MediaType)
107+
}
108+
109+
// (perhaps use a containerd content store??)
110+
var manifest ocispec.Manifest
111+
if err := obj.FetchJSON(ctx, &manifest); err != nil {
112+
return nil, err
113+
}
114+
return &manifest, nil
115+
}
116+
117+
func (obj ResolvedObject) IsImageManifest() bool {
118+
return obj.Desc.MediaType == ocispec.MediaTypeImageManifest || obj.Desc.MediaType == images.MediaTypeDockerSchema2Manifest
119+
}
120+
121+
func (obj ResolvedObject) IsImageIndex() bool {
122+
return obj.Desc.MediaType == ocispec.MediaTypeImageIndex || obj.Desc.MediaType == images.MediaTypeDockerSchema2ManifestList
123+
}
124+
125+
func Resolve(ctx context.Context, image string) (*ResolvedObject, error) {
126+
var (
127+
obj = ResolvedObject{
128+
Ref: image,
129+
}
130+
err error
131+
)
132+
133+
obj.Ref, obj.resolver, err = resolverHelper(obj.Ref)
134+
if err != nil {
135+
return nil, err
136+
}
137+
138+
obj.Ref, obj.Desc, err = obj.resolver.Resolve(ctx, obj.Ref)
139+
if err != nil {
140+
return nil, err
141+
}
142+
143+
obj.fetcher, err = obj.resolver.Fetcher(ctx, obj.Ref)
144+
if err != nil {
145+
return nil, err
146+
}
147+
148+
return &obj, nil
149+
}
150+
151+
func resolverHelper(image string) (string, remotes.Resolver, error) {
152+
ref, err := docker.ParseAnyReference(image)
153+
if err != nil {
154+
return "", nil, err
155+
}
156+
if namedRef, ok := ref.(docker.Named); ok {
157+
// add ":latest" if necessary
158+
namedRef = docker.TagNameOnly(namedRef)
159+
ref = namedRef
160+
}
161+
return ref.String(), dockerremote.NewResolver(dockerremote.ResolverOptions{
162+
// TODO port this to "Hosts:" (especially so we can return Scheme correctly) but requires reimplementing some of https://github.com/containerd/containerd/blob/v1.6.9/remotes/docker/resolver.go#L161-L184 😞
163+
Host: func(host string) (string, error) {
164+
if host == "docker.io" {
165+
if publicProxy := os.Getenv("DOCKERHUB_PUBLIC_PROXY"); publicProxy != "" {
166+
if publicProxyURL, err := url.Parse(publicProxy); err == nil {
167+
// TODO Scheme (also not sure if "host:port" will be satisfactory to containerd here, but 🤷)
168+
return publicProxyURL.Host, nil
169+
} else {
170+
return "", err
171+
}
172+
}
173+
return "registry-1.docker.io", nil // https://github.com/containerd/containerd/blob/1c90a442489720eec95342e1789ee8a5e1b9536f/remotes/docker/registry.go#L193
174+
}
175+
return host, nil
176+
},
177+
}), nil
178+
}

0 commit comments

Comments
 (0)