From 6340184df636eec2f7b41cc8b1ea9e6e2cb78838 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Thu, 15 Sep 2022 16:09:24 +0100 Subject: [PATCH] source: refactor reference parsing for oci-layout Instead of using custom parsing mechansisms for references in oci-layout, we use containerd's reference.Parse or docker distribution's reference.Parse (depending on where we do the parsing, and what's consistent with the file where it's already done). These operations are neater than manually parsing, and have hopefully more consistent error messages, and better handling of labels (for if/when those are introduced). Signed-off-by: Justin Chadwell --- client/llb/source.go | 4 +-- frontend/dockerfile/builder/build.go | 49 ++++++++++++++-------------- source/containerimage/ocilayout.go | 16 ++++----- source/containerimage/pull.go | 14 +++----- source/identifier.go | 15 ++++----- 5 files changed, 46 insertions(+), 52 deletions(-) diff --git a/client/llb/source.go b/client/llb/source.go index 5af51a467..e0b436348 100644 --- a/client/llb/source.go +++ b/client/llb/source.go @@ -455,7 +455,7 @@ func Differ(t DiffType, required bool) LocalOption { }) } -func OCILayout(contentStoreID string, dig digest.Digest, opts ...OCILayoutOption) State { +func OCILayout(store string, digest digest.Digest, opts ...OCILayoutOption) State { gi := &OCILayoutInfo{} for _, o := range opts { @@ -474,7 +474,7 @@ func OCILayout(contentStoreID string, dig digest.Digest, opts ...OCILayoutOption addCap(&gi.Constraints, pb.CapSourceOCILayout) - source := NewSource(fmt.Sprintf("oci-layout://%s@%s", contentStoreID, dig), attrs, gi.Constraints) + source := NewSource(fmt.Sprintf("oci-layout://%s@%s", store, digest), attrs, gi.Constraints) return NewState(source.Output()) } diff --git a/frontend/dockerfile/builder/build.go b/frontend/dockerfile/builder/build.go index 41a513deb..5763ae77a 100644 --- a/frontend/dockerfile/builder/build.go +++ b/frontend/dockerfile/builder/build.go @@ -33,7 +33,6 @@ import ( "github.com/moby/buildkit/solver/errdefs" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/gitutil" - digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -956,30 +955,30 @@ func contextByName(ctx context.Context, c client.Client, sessionID, name string, } return st, nil, nil case "oci-layout": - ref := strings.TrimPrefix(vv[1], "//") - // expected format is storeID@hash - parts := strings.SplitN(ref, "@", 2) - if len(parts) != 2 { - return nil, nil, errors.Errorf("invalid oci-layout format '%s', must be oci-layout:///content-store@sha256:digest", vv[1]) - } - storeID := parts[0] - dig, err := digest.Parse(parts[1]) + refSpec := strings.TrimPrefix(vv[1], "//") + ref, err := reference.Parse(refSpec) if err != nil { - return nil, nil, errors.Errorf("invalid digest format '%s', must be oci-layout:///content-store@sha256:digest", vv[1]) + return nil, nil, errors.Wrapf(err, "could not parse oci-layout reference %q", refSpec) + } + named, ok := ref.(reference.Named) + if !ok { + return nil, nil, errors.Errorf("oci-layout reference %q has no name", ref.String()) + } + if reference.Domain(named) != "" { + return nil, nil, errors.Errorf("oci-layout reference %q has domain", ref.String()) + } + if strings.Contains(reference.Path(named), "/") { + return nil, nil, errors.Errorf("oci-layout reference %q name is multi-part", ref.String()) + } + digested, ok := ref.(reference.Digested) + if !ok { + return nil, nil, errors.Errorf("oci-layout reference %q does not have digest", ref.String()) } - // the ref now is "content-store@sha256:digest" - // ResolveImageConfig will try to treat that as a valid reference, - // to be parsed with https://pkg.go.dev/github.com/containerd/containerd@v1.6.6/reference#Parse - // That will fail, because it will think that "content-store@sha256" is a host and "digest" is a port. - // To get it to pass, we need to jury-rig it with a host, so that it is a legitimate ref and can - // be processed. - // A reasonable format is storeID as host, so - // storeID/image@digest - // We do not support any image lookup for now, so any image will do; it is ignored. - - usableRef := fmt.Sprintf("%s/%s@%s", storeID, "image", dig) - _, data, err := c.ResolveImageConfig(ctx, usableRef, llb.ResolveImageConfigOpt{ + // We use store id as the host here, the image name will be ignored + // (since image lookup is not currently supported) + id := fmt.Sprintf("%s/image@%s", named.Name(), digested.Digest()) + _, data, err := c.ResolveImageConfig(ctx, id, llb.ResolveImageConfigOpt{ Platform: platform, ResolveMode: resolveMode, LogName: fmt.Sprintf("[context %s] load metadata for %s", name, ref), @@ -991,10 +990,12 @@ func contextByName(ctx context.Context, c client.Client, sessionID, name string, var img dockerfile2llb.Image if err := json.Unmarshal(data, &img); err != nil { - return nil, nil, err + return nil, nil, errors.Wrap(err, "could not parse oci-layout image config") } - st := llb.OCILayout(storeID, dig, + st := llb.OCILayout( + named.Name(), + digested.Digest(), llb.WithCustomName("[context "+name+"] OCI load from client"), llb.OCISessionID(c.BuildOpts().SessionID), ) diff --git a/source/containerimage/ocilayout.go b/source/containerimage/ocilayout.go index 86b0d9111..521a2e7b2 100644 --- a/source/containerimage/ocilayout.go +++ b/source/containerimage/ocilayout.go @@ -63,12 +63,11 @@ func (r *ociLayoutResolver) Fetch(ctx context.Context, desc ocispecs.Descriptor) func (r *ociLayoutResolver) Resolve(ctx context.Context, refString string) (string, ocispecs.Descriptor, error) { ref, err := reference.Parse(refString) if err != nil { - return "", ocispecs.Descriptor{}, errors.Wrapf(err, "invalid reference '%s'", refString) + return "", ocispecs.Descriptor{}, errors.Wrapf(err, "invalid reference %q", refString) } - - dig := ref.Digest() - if dig == "" { - return "", ocispecs.Descriptor{}, errors.Errorf("reference must have format @sha256:: %s", refString) + dgst := ref.Digest() + if dgst == "" { + return "", ocispecs.Descriptor{}, errors.Errorf("reference %q must have digest", refString) } info, err := r.info(ctx, ref) @@ -80,7 +79,7 @@ func (r *ociLayoutResolver) Resolve(ctx context.Context, refString string) (stri // This is necessary because we do not know the media-type of the descriptor, // and there are descriptor processing elements that expect it. desc := ocispecs.Descriptor{ - Digest: dig, + Digest: info.Digest, Size: info.Size, } rc, err := r.Fetch(ctx, desc) @@ -91,12 +90,13 @@ func (r *ociLayoutResolver) Resolve(ctx context.Context, refString string) (stri if err != nil { return "", ocispecs.Descriptor{}, errors.Wrap(err, "unable to read root manifest") } - // try it first as an index, then as a manifest + mediaType, err := imageutil.DetectManifestBlobMediaType(b) if err != nil { - return "", ocispecs.Descriptor{}, errors.Wrapf(err, "reference %s contains neither an index nor a manifest", refString) + return "", ocispecs.Descriptor{}, errors.Wrapf(err, "reference %q contains neither an index nor a manifest", refString) } desc.MediaType = mediaType + return refString, desc, nil } diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go index 74c928d3f..0b03d17c1 100644 --- a/source/containerimage/pull.go +++ b/source/containerimage/pull.go @@ -106,19 +106,13 @@ func (is *Source) ResolveImageConfig(ctx context.Context, ref string, opt llb.Re } rslvr = resolver.DefaultPool.GetResolver(is.RegistryHosts, ref, "pull", sm, g).WithImageStore(is.ImageStore, rm) case ResolverTypeOCILayout: - // with OCI layout, we always just "pull" rm = source.ResolveModeForcePull - // get the content store ID from the ref + parsed, err := reference.Parse(ref) if err != nil { - return "", nil, errors.Errorf("invalid oci-layout ref format '%s', must be content-store/image@sha256:digest", ref) + return "", nil, err } - if parsed.Digest() == "" { - return "", nil, errors.Errorf("oci-layout ref format '%s' missing digest, must be content-store/image@sha256:digest", ref) - } - storeID := parsed.Hostname() - - rslvr = getOCILayoutResolver(storeID, sm, "", g) + rslvr = getOCILayoutResolver(parsed.Hostname(), sm, "", g) } key += rm.String() res, err := is.g.Do(ctx, key, func(ctx context.Context) (interface{}, error) { @@ -172,7 +166,7 @@ func (is *Source) Resolve(ctx context.Context, id source.Identifier, sm *session } mode = source.ResolveModeForcePull // with OCI layout, we always just "pull" sessionID = ociIdentifier.SessionID - ref = reference.Spec{Locator: "oci-layout/dummy", Object: "@" + ociIdentifier.Manifest.String()} + ref = reference.Spec{Locator: "oci-layout/dummy", Object: "@" + ociIdentifier.Digest.String()} storeID = ociIdentifier.Name layerLimit = ociIdentifier.LayerLimit default: diff --git a/source/identifier.go b/source/identifier.go index 6e996cbea..be7bd21f8 100644 --- a/source/identifier.go +++ b/source/identifier.go @@ -292,23 +292,22 @@ func (*HTTPIdentifier) ID() string { type OCIIdentifier struct { Name string - Manifest digest.Digest + Digest digest.Digest Platform *ocispecs.Platform SessionID string LayerLimit *int } func NewOCIIdentifier(str string) (*OCIIdentifier, error) { - // OCI identifier arg is of the format: path@hash - parts := strings.SplitN(str, "@", 2) - if len(parts) != 2 { - return nil, errors.New("OCI must be in format of storeID@manifest-hash") + store, digSrc, found := strings.Cut(str, "@") + if !found { + return nil, errors.Errorf("invalid OCI identifier %s", str) } - dig, err := digest.Parse(parts[1]) + dig, err := digest.Parse(digSrc) if err != nil { - return nil, errors.Wrap(err, "OCI must be in format of storeID@manifest-hash, invalid digest") + return nil, errors.Wrapf(err, "invalid digest in OCI identifier %s", str) } - return &OCIIdentifier{Name: parts[0], Manifest: dig}, nil + return &OCIIdentifier{Name: store, Digest: dig}, nil } func (*OCIIdentifier) ID() string {