Docker image unpacker#715
Conversation
|
Is there a technical reason this depends on those other branches, or is it just a result of your local git tree? I'm assuming the latter, but want to know what I should look out for when reviewing. |
|
No, it doesn't technically depend on those other branches, it's just that I need all these features for the project I'm currently working on. |
rbs-jacob
left a comment
There was a problem hiding this comment.
Some small stuff, then this should be good to go!
| entry = entry.strip("/") | ||
| basename = os.path.basename(entry) |
There was a problem hiding this comment.
Might also want to call os.path.normpath in case there are leading ./.
| with open(manifest_path) as f: | ||
| manifest = json.load(f) | ||
|
|
||
| layers = manifest[0]["Layers"] |
There was a problem hiding this comment.
Why only pick the first entry? I'm pretty sure (~75%) Docker can bundle multiple images in the same output tarball. Might be worth at least emitting a warning here if there are more than one?
| raise NotImplementedError( | ||
| "Packing a DockerImage back into a Docker image tarball is not supported. " | ||
| "The current unpacker merges all layers into a single filesystem, so the original " | ||
| "layer structure cannot be reconstructed." | ||
| ) |
There was a problem hiding this comment.
Based on the comment in this error, I'm assuming the answer is "no," but: can we just use the TarPacker here to not specifically implement an image packer?
Since this is probably not possible, feel free to resolve this without comment if that's the case.
| class TestDockerImagePacker: | ||
| async def test_pack_not_implemented(self, ofrak_context: OFRAKContext): | ||
| with open(DOCKER_IMAGE_ASSET, "rb") as f: | ||
| data = f.read() | ||
| resource = await ofrak_context.create_root_resource("docker.tar", data) | ||
| await resource.identify() | ||
|
|
||
| await resource.unpack() | ||
| with pytest.raises(NotImplementedError): | ||
| await resource.pack() |
There was a problem hiding this comment.
I get that we might have to do this for test coverage, but it feels weird to assert that the packer is unimplemented in a test.
One sentence summary of this PR (This should go in the CHANGELOG!)
Docker image tarball unpacker.
Link to Related Issue(s)
Please describe the changes in your request.
Adds a Docker image unpacker: unpack a docker image tar ball to a filesystem that represents the filesystem from this image once run as a container. This unpacker walks the different image layers and applies them, including whiteouts and opaque whiteouts.
Anyone you think should look at this, specifically?
No