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.

Sign up for GitHub

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

Jump to bottom

[Bug]: CopyDirToContainer doesn't behave as expected #2780

Open
stevenh opened this issue Sep 13, 2024 · 1 comment · May be fixed by #2782
Open

[Bug]: CopyDirToContainer doesn't behave as expected #2780

stevenh opened this issue Sep 13, 2024 · 1 comment · May be fixed by #2782
Labels
bug An issue with the library

Comments

@stevenh
Copy link
Collaborator

stevenh commented Sep 13, 2024

Testcontainers version

0.33

Using the latest Testcontainers version?

Yes

Host OS

Linux

Host arch

amd64

Go version

1.23.1

Docker version

Client:
 Version:           27.2.0
 API version:       1.47
 Go version:        go1.21.13
 Git commit:        3ab4256
 Built:             Tue Aug 27 14:14:20 2024
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Desktop  ()
 Engine:
  Version:          27.2.0
  API version:      1.47 (minimum version 1.24)
  Go version:       go1.21.13
  Git commit:       3ab5c7d
  Built:            Tue Aug 27 14:15:15 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.7.20
  GitCommit:        8fc6bcff51318944179630522a095cc9dbf9f353
 runc:
  Version:          1.1.13
  GitCommit:        v1.1.13-0-g58aa920
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

Client:
 Version:    27.2.0
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.16.2-desktop.1
    Path:     /usr/local/lib/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.29.2-desktop.2
    Path:     /usr/local/lib/docker/cli-plugins/docker-compose
  debug: Get a shell into any image or container (Docker Inc.)
    Version:  0.0.34
    Path:     /usr/local/lib/docker/cli-plugins/docker-debug
  desktop: Docker Desktop commands (Alpha) (Docker Inc.)
    Version:  v0.0.15
    Path:     /usr/local/lib/docker/cli-plugins/docker-desktop
  dev: Docker Dev Environments (Docker Inc.)
    Version:  v0.1.2
    Path:     /usr/local/lib/docker/cli-plugins/docker-dev
  extension: Manages Docker extensions (Docker Inc.)
    Version:  v0.2.25
    Path:     /usr/local/lib/docker/cli-plugins/docker-extension
  feedback: Provide feedback, right in your terminal! (Docker Inc.)
    Version:  v1.0.5
    Path:     /usr/local/lib/docker/cli-plugins/docker-feedback
  init: Creates Docker-related starter files for your project (Docker Inc.)
    Version:  v1.3.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-init
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc.)
    Version:  0.6.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-sbom
  scout: Docker Scout (Docker Inc.)
    Version:  v1.13.0
    Path:     /usr/local/lib/docker/cli-plugins/docker-scout
WARNING: Plugin "/usr/local/lib/docker/cli-plugins/docker-scan" is not valid: failed to fetch metadata: fork/exec /usr/local/lib/docker/cli-plugins/docker-scan: no such file or directory

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 379
 Server Version: 27.2.0
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 nvidia runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 8fc6bcff51318944179630522a095cc9dbf9f353
 runc version: v1.1.13-0-g58aa920
 init version: de40ad0
 Security Options:
  seccomp
   Profile: unconfined
 Kernel Version: 5.15.153.1-microsoft-standard-WSL2
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 32
 Total Memory: 31.2GiB
 Name: docker-desktop
 ID: 087461d5-735b-4132-9fe3-1b275179a4fe
 Docker Root Dir: /var/lib/docker
 Debug Mode: true
  File Descriptors: 55
  Goroutines: 84
  System Time: 2024-09-13T23:07:41.276467563Z
  EventsListeners: 16
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Labels:
  com.docker.desktop.address=unix:///var/run/docker-cli.sock
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5555
  127.0.0.0/8
  172.16.0.0/12
  192.168.0.0/16
  10.0.0.0/8
 Live Restore Enabled: false

WARNING: No blkio throttle.read_bps_device support
WARNING: No blkio throttle.write_bps_device support
WARNING: No blkio throttle.read_iops_device support
WARNING: No blkio throttle.write_iops_device support
WARNING: daemon is not using the default seccomp profile

What happened?

