Merge pull request #1277 from crazy-max/fix-compose-merge

bake(compose): fix unskipped services without build context

bake/bake.go

@@ -200,15 +200,15 @@ func ParseFiles(files []File, defaults map[string]string) (_ *Config, err error)
 	}()
 
 	var c Config
-	var fs []*hcl.File
+	var composeFiles []File
+	var hclFiles []*hcl.File
 	for _, f := range files {
-		cfg, isCompose, composeErr := ParseComposeFile(f.Data, f.Name)
+		isCompose, composeErr := validateComposeFile(f.Data, f.Name)
 		if isCompose {
 			if composeErr != nil {
 				return nil, composeErr
 			}
-			c = mergeConfig(c, *cfg)
-			c = dedupeConfig(c)
+			composeFiles = append(composeFiles, f)
 		}
 		if !isCompose {
 			hf, isHCL, err := ParseHCLFile(f.Data, f.Name)
@@ -216,7 +216,7 @@ func ParseFiles(files []File, defaults map[string]string) (_ *Config, err error)
 				if err != nil {
 					return nil, err
 				}
-				fs = append(fs, hf)
+				hclFiles = append(hclFiles, hf)
 			} else if composeErr != nil {
 				return nil, fmt.Errorf("failed to parse %s: parsing yaml: %v, parsing hcl: %w", f.Name, composeErr, err)
 			} else {
@@ -225,8 +225,17 @@ func ParseFiles(files []File, defaults map[string]string) (_ *Config, err error)
 		}
 	}
 
