builder: do not set network.host entitlement flag if already set in b…

…uildkitd conf Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
crazy-max · Sep 11, 2024 · 617d59d · 617d59d
1 parent 40f444f
commit 617d59d
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 22 deletions.
diff --git a/builder/builder.go b/builder/builder.go
@@ -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
 	}
@@ -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
 	}
@@ -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 {
@@ -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")

diff --git a/builder/builder_test.go b/builder/builder_test.go
@@ -1,6 +1,8 @@
 package builder
 
 import (
+	"os"
+	"path"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -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",
 			},
@@ -50,6 +67,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
 			"",
 			"kubernetes",
 			nil,
+			"",
 			[]string{
 				"--allow-insecure-entitlement=network.host",
 			},
@@ -60,6 +78,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
 			"",
 			"remote",
 			nil,
+			"",
 			nil,
 			false,
 		},
@@ -68,6 +87,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
 			"--allow-insecure-entitlement=security.insecure",
 			"docker-container",
 			nil,
+			"",
 			[]string{
 				"--allow-insecure-entitlement=security.insecure",
 			},
@@ -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",
@@ -89,6 +110,7 @@ func TestParseBuildkitdFlags(t *testing.T) {
 			"",
 			"docker-container",
 			map[string]string{"network": "host"},
+			"",
 			[]string{
 				"--allow-insecure-entitlement=network.host",
 			},
@@ -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",
 			},
@@ -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

diff --git a/util/confutil/config.go b/util/confutil/config.go
@@ -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) {

diff --git a/util/confutil/container.go b/util/confutil/container.go
@@ -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
 	}