compose-spec / compose-go

Reference library for parsing and loading Compose YAML files
https://compose-spec.io
Apache License 2.0
350 stars 109 forks source link

feat(loader): made interpolation after merge #644

Closed idsulik closed 2 months ago

idsulik commented 2 months ago

What this PR does / why we need it: Changed interpolation order, it used to interpolate for each config file separately, but now it interpolates after merge.
To be honest, I'm not sure it's a good idea, maybe it'll be better to add an option to control it, like --interpolate-after-merge

Which issue(s) this PR fixes: Fixes https://github.com/docker/compose/issues/11925

ndeloof commented 2 months ago

This was my initial intent during compose-go/v2 refactoring, but this breaks when include or extends uses variables for target file/service. Need at least partial interpolation for those - I never took time to look into this, would you like to give it a try ?

idsulik commented 2 months ago

but this breaks when include or extends uses variables for target file/service

@ndeloof could you please provide an example?

ndeloof commented 2 months ago

A typical use:

services:
  prod:
    image: a

  dev:
    image: a-dev

  app:
    extends: ${BASE:-prod}
idsulik commented 2 months ago

@ndeloof pushed changes to interpolate include and extends before merging and interpolate everything after the merge, but I think it becomes too complicated.

ndeloof commented 2 months ago

Checking this PR again I went with an alternate implementation, reusing your test case: https://github.com/compose-spec/compose-go/pull/651

wdyt ?

Anyway, the current compose-spec schema declare types for many attributes which can actually be set by a variable, and validation fails. Would need to first fix the schema

ndeloof commented 2 months ago

Closing as https://github.com/compose-spec/compose-go/pull/651 has been merged