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

podman cp chowns files #10801

Closed
matejvasek opened this issue Jun 28, 2021 · 11 comments · Fixed by #10804
Closed

podman cp chowns files #10801

matejvasek opened this issue Jun 28, 2021 · 11 comments · Fixed by #10804
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@matejvasek
Copy link
Contributor

Sub-comman podman cp doesn't preserve uid/git from tar. It deliberately chowns everything to uid/gid the container is running under at the moment. docker however preserve uid/gid. Since podman is drop in replacement we should do that too.
If not for the cp CLI sub-comman then an least for docker APIv2.

@matejvasek
Copy link
Contributor Author

/cc @mheon @vrothberg

@mheon mheon added the kind/bug Categorizes issue or PR as related to a bug. label Jun 28, 2021
@rhatdan
Copy link
Member

rhatdan commented Jun 28, 2021

@matejvasek Could you write a repeater example?

@matejvasek
Copy link
Contributor Author

~> echo 'L3dvcmtzcGFjZS9hLnR4dAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAwMDA2NDQAMDAwMTc1MAAwMDAxNzUxADAwMDAwMDAwMDAwADE0MDY2MzYxMDE1ADAxMjMyNQAgMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB1c3RhcgAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwMDAwMDAwADAwMDAwMDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAvd29ya3NwYWNlL2IudHh0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMDAwMDY0NAAwMDAxNzUwADAwMDE3NTEAMDAwMDAwMDAwMDAAMTQwNjYzNjEwMTcAMDEyMzMwACAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHVzdGFyADAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAwMDAwMDAAMDAwMDAwMAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=' | base64 -d > a.tar

~> podman container create --name show-ws quay.io/boson/faas-go-builder:v0.8.3 /usr/bin/ls -la /tmp/workspace
~> podman cp - show-ws:/tmp < a.tar
~> podman start show-ws -a
total 0
drwxr-xr-x. 2 root root 60 Jun 28 17:24 .
drwxrwxrwt. 3 root root 18 Jun 28 17:24 ..
-rw-r--r--. 1 root root  0 Jun 28 14:51 a.txt
-rw-r--r--. 1 root root  0 Jun 28 14:51 b.txt

~> docker create --name show-ws quay.io/boson/faas-go-builder:v0.8.3 /usr/bin/ls -la /tmp/workspace
~> docker cp - show-ws:/tmp < a.tar
~> docker start show-ws  -a
total 0
drwxr-xr-x. 1 root root 20 Jun 28 17:22 .
drwxrwxrwt. 1 root root 90 Jun 28 17:22 ..
-rw-r--r--. 1 cnb  cnb   0 Jun 28 14:51 a.txt
-rw-r--r--. 1 cnb  cnb   0 Jun 28 14:51 b.txt

@matejvasek
Copy link
Contributor Author

diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go
index 0ab322829..09a69f8f5 100644
--- a/libpod/container_copy_linux.go
+++ b/libpod/container_copy_linux.go
@@ -63,12 +63,12 @@ func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.
        }
 
        // Make sure we chown the files to the container's main user and group ID.
-       user, err := getContainerUser(c, mountPoint)
-       if err != nil {
-               unmount()
-               return nil, err
-       }
-       idPair := idtools.IDPair{UID: int(user.UID), GID: int(user.GID)}
+       //user, err := getContainerUser(c, mountPoint)
+       //if err != nil {
+       //      unmount()
+       //      return nil, err
+       //}
+       //idPair := idtools.IDPair{UID: int(user.UID), GID: int(user.GID)}
 
        decompressed, err := archive.DecompressStream(reader)
        if err != nil {
@@ -84,8 +84,8 @@ func (c *Container) copyFromArchive(ctx context.Context, path string, reader io.
                putOptions := buildahCopiah.PutOptions{
                        UIDMap:     c.config.IDMappings.UIDMap,
                        GIDMap:     c.config.IDMappings.GIDMap,
-                       ChownDirs:  &idPair,
-                       ChownFiles: &idPair,
+                       //ChownDirs:  &idPair,
+                       //ChownFiles: &idPair,
                }
 
                return c.joinMountAndExec(ctx,

this should fix it

@matejvasek
Copy link
Contributor Author

btw docker can chown files like podman does but -a flag has to be used.

I find it weird because:

Options:
  -a, --archive       Archive mode (copy all uid/gid information)

I would expect that it would works exactly opposite: if -a flag is present it IMHO should preserve uid/gid from tar, but opposite is true, if -a flag is present files are chowned. Am I misunderstanding anything here @rhatdan?

@matejvasek
Copy link
Contributor Author

Maybe we could add the -a flag to the podman?

@rhatdan
Copy link
Member

rhatdan commented Jun 28, 2021

SGTM
Interested in opening a PR.
@vrothberg PTAL

@matejvasek
Copy link
Contributor Author

The flag is by default false meaning that by default files are not chowned as opposed to current podman behavior. May I do such a change?

@rhatdan
Copy link
Member

rhatdan commented Jun 28, 2021

Yes we should match the Docker behaviour. We can add a flag to containers.conf, if people want to change the default value of this field.

@vrothberg
Copy link
Member

Note that podman cp has never been fully compatible with docker cp. Changing the behavior now may cause regressions for Podman users.

@rhatdan, do you want to continue? I am not necessarily against it but want to weigh in.

@rhatdan
Copy link
Member

rhatdan commented Jun 29, 2021

I think it is best to add the -a function and default to true to match current podman behaviour, then lets add a flag to containers.conf if people want default to Docker behaviour.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants