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

Chaining commands doesn't work anymore #124

Closed
zaratan opened this issue Apr 20, 2021 · 4 comments
Closed

Chaining commands doesn't work anymore #124

zaratan opened this issue Apr 20, 2021 · 4 comments
Labels

Comments

@zaratan
Copy link
Contributor

zaratan commented Apr 20, 2021

Describe the bug

The fix for #113 broke our own dip.ymls with no beautiful way to fix it.

We have many command chains in them like command: nvm use 12 && nvm run test. Since #113, the second part of the command will be executed on the host and not inside the container. Also note that it also breaks any backtick ` as the command will be ran on the host.

To Reproduce

entrypoint.sh:

#!/bin/bash
set -e

echo "$@"
eval "$@"

Dockerfile:

FROM ruby:2.7
COPY entrypoint.sh /
ENTRYPOINT ["/entrypoint.sh"]

docker-compose.yml

version: '3.7'
services:
  app:
    build: .

dip.yml

version: "6"

compose:
  files:
    - docker-compose.yml

interaction:
  test_chain:
    service: app
    command: pwd && bash -c 'pwd'
  test_backquote:
    service: app
    command: echo `pwd`

All output are created with DIP_ENV=debug

With dip 6.1.0 (expected behaviour)

  • dip test_chain
[{}, "docker", ["inspect", "--format", "{{ .NetworkSettings.Networks.frontend.IPAddress }}", "dnsdock"]]
[{"DIP_DNS"=>"127.0.0.11"}, "docker-compose", ["--file", "/Users/pasind/tmp/test_dip/docker-compose.yml", "run", "--rm", "app", "pwd", "&&", "bash", "-c", "pwd"]]
Creating test_dip_app_run ... done
pwd && bash -c pwd
/
/
  • dip test_backquote
[{}, "docker", ["inspect", "--format", "{{ .NetworkSettings.Networks.frontend.IPAddress }}", "dnsdock"]]
[{"DIP_DNS"=>"127.0.0.11"}, "docker-compose", ["--file", "/Users/pasind/tmp/test_dip/docker-compose.yml", "run", "--rm", "app", "echo", "`pwd`"]]
Creating test_dip_app_run ... done
echo `pwd`
/

With dip 7.0.1 (new behaviour)

  • dip test_chain
[{}, "docker inspect --format '{{ .NetworkSettings.Networks.frontend.IPAddress }}' dnsdock"]
[{"DIP_DNS"=>"127.0.0.11"}, "docker-compose --file /Users/pasind/tmp/test_dip/docker-compose.yml run --rm app pwd && bash -c 'pwd'"]
Creating test_dip_app_run ... done
pwd
/
/Users/pasind/tmp/test_dip
  • dip test_backquote
[{}, "docker inspect --format '{{ .NetworkSettings.Networks.frontend.IPAddress }}' dnsdock"]
[{"DIP_DNS"=>"127.0.0.11"}, "docker-compose --file /Users/pasind/tmp/test_dip/docker-compose.yml run --rm app echo `pwd`"]
Creating test_dip_app_run ... done
echo /Users/pasind/tmp/test_dip
/Users/pasind/tmp/test_dip

Context

  • OS: MacOS
  • Version <7 and 7+

Workaround and proposed solution

There's a workaround that works for us but makes the dip.yml quite ugly. We could wrap any command in '''original command''' or "'original command'". This work around works both in 6.1 and 7.0.

The solutions I can see are:

  • Providing an option for a command: in the dip.yml, for an interaction specifying safe: true will automatically wrap the command in quotes.
  • Automatically detect if there's a , a ||or a&&` in the command and then automatically wrapping.
  • Always wrap
  • Instead of wrapping we could also use the old code/behaviour.

In any case, if we can find a solution that works for everyone I will gladly make the PR for this.

@bibendi
Copy link
Owner

bibendi commented Apr 20, 2021

Hi Denis,
Thank you for the detailed bug report!
It's very hard to straddle both worlds :(
I'll see this week what I can do.

@bibendi bibendi added the bug label Apr 20, 2021
@zaratan
Copy link
Contributor Author

zaratan commented Apr 20, 2021

No worries @bibendi I'll also try to see if I find a better solution. Take your time, we will stay on 6.1 for now.

@bibendi
Copy link
Owner

bibendi commented Apr 30, 2021

Fixed at 7.1.0

@bibendi bibendi closed this as completed Apr 30, 2021
@zaratan
Copy link
Contributor Author

zaratan commented Apr 30, 2021

Awesome thanks! I'll try this next week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants