lifecycle icon indicating copy to clipboard operation
lifecycle copied to clipboard

Default process type with `-e` argument not sent to command

Open schneems opened this issue 3 years ago • 9 comments

Args with -e are somehow eaten from default process types. In this case, we create a simple bash script that echos the args that are passed to it:

#!/usr/bin/env bash

echo "Printing args"
printf '%s\n' "$*"

Here's the default process with args:

$ pack inspect sample | grep Processes -A 5
Processes:
  TYPE                        SHELL        COMMAND                                              ARGS                                 WORK DIR
  print-args (default)        bash         /layers/sample-counter/sys-info/print-args.sh        before-dash-e -e after-dash-e        /workspace

When we run this, you would expect to see -e in the output, but it is not present:

Actual

$ docker run -it --rm sample
Printing args
before-dash-e after-dash-e

Note that the -e is missing

Expected

$ docker run -it --rm sample
Printing args
before-dash-e -e after-dash-e

Reproduce

Clone the code:

git clone https://github.com/schneems/cnb-dash-e-bug
cd cnb-dash-e-bug

Build the app:

$ pack build sample --buildpack ./ --path ./
22: Pulling from heroku/builder
Digest: sha256:e7916849c5cddcad0877666e672ddd37bcfdaf5064dba6967032b780291e66f5
Status: Image is up to date for heroku/builder:22
22-cnb: Pulling from heroku/heroku
Digest: sha256:48752e45ae12ea577f519b734a9578c7d0dab08359edc2c252253bb67a1c6564
Status: Image is up to date for heroku/heroku:22-cnb
Restoring data for SBOM from previous image
===> DETECTING
sample-counter 0.1.0
===> RESTORING
Restoring metadata for "sample-counter:sys-info" from app image
===> BUILDING
---> Hello processes buildpack
---> Adding sys-info process
---> Done
===> EXPORTING
Reusing layer 'sample-counter:sys-info'
Reusing layer 'launch.sbom'
Adding 1/1 app layer(s)
Reusing layer 'launcher'
Reusing layer 'config'
Reusing layer 'process-types'
Adding label 'io.buildpacks.lifecycle.metadata'
Adding label 'io.buildpacks.build.metadata'
Adding label 'io.buildpacks.project.metadata'
Setting default process type 'print-args'
Saving sample...
*** Images (6992ee87a261):
      sample
Successfully built image sample

Execute the default process:

$ docker run -it --rm sample
Printing args
before-dash-e  after-dash-e

Context

lifecycle version
Lifecycle:
  Version: 0.14.1
platform version(s)
$ pack report
Pack:
  Version:  0.27.0+git-f4f5be1.build-3382
  OS/Arch:  darwin/amd64

Default Lifecycle Version:  0.14.1

Supported Platform APIs:  0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9

Config:
  default-builder-image = "[REDACTED]"

  [[trusted-builders]]
    name = "[REDACTED]"

  [[trusted-builders]]
    name = "[REDACTED]"
$ docker info
Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.8.2)
  compose: Docker Compose (Docker Inc., v2.6.0)
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc., 0.6.0)
  scan: Docker Scan (Docker Inc., v0.17.0)
Server:
 Containers: 136
  Running: 0
  Paused: 0
  Stopped: 136
 Images: 47
 Server Version: 20.10.16
anything else?

Nope

schneems avatar Nov 08 '22 18:11 schneems

This seems like a bug in launcher to me.

I believe the -e is being swallowed by the echo in the "$(eval echo \"$0\")" here (since -e is a valid echo option): https://github.com/buildpacks/lifecycle/blob/caf3d06c0073ff24fa1b3f4c7ac59b9eac111784/launch/bash.go#L58

edmorley avatar Nov 08 '22 18:11 edmorley

To clarify, it evaluates to $(eval echo "-e"), if a given argument is only -e. Could also happen with -E, -n, and whatever other options Bash's echo builtin gets in the future.

In zsh, echo is a builtin that has a special argument terminator, -, but that does not work with a standard POSIX /bin/echo or with Bash's builtin, unfortunately.

dzuelke avatar Nov 08 '22 21:11 dzuelke

By the way, it is not necessary to escape the double quotes for the echo call in the eval subshell. You can arbitrarily nest subshells that use $() without any special quoting.

Example: echo $(echo "hi $(echo "there")")

(that's one of the main selling points of $() over backticks)

dzuelke avatar Nov 08 '22 21:11 dzuelke

Maybe this works?

commandScript += fmt.Sprintf(` "$(eval cat <<< "${%d}")"`, i) 

-edit-

It does not (won't expand variables in an argument).

dzuelke avatar Nov 09 '22 18:11 dzuelke

@schneems thanks for raising this issue. I have a question, is it at all possible for you to convert the process to direct = true? You can still provide bash as the command if desired, but the invocation will follow a different code path that shouldn't have this issue.

natalieparellano avatar Nov 09 '22 19:11 natalieparellano

In my real-life use case, I need to pass an env var to a command arg so I need bash to do the variable substitution https://github.com/heroku/buildpacks-ruby/blob/fd0402b5fad4785edcce5ccd3822a8bdd84ccebe/buildpacks/ruby/src/main.rs#L118-L121.

I'm able to work around it in the short term as --environment can be used instead of -e.

schneems avatar Nov 09 '22 21:11 schneems

Thanks @schneems - as you may know, we removed direct = false processes from buildpack 0.9. Going forward, we expect to have some way to provide variable references that can be evaluated in the container, but the exact mechanism is yet-to-be specified. I believe @samj1912 has some ideas on this.

natalieparellano avatar Nov 09 '22 21:11 natalieparellano

Thanks for chiming in. I heard the talk but was fuzzy on the details. I just read the RFC https://github.com/buildpacks/rfcs/pull/168/files. It sounds like a proprietary format was introduced to support dynamically replacing env vars.

If this functionality is dead in the water it's not worth fixing then, I guess. I want to ensure that future versions have a version that will allow some kind of bash execution as our customers rely heavily on the ability to write and run arbitrary bash scripts, specifically through the Procfile https://github.com/heroku/procfile-cnb. People rely on things like being able to pipe and fork and, just about anything else you can do in a oneliner. It looks like we can mitigate that by running it with a direct=true with bash -c <command>.

schneems avatar Nov 14 '22 20:11 schneems

@schneems I think this issue should remain open until such time as bash support is fully dropped (ie: when buildpack API 0.8 is dropped eventually in the future) - in order to:

  1. Aid discovery of the bug/workarounds
  2. Indicate that a fix would be accepted, if someone has the time to work on it.

edmorley avatar Nov 14 '22 20:11 edmorley