Merge pull request #259 from tiborvass/fix-inherits-override

bake: fix override bug with inheritance

bake/bake.go

@@ -23,13 +23,14 @@ func ReadTargets(ctx context.Context, files, targets, overrides []string) (map[s
 		}
 		c = mergeConfig(c, *cfg)
 	}
-	if err := c.setOverrides(overrides); err != nil {
+	o, err := c.newOverrides(overrides)
+	if err != nil {
 		return nil, err
 	}
 	m := map[string]Target{}
 	for _, n := range targets {
 		for _, n := range c.ResolveGroup(n) {
-			t, err := c.ResolveTarget(n)
+			t, err := c.ResolveTarget(n, o)
 			if err != nil {
 				return nil, err
 			}
@@ -105,23 +106,24 @@ func mergeConfig(c1, c2 Config) Config {
 	return c1
 }
 
-func (c Config) setOverrides(v []string) error {
+func (c Config) newOverrides(v []string) (map[string]Target, error) {
+	m := map[string]Target{}
 	for _, v := range v {
 
 		parts := strings.SplitN(v, "=", 2)
 		keys := strings.SplitN(parts[0], ".", 3)
 		if len(keys) < 2 {
-			return errors.Errorf("invalid override key %s, expected target.name", parts[0])
+			return nil, errors.Errorf("invalid override key %s, expected target.name", parts[0])
 		}
 
 		name := keys[0]
 		if len(parts) != 2 && keys[1] != "args" {
-			return errors.Errorf("invalid override %s, expected target.name=value", v)
+			return nil, errors.Errorf("invalid override %s, expected target.name=value", v)
 		}
 
-		t, ok := c.Target[name]
-		if !ok {
-			return errors.Errorf("unknown target %s", name)
+		t := m[name]
+		if _, ok := c.Target[name]; !ok {
+			return nil, errors.Errorf("unknown target %s", name)
 		}
 
 		switch keys[1] {
@@ -131,7 +133,7 @@ func (c Config) setOverrides(v []string) error {
 			t.Dockerfile = &parts[1]
 		case "args":
 			if len(keys) != 3 {
-				return errors.Errorf("invalid key %s, args requires name", parts[0])
+				return nil, errors.Errorf("invalid key %s, args requires name", parts[0])
 			}
 			if t.Args == nil {
 				t.Args = map[string]string{}
@@ -146,7 +148,7 @@ func (c Config) setOverrides(v []string) error {
 			}
 		case "labels":
 			if len(keys) != 3 {
-				return errors.Errorf("invalid key %s, lanels requires name", parts[0])
+				return nil, errors.Errorf("invalid key %s, lanels requires name", parts[0])
 			}
 			if t.Labels == nil {
 				t.Labels = map[string]string{}
@@ -170,11 +172,11 @@ func (c Config) setOverrides(v []string) error {
 		case "output":
 			t.Outputs = append(t.Outputs, parts[1])
 		default:
-			return errors.Errorf("unknown key: %s", keys[1])
+			return nil, errors.Errorf("unknown key: %s", keys[1])
 		}
-		c.Target[name] = t
+		m[name] = t
 	}
-	return nil
+	return m, nil
 }
 
 func (c Config) ResolveGroup(name string) []string {
@@ -197,8 +199,8 @@ func (c Config) group(name string, visited map[string]struct{}) []string {
 	return targets
 }
 
-func (c Config) ResolveTarget(name string) (*Target, error) {
-	t, err := c.target(name, map[string]struct{}{})
+func (c Config) ResolveTarget(name string, overrides map[string]Target) (*Target, error) {
+	t, err := c.target(name, map[string]struct{}{}, overrides)
 	if err != nil {
 		return nil, err
 	}
@@ -213,7 +215,7 @@ func (c Config) ResolveTarget(name string) (*Target, error) {
 	return t, nil
 }
 
-func (c Config) target(name string, visited map[string]struct{}) (*Target, error) {
+func (c Config) target(name string, visited map[string]struct{}, overrides map[string]Target) (*Target, error) {
 	if _, ok := visited[name]; ok {
 		return nil, nil
 	}
@@ -224,7 +226,7 @@ func (c Config) target(name string, visited map[string]struct{}) (*Target, error
 	}
 	var tt Target
 	for _, name := range t.Inherits {
-		t, err := c.target(name, visited)
+		t, err := c.target(name, visited, overrides)
 		if err != nil {
 			return nil, err
 		}
@@ -233,7 +235,7 @@ func (c Config) target(name string, visited map[string]struct{}) (*Target, error
 		}
 	}
 	t.Inherits = nil
-	tt = merge(merge(defaultTarget(), t), tt)
+	tt = merge(merge(merge(defaultTarget(), tt), t), overrides[name])
 	tt.normalize()
 	return &tt, nil
 }

bake/bake_test.go

@@ -19,10 +19,17 @@ func TestReadTargets(t *testing.T) {
 	fp := filepath.Join(tmpdir, "config.hcl")
 	err = ioutil.WriteFile(fp, []byte(`
 target "dep" {
+	args {
+		VAR_INHERITED = "dep"
+		VAR_BOTH = "dep"
+	}
 }
 
 target "webapp" {
 	dockerfile = "Dockerfile.webapp"
+	args {
+		VAR_BOTH = "webapp"
+	}
 	inherits = ["dep"]
 }`), 0600)
 	require.NoError(t, err)
@@ -32,12 +39,21 @@ target "webapp" {
 	t.Run("NoOverrides", func(t *testing.T) {
 		m, err := ReadTargets(ctx, []string{fp}, []string{"webapp"}, nil)
 		require.NoError(t, err)
+		require.Equal(t, 1, len(m))
 
 		require.Equal(t, "Dockerfile.webapp", *m["webapp"].Dockerfile)
 		require.Equal(t, ".", *m["webapp"].Context)
+		require.Equal(t, "dep", m["webapp"].Args["VAR_INHERITED"])
+	})
+
+	t.Run("InvalidTargetOverrides", func(t *testing.T) {
+		_, err := ReadTargets(ctx, []string{fp}, []string{"webapp"}, []string{"nosuchtarget.context=foo"})
+		require.NotNil(t, err)
+		require.Equal(t, err.Error(), "unknown target nosuchtarget")
 	})
 
 	t.Run("ArgsOverrides", func(t *testing.T) {
+		t.Run("leaf", func(t *testing.T) {
 			os.Setenv("VAR_FROMENV"+t.Name(), "fromEnv")
 			defer os.Unsetenv("VAR_FROM_ENV" + t.Name())
 
@@ -46,6 +62,8 @@ target "webapp" {
 				"webapp.args.VAR_EMPTY=",
 				"webapp.args.VAR_SET=bananas",
 				"webapp.args.VAR_FROMENV" + t.Name(),
+				"webapp.args.VAR_INHERITED=override",
+				// not overriding VAR_BOTH on purpose
 			})
 			require.NoError(t, err)
 
@@ -61,6 +79,21 @@ target "webapp" {
 			require.Equal(t, m["webapp"].Args["VAR_SET"], "bananas")
 
 			require.Equal(t, m["webapp"].Args["VAR_FROMENV"+t.Name()], "fromEnv")
+
+			require.Equal(t, m["webapp"].Args["VAR_BOTH"], "webapp")
+			require.Equal(t, m["webapp"].Args["VAR_INHERITED"], "override")
+		})
+
+		// building leaf but overriding parent fields
+		t.Run("parent", func(t *testing.T) {
+			m, err := ReadTargets(ctx, []string{fp}, []string{"webapp"}, []string{
+				"dep.args.VAR_INHERITED=override",
+				"dep.args.VAR_BOTH=override",
+			})
+			require.NoError(t, err)
+			require.Equal(t, m["webapp"].Args["VAR_INHERITED"], "override")
+			require.Equal(t, m["webapp"].Args["VAR_BOTH"], "webapp")
+		})
 	})
 
 	t.Run("ContextOverride", func(t *testing.T) {