-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
process extends
recursively
#547
Conversation
a824217
to
b93a956
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
loader/extends.go
Outdated
extendsOpts.ResourceLoaders = append([]ResourceLoader{}, opts.ResourceLoaders...) | ||
// replace localResourceLoader with a new flavour, using extended file base path | ||
extendsOpts.ResourceLoaders[len(opts.ResourceLoaders)-1] = localResourceLoader{ | ||
WorkingDir: localdir, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of assuming that localResourceLoader
is the last in the slice, it might be better to loop through and attempt to cast them until you find it and then replace it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't an assumption, local resource loader has to be the last in the slice as all "remote" resource loaders are checked first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduced RemoteResourceLoaders
to check this implicit rule
99d2fbd
to
8ef0fb3
Compare
Signed-off-by: Nicolas De Loof <[email protected]>
Signed-off-by: Nicolas De Loof <[email protected]>
fixes docker/compose#11394
issue takes place when there's a chain of extending services:
The root cause for this issue is that we try to extends a service before the compose model has been fully resolved. Typically loading this illustration example, if first service (which go map iteration makes random) to be processed is
service
, it will merge with siblinglevel2
. This results in abuild
mapping defined without acontext
, and will default to.
. But iflevel2
is loaded first, it will havebuild.context
set to..
, and thenservice
will do as well.This PR changes the logic so that we force
extends
to be applied recursively on services using new funcapplyServiceExtends
. This one will process a service to applyextends
if needed (otherwise is no-op). Once it has identified the base service used by extends, it executes recursively to enforce this one to be fully defined