Add initial Builder: oci-import support
#20
Conversation
This includes a workaround for the bug it helped me find (and fix), but I don't think it needs to wait for the fix because the workaround is really trivial / straightforward. The biggest benefit here (besides being one step closer to being able to switch to the new method for _everything_) is that this new way we build images will be significantly less susceptible to the downsides of the previous `oci-import` support (most notably, that the tarballs round-tripping through Docker recompresses them -- the containerd image storage in Docker itself should also help make that much better).
| @@ -0,0 +1,15 @@ | |||
| # https://github.com/docker-library/official-images/blob/fe9c059402181390eac083cbdd7229b5d123236e/library/ubuntu but intentionally slimmed down (just "latest" on one architecture, no email addresses) | |||
|
|
|||
| Maintainers: Tomáš Virtus (@woky), Cristóvão Cordeiro (@cjdcordeiro) | |||
There was a problem hiding this comment.
UTF-8 support established
| GitRepo: https://git.launchpad.net/cloud-images/+oci/ubuntu-base | ||
| GitCommit: fa42be9027eccb928a1f0f43d95ffd9a45d36737 | ||
| Builder: oci-import | ||
| File: index.json |
There was a problem hiding this comment.
Is this necessary? It has to be index.json, right?
https://github.com/opencontainers/image-spec/blob/main/image-layout.md
There was a problem hiding this comment.
The oci-import builder is a bashbrew construction that uses OCI layouts, but does not mandate the use of index.json explicitly -- see https://github.com/docker-library/bashbrew/blob/5152c0df682515cbe7ac62b68bcea4278856429f/cmd/bashbrew/oci-builder.go#L117-L134
If File: is not set to the literal string index.json, then it is assumed to point to a JSON file that contains a single OCI content descriptor for the image manifest (which will then be looked up under blobs/sha256/xxxx).
See also https://github.com/docker-library/meta-scripts/pull/20/files#diff-a64244565ecee6714e591bafe1b0771f08c5217287fb88a6ff0e9f1457eb8ac3R296-R299 for where in this PR those get "upgraded" into a correct index.json so that we don't have to handle both cases exceptionally here.
(Technically, the code I've written there to "upgrade" that case to index.json would break if the object under blobs/sha256/xxx is a symlink to index.json at the root of the OCI layout, but given we've established that the format is "OCI layout", it's fair for us to assume that index.json at the root has special meaning and whatever is happening in that theoretical case is wrong.)
| "_commit", | ||
|
|
||
| # TODO figure out a good, safe place to store our temporary build/push directory (maybe this is fine? we do it for buildx build too) | ||
| "mkdir temp", |
There was a problem hiding this comment.
mktemp -d not available?
local tmpdir=$(umask 077 && d=ztemp-doi-bb-$$-$RANDOM; mkdir "$d" && echo "$d")There was a problem hiding this comment.
It is, but build_command and push_command both need access to it, so it needs to be stored in a common location. I think this really is fine as-is -- we've so far used the model that PWD is fair game for these commands to all store/transmit state, and it works reasonably well.
| error(\"unsupported descriptor mediaType: \" + .mediaType) | ||
| else . end | ||
| # TODO validate .digest somehow (`crane validate`? see below) - would also be good to validate all descriptors recursively (not sure if `crane push` does that) | ||
| | if .size < 0 then |
There was a problem hiding this comment.
Is a size of zero valid?
There was a problem hiding this comment.
It's valid for the descriptor format, but in this case it would mean the image manifest itself is zero bytes, which isn't valid.
| "buildId": "93476ae64659d71f4ee7fac781d6d1890df8926682e2fa6bd647a246b33ad9bf", | ||
| "build": { | ||
| "img": "oisupport/staging-amd64:93476ae64659d71f4ee7fac781d6d1890df8926682e2fa6bd647a246b33ad9bf", | ||
| "resolved": null, |
There was a problem hiding this comment.
With docker-library/official-images@795e049, this is now even safer as part of our tests because this exact buildId will never be officially built. 😄 😇
See also:
This includes a workaround for the bug it helped me find (and fix in docker-library/bashbrew#92), but I don't think it needs to wait for the fix because the workaround is really trivial / straightforward.
The biggest benefit here (besides being one step closer to being able to switch to the new method for everything) is that this new way we build images will be significantly less susceptible to the downsides of the previous
oci-importsupport (most notably, that the tarballs round-tripping through Docker recompresses them -- the containerd image storage in Docker itself should also help make that much better).That also means this would make me feel comfortable with finally replying to docker-library/official-images#527 to point them at the
oci-importsupport that they might be interested in (and thus resolving docker-library/meta#22 one way or another). 👀