jason-johnson / azure-pipelines-tasks-terraform

Azure Pipelines extension for Terraform
MIT License
134 stars 58 forks source link

Terraform CLI in Azure Devops parsing issue when passing variables with double quotes #347

Open akordowski opened 1 year ago

akordowski commented 1 year ago

Describe the bug When passing variables with double quotes to the command using the -var option, in the first parameter the " is removed instead of the \ escape character.

This behaviour occured on the import command, but propably occurs on every command with commandOptions.

To Reproduce

- task: TerraformCLI@0
  inputs:
    command: 'import'
    commandOptions: '-input=false
      -var=\"resource_group_location=location\"
      -var=\"resource_group_name=name\"'

Output: /opt/hostedtoolcache/terraform/1.4.6/x64/terraform import -var=\resource_group_location=location" -var="resource_group_name=name"

Expected behavior Correct double quotes parsing/escaping.

Noel-Jones commented 1 year ago

Anyone got a workaround for this? I've spent best part of a working day trying different forms of quotes. The clue that the " is removed instead of the \ looked promising. Just add an extra " and you can get it all looking right but it still doesn't work correctly.

Here I added an extra variable x with a leading ". All looks good and I can copy the final command to a bash shell and it works (caveat, the extra variable x is not known so it has to be removed again). In the pipeline I'm told the connectors variable is not defined.

##[debug]commandOptions=-var='"x=1' -var='connectors=[\"SF_Customers\",\"Customer_Updates\",\"Employees\",\"Activities\"]' -var='env=dev' -var='service=bob' --out SF_dev_infServ_63934.plan
##[debug]commandOptions=-var='"x=1' -var='connectors=[\"SF_Customers\",\"Customer_Updates\",\"Employees\",\"Activities\"]' -var='env=dev' -var='service=bob' --out SF_dev_infServ_63934.plan
##[debug]command=plan
##[debug]workingDirectory=/agent/_work/1/s/confluent_cloud_connectors
##[debug]which 'terraform'
##[debug]found: '/opt/hostedtoolcache/terraform/1.5.7/x64/terraform'
##[debug]which '/opt/hostedtoolcache/terraform/1.5.7/x64/terraform'
##[debug]found: '/opt/hostedtoolcache/terraform/1.5.7/x64/terraform'
##[debug]/opt/hostedtoolcache/terraform/1.5.7/x64/terraform arg: plan
##[debug]/opt/hostedtoolcache/terraform/1.5.7/x64/terraform arg: -var='x=1' -var='connectors=["SF_Customers","Customer_Updates","Employees","Activities"]' -var='env=dev' -var='service=bob' --out SF_dev_infServ_63934.plan
##[debug]/opt/hostedtoolcache/terraform/1.5.7/x64/terraform arg: -detailed-exitcode
##[debug]exec tool: /opt/hostedtoolcache/terraform/1.5.7/x64/terraform
##[debug]arguments:
##[debug]   plan
##[debug]   -var='x=1' -var='connectors=["SF_Customers","Customer_Updates","Employees","Activities"]' -var='env=dev' -var='service=bob' --out SF_dev_infServ_63934.plan
##[debug]   -detailed-exitcode
/opt/hostedtoolcache/terraform/1.5.7/x64/terraform plan -var='x=1' -var='connectors=["SF_Customers","Customer_Updates","Employees","Activities"]' -var='env=dev' -var='service=bob' --out SF_dev_infServ_63934.plan -detailed-exitcode
var.connectors
  A list of connectors to create in the environment for the service must be specified.
Noel-Jones commented 1 year ago

I've had a look through the code and the issue lies unsurprisingly within tasks/terraform-cli/src/runners/builders/run-with-command-options.ts. It all appears to be to split the string with multiple arguments into an array of strings (would be much easier if the commandOptions was an array of strings in the first place!). So the sort of things it needs to handle is

  1. -var=xyz=Hello
  2. -var=xyz="Hello"
  3. -var=xyz="Hello World"
  4. -var=xyz=\"Hello World\"
  5. -var=xyz="[\"Hello World\"]"
  6. -var=xyz='["Hello World"]'
  7. -var=xyz=["Hello World","Bye World"]' #note no space between strings!!

UPDATE: I'm not sure that case 4 is actually valid, or indeed the OP's example, case 3 is sufficient. Only unescaped quotes (of either flavour) should prevent splitting the command on the next space. Case 5 and 6 are probably also not needed since the appropriate form for a list of strings is case 7. Case 7 implies that we should consider brackets too!! I'd also disagree with the comment/action in the code that only double quotes are escaped - if someone wants to or needs to escape any character then why should the code stop that?

Lines 50 through 58 handle the quotes. If the quote is not escaped then the "inQuotes" flag is toggled and the quote is thrown away. So that will remove the quotes from cases 2 and 3 for a start. Case 2 should still work and case 3 depends upon the command calling mechanism.

On to case 4, where the quote is escaped. At the first \ line 60 (not escaped) does not match, nor does line 54 (not in quotes) so it drops down to the append function, which does just that. The "escaped" flag does not get set. At this point we are back to the previous paragraph. It has appended the \ and will throw away the quote (but remember the "inQuotes" flag is set). At the next \ line 65 is matched and the escaped flag is set. The append function will correctly yield \".

Bluntly I think the loop needs a bit of a rework. It's hard to see a simple change that resolves the issue. Not to say it doesn't exist, it's just very hard to work through the logic.

That said, I did catch on to a workaround! I'm not sure if I should mention it because anyone that uses it will find their pipeline broken if the issue is fixed properly. As the OP clued us in, the first escaped quote should be "\" but this does not work for the second one. That needs to be \"", e.g.

##[debug]commandOptions=-var=connectors=["\"SF_Customers\"","\"Customer_Updates\"","\"Employees\"","\"Activities\""] -var=env=dev -var=service=salesforce --out SF_dev_infServ_63957.plan