Write temp script instead of running with sh -c

[lkysow]

Currently we run TF commands via sh -c "<cmds>" so that environment variables from the local environment can be used within additional args like -var mysecret=$SECRET.

The problem with this is that we need to manually escape what we pass in, which leads to problems like #302. Instead what we should do is write a temporary script and then execute that.

[majormoses]

can we have it cat the debug script when in a debug mode? If there are issues it's super helpful to output what it attempted to do.

[kipkoan]

Have you tried using `exec` instead of `sh -c`? I'm not sure it would work better but may be worth a shot to keep it simple.

…

On Oct 30, 2018, at 11:06 AM, Luke Kysow ***@***.***> wrote: Currently we run TF commands via sh -c "<cmds>" so that environment variables from the local environment can be used within additional args like -var mysecret=$SECRET. The problem with this is that we need to manually escape what we pass in, which leads to problems like #302. Instead what we should do is write a temporary script and then execute that. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[lkysow]

@majormoses yeah some sort of debugging ability will be necessary. Right now it logs the exact command that it's running so we'd want something similar.

@kipkoan it uses exec: https://github.com/runatlantis/atlantis/blob/master/server/events/terraform/terraform_client.go#L119 The problem is that if you use exec without sh -c, if someone has config like:

workflows:
  custom:
    plan:
      steps:
      - init
      - plan:
         extra_args: [-var-file, $WORKSPACE.tfvars]

Then if you were doing exec.Command("terraform", "plan", "-var-file", "$WORKSPACE.tfvars") the environment variable wouldn't be used, it would use that string. So instead we do exec.Command("sh", "-c", "terraform plan -var-file $WORKSPACE.tfvars").

What I think is a better practice is to write a temp script:

#!/bin/sh
set -e
terraform plan -var-file $WORKSPACE.tfvars

[kipkoan]

Sorry I meant the `exec` shell command. `exec.Command("exec", "terraform", "plan", ...)` I don't think it requires quoting the command like `sh -c` does, so we wouldn't need to escape anything. I can try that out and report back. But the temp shell script may be a cleaner solution anyway! :)

…

On Oct 31, 2018, at 7:47 AM, Luke Kysow ***@***.***> wrote: @majormoses yeah some sort of debugging ability will be necessary. Right now it logs the exact command that it's running so we'd want something similar. @kipkoan it uses exec: https://github.com/runatlantis/atlantis/blob/master/server/events/terraform/terraform_client.go#L119 The problem is that if you use exec without sh -c, if someone has config like: workflows: custom: plan: steps: - init - plan: extra_args: [-var-file, $WORKSPACE.tfvars] Then if you were doing exec.Command("terraform", "plan", "-var-file", "$WORKSPACE.tfvars") the environment variable wouldn't be used, it would use that string. So instead we do exec.Command("sh", "-c", "terraform plan -var-file $WORKSPACE.tfvars"). What I think is a better practice is to write a temp script: #!/bin/sh set -e terraform plan -var-file $WORKSPACE.tfvars — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[tomer-1]

won't it be a problem if you'd like to pass secrets to the command? it will have to write the secrets to the machine...

[lkysow]

This will also fix an issue where if terraform crashes, Atlantis isn't left hanging.

I think this is related to golang/go#18874