build-args newline split

[ruslandoga]

Fixes #726

redhat-actions/buildah-build seem to be splitting by newlines

[crazy-max]

Thank you for contributing! That's SGTM but it appears your commit message is missing a DCO sign-off, causing the DCO check to fail: https://github.com/docker/build-push-action/pull/736/checks?check_run_id=9861389571

We require all commit messages to have a Signed-off-by line with your name and e-mail, which looks something like:

Signed-off-by: YourFirsName YourLastName <yourname@example.org>

Sorry for the hassle and let me know if you need help or more detailed instructions!

[ruslandoga]

DCO complained about Signed-off-by: Ruslan Doga <rusl@n-do.ga>:

Commit sha: ab9c0b3, Author: ruslandoga, Committer: ruslandoga; Expected "ruslandoga 67764432+ruslandoga@users.noreply.github.com", but got "Ruslan Doga rusl@n-do.ga".

Using my username and github noreply email made it pass. I hope it's OK.

[crazy-max]

You forgot to build javascript artifacts with docker buildx bake pre-checkin: https://github.com/docker/build-push-action/blob/master/.github/CONTRIBUTING.md#submitting-a-pull-request

https://github.com/docker/build-push-action/actions/runs/3908784489/jobs/6680628395#step:3:308

#29 0.323 ERROR: Build result differs. Please build first with "docker buildx bake build"
#29 0.326  M dist/index.js
#29 0.326  M dist/index.js.map

[ruslandoga]

Done: 3bfdd83

[crazy-max]

LGTM thanks

[crazy-max]

@ruslandoga Hum I just recall it will break users that have multi line build arg value: #374

[crazy-max]

@ruslandoga Does it work if you add quotes with current version?:

- uses: docker/build-push-action@v3
  with:
    build-args: |
      "BUILD_INFO=${{ steps.meta.outputs.json }}"

[ruslandoga]

@crazy-max I think I've tried it and it didn't work because the quotes have to be escaped in JSON. I'll try again.

[ruslandoga]

It doesn't work: https://github.com/ruslandoga/docker-build-info/actions/runs/3909945739/jobs/6681579929

--build-arg "BUILD_INFO={"tags":["ghcr.io/ruslandoga/docker-build-info:pr-2",ghcr.io/ruslandoga/docker-build-info:quotes,ghcr.io/ruslandoga/docker-build-info:5709796,

[crazy-max]

And using toJSON expression?:

- uses: docker/build-push-action@v3
  with:
    build-args: |
      BUILDINFO=${{ toJSON(steps.meta.outputs.json) }}

Alternatively I think we could just create env vars in metadata-action for each output named:

• DOCKER_METADATA_VERSION
• ...
• DOCKER_METADATA_JSON

So you could just set:

- uses: docker/build-push-action@v3
  with:
    build-args: |
      DOCKER_METADATA_JSON

WDYT?

[ruslandoga]

toJSON (ruslandoga/docker-build-info#3) results in invalid (extra quotes) BUILD_INFO as well:

/ # printenv BUILD_INFO
"{\"tags\":[\"ghcr.io/ruslandoga/docker-build-info:pr-3\",\"ghcr.io/ruslandoga/docker-build-info:toJson\",\"ghcr.io/ruslandoga/docker-build-info:ca87063\",\"ghcr.io/ruslandoga/docker-build-info:toJson-ca87063\"],\"labels\":{\"org.opencontainers.image.title\":\"docker-build-info\",\"org.opencontainers.image.description\":\"\",\"org.opencontainers.image.url\":\"https://github.com/ruslandoga/docker-build-info\",\"org.opencontainers.image.source\":\"https://github.com/ruslandoga/docker-build-info\",\"org.opencontainers.image.version\":\"pr-3\",\"org.opencontainers.image.created\":\"2023-01-13T09:45:48.327Z\",\"org.opencontainers.image.revision\":\"ca8706329ed9b7bfad723d9cc0cb96d08c9fba79\",\"org.opencontainers.image.licenses\":\"\"}}"

I think the proper fix is to replace the CSV parser (it's splitting by commas that breaks arrays in JSON) with something more specific to the use-case:

• newlines when quoted > 1 are escaped
• newlines when quoted = 0 mean next build_arg (can have extra check for <key>= prefix after \n)

quoted is a "nesting" counter that's incremented each time an unmatched " or "" or """ is encountered and decremented on a matched one.

Or maybe switching from build-args being a list to a dict could also be done. Then the values could use something like https://yaml-multiline.info if needed. In my use-case it probably wouldn't be required.

- uses: docker/build-push-action@v3
  with:
    build-args:
      BUILD_METADATA: ${{ steps.meta.outputs.json  }}

---

I think switching to DOCKER_METADATA_JSON would require a change in the Dockerfile which I'd like to avoid if possible.

[crazy-max]

@ruslandoga I made some changes to the metadata action to stringify the JSON output and looks to work on my side: https://github.com/docker/metadata-action/pull/257/files#diff-4fab5baaca5c14d2de62d8d2fceef376ddddcc8e9509d86cfa5643f51b89ce3dR73

- uses: docker/build-push-action@v3
  with:
    build-args:
      BUILD_METADATA=${{ steps.meta.outputs.json  }}

FROM alpine
RUN apk add --no-cache jq
ARG BUILD_METADATA
RUN echo $BUILD_METADATA | jq

Can you try with crazy-max/docker-metadata-action@env-output and let me know?

[ruslandoga]

@crazy-max echo strips the outer quotes from its input. I think this issue should be tested with printenv

[crazy-max]

@ruslandoga Don't think it matters with jq but here it is: https://github.com/docker/metadata-action/actions/runs/3910565457/jobs/6682911373#step:4:207

[ruslandoga]

jq is irrelevant. The JSON payload should be valid (not quoted as a string) for most non-cli parsers to work, otherwise they'd just return a string.

[ruslandoga]

Still, env-output branch doesn't work either: https://github.com/ruslandoga/docker-build-info/pull/4/checks, results in invalid JSON as before, because CSV parser splits JSON arrays incorrectly.

[crazy-max]

Added a new step to check this and looks fine to me: https://github.com/docker/metadata-action/actions/runs/3910598664/jobs/6682982486#step:5:188

• workflow: https://github.com/crazy-max/docker-metadata-action/blob/04861f5102fc7042b8529e3f426856244a8cfba2/.github/workflows/ci.yml#L230-L237
• dockerfile: https://github.com/crazy-max/docker-metadata-action/blob/04861f5102fc7042b8529e3f426856244a8cfba2/test/json.Dockerfile

[ruslandoga]

Try adding more tags (I don't have problems with two tags in json.tags either). Or just try using my repro #726 :)

[crazy-max]

Ah yes indeed 😞 If you're willing to open a new PR with your suggestion to the CSV parser, feel free to do so. I will look later today at this issue if you have nothing yet.