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

Add support for --follow-link option in podman cp #16585

Open
htjain opened this issue Nov 22, 2022 · 22 comments · May be fixed by containers/buildah#4703 or #18020
Open

Add support for --follow-link option in podman cp #16585

htjain opened this issue Nov 22, 2022 · 22 comments · May be fixed by containers/buildah#4703 or #18020
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@htjain
Copy link

htjain commented Nov 22, 2022

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature

Description

Currently podman cp doesn't support for --follow-link like docker cp does.

Steps to reproduce the issue:

  1. podman cp --follow-link xyz.tar container:/tmp/

Describe the results you received:
Error: unknown flag: --follow-link

Describe the results you expected:
File cp command should copy target file to container.

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

$ podman version
Version:      3.4.7
API Version:  3.4.7
Go Version:   go1.19.2
Built:        Thu Jan  1 05:30:00 1970
OS/Arch:      linux/arm64

Output of podman info:

$ podman info
host:
  arch: arm64
  buildahVersion: 1.23.1
  cgroupControllers:
  - cpu
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: 'conmon: /usr/bin/conmon'
    path: /usr/bin/conmon
    version: 'conmon version 2.1.3, commit: unknown'
  cpus: 4
  distribution:
    codename: bookworm
    distribution: debian
    version: unknown
  eventLogger: journald
  hostname: raspberrypi
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
  kernel: 5.15.76-v8+
  linkmode: dynamic
  logDriver: journald
  memFree: 201830400
  memTotal: 3529732096
  ociRuntime:
    name: runc
    package: 'runc: /usr/bin/runc'
    path: /usr/bin/runc
    version: |-
      runc version 1.1.4+ds1
      commit: 1.1.4+ds1-1
      spec: 1.0.2-dev
      go: go1.19
      libseccomp: 2.5.4
  os: linux
  remoteSocket:
    path: /run/user/1000/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: false
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: 'slirp4netns: /usr/bin/slirp4netns'
    version: |-
      slirp4netns version 1.2.0
      commit: 656041d45cfca7a4176f6b7eed9e4fe6c11e8383
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.4
  swapFree: 26738688
  swapTotal: 104853504
  uptime: 69h 57m 46.53s (Approximately 2.88 days)
plugins:
  log:
  - k8s-file
  - none
  - journald
  network:
  - bridge
  - macvlan
  volume:
  - local
registries: {}
store:
  configFile: /home/pi/.config/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: vfs
  graphOptions: {}
  graphRoot: /home/pi/.local/share/containers/storage
  graphStatus: {}
  imageStore:
    number: 19
  runRoot: /run/user/1000/containers
  volumePath: /home/pi/.local/share/containers/storage/volumes
version:
  APIVersion: 3.4.7
  Built: 0
  BuiltTime: Thu Jan  1 05:30:00 1970
  GitCommit: ""
  GoVersion: go1.19.2
  OsArch: linux/arm64
  Version: 3.4.7

Package info (e.g. output of rpm -q podman or apt list podman or brew info podman):

$ apt list podman
Listing... Done
podman/testing,now 3.4.7+ds1-3+b3 arm64 [installed]
podman/testing 3.4.7+ds1-3+b3 armhf

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/main/troubleshooting.md)

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):
Same behavior on other OSs

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 22, 2022
@rhatdan
Copy link
Member

rhatdan commented Nov 22, 2022

@htjain I started working on this a while back, but have too many other higher priorities now to finish in a reasonable amount of time.

If you were interested in taking it over, we could move it forward.

@htjain
Copy link
Author

htjain commented Nov 23, 2022

@rhatdan I can take it up. #15730 this PR you are talking about ?

@rhatdan
Copy link
Member

rhatdan commented Nov 23, 2022

Yes. It is all yours.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jan 3, 2023

@htjain any progress?

@github-actions
Copy link

github-actions bot commented Feb 3, 2023

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Feb 3, 2023

@flouthoc PTAL

@github-actions
Copy link

github-actions bot commented Mar 6, 2023

A friendly reminder that this issue had no activity for 30 days.

@danishprakash
Copy link
Contributor

@rhatdan I'd like to look into this.

@flouthoc
Copy link
Collaborator

@danishprakash Thanks assigned it to you :)

@danishprakash
Copy link
Contributor

While triaging, I found some issues with what we're trying to achieve which I'd like to clear before I move on. It looks like

In docker cp, if you don't specify the --follow-link flag, docker would just copy the link and not the destination file. But if you do specify that flag, it would follow the link and copy the target to the destination.

$ docker run -d --rm -it --name test opensuse/tumbleweed bash

$ ls -al /tmp/file
/tmp/file -> /home/user/config/target

$ docker cp /tmp/file test:/tmp/file
$ docker exec test ls -al /tmp/file
/tmp/file -> /home/user/config/target 	# this doesn't exist of course

$ docker cp --follow-link /tmp/file test:/tmp/file
$ docker exec test ls -al /tmp/file
/tmp/file 								# with the contents for the symbolic link target

Now, podman, by default, follows the link without the --follow-link flag. So the question would be, do we change the current cp behavior to conform to docker's and add the --follow-link flag to do what it does right now. Or we leave it as is and document the behaviour? Or I might've missed something and the above is not right, in which case, I'd request either @rhatdan or @flouthoc to corroborate this once, thanks!

@flouthoc
Copy link
Collaborator

Okay podman indeed by default follows the symlink, i'd say we should default to how docker does and introduce a --follow-link, my only concern is that this is a breaking change and might break old users.

Again my vote is to default how docker does it but lets wait for others to take a look.

@rhatdan @containers/podman-maintainers WDYT ?

@giuseppe
Copy link
Member

+1 to the Docker behavior. It is safer to not follow symlinks by default

@vrothberg
Copy link
Member

Agreed. Docker compat is also a compelling reason to change the default.

@danishprakash
Copy link
Contributor

That sounds great but as @flouthoc said, I'm also concerned that this is a breaking change and might break existing workflows. Either way, I'll start looking into the changes.

@rhatdan
Copy link
Member

rhatdan commented Mar 17, 2023

We need a boolean to change the default globally in containers.conf, then users who want the preivous default can customize their environment.

@danishprakash
Copy link
Contributor

Tried a sample implementation here which included changes to both buildah and podman. The idea is to maintain the symlink even if the target isn't present on the container which would inevitably lead to an error when accessed. This is docker's behaviour too.

While passing --follow-link flag, podman would continue to do what it currently does.

We need a boolean to change the default globally in containers.conf, then users who want the preivous default can customize their environment.

I can go ahead and finalize the PRs in both the repos if the approach above sounds reasonable. Thoughts?

@giuseppe
Copy link
Member

giuseppe commented Apr 3, 2023

yes please open the PRs so we can comment on them

@fantognazza
Copy link

I noticed that the PR in buildah got stale. In the meantime, could we introduce the --follow-link and -L flags just to improve compatibility with docker-based scripts that use them (e.g. mflowgen)? This shouldn't break any existing workflow.

@danishprakash
Copy link
Contributor

Are you suggesting a dummy flag so that podman doesn't complain?

@fantognazza
Copy link

Exactly, mflowgen extensively uses docker cp -aL <...>, which results in podman complaining of non-existent -L flag

@danishprakash
Copy link
Contributor

Perhaps @rhatdan @giuseppe can chime in here. Either way, it'd be nice if we can move ahead on the PR, waiting on review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
7 participants