Validation false positive for nested interpolation in docker-compose

[AlexSkrypnyk]

When provisioning in Lagoon, the following variable assignment in the docker-compose.yml is reported as invalid, while Docker Compose considers this valid and can process it accordingly.

MYVAR1: ${MYVAR1:-${MYVAR2:-defaultvalue}substring}

When provisioning in Lagoon, the following error is received:

There are issues with your docker compose file that lagoon uses that should be fixed.

Please note that any modern shell supports nested variable interpolation, so supporting this in Lagoon seems a reasonable expectation.

[shreddedbacon]

Interesting, because this uses the docker-compose spec library to validate. There could be something missed though.

Can you provide how you're defining this (full docker-compose file would be nice if possible)

[AlexSkrypnyk]

x-environment: &default-environment
  MYVAR1: ${MYVAR1:-${MYVAR2:-defaultvalue}substring}

services:
  cli:
    image: uselagoon/php-8.1-cli-drupal:23.10.0
    environment:
      <<: *default-environment

[AlexSkrypnyk]

@shreddedbacon

this uses the docker-compose spec library to validate

I only was able to find this

build-deploy-tool/legacy/build-deploy-docker-compose.sh

Line 203 in 6bd767a

### use the `build-validate` built in validater to run over the provided docker-compose file

Not sure how to follow to the library though.

The official docker compose docs state that nested interpolation is supported
https://docs.docker.com/compose/compose-file/12-interpolation/

Looking at the tests here, https://github.com/uselagoon/build-deploy-tool/tree/6bd767a68605db8e16a553d0f61766969852d3ca/test-resources/docker-compose, there is not a single instance of nested interpolation of variables among those fixtures.

[shreddedbacon]

We use the github.com/compose-spec/compose-go library to parse the docker-compose files. The error you see is from it.

We don't have a test case for nested interpolation because until today you're the first person to do it.

When I run your provided nested variable into a test case, I get the following

invalid interpolation format for x-environment.MYVAR1: "${MYVAR2:-defaultvalue". You may need to escape any $ with another $

but if I do it like the docker docs state and use ${MYVAR1:-${MYVAR2:-defaultvalue}}, it parses fine.

[shreddedbacon]

For some reason, maybe there is a fix in a newer compose-go library version, it doesn't like the substring in your provided nested variable.

I'll check the compose-go library if there is a fix in a newer version, otherwise will have to log an issue with them.

[AlexSkrypnyk]

What is the version of the compose-go are you using?
There is a bug that was fixed in https://github.com/compose-spec/compose-go/releases/tag/v1.13.5

https://github.com/compose-spec/compose-go/pull/405/files

[shreddedbacon]

Yes, just saw that. I'll see if we can update our version. We are running a forked version at the moment due to some Lagoon-isms though that I'll need to backport.

[shreddedbacon]

Will see what I can do, but it may not be today.

[tobybellwood]

The provided code does not build with docker compose v2, only v1

It can't handle the "substring" appended inside the variable
MYVAR1: ${MYVAR1:-${MYVAR2:-defaultvalue}}substring

[AlexSkrypnyk]

The use case is quite wide: base a value on COMPOSE_PROJECT_NAME but allow to override it from internal variables.

Example from DrevOps:

DREVOPS_LOCALDEV_URL: &default-url ${DREVOPS_LOCALDEV_URL:-${COMPOSE_PROJECT_NAME:-example-site}.docker.amazee.io}

I want the default URL to be based on COMPOSE_PROJECT_NAME by default (which defaults to the current directory) - this allows to run the same project codebase from multiple directories and the URL will be auto-prefixed with the current project dir name. But it also will allow to override that URL locally by providing DREVOPS_LOCALDEV_URL if needed without changing any code in docker-compose.yml.

[AlexSkrypnyk]

@tobybellwood
please see the link above - we have pin-pointed the issue - it is due to the outdated composer-go library used by Lagoon.

[tobybellwood]

oh - ahahaha - I realised that i'd pinned my docker compose v2 to v2.17.3 for testing last week... And hence why it was broken on my local (and hence was hitting the exact same bug)

[AlexSkrypnyk]

a gentle bump on this one. consumer projects need to use workarounds that would need to be undone later.

I'm happy to sponsor this work.

[shreddedbacon]

Edit: TLDR; we are addressing it, but unfortunately we have to tackle it over some time to gently nudge people.

There is a linked issue where we are looking to incorporate the changes required to do this, but there are issues that we face with changing the library to a newer version. See the linked PR #253 for the change to upgrade the library.

PR #255 was created to take care the other issue, we can't implement the changes for #253 as they would immediately fail almost every single build in AIO cloud Lagoon, and probably anyone that has used our old examples.

We use #255 to warn people now in their builds to say that they have things that they need to address before we can incorporate the changes in #253.

[AlexSkrypnyk]

@shreddedbacon
do you have any ETA on this. It has been 3 months and I would like to know what to expect. thanks

[shreddedbacon]

@shreddedbacon do you have any ETA on this. It has been 3 months and I would like to know what to expect. thanks

No ETA. As mentioned in the last comment there are a lot of other considerations here before we can use the newer version of the compose spec. We can't just fail every build, we have gradually been rolling out warnings in the builds for things that the upgrade of the compose spec would cause failures for.

We are working on it, but as this is the only report of the bug, it isn't as high on the list as slowly informing people that we are about to break their builds is.

[AlexSkrypnyk]

This just took a new twist: there is now a support for defining several .env files in docker-compose.yml.

  env_file:
    - path: .env
      required: false
    - path: .env.local
      required: false

Doing so, does not pass the validator:

##############################################
STEP Docker Compose Validation: Completed at 2024-04-25 06:50:46 (UTC) Duration 00:00:01 Elapsed 00:00:02
##############################################

##############################################
Warning!
There are issues with your docker compose file that lagoon uses that should be fixed.
You can run docker compose config locally to check that your docker-compose file is valid.
##############################################

2 error(s) decoding: * 'env_file[0]' expected type 'string', got unconvertible type 'map[string]interface {}', value: 'map[path:.env required:false]' * 'env_file[1]' expected type 'string', got unconvertible type 'map[string]interface {}', value: 'map[path:.env.local required:false]'

##############################################

So this is a second case where we really need the support for a new docker-copose.yml.

Docker Compose will keep updating the format and this new format would need to be supported at some point anyway. Can this be prioritised?

[shreddedbacon]

Yes, we are aware of all of these. Unfortunately as said before, the tradeoff with moving to the latest version is that people will get build failures for things that we would no longer be able to check for. We need to make a call on our end as to when we will do that.

[shreddedbacon]

This PR is work on getting it done, you can see that support for env_file is there #304