-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for oci-layout build-context #1173
Conversation
Pulling in the updated buildkit lib updated a whole bunch of other things. go mod worked its magic. It all worked and compiled, so I didn't complain. |
build/build.go
Outdated
target.OCIStores = map[string]content.Store{ | ||
k: store, | ||
} | ||
target.FrontendAttrs["context:"+k] = "oci-layout:" + k |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value in here nees to include digest that you are expecting in https://github.com/moby/buildkit/blob/master/frontend/dockerfile/builder/build.go#L892
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that is it. I went through our buildkit PR, and could not recall which part did this. That helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Take a look, I think I got it right.
Source an image from a local [OCI layout compliant directory](https://github.com/opencontainers/image-spec/blob/main/image-layout.md): | ||
|
||
```console | ||
$ docker buildx build --build-context foo=oci-layout:///path/to/local/layout@sha256:abcd12345 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should allow this input without the digest as well in here. If no digest is set it can look up index.json
inside this dir and see if it has one descriptor and then set it as the expected digest. If there is ambiguity then it can produce an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am hesitant about this, as it wades into a deep unresolved issue around what actually should be in index.json
. I have seen those that only include tags, others that include entire image refs, making the layout directory essentially into a full registry. I have implemented. both ways.
containerd does a bit of a hybrid: directory is a full registry, but tag->manifest-hash is in the boltdb.
Let's get this working with the hash required, as is, and then let's deal with supporting hash-less - both in buildkit and in buildx - as a separate iteration.
ce076d3
to
64efc7d
Compare
Most things resolved, one open to be sure that this is right, another subject to discussion. |
64efc7d
to
03e0ce4
Compare
03e0ce4
to
d706310
Compare
Fixed the listing issue, and rebased on |
d706310
to
92a615d
Compare
Fixed the go mod issues. Phew. I am seeing vet issues with:
I didn't think I changed those, but perhaps downstream dependencies? |
92a615d
to
cc135ac
Compare
Yeah, it looks like the definition of func NewDockerAuthProvider(stderr io.Writer) session.Attachable {
return &authProvider{
config: config.LoadDefaultConfigFile(stderr),
seeds: &tokenSeeds{dir: config.Dir()},
loggerCache: map[string]struct{}{},
}
} to func NewDockerAuthProvider(cfg *configfile.ConfigFile) session.Attachable {
return &authProvider{
authConfigCache: map[string]*types.AuthConfig{},
config: cfg,
seeds: &tokenSeeds{dir: config.Dir()},
loggerCache: map[string]struct{}{},
}
} I got it. |
8dba3d4
to
faad929
Compare
Figured out the failing CI test, updated, should be good now. |
How do we stand here @tonistiigi ? |
2777474
to
068c31c
Compare
Been almost a month. I just rebased. Is there anything else we need to get this merged in? |
@@ -126,6 +126,30 @@ FROM alpine | |||
COPY --from=project myfile / | |||
``` | |||
|
|||
Source an image from a local [OCI layout compliant directory](https://github.com/opencontainers/image-spec/blob/main/image-layout.md): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update line 109 with OCI layout directory and add an intented subtitle for OCI layout for the new content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Back to you @tonistiigi
Signed-off-by: Avi Deitcher <avi@deitcher.net>
068c31c
to
02bae94
Compare
Adds to buildx the oci-layout: build-context capabilities added to buildkit in buildkit 2827.
Recommended by @tonistiigi . Remains incomplete, some things I wasn't sure about. Will comment inline.