testcontainers-rs icon indicating copy to clipboard operation
testcontainers-rs copied to clipboard

[feature request] Add file permission to `CopyToContainer`

Open stormshield-gt opened this issue 7 months ago • 1 comments

CopyToContainer will perform the copy as a root user, which will cause a permission issue for containers running with an unprivileged user.

To work around this, the go-testcontainers have a mode parameters that allow to configure the permission of the copied file.

I think it will be a nice addition to this library.

stormshield-gt avatar Apr 24 '25 16:04 stormshield-gt

The go library seems to do it by adjusting the tar permission.

This example of tokio_tar, the lib we use, make me think it would be possible to do the same

stormshield-gt avatar Apr 24 '25 17:04 stormshield-gt

Hi everyone — I'd like to push this issue forward, but the change would touch a widely used API, so I'd appreciate consensus before coding. Summary of the candidate directions:

  1. Option A – Additive forever

    • Keep the current with_copy_to(target, source) untouched and add with_copy_to_opt(CopyOptions) alongside it.
    • Example:
      image.with_copy_to("/app/config", "./config/local.toml");
      
      let opts = CopyOptions::new("./config/local.toml")
          .target("/app/config")
          .mode(0o600);
      image.with_copy_to_opt(opts);
      
    • Pros: zero breakage; users only move to the new API when they need extra configuration.
    • Cons: we permanently carry two methods, docs/IDE hints must explain the difference, and the longer name never disappears.
    • Note: we could implement with_copy_to in terms of with_copy_to_opt so ImageExt implementers only override the _opt variant, reducing duplication even if both names stay public.
  2. Option B – Additive now, rename later (with deprecation)

    • Same as Option A initially, but we immediately mark with_copy_to(target, source) as deprecated.
    • Next major release: remove the old signature and rename with_copy_to_opt back to with_copy_to(CopyOptions).
    • Migration sketch:
      // Transition period
      let opts = CopyOptions::new("./config/local.toml")
          .target("/app/config")
          .mode(0o600);
      image.with_copy_to_opt(opts); // legacy with_copy_to warns
      
      // Next major release
      image.with_copy_to(opts);
      
    • Pros: smooth migration today, eventual convergence to the concise name, and time to validate the CopyOptions design.
    • Cons: users touch call sites twice (move to _opt, later drop the suffix) and we must communicate the timeline clearly.
  3. Option C – Immediate breaking change

    • Replace the existing signature now so with_copy_to only accepts CopyOptions; no parallel method or deprecation window.
    • Example:
      let opts = CopyOptions::new("./config/local.toml")
          .target("/app/config")
          .mode(0o600);
      image.with_copy_to(opts); // sole entry point
      
    • Pros: the API stays clean and unambiguous, and implementation/docs cover a single code path.
    • Cons: immediate breaking change, so it must ship with a major release or a tight migration guide; every downstream user updates at once.

Given the above, I'm leaning toward Option A: it keeps today’s API intact, we can default with_copy_to to delegate to with_copy_to_opt, and it keeps the migration cost minimal even if we have to live with the longer name. Still open to other preferences—feedback welcome!

DCjanus avatar Nov 14 '25 10:11 DCjanus

I think A is a good compromise to deliver it faster and not to force people to update their usage even after upgrade

Another option could be to extend target argument

Currently with_copy_to accepts two params: target: impl Into<String>, and source: impl Into<CopyDataSource> So we could introduce TargetOptions and expect target: impl Into<TargetOptions> Implement From for any T: Into<String> while allowing users to pass TargetOptions as is

let target = CopyTargetOptions::new("config.toml").mode(0o600)`;
image.with_copy_to(target, "./config.toml");
// and this way also will work, but without `mode`
image.with_copy_to("config.toml", "./config.toml");

I'm not completely sure if it's clear/transparent enough, but it will preserve compatibility and won't require to add new methods Single CopyOptions might be more understandable

WDYT?

DDtKey avatar Nov 14 '25 17:11 DDtKey

While adding with_mode I hit a semantics question and would like feedback on what avoids surprising users:

  • Current behavior: when the source is a directory we use append_dir_all; mode only applies to the top-level target directory, while files/subdirs keep their host permissions (or defaults).
  • Options:
    1. Define that mode affects only the target directory; children always inherit host permissions—no unexpected deep changes.
    2. Apply the same mode recursively to every entry in the tree—convenient for uniform permissions but could quietly change deep files.
    3. Allow separate modes for directories vs files—more complex API.

Which semantics feels most intuitive and least surprising? Thanks!

DCjanus avatar Nov 18 '25 03:11 DCjanus

Perhaps it should be a separate opt-in option to apply mode recursively 🤔

DDtKey avatar Nov 25 '25 15:11 DDtKey