Skip to content

Commit

Permalink
Merge pull request #2690 from crazy-max/v0.17.1_cherry-picks
Browse files Browse the repository at this point in the history
[v0.17] cherry-picks for v0.17.1
  • Loading branch information
tonistiigi committed Sep 13, 2024
2 parents 78c8c28 + dd0d53e commit dbccfa6
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 23 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ jobs:
permissions:
# required to write sarif report
security-events: write
# required to check out the repository
contents: read
steps:
-
name: Checkout
Expand Down
2 changes: 1 addition & 1 deletion bake/bake.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ type Target struct {
Outputs []string `json:"output,omitempty" hcl:"output,optional" cty:"output"`
Pull *bool `json:"pull,omitempty" hcl:"pull,optional" cty:"pull"`
NoCache *bool `json:"no-cache,omitempty" hcl:"no-cache,optional" cty:"no-cache"`
NetworkMode *string `json:"network" hcl:"network" cty:"network"`
NetworkMode *string `json:"network,omitempty" hcl:"network,optional" cty:"network"`
NoCacheFilter []string `json:"no-cache-filter,omitempty" hcl:"no-cache-filter,optional" cty:"no-cache-filter"`
ShmSize *string `json:"shm-size,omitempty" hcl:"shm-size,optional"`
Ulimits []string `json:"ulimits,omitempty" hcl:"ulimits,optional"`
Expand Down
41 changes: 29 additions & 12 deletions builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,16 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre
return nil, err
}

buildkitdFlags, err := parseBuildkitdFlags(opts.BuildkitdFlags, driverName, driverOpts)
buildkitdConfigFile := opts.BuildkitdConfigFile
if buildkitdConfigFile == "" {
// if buildkit daemon config is not provided, check if the default one
// is available and use it
if f, ok := confutil.DefaultConfigFile(dockerCli); ok {
buildkitdConfigFile = f
}
}

buildkitdFlags, err := parseBuildkitdFlags(opts.BuildkitdFlags, driverName, driverOpts, buildkitdConfigFile)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -496,15 +505,6 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre
setEp = false
}

buildkitdConfigFile := opts.BuildkitdConfigFile
if buildkitdConfigFile == "" {
// if buildkit daemon config is not provided, check if the default one
// is available and use it
if f, ok := confutil.DefaultConfigFile(dockerCli); ok {
buildkitdConfigFile = f
}
}

if err := ng.Update(opts.NodeName, ep, opts.Platforms, setEp, opts.Append, buildkitdFlags, buildkitdConfigFile, driverOpts); err != nil {
return nil, err
}
Expand Down Expand Up @@ -641,7 +641,7 @@ func validateBuildkitEndpoint(ep string) (string, error) {
}

