Skip to content
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

render::unpack fails for some images. #149

Open
akhramov opened this issue Apr 2, 2020 · 6 comments
Open

render::unpack fails for some images. #149

akhramov opened this issue Apr 2, 2020 · 6 comments
Labels

Comments

@akhramov
Copy link

akhramov commented Apr 2, 2020

Hi, thanks for your awesome library! I think I found a little bug, described below. Aside from that -- great overall experience, thanks!

Description
Alpine-based images make a heavy use of symlinks.
As it stands now, tar-rs overwrites files and not symlinks. (alexcrichton/tar-rs/pull/217)
If layers create a symlink in the same location, render::unpack will fail.

Steps to reproduce:

  1. Run cargo run --example image registry-1.docker.io nginxdemos/hello

Expected result:

  1. Layers are downloaded and unpacked

Actual result:

  1. The command fails with
[registry-1.docker.io] failed to unpack `/usr/home/akhramov/dkregistry-rs/nginxdemos_hello:latest/usr/bin/strings`

Known workaround:

None yet, shall tar-rs be fixed?

@steveej
Copy link
Contributor

steveej commented Jun 9, 2020

Thanks for reporting this. I would expect render::unpack to overwrite the previous layers' contents, so I agree this is a bug.

None yet, shall tar-rs be fixed?

I think this should be the goal, yes. It seems that the PR you linked has a larger scope than this and it would probably be better to separate that.

@steveej steveej added the bug label Jun 9, 2020
@lucab
Copy link
Member

lucab commented Jun 9, 2020

Cross-referencing: alexcrichton/tar-rs#132.

@steveej
Copy link
Contributor

steveej commented Jun 9, 2020

Cross-referencing: alexcrichton/tar-rs#132.

Thanks, but what if just the symlink is changed to point to a different path? That's not the same as following a link and overwriting the target file.

@akhramov
Copy link
Author

JFYI, I am implementing an application which partially overlaps this crate's functionality. I ended up replacing tar-rs with libarchive.

There are existing libarchive bindings, yet those are poorly maintained, so I created my own.

Unpacker:
https://github.com/akhramov/werft/blob/master/baustelle/src/unpacker.rs

libarchive glue code:
https://github.com/akhramov/werft/blob/master/baustelle/src/archive.rs - Provides Rust-y API for unpacker.

https://github.com/akhramov/werft/blob/master/baustelle/src/archive/resource.rs -- reads & extracts archives
https://github.com/akhramov/werft/blob/master/baustelle/src/archive/entry.rs -- reads & sets entries path.

@moschroe
Copy link

moschroe commented Jun 11, 2020

I ended up replacing tar-rs with libarchive.

Shame, since it's a public holiday here, I just got around to tackling this. Still created a more focused PR. (alexcrichton/tar-rs#229)

All the best!

@steveej
Copy link
Contributor

steveej commented Jun 19, 2020

@moschroe thanks! That PR has landed by now so we'll have this in the next tar-rs release 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants