YAML is not parsed correctly

[blampe]

I originally discovered this as a bug where a docker-compose file containing image: would also require setting build: context:

For example, this docker-compose file should build successfully:

version: "3"
services:
  nginx:
    image: registry.hub.docker.com/library/nginx

using this workflow:

name: foo
jobs:
  foo:
    steps:
      - uses: whoan/docker-build-with-cache-action@v5.11.1
        with:
          registry: registry.hub.docker.com
          compose_file: docker-compose.yml

however it fails with the following:

[Compose file] Building image: registry.hub.docker.com/library/nginx
Failed to get context

Expanding the docker-compose to

version: "3"
services:
  nginx:
    image: registry.hub.docker.com/library/nginx
    build:
      context: .

resolves the issue, but this should not be needed -- we have all of the information we need to build the image.

After poking around more I realized this is due to how the tool attempts to parse YAML, which ultimately creates a number of additional bugs:

• Adding additional but valid spaces in front of the foo example above causes a failure with Failed to get service name, because the parser doesn't account for inconsistent indentation levels (valid in YAML).
• Adding a comment anywhere after a key causes the entire line to be consumed by the tool. So for example image: nginx # ignore me attempts to fetch an image called nginx # ignore me from the registry.

Instead of trying to re-implement YAML parsing, the tool should instead consume the compose file via an existing YAML parser. This would simplify service & registry discovery and make for a more user-friendly API.

[whoan]

HI @blampe . Sorry I haven't had time to work on this action before.

So we have a bunch of things here. Let me try to summarize and understand them:

1. The compose file needs a context so the actions can build the image: I think the action should use . by default. So your image is not building, or it's just that the log is misleading?
2. Adding additional but valid spaces in front of the foo example above causes a failure: I didn't understand this one. The foo example is in the workflow, so I think you are trying to use the workflow as the compose_file input, which is not expected. Could you please clarify?
3. The action should use a yaml parser: True, I just didn't want to add a new dependency but I am going to consider it

[blampe]

HI @blampe . Sorry I haven't had time to work on this action before.

So we have a bunch of things here. Let me try to summarize and understand them:

1. The compose file needs a context so the actions can build the image: I think the action should use . by default. So your image is not building, or it's just that the log is misleading?
2. Adding additional but valid spaces in front of the foo example above causes a failure: I didn't understand this one. The foo example is in the workflow, so I think you are trying to use the workflow as the compose_file input, which is not expected. Could you please clarify?
3. The action should use a yaml parser: True, I just didn't want to add a new dependency but I am going to consider it

It's been a while and I don't have the repo in front of me any more, so I'm going by my best recollection.

I think the action should use . by default. So your image is not building, or it's just that the log is misleading?

This log line shows up which (IIRC) kills the whole build because we're using set -e and that log line exits with false. So I think the intent is to default to . but that wasn't actually happening.

Your points 2 and 3 are essentially the same thing. Specifically, something like this (which is valid YAML) wouldn't be handled correctly by the tool:

version: "3"
services:
  nginx:
        image: registry.hub.docker.com/library/nginx
  other:
    image: registry.hub.docker.com/library/nginx # this comment also breaks things

[whoan]

Thanks for your prompt answer @blampe
I will fix the default context bug on this issue and will create a separate issue for the comments in the yaml as I think that using a yaml parser will be a bigger effort and will take more time.