-	if len(fs) > 0 {
-		if err := hclparser.Parse(hcl.MergeFiles(fs), hclparser.Opt{
+	if len(composeFiles) > 0 {
+		cfg, cmperr := ParseComposeFiles(composeFiles)
+		if cmperr != nil {
+			return nil, errors.Wrap(cmperr, "failed to parse compose file")
+		}
+		c = mergeConfig(c, *cfg)
+		c = dedupeConfig(c)
+	}
+
+	if len(hclFiles) > 0 {
+		if err := hclparser.Parse(hcl.MergeFiles(hclFiles), hclparser.Opt{
 			LookupVar:     os.LookupEnv,
 			Vars:          defaults,
 			ValidateLabel: validateTargetName,
@@ -234,18 +243,25 @@ func ParseFiles(files []File, defaults map[string]string) (_ *Config, err error)
 			return nil, err
 		}
 	}
+
 	return &c, nil
 }
 
 func dedupeConfig(c Config) Config {
 	c2 := c
+	c2.Groups = make([]*Group, 0, len(c2.Groups))
+	for _, g := range c.Groups {
+		g1 := *g
+		g1.Targets = dedupSlice(g1.Targets)
+		c2.Groups = append(c2.Groups, &g1)
+	}
 	c2.Targets = make([]*Target, 0, len(c2.Targets))
-	m := map[string]*Target{}
+	mt := map[string]*Target{}
 	for _, t := range c.Targets {
-		if t2, ok := m[t.Name]; ok {
+		if t2, ok := mt[t.Name]; ok {
 			t2.Merge(t)
 		} else {
-			m[t.Name] = t
+			mt[t.Name] = t
 			c2.Targets = append(c2.Targets, t)
 		}
 	}
@@ -256,26 +272,6 @@ func ParseFile(dt []byte, fn string) (*Config, error) {
 	return ParseFiles([]File{{Data: dt, Name: fn}}, nil)
 }
 
-func ParseComposeFile(dt []byte, fn string) (*Config, bool, error) {
-	envs := sliceToMap(os.Environ())
-	if wd, err := os.Getwd(); err == nil {
-		envs, err = loadDotEnv(envs, wd)
-		if err != nil {
-			return nil, true, err
-		}
-	}
-	fnl := strings.ToLower(fn)
-	if strings.HasSuffix(fnl, ".yml") || strings.HasSuffix(fnl, ".yaml") {
-		cfg, err := ParseCompose(dt, envs)
-		return cfg, true, err
-	}
-	if strings.HasSuffix(fnl, ".json") || strings.HasSuffix(fnl, ".hcl") {
-		return nil, false, nil
-	}
-	cfg, err := ParseCompose(dt, envs)
-	return cfg, err == nil, err
-}
-
 type Config struct {
 	Groups  []*Group  `json:"group" hcl:"group,block"`
 	Targets []*Target `json:"target" hcl:"target,block"`

bake/bake_test.go

@@ -530,7 +530,8 @@ func TestReadEmptyTargets(t *testing.T) {
 		Name: "docker-compose.yml",
 		Data: []byte(`
 services:
-  app2: {}
+  app2:
+    build: {}
 `),
 	}
 
@@ -1226,3 +1227,35 @@ target "f" {
 		})
 	}
 }
+
+func TestUnknownExt(t *testing.T) {
+	dt := []byte(`
+		target "app" {
+			context = "dir"
+			args = {
+				v1 = "foo"
+			}
+		}
+		`)
+	dt2 := []byte(`
+services:
+  app:
+    build:
+      dockerfile: Dockerfile-alternate
+      args:
+        v2: "bar"
+`)
+
+	c, err := ParseFiles([]File{
+		{Data: dt, Name: "c1.foo"},
+		{Data: dt2, Name: "c2.bar"},
+	}, nil)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, len(c.Targets))
+	require.Equal(t, "app", c.Targets[0].Name)
+	require.Equal(t, "foo", c.Targets[0].Args["v1"])
+	require.Equal(t, "bar", c.Targets[0].Args["v2"])
+	require.Equal(t, "dir", *c.Targets[0].Context)
+	require.Equal(t, "Dockerfile-alternate", *c.Targets[0].Dockerfile)
+}

bake/compose.go

@@ -1,7 +1,6 @@
 package bake
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -13,27 +12,31 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-// errComposeInvalid is returned when a compose file is invalid
-var errComposeInvalid = errors.New("invalid compose file")
+func ParseComposeFiles(fs []File) (*Config, error) {
+	envs, err := composeEnv()
+	if err != nil {
+		return nil, err
+	}
+	var cfgs []compose.ConfigFile
+	for _, f := range fs {
+		cfgs = append(cfgs, compose.ConfigFile{
+			Filename: f.Name,
+			Content:  f.Data,
+		})
+	}
+	return ParseCompose(cfgs, envs)
+}
 
-func ParseCompose(dt []byte, envs map[string]string) (*Config, error) {
+func ParseCompose(cfgs []compose.ConfigFile, envs map[string]string) (*Config, error) {
 	cfg, err := loader.Load(compose.ConfigDetails{
-		ConfigFiles: []compose.ConfigFile{
-			{
-				Content: dt,
-			},
-		},
+		ConfigFiles: cfgs,
 		Environment: envs,
 	}, func(options *loader.Options) {
 		options.SkipNormalization = true
-		options.SkipConsistencyCheck = true
 	})
 	if err != nil {
 		return nil, err
 	}
-	if err = composeValidate(cfg); err != nil {
-		return nil, err
-	}
 
 	var c Config
 	if len(cfg.Services) > 0 {
@@ -44,7 +47,7 @@ func ParseCompose(dt []byte, envs map[string]string) (*Config, error) {
 
 		for _, s := range cfg.Services {
 			if s.Build == nil {
-				s.Build = &compose.BuildConfig{}
+				continue
 			}
 
 			targetName := sanitizeTargetName(s.Name)
@@ -110,6 +113,50 @@ func ParseCompose(dt []byte, envs map[string]string) (*Config, error) {
 	return &c, nil
 }
 
+func validateComposeFile(dt []byte, fn string) (bool, error) {
+	envs, err := composeEnv()
+	if err != nil {
+		return true, err
+	}
+	fnl := strings.ToLower(fn)
+	if strings.HasSuffix(fnl, ".yml") || strings.HasSuffix(fnl, ".yaml") {
+		return true, validateCompose(dt, envs)
+	}
+	if strings.HasSuffix(fnl, ".json") || strings.HasSuffix(fnl, ".hcl") {
+		return false, nil
+	}
+	err = validateCompose(dt, envs)
+	return err == nil, err
+}
+
+func validateCompose(dt []byte, envs map[string]string) error {
+	_, err := loader.Load(compose.ConfigDetails{
+		ConfigFiles: []compose.ConfigFile{
+			{
+				Content: dt,
+			},
+		},
+		Environment: envs,
+	}, func(options *loader.Options) {
+		options.SkipNormalization = true
+		// consistency is checked later in ParseCompose to ensure multiple
+		// compose files can be merged together
+		options.SkipConsistencyCheck = true
+	})
+	return err
+}
+
+func composeEnv() (map[string]string, error) {
+	envs := sliceToMap(os.Environ())
+	if wd, err := os.Getwd(); err == nil {
+		envs, err = loadDotEnv(envs, wd)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return envs, nil
+}
+
 func loadDotEnv(curenv map[string]string, workingDir string) (map[string]string, error) {
 	if curenv == nil {
 		curenv = make(map[string]string)
@@ -248,28 +295,6 @@ func (t *Target) composeExtTarget(exts map[string]interface{}) error {
 	return nil
 }
 
-// composeValidate validates a compose file
-func composeValidate(project *compose.Project) error {
-	for _, s := range project.Services {
-		if s.Build != nil {
-			for _, secret := range s.Build.Secrets {
-				if _, ok := project.Secrets[secret.Source]; !ok {
-					return errors.Wrap(errComposeInvalid, fmt.Sprintf("service %q refers to undefined build secret %s", sanitizeTargetName(s.Name), secret.Source))
-				}
-			}
-		}
-	}
-	for name, secret := range project.Secrets {
-		if secret.External.External {
-			continue
-		}
-		if secret.File == "" && secret.Environment == "" {
-			return errors.Wrap(errComposeInvalid, fmt.Sprintf("secret %q must declare either `file` or `environment`", name))
-		}
-	}
-	return nil
-}
-
 // composeToBuildkitSecret converts secret from compose format to buildkit's
 // csv format.
 func composeToBuildkitSecret(inp compose.ServiceSecretConfig, psecret compose.SecretConfig) (string, error) {

bake/compose_test.go

@@ -6,6 +6,8 @@ import (
 	"sort"
 	"testing"
 
+	compose "github.com/compose-spec/compose-go/types"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -38,7 +40,7 @@ secrets:
     file: /root/.aws/credentials
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 
 	require.Equal(t, 1, len(c.Groups))
@@ -76,9 +78,10 @@ services:
     webapp:
         build: ./db
 `)
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(c.Groups))
+	require.Equal(t, 1, len(c.Targets))
 }
 
 func TestParseComposeTarget(t *testing.T) {
@@ -94,7 +97,7 @@ services:
       target: webapp
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 
 	require.Equal(t, 2, len(c.Targets))
@@ -119,7 +122,7 @@ services:
       target: webapp
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 	require.Equal(t, 2, len(c.Targets))
 	sort.Slice(c.Targets, func(i, j int) bool {
@@ -153,7 +156,7 @@ services:
 	os.Setenv("ZZZ_BAR", "zzz_foo")
 	defer os.Unsetenv("ZZZ_BAR")
 
-	c, err := ParseCompose(dt, sliceToMap(os.Environ()))
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, sliceToMap(os.Environ()))
 	require.NoError(t, err)
 	require.Equal(t, "bar", c.Targets[0].Args["FOO"])
 	require.Equal(t, "zzz_foo", c.Targets[0].Args["BAR"])
@@ -167,8 +170,8 @@ services:
     entrypoint: echo 1
 `)
 
-	_, err := ParseCompose(dt, nil)
-	require.NoError(t, err)
+	_, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
+	require.Error(t, err)
 }
 
 func TestAdvancedNetwork(t *testing.T) {
@@ -192,7 +195,7 @@ networks:
           gateway: 10.5.0.254
 `)
 
-	_, err := ParseCompose(dt, nil)
+	_, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 }
 
@@ -209,7 +212,7 @@ services:
         - bar
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 	require.Equal(t, []string{"foo", "bar"}, c.Targets[0].Tags)
 }
@@ -246,7 +249,7 @@ networks:
     name: test-net
 `)
 
-	_, err := ParseCompose(dt, nil)
+	_, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 }
 
@@ -299,7 +302,7 @@ services:
         no-cache: true
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 	require.Equal(t, 2, len(c.Targets))
 	sort.Slice(c.Targets, func(i, j int) bool {
@@ -343,7 +346,7 @@ services:
           - type=local,dest=path/to/cache
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(c.Targets))
 	require.Equal(t, []string{"ct-addon:foo", "ct-addon:baz"}, c.Targets[0].Tags)
@@ -376,7 +379,7 @@ services:
       - ` + envf.Name() + `
 `)
 
-	c, err := ParseCompose(dt, nil)
+	c, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 	require.Equal(t, map[string]string{"CT_ECR": "foo", "FOO": "bsdf -csdf", "NODE_ENV": "test"}, c.Targets[0].Args)
 }
@@ -397,7 +400,10 @@ services:
 `)
 
 	chdir(t, tmpdir)
-	c, _, err := ParseComposeFile(dt, "docker-compose.yml")
+	c, err := ParseComposeFiles([]File{{
+		Name: "docker-compose.yml",
+		Data: dt,
+	}})
 	require.NoError(t, err)
 	require.Equal(t, map[string]string{"FOO": "bar"}, c.Targets[0].Args)
 }
@@ -419,7 +425,7 @@ services:
         published: "3306"
         protocol: tcp
 `)
-	_, err := ParseCompose(dt, nil)
+	_, err := ParseCompose([]compose.ConfigFile{{Content: dt}}, nil)
 	require.NoError(t, err)
 }
 
@@ -465,12 +471,12 @@ func TestServiceName(t *testing.T) {
 	for _, tt := range cases {
 		tt := tt
 		t.Run(tt.svc, func(t *testing.T) {
-			_, err := ParseCompose([]byte(`
+			_, err := ParseCompose([]compose.ConfigFile{{Content: []byte(`
 services:
-  `+tt.svc+`:
+  ` + tt.svc + `:
     build:
       context: .
-`), nil)
+`)}}, nil)
 			if tt.wantErr {
 				require.Error(t, err)
 			} else {
@@ -536,7 +542,84 @@ services:
 	for _, tt := range cases {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := ParseCompose(tt.dt, nil)
+			_, err := ParseCompose([]compose.ConfigFile{{Content: tt.dt}}, nil)
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestValidateComposeFile(t *testing.T) {
+	cases := []struct {
+		name      string
+		fn        string
+		dt        []byte
+		isCompose bool
+		wantErr   bool
+	}{
+		{
+			name: "empty service",
+			fn:   "docker-compose.yml",
+			dt: []byte(`
+services:
+  foo:
+`),
+			isCompose: true,
+			wantErr:   true,
+		},
+		{
+			name: "build",
+			fn:   "docker-compose.yml",
+			dt: []byte(`
+services:
+  foo:
+    build: .
+`),
+			isCompose: true,
+			wantErr:   false,
+		},
+		{
+			name: "image",
+			fn:   "docker-compose.yml",
+			dt: []byte(`
+services:
+  simple:
+    image: nginx
+`),
+			isCompose: true,
+			wantErr:   false,
+		},
+		{
+			name: "unknown ext",
+			fn:   "docker-compose.foo",
+			dt: []byte(`
+services:
+  simple:
+    image: nginx
+`),
+			isCompose: true,
+			wantErr:   false,
+		},
+		{
+			name: "hcl",
+			fn:   "docker-bake.hcl",
+			dt: []byte(`
+target "default" {
+  dockerfile = "test"
+}
+`),
+			isCompose: false,
+			wantErr:   false,
+		},
+	}
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			isCompose, err := validateComposeFile(tt.dt, tt.fn)
+			assert.Equal(t, tt.isCompose, isCompose)
 			if tt.wantErr {
 				require.Error(t, err)
 			} else {