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

bake(compose): fix unskipped services without build context

bake/bake.go

@@ -200,23 +200,23 @@ 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)
 			if isHCL {
 				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,27 +225,43 @@ 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,
 		}, &c); err.HasErrors() {
 			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) {