-
-
Notifications
You must be signed in to change notification settings - Fork 201
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 setting user
, workingDir
and env
when executing a command in a container
#668
Add support for setting user
, workingDir
and env
when executing a command in a container
#668
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the addition of these options is very useful and necessary when executing commands within a container. 💯
Just some nits relevant to the format that should give us prettier.
packages/testcontainers/src/container-runtime/clients/container/container-client.ts
Outdated
Show resolved
Hide resolved
packages/testcontainers/src/container-runtime/clients/container/container-client.ts
Outdated
Show resolved
Hide resolved
packages/testcontainers/src/container-runtime/clients/container/docker-container-client.ts
Outdated
Show resolved
Hide resolved
packages/testcontainers/src/container-runtime/clients/container/podman-container-client.ts
Outdated
Show resolved
Hide resolved
What difference and advantage we have doing const container = await new GenericContainer("alpine")
.withCommand(["sleep", "infinity"])
.start();
const { output, exitCode } = await container.exec(["echo", "hello", "world"], {
workingDir: "/app/src/",
user: "alpine",
env: {
"VAR1": "enabled",
"VAR2": "full",
}
}); instead or doing 🤔 ? const startedContainer = await new GenericContainer("alpine")
.withWorkingDir("/app/src/")
.withUser("alpine")
.withEnvironment({
"VAR1": "enabled",
"VAR2": "full",
})
.withCommand(["sleep", "infinity"])
.start();
const { output, exitCode } = await startedContainer.exec(["echo", "hello", "world"]); apart from being able to redefine these options in each execution of a command? |
The two things are different.
The first, which I added support for, specifies the current working directory for the specific command you are running in an already-running container. So, you are only affecting the command you execute. You must specify the The use case here is:
For example, I want to modify on-the-fly a configuration file stored in
This second case affects the current working directory of the new container you will create after calling This For reference: |
The feature has really sense to me @LNSD thanks to clarify with all this great explanation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @LNSD, first thank you so much for opening the PR and for providing such a valid use case. We love contributions!!
Second, I'm core maintainer of Testcontainer Go, stepping ahead because @cristianrgreco is on PTO, and seeing this PR with a lot of value. So please take my review comments as a non-typescript developer at all 🙏 As you'll read, my comments are mostly related to style and consistency with existing patterns, not related at all with the "business" of the code, which initially LGTM.
packages/testcontainers/src/container-runtime/clients/container/docker-container-client.ts
Show resolved
Hide resolved
packages/testcontainers/src/container-runtime/clients/container/container-client.ts
Show resolved
Hide resolved
packages/testcontainers/src/container-runtime/clients/container/podman-container-client.ts
Show resolved
Hide resolved
packages/testcontainers/src/container-runtime/clients/container/docker-container-client.ts
Show resolved
Hide resolved
BTW it'd be important if you could add tests demonstrating the new behaviour 🙏 |
Added an integration test 🧪 covering the new functionality: faa32d7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, added a comment regarding the added test, but other than that, thank you for this contribution. Looks a great improvement!
BTW I'll not merge it immediately, as I'd like to give @cristianrgreco a couple of days to have the chance to take a look if needed.
Rebased the PR on top of the CI fix 🔁 |
Thanks for raising the PR @LNSD, very well tested and documented 🙂 Because
So that a user could do for example: import { ExecOptions, ExecResult } from "testcontainers";
const options: ExecOptions = { ... };
const result = await container.exec(..., options); Other than that it LGTM |
@@ -19,7 +19,7 @@ export { Network, StartedNetwork, StoppedNetwork } from "./network/network"; | |||
export { Wait } from "./wait-strategies/wait"; | |||
export { StartupCheckStrategy, StartupStatus } from "./wait-strategies/startup-check-strategy"; | |||
export { PullPolicy, ImagePullPolicy } from "./utils/pull-policy"; | |||
export { InspectResult, Content, ExecResult } from "./types"; | |||
export { InspectResult, Content, ExecOptions, ExecResult } from "./types"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done ✔️ 😁
user
, workingDir
and env
when executing a command in a container
This pull request introduces support for specifying
User
,WorkingDir
, andEnv
options during container execution in both Docker (API docs) and Podman (API docs), enhancing customization.Changes:
Added support to
StartedTestContainer.exec(command, opts)
function to provide the following options:user
:user
,user:group
,uid
, oruid:gid
.workingDir
:WorkingDir
option allows users to define the working directory for the exec process inside the container.env
:Env
option when executing commands, allowing for environment variable customization.Example:
Your feedback and testing contributions are appreciated.