// parseBuildkitdFlags parses buildkit flags
func parseBuildkitdFlags(inp string, driver string, driverOpts map[string]string) (res []string, err error) {
func parseBuildkitdFlags(inp string, driver string, driverOpts map[string]string, buildkitdConfigFile string) (res []string, err error) {
if inp != "" {
res, err = shlex.Split(inp)
if err != nil {
Expand All @@ -663,10 +663,27 @@ func parseBuildkitdFlags(inp string, driver string, driverOpts map[string]string
}
}

var hasNetworkHostEntitlementInConf bool
if buildkitdConfigFile != "" {
btoml, err := confutil.LoadConfigTree(buildkitdConfigFile)
if err != nil {
return nil, err
} else if btoml != nil {
if ies := btoml.GetArray("insecure-entitlements"); ies != nil {
for _, e := range ies.([]string) {
if e == "network.host" {
hasNetworkHostEntitlementInConf = true
break
}
}
}
}
}

if v, ok := driverOpts["network"]; ok && v == "host" && !hasNetworkHostEntitlement && driver == "docker-container" {
// always set network.host entitlement if user has set network=host
res = append(res, "--allow-insecure-entitlement=network.host")
} else if len(allowInsecureEntitlements) == 0 && (driver == "kubernetes" || driver == "docker-container") {
} else if len(allowInsecureEntitlements) == 0 && !hasNetworkHostEntitlementInConf && (driver == "kubernetes" || driver == "docker-container") {
// set network.host entitlement if user does not provide any as
// network is isolated for container drivers.
res = append(res, "--allow-insecure-entitlement=network.host")
Expand Down
48 changes: 41 additions & 7 deletions builder/builder_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package builder

import (
"os"
"path"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -27,19 +29,34 @@ func TestCsvToMap(t *testing.T) {
}

func TestParseBuildkitdFlags(t *testing.T) {
buildkitdConf := `
# debug enables additional debug logging
debug = true
# insecure-entitlements allows insecure entitlements, disabled by default.
insecure-entitlements = [ "network.host", "security.insecure" ]
[log]
# log formatter: json or text
format = "text"
`
dirConf := t.TempDir()
buildkitdConfPath := path.Join(dirConf, "buildkitd-conf.toml")
require.NoError(t, os.WriteFile(buildkitdConfPath, []byte(buildkitdConf), 0644))

testCases := []struct {
name string
flags string
driver string
driverOpts map[string]string
expected []string
wantErr bool
name string
flags string
driver string
driverOpts map[string]string
buildkitdConfigFile string
expected []string
wantErr bool
}{
{
"docker-container no flags",
"",
"docker-container",
nil,
"",
[]string{
"--allow-insecure-entitlement=network.host",
},
Expand All @@ -50,6 +67,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
"",
"kubernetes",
nil,
"",
[]string{
"--allow-insecure-entitlement=network.host",
},
Expand All @@ -60,6 +78,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
"",
"remote",
nil,
"",
nil,
false,
},
Expand All @@ -68,6 +87,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
"--allow-insecure-entitlement=security.insecure",
"docker-container",
nil,
"",
[]string{
"--allow-insecure-entitlement=security.insecure",
},
Expand All @@ -78,6 +98,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
"--allow-insecure-entitlement=network.host --allow-insecure-entitlement=security.insecure",
"docker-container",
nil,
"",
[]string{
"--allow-insecure-entitlement=network.host",
"--allow-insecure-entitlement=security.insecure",
Expand All @@ -89,6 +110,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
"",
"docker-container",
map[string]string{"network": "host"},
"",
[]string{
"--allow-insecure-entitlement=network.host",
},
Expand All @@ -99,6 +121,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
"--allow-insecure-entitlement=network.host",
"docker-container",
map[string]string{"network": "host"},
"",
[]string{
"--allow-insecure-entitlement=network.host",
},
Expand All @@ -109,25 +132,36 @@ func TestParseBuildkitdFlags(t *testing.T) {
"--allow-insecure-entitlement=network.host --allow-insecure-entitlement=security.insecure",
"docker-container",
map[string]string{"network": "host"},
"",
[]string{
"--allow-insecure-entitlement=network.host",
"--allow-insecure-entitlement=security.insecure",
},
false,
},
{
"docker-container with buildkitd conf setting network.host entitlement",
"",
"docker-container",
nil,
buildkitdConfPath,
nil,
false,
},
{
"error parsing flags",
"foo'",
"docker-container",
nil,
"",
nil,
true,
},
}
for _, tt := range testCases {
tt := tt
t.Run(tt.name, func(t *testing.T) {
flags, err := parseBuildkitdFlags(tt.flags, tt.driver, tt.driverOpts)
flags, err := parseBuildkitdFlags(tt.flags, tt.driver, tt.driverOpts, tt.buildkitdConfigFile)
if tt.wantErr {
require.Error(t, err)
return
Expand Down
20 changes: 20 additions & 0 deletions tests/bake.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,26 @@ target "build" {
require.Equal(t, ".", *def.Target["build"].Context)
require.Equal(t, "Dockerfile", *def.Target["build"].Dockerfile)
require.Equal(t, map[string]*string{"HELLO": ptrstr("foo")}, def.Target["build"].Args)

require.Equal(t, `{
"group": {
"default": {
"targets": [
"build"
]
}
},
"target": {
"build": {
"context": ".",
"dockerfile": "Dockerfile",
"args": {
"HELLO": "foo"
}
}
}
}
`, stdout.String())
}

func testBakeLocal(t *testing.T, sb integration.Sandbox) {
Expand Down
4 changes: 2 additions & 2 deletions util/confutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func DefaultConfigFile(dockerCli command.Cli) (string, bool) {
return "", false
}

// loadConfigTree loads BuildKit config toml tree
func loadConfigTree(fp string) (*toml.Tree, error) {
// LoadConfigTree loads BuildKit config toml tree
func LoadConfigTree(fp string) (*toml.Tree, error) {
f, err := os.Open(fp)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
Expand Down
2 changes: 1 addition & 1 deletion util/confutil/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func LoadConfigFiles(bkconfig string) (map[string][]byte, error) {
}

// Load config tree
btoml, err := loadConfigTree(bkconfig)
btoml, err := LoadConfigTree(bkconfig)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit dbccfa6

Please sign in to comment.