Repository: compose-spec/compose-go
Pull request #434 (2023-07-13 03:35:37)

loader: consistently resolve `build.context` path
When using `extends.file` to load a service from another Compose file, if the `build.context` field wasn't explicitly set, it would get ignored by the path resolution.

As a result, the build context would end up as the working directory of the _root_ Compose file instead of the working directory of the _loaded_ Compose file.

This was because the relative path resolution logic ignored empty `build.context` values. Unfortunately, removing that restriction is itself not sufficient, as it then attempted to verify that the local path existed in an attempt to avoid unintentionally joining the working directory with a remote context value (e.g. `/foo/https://github.com/my/repo.git`).

This is problematic because the working directory itself might be relative (rooted to an unknown location that is != `.`), so it will be resolved relative to the current process working directory, and thus fail the existence check.

In practice, this happens when using `extends.file`, where we do resolve paths but intentionally pass a potentially relative value for the working dir, thus making it unsafe to be doing real filesystem operations here.

In fact, even if a context was specified, it was possible to break this by running Compose from a different directory than the _root_ Compose file while specifying the path to it.

As there's no formal specification for determining local path vs. remote build contexts (see discussion in #385), I'm simply eliminating the existence check. This COULD mean that Compose begins to erroneously consider remote context values as local paths if builders add new unsupported syntaxes, but I think it's fair for us to be more restrictive.

Additionally, I've ensured that when path resolution is happening, it _always_ resolves the `build.context` to an absolute path for consistency. In particular, this should help make it easier to use the output of `docker compose config` from arbitrary working directories.

There's a new test that covers the `extends.file` + `build.context` behavior, and everal existing test adjustments to account for the fact that Compose was emitting relative `build.context` paths from `docker compose config` despite everything else being absolute (such as volumes).

Fixes:
 * docker/for-mac#6912


Patch:
diff --git a/loader/full-example.yml b/loader/full-example.yml
index 9f818f8..e2df509 100644
--- a/loader/full-example.yml
+++ b/loader/full-example.yml
@@ -24,7 +24,7 @@ services:
         - bar
       labels: [FOO=BAR]
       additional_contexts:
-        foo: /bar
+        foo: ./bar
       secrets:
         - secret1
         - source: secret2
diff --git a/loader/paths.go b/loader/paths.go
index 85b687d..45bbc54 100644
--- a/loader/paths.go
+++ b/loader/paths.go
@@ -68,13 +68,9 @@ func ResolveRelativePaths(project *types.Project) error {
 }
 
 func ResolveServiceRelativePaths(workingDir string, s *types.ServiceConfig) {
-	if s.Build != nil && s.Build.Context != "" && !isRemoteContext(s.Build.Context) {
-		// Build context might be a remote http/git context. Unfortunately supported "remote"
-		// syntax is highly ambiguous in moby/moby and not defined by compose-spec,
-		// so let's assume runtime will check
-		localContext := absPath(workingDir, s.Build.Context)
-		if _, err := os.Stat(localContext); err == nil {
-			s.Build.Context = localContext
+	if s.Build != nil {
+		if !isRemoteContext(s.Build.Context) {
+			s.Build.Context = absPath(workingDir, s.Build.Context)
 		}
 		for name, path := range s.Build.AdditionalContexts {
 			if strings.Contains(path, "://") { // `docker-image://` or any builder specific context type
@@ -83,10 +79,7 @@ func ResolveServiceRelativePaths(workingDir string, s *types.ServiceConfig) {
 			if isRemoteContext(path) {
 				continue
 			}
-			path = absPath(workingDir, path)
-			if _, err := os.Stat(path); err == nil {
-				s.Build.AdditionalContexts[name] = path
-			}
+			s.Build.AdditionalContexts[name] = absPath(workingDir, path)
 		}
 	}
 	for j, f := range s.EnvFile {
@@ -127,8 +120,13 @@ func absComposeFiles(composeFiles []string) ([]string, error) {
 	return composeFiles, nil
 }
 
+// isRemoteContext returns true if the value is a Git reference or HTTP(S) URL.
+//
+// Any other value is assumed to be a local filesystem path and returns false.
+//
+// See: https://github.com/moby/buildkit/blob/18fc875d9bfd6e065cd8211abc639434ba65aa56/frontend/dockerui/context.go#L76-L79
 func isRemoteContext(maybeURL string) bool {
-	for _, prefix := range []string{"https://", "http://", "git://", "github.com/", "git@"} {
+	for _, prefix := range []string{"https://", "http://", "git://", "ssh://", "github.com/", "git@"} {
 		if strings.HasPrefix(maybeURL, prefix) {
 			return true
 		}


Test patch:
diff --git a/loader/full-struct_test.go b/loader/full-struct_test.go
index 513e245..ef2b55f 100644
--- a/loader/full-struct_test.go
+++ b/loader/full-struct_test.go
@@ -53,14 +53,14 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 				"com.example.foo": "bar",
 			},
 			Build: &types.BuildConfig{
-				Context:            "./dir",
+				Context:            filepath.Join(workingDir, "dir"),
 				Dockerfile:         "Dockerfile",
 				Args:               map[string]*string{"foo": strPtr("bar")},
 				SSH:                []types.SSHKey{{ID: "default", Path: ""}},
 				Target:             "foo",
 				Network:            "foo",
 				CacheFrom:          []string{"foo", "bar"},
-				AdditionalContexts: types.Mapping{"foo": "/bar"},
+				AdditionalContexts: types.Mapping{"foo": filepath.Join(workingDir, "bar")},
 				Labels:             map[string]string{"FOO": "BAR"},
 				Secrets: []types.ServiceSecretConfig{
 					{
@@ -438,6 +438,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 		{
 			Name: "bar",
 			Build: &types.BuildConfig{
+				Context:          workingDir,
 				DockerfileInline: "FROM alpine\nRUN echo \"hello\" > /world.txt\n",
 			},
 			Environment: types.MappingWithEquals{},
@@ -598,6 +599,7 @@ func fullExampleYAML(workingDir, homeDir string) string {
 services:
   bar:
     build:
+      context: %s
       dockerfile_inline: |
         FROM alpine
         RUN echo "hello" > /world.txt
@@ -605,7 +607,7 @@ services:
     annotations:
       com.example.foo: bar
     build:
-      context: ./dir
+      context: %s
       dockerfile: Dockerfile
       args:
         foo: bar
@@ -617,7 +619,7 @@ services:
         - foo
         - bar
       additional_contexts:
-        foo: /bar
+        foo: %s
       network: foo
       target: foo
       secrets:
@@ -1037,6 +1039,9 @@ x-nested:
   bar: baz
   foo: bar
 `,
+		workingDir,
+		filepath.Join(workingDir, "dir"),
+		filepath.Join(workingDir, "bar"),
 		filepath.Join(workingDir, "example1.env"),
 		filepath.Join(workingDir, "example2.env"),
 		workingDir,
@@ -1148,6 +1153,7 @@ func fullExampleJSON(workingDir, homeDir string) string {
   "services": {
     "bar": {
       "build": {
+        "context": "%s",
         "dockerfile_inline": "FROM alpine\nRUN echo \"hello\" \u003e /world.txt\n"
       },
       "command": null,
@@ -1158,7 +1164,7 @@ func fullExampleJSON(workingDir, homeDir string) string {
         "com.example.foo": "bar"
       },
       "build": {
-        "context": "./dir",
+        "context": "%s",
         "dockerfile": "Dockerfile",
         "args": {
           "foo": "bar"
@@ -1174,7 +1180,7 @@ func fullExampleJSON(workingDir, homeDir string) string {
           "bar"
         ],
         "additional_contexts": {
-          "foo": "/bar"
+          "foo": "%s"
         },
         "network": "foo",
         "target": "foo",
@@ -1683,6 +1689,9 @@ func fullExampleJSON(workingDir, homeDir string) string {
 		toPath(workingDir, "config_data"),
 		toPath(homeDir, "config_data"),
 		toPath(workingDir, "secret_data"),
+		toPath(workingDir),
+		toPath(workingDir, "dir"),
+		toPath(workingDir, "bar"),
 		toPath(workingDir, "example1.env"),
 		toPath(workingDir, "example2.env"),
 		toPath(workingDir),
diff --git a/loader/loader_test.go b/loader/loader_test.go
index b8b889a..0a35d68 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -360,6 +360,67 @@ services:
 	assert.DeepEqual(t, service.Command, types.ShellCommand{"/bin/ash", "-c", "rm -rf /tmp/might-not-exist"})
 }
 
+func TestLoadExtendsMultipleFiles(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Test creates real files on disk")
+	}
+
+	tmpdir := t.TempDir()
+
+	aDir := filepath.Join(tmpdir, "a")
+	assert.NilError(t, os.Mkdir(aDir, 0o700))
+	aYAML := `
+services:
+  a:
+    build: .
+`
+	assert.NilError(t, os.WriteFile(filepath.Join(tmpdir, "a", "compose.yaml"), []byte(aYAML), 0o600))
+
+	bDir := filepath.Join(tmpdir, "b")
+	assert.NilError(t, os.Mkdir(bDir, 0o700))
+	bYAML := `
+services:
+  b:
+    build:
+      target: fake
+`
+	assert.NilError(t, os.WriteFile(filepath.Join(tmpdir, "b", "compose.yaml"), []byte(bYAML), 0o600))
+
+	rootYAML := `
+services:
+  a:
+    extends:
+      file: ./a/compose.yaml
+      service: a
+  b:
+    extends:
+      file: ./b/compose.yaml
+      service: b
+`
+	assert.NilError(t, os.WriteFile(filepath.Join(tmpdir, "compose.yaml"), []byte(rootYAML), 0o600))
+
+	actual, err := Load(types.ConfigDetails{
+		WorkingDir: tmpdir,
+		ConfigFiles: []types.ConfigFile{{
+			Filename: filepath.Join(tmpdir, "compose.yaml"),
+		}},
+		Environment: nil,
+	}, func(options *Options) {
+		options.SkipNormalization = true
+		options.SkipConsistencyCheck = true
+	})
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(actual.Services, 2))
+
+	svcA, err := actual.GetService("a")
+	assert.NilError(t, err)
+	assert.Equal(t, svcA.Build.Context, aDir)
+
+	svcB, err := actual.GetService("b")
+	assert.NilError(t, err)
+	assert.Equal(t, svcB.Build.Context, bDir)
+}
+
 func TestLoadCredentialSpec(t *testing.T) {
 	actual, err := loadYAML(`
 name: load-credential-spec
diff --git a/loader/paths_test.go b/loader/paths_test.go
index 42abd89..1a62347 100644
--- a/loader/paths_test.go
+++ b/loader/paths_test.go
@@ -84,6 +84,7 @@ func TestResolveBuildContextPaths(t *testing.T) {
 
 func TestResolveAdditionalContexts(t *testing.T) {
 	wd, _ := filepath.Abs(".")
+	absSubdir := filepath.Join(wd, "dir")
 	project := types.Project{
 		Name:       "myProject",
 		WorkingDir: wd,
@@ -96,7 +97,7 @@ func TestResolveAdditionalContexts(t *testing.T) {
 					AdditionalContexts: map[string]string{
 						"image":    "docker-image://foo",
 						"oci":      "oci-layout://foo",
-						"abs_path": "/tmp",
+						"abs_path": absSubdir,
 						"github":   "github.com/compose-spec/compose-go",
 						"rel_path": "./testdata",
 					},
@@ -117,7 +118,7 @@ func TestResolveAdditionalContexts(t *testing.T) {
 					AdditionalContexts: map[string]string{
 						"image":    "docker-image://foo",
 						"oci":      "oci-layout://foo",
-						"abs_path": "/tmp",
+						"abs_path": absSubdir,
 						"github":   "github.com/compose-spec/compose-go",
 						"rel_path": filepath.Join(wd, "testdata"),
 					},
diff --git a/loader/types_test.go b/loader/types_test.go
index 59628ef..8917ca1 100644
--- a/loader/types_test.go
+++ b/loader/types_test.go
@@ -25,7 +25,7 @@ import (
 	is "gotest.tools/v3/assert/cmp"
 )
 
-func TestMarshallProject(t *testing.T) {
+func TestMarshalProject(t *testing.T) {
 	workingDir, err := os.Getwd()
 	assert.NilError(t, err)
 	homeDir, err := os.UserHomeDir()
@@ -45,7 +45,7 @@ func TestMarshallProject(t *testing.T) {
 	assert.NilError(t, err)
 }
 
-func TestJSONMarshallProject(t *testing.T) {
+func TestJSONMarshalProject(t *testing.T) {
 	workingDir, err := os.Getwd()
 	assert.NilError(t, err)
 	homeDir, err := os.UserHomeDir()
diff --git a/loader/with-version-struct_test.go b/loader/with-version-struct_test.go
index 458899f..f2d0c55 100644
--- a/loader/with-version-struct_test.go
+++ b/loader/with-version-struct_test.go
@@ -17,6 +17,8 @@
 package loader
 
 import (
+	"path/filepath"
+
 	"github.com/compose-spec/compose-go/types"
 )
 
@@ -29,12 +31,13 @@ func withVersionExampleConfig() *types.Config {
 }
 
 func withVersionServices() []types.ServiceConfig {
+	buildCtx, _ := filepath.Abs("./Dockerfile")
 	return []types.ServiceConfig{
 		{
 			Name: "web",
 
 			Build: &types.BuildConfig{
-				Context: "./Dockerfile",
+				Context: buildCtx,
 			},
 			Environment: types.MappingWithEquals{},
 			Networks: map[string]*types.ServiceNetworkConfig{