Skip to content

bake: support += operator to append with overrides #3031

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 4, 2025
Merged

bake: support += operator to append with overrides #3031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 58 additions & 18 deletions bake/bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type File struct {
type Override struct {
Value string
ArrValue []string
Append bool
}

func defaultFilenames() []string {
Expand Down Expand Up @@ -528,9 +529,12 @@ func (c Config) newOverrides(v []string) (map[string]map[string]Override, error)
m := map[string]map[string]Override{}
for _, v := range v {
parts := strings.SplitN(v, "=", 2)
keys := strings.SplitN(parts[0], ".", 3)

skey := strings.TrimSuffix(parts[0], "+")
appendTo := strings.HasSuffix(parts[0], "+")
keys := strings.SplitN(skey, ".", 3)
if len(keys) < 2 {
return nil, errors.Errorf("invalid override key %s, expected target.name", parts[0])
return nil, errors.Errorf("invalid override key %s, expected target.name", skey)
}

pattern := keys[0]
Expand All @@ -543,23 +547,23 @@ func (c Config) newOverrides(v []string) (map[string]map[string]Override, error)
return nil, err
}

kk := strings.SplitN(parts[0], ".", 2)

okey := strings.Join(keys[1:], ".")
for _, name := range names {
t, ok := m[name]
if !ok {
t = map[string]Override{}
m[name] = t
}

o := t[kk[1]]
override := t[okey]

// IMPORTANT: if you add more fields here, do not forget to update
// docs/reference/buildx_bake.md (--set) and https://docs.docker.com/build/bake/overrides/
switch keys[1] {
case "output", "cache-to", "cache-from", "tags", "platform", "secrets", "ssh", "attest", "entitlements", "network", "annotations":
if len(parts) == 2 {
o.ArrValue = append(o.ArrValue, parts[1])
override.Append = appendTo
override.ArrValue = append(override.ArrValue, parts[1])
}
case "args":
if len(keys) != 3 {
Expand All @@ -570,7 +574,7 @@ func (c Config) newOverrides(v []string) (map[string]map[string]Override, error)
if !ok {
continue
}
o.Value = v
override.Value = v
}
fallthrough
case "contexts":
Expand All @@ -580,11 +584,11 @@ func (c Config) newOverrides(v []string) (map[string]map[string]Override, error)
fallthrough
default:
if len(parts) == 2 {
o.Value = parts[1]
override.Value = parts[1]
}
}

t[kk[1]] = o
t[okey] = override
}
}
return m, nil
Expand Down Expand Up @@ -896,13 +900,21 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon
}
t.Labels[keys[1]] = &value
case "tags":
t.Tags = o.ArrValue
if o.Append {
t.Tags = append(t.Tags, o.ArrValue...)
} else {
t.Tags = o.ArrValue
}
case "cache-from":
cacheFrom, err := buildflags.ParseCacheEntry(o.ArrValue)
if err != nil {
return err
}
t.CacheFrom = cacheFrom
if o.Append {
t.CacheFrom = t.CacheFrom.Merge(cacheFrom)
} else {
t.CacheFrom = cacheFrom
}
for _, c := range t.CacheFrom {
if c.Type == "local" {
if v, ok := c.Attrs["src"]; ok {
Expand All @@ -915,7 +927,11 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon
if err != nil {
return err
}
t.CacheTo = cacheTo
if o.Append {
t.CacheTo = t.CacheTo.Merge(cacheTo)
} else {
t.CacheTo = cacheTo
}
for _, c := range t.CacheTo {
if c.Type == "local" {
if v, ok := c.Attrs["dest"]; ok {
Expand All @@ -932,7 +948,11 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon
if err != nil {
return errors.Wrap(err, "invalid value for outputs")
}
t.Secrets = secrets
if o.Append {
t.Secrets = t.Secrets.Merge(secrets)
} else {
t.Secrets = secrets
}
for _, s := range t.Secrets {
if s.FilePath != "" {
ent.FSRead = append(ent.FSRead, s.FilePath)
Expand All @@ -943,18 +963,30 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon
if err != nil {
return errors.Wrap(err, "invalid value for outputs")
}
t.SSH = ssh
if o.Append {
t.SSH = t.SSH.Merge(ssh)
} else {
t.SSH = ssh
}
for _, s := range t.SSH {
ent.FSRead = append(ent.FSRead, s.Paths...)
}
case "platform":
t.Platforms = o.ArrValue
if o.Append {
t.Platforms = append(t.Platforms, o.ArrValue...)
} else {
t.Platforms = o.ArrValue
}
case "output":
outputs, err := parseArrValue[buildflags.ExportEntry](o.ArrValue)
if err != nil {
return errors.Wrap(err, "invalid value for outputs")
}
t.Outputs = outputs
if o.Append {
t.Outputs = t.Outputs.Merge(outputs)
} else {
t.Outputs = outputs
}
for _, o := range t.Outputs {
if o.Destination != "" {
ent.FSWrite = append(ent.FSWrite, o.Destination)
Expand Down Expand Up @@ -984,11 +1016,19 @@ func (t *Target) AddOverrides(overrides map[string]Override, ent *EntitlementCon
}
t.NoCache = &noCache
case "no-cache-filter":
t.NoCacheFilter = o.ArrValue
if o.Append {
t.NoCacheFilter = append(t.NoCacheFilter, o.ArrValue...)
} else {
t.NoCacheFilter = o.ArrValue
}
case "shm-size":
t.ShmSize = &value
case "ulimits":
t.Ulimits = o.ArrValue
if o.Append {
t.Ulimits = append(t.Ulimits, o.ArrValue...)
} else {
t.Ulimits = o.ArrValue
}
case "network":
t.NetworkMode = &value
case "pull":
Expand Down
68 changes: 68 additions & 0 deletions bake/bake_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ target "webapp" {
annotations = [
"index,manifest:org.opencontainers.image.authors=dvdksn"
]
attest = [
"type=provenance,mode=max"
]
platforms = [
"linux/amd64"
]
secret = [
"id=FOO,env=FOO"
]
inherits = ["webDEP"]
}`),
}
Expand Down Expand Up @@ -127,6 +136,22 @@ target "webapp" {
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})

t.Run("AttestOverride", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.attest=type=sbom"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Len(t, m["webapp"].Attest, 2)
require.Equal(t, "provenance", m["webapp"].Attest[0].Type)
require.Equal(t, "sbom", m["webapp"].Attest[1].Type)
})

t.Run("AttestAppend", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.attest+=type=sbom"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Len(t, m["webapp"].Attest, 2)
require.Equal(t, "provenance", m["webapp"].Attest[0].Type)
require.Equal(t, "sbom", m["webapp"].Attest[1].Type)
})

t.Run("ContextOverride", func(t *testing.T) {
t.Parallel()
_, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.context"}, nil, &EntitlementConf{})
Expand All @@ -148,6 +173,49 @@ target "webapp" {
require.Equal(t, []string{"webapp"}, g["default"].Targets)
})

t.Run("PlatformOverride", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.platform=linux/arm64"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Equal(t, []string{"linux/arm64"}, m["webapp"].Platforms)
})

t.Run("PlatformAppend", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.platform+=linux/arm64"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Equal(t, []string{"linux/amd64", "linux/arm64"}, m["webapp"].Platforms)
})

t.Run("PlatformAppendMulti", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.platform+=linux/arm64", "webapp.platform+=linux/riscv64"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Equal(t, []string{"linux/amd64", "linux/arm64", "linux/riscv64"}, m["webapp"].Platforms)
})

t.Run("PlatformAppendMultiLastOverride", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.platform+=linux/arm64", "webapp.platform=linux/riscv64"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Equal(t, []string{"linux/arm64", "linux/riscv64"}, m["webapp"].Platforms)
})
Comment on lines +194 to +198
Copy link
Member Author

@crazy-max crazy-max Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is edgy and don't think current implementation is right. If last override does not append then it should only take this value and not append with previous one:

target "default" {
  platforms = ["linux/amd64"]
}
$ docker buildx bake --set *.platform+=linux/arm64 --set *.platform=linux/riscv64 --print
#1 [internal] load local bake definitions
#1 reading docker-bake.hcl 51B / 51B done
#1 DONE 0.0s
{
  "group": {
    "default": {
      "targets": [
        "default"
      ]
    }
  },
  "target": {
    "default": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "platforms": [
        "linux/arm64",
        "linux/riscv64"
      ]
    }
  }
}

Should be instead:

$ docker buildx bake --set *.platform+=linux/arm64 --set *.platform=linux/riscv64 --print
#1 [internal] load local bake definitions
#1 reading docker-bake.hcl 51B / 51B done
#1 DONE 0.0s
{
  "group": {
    "default": {
      "targets": [
        "default"
      ]
    }
  },
  "target": {
    "default": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "platforms": [
        "linux/riscv64"
      ]
    }
  }
}

Maybe that's fine for this PR to start with but I think we should refactor the override logic for such case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there ever a case where = and += for same override key make sense? It would not be append anyway. Even if you follow the order of arguments, then one way it is a replace, and the other way some overrides get ignored.


t.Run("SecretsOverride", func(t *testing.T) {
t.Setenv("FOO", "foo")
t.Setenv("BAR", "bar")
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.secrets=id=BAR,env=BAR"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Len(t, m["webapp"].Secrets, 1)
require.Equal(t, "BAR", m["webapp"].Secrets[0].ID)
})

t.Run("SecretsAppend", func(t *testing.T) {
t.Setenv("FOO", "foo")
t.Setenv("BAR", "bar")
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.secrets+=id=BAR,env=BAR"}, nil, &EntitlementConf{})
require.NoError(t, err)
require.Len(t, m["webapp"].Secrets, 2)
require.Equal(t, "FOO", m["webapp"].Secrets[0].ID)
require.Equal(t, "BAR", m["webapp"].Secrets[1].ID)
})
Comment on lines +209 to +217
Copy link
Member Author

@crazy-max crazy-max Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR but if we append a secret with the same ID it doesn't seem to override the existing one from the bake definition but append instead:

target "default" {
  secret = [
    "id=FOO,env=FOO"
  ]
}
$ FOO=foo BAR=bar docker buildx bake -f secret.hcl --set *.secrets+=id=FOO,env=BAR --print
#1 [internal] load local bake definitions
#1 reading secret.hcl 59B / 59B done
#1 DONE 0.0s
{
  "group": {
    "default": {
      "targets": [
        "default"
      ]
    }
  },
  "target": {
    "default": {
      "context": ".",
      "dockerfile": "Dockerfile",
      "secret": [
        {
          "id": "FOO",
          "env": "FOO"
        },
        {
          "id": "FOO",
          "env": "BAR"
        }
      ]
    }
  }
}

Building with this Dockerfile:

FROM busybox
RUN --mount=type=secret,id=FOO cat /run/secrets/FOO

We have:

#0 building with "default" instance using docker driver

#1 [internal] load local bake definitions
#1 reading secret.hcl 59B / 59B done
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 102B done
#2 DONE 0.1s

#3 [internal] load metadata for docker.io/library/busybox:latest
#3 DONE 0.0s

#4 [internal] load .dockerignore
#4 transferring context:
#4 transferring context: 2B done
#4 DONE 0.1s

#5 [stage-0 1/2] FROM docker.io/library/busybox:latest
#5 DONE 0.0s

#6 [stage-0 2/2] RUN --mount=type=secret,id=FOO cat /run/secrets/FOO
#6 0.335 bar
#6 DONE 0.4s

#7 exporting to image
#7 exporting layers 0.0s done
#7 writing image sha256:0d8fffa9b8b5bee5151adc9fd83c9477653621c751f143a5fe6e44600bbbed52 done
#7 DONE 0.1s

Which is ok as it takes the last one defined but we should dedup early.

@jsternberg Maybe having a custom dedup logic for secrets in

buildx/util/buildflags/secrets.go

Line 30 in 4ed1e07

return removeDupes(s)

Copy link
Member Author

@crazy-max crazy-max Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #3032


t.Run("ShmSizeOverride", func(t *testing.T) {
m, _, err := ReadTargets(ctx, []File{fp}, []string{"webapp"}, []string{"webapp.shm-size=256m"}, nil, &EntitlementConf{})
require.NoError(t, err)
Expand Down
26 changes: 23 additions & 3 deletions docs/reference/buildx_bake.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,19 +347,22 @@ is defined in https://golang.org/pkg/path/#Match.
```console
$ docker buildx bake --set target.args.mybuildarg=value
$ docker buildx bake --set target.platform=linux/arm64
$ docker buildx bake --set foo*.args.mybuildarg=value # overrides build arg for all targets starting with 'foo'
$ docker buildx bake --set *.platform=linux/arm64 # overrides platform for all targets
$ docker buildx bake --set foo*.no-cache # bypass caching only for targets starting with 'foo'
$ docker buildx bake --set foo*.args.mybuildarg=value # overrides build arg for all targets starting with 'foo'
$ docker buildx bake --set *.platform=linux/arm64 # overrides platform for all targets
$ docker buildx bake --set foo*.no-cache # bypass caching only for targets starting with 'foo'
$ docker buildx bake --set target.platform+=linux/arm64 # appends 'linux/arm64' to the platform list
```

You can override the following fields:

* `annotations`
* `attest`
* `args`
* `cache-from`
* `cache-to`
* `context`
* `dockerfile`
* `entitlements`
* `labels`
* `load`
* `no-cache`
Expand All @@ -372,3 +375,20 @@ You can override the following fields:
* `ssh`
* `tags`
* `target`

You can append using `+=` operator for the following fields:

* `annotations`¹
* `attest`¹
* `cache-from`
* `cache-to`
* `entitlements`¹
* `no-cache-filter`
* `output`
* `platform`
* `secrets`
* `ssh`
* `tags`

> [!NOTE]
> ¹ These fields already append by default.
Loading