Remove /bin/sh wrapping from container shell commands (#464)

Previously `libcnb-test` wrapped the shell commands for `PrepareContainerContext::start_with_shell_command` and `ContainerContext::shell_exec` in `/bin/sh -c '...'` invocation. This is unnecessary, since `launcher` already runs the passed commands using bash: https://buildpacks.io/docs/app-developer-guide/run-an-app/#user-provided-shell-process https://buildpacks.io/docs/app-developer-guide/run-an-app/#user-provided-shell-process-with-bash-script In addition, it's not necessary to hardcode the full launcher path, since `/cnb/lifecycle` is automatically put on `PATH` during image creation. Now, the way the commands are invoked are consistent with how users are instructed to run processes in the upstream CNB docs. GUS-W-11415544.
heroku · Jul 12, 2022 · aca4eeb · aca4eeb
1 parent a39963f
commit aca4eeb
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 19 deletions.
diff --git a/libcnb-test/CHANGELOG.md b/libcnb-test/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Add `TestConfig::cargo_profile` and `CargoProfile` to support compilation of buildpacks in release mode. ([#456](https://github.com/heroku/libcnb.rs/pull/456))
 - Improve the error message shown when Pack CLI is not installed. ([#455](https://github.com/heroku/libcnb.rs/pull/455))
 - Check that `TestConfig::app_dir` is a valid directory before applying `TestConfig::app_dir_preprocessor` or invoking Pack, so that a clearer error message can be displayed. ([#452](https://github.com/heroku/libcnb.rs/pull/452))
+- `PrepareContainerContext::start_with_shell_command` and `ContainerContext::shell_exec` now use `bash` instead of `/bin/sh`. ([#464](https://github.com/heroku/libcnb.rs/pull/464))
 - Use the crate's `README.md` as the root module's rustdocs, so that all of the crate's documentation can be seen in one place on `docs.rs`. ([#460](https://github.com/heroku/libcnb.rs/pull/460))
 - Add rustdocs with code examples for all public APIs. ([#441](https://github.com/heroku/libcnb.rs/pull/441))
 - Fix rustdocs for `LogOutput`. ([#440](https://github.com/heroku/libcnb.rs/pull/440))

diff --git a/libcnb-test/src/container_context.rs b/libcnb-test/src/container_context.rs
@@ -218,7 +218,7 @@ impl<'a> PrepareContainerContext<'a> {
     /// Creates and starts the container configured by this context using the given shell command.
     ///
     /// The CNB lifecycle launcher will be implicitly used. Environment variables will be set. Uses
-    /// `/bin/sh` as the shell.
+    /// `bash` as the shell.
     ///
     /// See: [CNB App Developer Guide: Run a multi-process app - User-provided shell process](https://buildpacks.io/docs/app-developer-guide/run-an-app/#user-provided-shell-process)
     ///
@@ -243,12 +243,8 @@ impl<'a> PrepareContainerContext<'a> {
         f: F,
     ) {
         self.start_internal(
-            Some(vec![String::from(CNB_LAUNCHER_PATH)]),
-            Some(vec![
-                String::from(SHELL_PATH),
-                String::from("-c"),
-                command.into(),
-            ]),
+            Some(vec![String::from(CNB_LAUNCHER_BINARY)]),
+            Some(vec![command.into()]),
             f,
         );
     }
@@ -472,7 +468,7 @@ impl<'a> ContainerContext<'a> {
                 .create_exec(
                     &self.container_name,
                     CreateExecOptions {
-                        cmd: Some(vec![CNB_LAUNCHER_PATH, SHELL_PATH, "-c", command.as_ref()]),
+                        cmd: Some(vec![CNB_LAUNCHER_BINARY, command.as_ref()]),
                         attach_stdout: Some(true),
                         ..CreateExecOptions::default()
                     },
@@ -516,5 +512,4 @@ impl<'a> Drop for ContainerContext<'a> {
     }
 }
 
-const CNB_LAUNCHER_PATH: &str = "/cnb/lifecycle/launcher";
-const SHELL_PATH: &str = "/bin/sh";
+const CNB_LAUNCHER_BINARY: &str = "launcher";
diff --git a/libcnb-test/test-fixtures/procfile/Procfile b/libcnb-test/test-fixtures/procfile/Procfile
@@ -1,3 +1,3 @@
-web: python3 -u -m http.server
+web: python3 -u -m http.server ${PORT:+"${PORT}"}
 worker: echo 'this is the worker process!'
 echo-args: echo
diff --git a/libcnb-test/tests/integration_test.rs b/libcnb-test/tests/integration_test.rs
@@ -66,6 +66,7 @@ fn starting_containers() {
         |context| {
             context
                 .prepare_container()
+                .env("PORT", "5678")
                 .start_with_default_process(|container| {
                     // Give the server time to boot up.
                     // TODO: Make requests to the server using a client that retries, and fetch logs after
@@ -76,10 +77,10 @@ fn starting_containers() {
                     assert_empty!(log_output_until_now.stderr);
                     assert_contains!(
                         log_output_until_now.stdout,
-                        "Serving HTTP on 0.0.0.0 port 8000"
+                        "Serving HTTP on 0.0.0.0 port 5678"
                     );
 
-                    let exec_log_output = container.shell_exec("ps");
+                    let exec_log_output = container.shell_exec("ps | grep python3");
                     assert_empty!(exec_log_output.stderr);
                     assert_contains!(exec_log_output.stdout, "python3");
                 });
@@ -107,14 +108,14 @@ fn starting_containers() {
                 },
             );
 
-            context
-                .prepare_container()
-                .env("TEST_VAR", "TEST_VALUE")
-                .start_with_shell_command("env", |container| {
+            context.prepare_container().start_with_shell_command(
+                "for i in {1..3}; do echo \"${i}\"; done",
+                |container| {
                     let all_log_output = container.logs_wait();
                     assert_empty!(all_log_output.stderr);
-                    assert_contains!(all_log_output.stdout, "TEST_VAR=TEST_VALUE");
-                });
+                    assert_eq!(all_log_output.stdout, "1\n2\n3\n");
+                },
+            );
         },
     );
 }