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.

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

[buildpacks] debug detect direct processes with /bin/sh -c ... #4345

Merged

Conversation

briandealwis
Copy link
Member

Description

GCP Buildpacks turn Procfile processes into direct processes that invoke /bin/bash with arguments -c procscript. Although this seems odd, it's really a special case of the script-type process launch.

@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

Merging #4345 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4345   +/-   ##
=======================================
  Coverage   71.74%   71.75%           
=======================================
  Files         324      324           
  Lines       12511    12515    +4     
=======================================
+ Hits         8976     8980    +4     
  Misses       2966     2966           
  Partials      569      569           
Impacted Files Coverage Δ
pkg/skaffold/debug/cnb.go 93.10% <100.00%> (+0.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4baad7c...f5a251d. Read the comment docs.

return append([]string{"--"}, transformed...)
// Detect and unwrap `/bin/sh -c ...`-style command lines; GCP Buildpacks turn Procfiles into `/bin/bash -c ...`
if (p.Command == "/bin/sh" || p.Command == "/bin/bash") && len(p.Args) >= 2 && p.Args[0] == "-c" {
p.Command = p.Args[1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the command is Command: "/bin/sh", Args: []string{"-c", "command arg1 arg2"} as in your test case, then
p.Command = strings.Split(p.Args[2], " ")[0] and
p.Args = strings.Split(p.Args[2], " ")[1:]

or is it the case at this point, p.Args is already a space separated array of all args?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So p.Args[1] is the script provided to sh -c. p.Args[2:] are then mapped to arguments for that shell script. So it's very rarely done:

$ sh -c 'echo "0 - $0"; i=0; for arg in $*; do i=$(($i + 1)); echo "$i - $arg"; done' hi there how are you
0 - hi
1 - there
2 - how
3 - are
4 - you

Note that strings.Split(..., " ") doesn't take into account quoted strings, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It's entirely possible that they provide /bin/sh -x -c '...' but I'm fine with giving up and saying "🙅‍♂️ no debug for you".)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol.

@tejal29 tejal29 self-assigned this Jun 18, 2020
GCP Buildpacks turn Procfile processes into direct processes
that invoke `/bin/bash` with arguments `-c procscript`.
Although this seems odd, it's really a special case of the
script-type process launch.
@briandealwis briandealwis merged commit b1183d4 into GoogleContainerTools:master Jun 18, 2020
@briandealwis briandealwis deleted the cnb-procfiles branch June 18, 2020 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants