Version 2.2.0 ignores quotation marks in input parameter "driver-opts"

[sugibuchi]

Behaviour

Since version v.2.2.0, docker/setup-buildx-action does not pass quotation marks in input param driver-opts to command line arguments of docker buildx create.

Steps to reproduce this issue

1. Run the following POC workflow.

on: [ workflow_dispatch ]

jobs:
  test:
    runs-on: [ ***** ]
    steps:
      - run: docker context create test
    
      - uses: docker/setup-buildx-action@v2.1.0
        with:
          endpoint: test
          driver-opts: |
            "env.no_proxy=localhost,127.0.0.1,.mydomain"

      - uses: docker/setup-buildx-action@v2.2.0
        with:
          endpoint: test
          driver-opts: |
            "env.no_proxy=localhost,127.0.0.1,.mydomain"

2. Compare output of "Creating a new builder instance" step between v2.1.0 and v2.2.0

Expected behaviour

In both versions, value of --driver-opt should be enclosed by double quotes like --driver-opt "env.no_proxy=localhost,127.0.0.1,.mydomain"

Actual behaviour

Run docker/setup-buildx-action@v2.1.0
...
Creating a new builder instance
  /usr/local/bin/docker buildx create --name builder-eab86ad6-6a45-4c69-b626-e059ae515957 --driver docker-container --driver-opt "env.no_proxy=localhost,127.0.0.1,.mydomain" --buildkitd-flags --allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host --use test

Run docker/setup-buildx-action@v2.2.0
...
Creating a new builder instance
  /usr/local/bin/docker buildx create --name builder-50be5891-6e8d-43b6-8c23-bc75aa44d03a --driver docker-container --driver-opt env.no_proxy=localhost,127.0.0.1,.mydomain --buildkitd-flags --allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host --use test
ERROR: invalid value "127.0.0.1", expecting k=v

As you can see, the value of --driver-opt is not enclosed by " in the execution of v2.2.0.

Configuration

• GitHub Enterprise Server 3.6.2
• Self-hosted runner deployed by ARC
• Runner version: 2.298.2

[crazy-max]

Looks related to #165. Not sure why it behaves like this now as quotes are still relaxed when parsing inputs:

setup-buildx-action/src/context.ts

Line 131 in c74574e

relaxQuotes: true,

With 2.1.0, following args are used with exec command:

setup-buildx-action/src/main.ts

Line 71 in c74574e

await exec.exec(createCmd.commandLine, createCmd.args);

    [
      'create',
      '--name',
      'builder-9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d',
      '--driver',
      'docker-container',
      '--driver-opt',
      '"env.no_proxy=localhost,127.0.0.1,.mydomain"',
      '--buildkitd-flags',
      '--allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host'
    ]

With 2.2.0:

    [
      'create',
      '--name',
      'builder-9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d',
      '--driver',
      'docker-container',
      '--driver-opt',
      'env.no_proxy=localhost,127.0.0.1,.mydomain',
      '--buildkitd-flags',
      '--allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host'
    ]

So yes with 2.1.0 quotes are still their but they shouldn't! And as we can see in your case this doesn't work without quotes where it should 😟

Looking at the toolkit api, args passed to exec are not quoted correctly: https://github.com/actions/toolkit/blob/9b58167dc9923ca0399b2451f4b3b2c6d4f25f3b/packages/exec/src/toolrunner.ts#L71-L77, that's sad... I guess that's because they use child_process.spawn() 😣.

So there are actually two issues here. I will take a look, thanks for your feedback.

[sugibuchi]

@crazy-max
Thank you for your prompt response.

Regarding quotes in driver-opts, we need to preserve them because --driver-opt of docker buildx create accepts a CSV of key-value pairs.

https://github.com/docker/buildx/blob/611329fc7f1365556789bff4747f608d40cdc8a9/commands/create.go#L328-L338

docker buildx create --driver-opt env.https_proxy=http://localhost:3128 --driver-opt "env.no_proxy=localhost,127.0.0.1,.mydomain" ...

is equivalent to:

docker buildx create --driver-opt env.https_proxy=http://localhost:3128,"env.no_proxy=localhost,127.0.0.1,.mydomain" ...

In this example, we need to enclose the second key-value pair by quotes to prevent CSV reader from splitting the value of the second KV.

[crazy-max]

Don't think that's the issue, driver-opt is already a stringArray. For stringSlice, each opt would need a new flag or comma separated:

https://pkg.go.dev/github.com/spf13/pflag#StringArray

StringArray defines a string flag with specified name, default value, and usage string. The return value is the address of a []string variable that stores the value of the flag. The value of each argument will not try to be separated by comma. Use a StringSlice for that.

https://pkg.go.dev/github.com/spf13/pflag#StringSlice

StringSlice defines a string flag with specified name, default value, and usage string. The return value is the address of a []string variable that stores the value of the flag. Compared to StringArray flags, StringSlice flags take comma-separated value as arguments and split them accordingly. For example:

--ss="v1,v2" --ss="v3"

will result in

[]string{"v1", "v2", "v3"}

[crazy-max]

Oh the error message comes from here actually: https://github.com/docker/buildx/blob/ac85f590ba7d4511f16684cc853a67538bf97ed9/commands/create.go#L328-L339

I guess the csv reader should not interpret commas. This looks also linked to docker/buildx#617:

Here

--driver-opt replicas=1,nodeselector="kubernetes.io/arch=arm64,kubernetes.io/arch=arm"

Should be

--driver-opt replicas=1 --driver-opt nodeselector=kubernetes.io/arch=arm64,kubernetes.io/arch=arm

..as driver-opt is a stringArray and not a stringSlice.

But the csvToMap func in buildx will return:

• replicas=1
• nodeselector=kubernetes.io/arch=arm64
• kubernetes.io/arch=arm

It should return instead:

• replicas=1
• nodeselector=kubernetes.io/arch=arm64,kubernetes.io/arch=arm

Same in your case where:

--driver-opt env.no_proxy=localhost,127.0.0.1,.mydomain

will return

• env.no_proxy=localhost
• 127.0.0.1
• .mydomain

instead of

• env.no_proxy=localhost,127.0.0.1,.mydomain

cc @tonistiigi @jedevc