Calling CopyDirToContainer with hostDirPath that is a host directory and a containerParentPath that container directory the contents of hostDirPath was copied to the parent of containerParentPath instead of the path itself.

For example:

container.CopyDirToContainer(context.Background, "testdata", "/scripts", 0o700)

This resulted in testdata in the root directory of the container.

I would expect this to match docker cp behaviour so if /scripts didn't exist the contents of testdata would be copied into the container creating /scripts. If the directory does exist it should copy it into the /scripts resulting in /scripts/testdata.

In addition to this the fileMode parameter overrides the permission of all files within the source when copied, with no option to preserve source permissions, which is frustrating.

Relevant log output

No response

Additional information

No response
@stevenh stevenh added the bug An issue with the library label Sep 13, 2024
@stevenh
Copy link
Collaborator Author

stevenh commented Sep 15, 2024

This means that TestCopyDirectoryToRunningContainerAsFile and TestCopyDirectoryToRunningContainerAsDir are broken even though they are passing as they missing validation as to the resulting file structure, i.e. the check for done in the log output

stevenh added a commit that referenced this issue Sep 16, 2024
@stevenh
feat: copy from host to container
eba1930 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fixes #2780
@stevenh stevenh linked a pull request Sep 16, 2024 that will close this issue
Draft
stevenh added a commit that referenced this issue Sep 16, 2024
@stevenh
feat: copy from host to container
6d7eb7b 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fixes #2780
stevenh added a commit that referenced this issue Sep 16, 2024
@stevenh
feat: copy from host to container
8ed34c6 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fix testdata wait scripts.

Fix invalid FileMode values, so tests don't fail with the new FileMode
validation.

Switch to exists check for copy instead of running it as a shell.

Fixes #2780
stevenh added a commit that referenced this issue Sep 16, 2024
@stevenh
feat: copy from host to container
3e34029 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fix testdata wait scripts.

Fix invalid FileMode values, so tests don't fail with the new FileMode
validation.

Switch to exists check for copy instead of running it as a shell.

Fixes #2780
stevenh added a commit that referenced this issue Sep 17, 2024
@stevenh
feat: copy from host to container
e81fdcb 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fix testdata wait scripts.

Fix invalid FileMode values, so tests don't fail with the new FileMode
validation.

Switch to exists check for copy instead of running it as a shell.

Fixes #2780
stevenh added a commit that referenced this issue Sep 17, 2024
@stevenh
feat: copy from host to container
3f6c23d 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fix testdata wait scripts.

Fix invalid FileMode values, so tests don't fail with the new FileMode
validation.

Switch to exists check for copy instead of running it as a shell.

Fixes #2780
stevenh added a commit that referenced this issue Sep 17, 2024
@stevenh
feat: copy from host to container
edb9d04 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fix testdata wait scripts.

Fix invalid FileMode values, so tests don't fail with the new FileMode
validation.

Switch to exists check for copy instead of running it as a shell.

Disable linter check for deprecated methods as we use them internally
and test them.

Fixes #2780
stevenh added a commit that referenced this issue Sep 17, 2024
@stevenh
feat: copy from host to container
2c6196f 
Add CopyHostPathTo to container methods, which is capable of copying files
and directories from the host to a container. It identifies the correct
copy semantics based on inspecting the source and target, replicating
the behaviour of docker cp and other OS copy tools.

This deprecates CopyDirToContainer and CopyFileToContainer while still
correcting their behaviour to also match docker cp behaviour.

Replace nonamedreturns linter with nakedret, as nonamedreturns prevents
naming returned parameters which has a number of valid uses including:
disambiguating return values and error checking.

Copying files to the container now doesn't compression as this is
slower and consumes more resources for the typical local transfer case.

Fix docker copy tests to they validate the correct behaviour by ensuring
that done is reported to the container log.

Clean up some error wrapping.

Fix testdata wait scripts.

Fix invalid FileMode values, so tests don't fail with the new FileMode
validation.

Switch to exists check for copy instead of running it as a shell.

Disable linter check for deprecated methods as we use them internally
and test them.

Fixes #2780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug An issue with the library
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: copy from host to container testcontainers/testcontainers-go
1 participant
@stevenh