Merge pull request #2266 from crazy-max/container-driver-host-entl

driver: set network.host entitlement by default for container drivers

builder/builder.go

@@ -26,6 +26,7 @@ import (
 	"github.com/google/shlex"
 	"github.com/moby/buildkit/util/progress/progressui"
 	"github.com/pkg/errors"
+	"github.com/spf13/pflag"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -429,12 +430,14 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre
 		}
 	}
 
-	var buildkitdFlags []string
+	driverOpts, err := csvToMap(opts.DriverOpts)
-	if opts.BuildkitdFlags != "" {
+	if err != nil {
-		buildkitdFlags, err = shlex.Split(opts.BuildkitdFlags)
+		return nil, err
-		if err != nil {
+	}
-			return nil, errors.Wrap(err, "failed to parse BuildKit daemon flags")
+
-		}
+	buildkitdFlags, err := parseBuildkitdFlags(opts.BuildkitdFlags, driverName, driverOpts)
+	if err != nil {
+		return nil, err
 	}
 
 	var ep string
@@ -493,11 +496,6 @@ func Create(ctx context.Context, txn *store.Txn, dockerCli command.Cli, opts Cre
 		setEp = false
 	}
 
-	driverOpts, err := csvToMap(opts.DriverOpts)
-	if err != nil {
-		return nil, err
-	}
-
 	buildkitdConfigFile := opts.BuildkitdConfigFile
 	if buildkitdConfigFile == "" {
 		// if buildkit daemon config is not provided, check if the default one
@@ -642,3 +640,38 @@ func validateBuildkitEndpoint(ep string) (string, error) {
 	}
 	return ep, nil
 }
+
+// parseBuildkitdFlags parses buildkit flags
+func parseBuildkitdFlags(inp string, driver string, driverOpts map[string]string) (res []string, err error) {
+	if inp != "" {
+		res, err = shlex.Split(inp)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to parse buildkit flags")
+		}
+	}
+
+	var allowInsecureEntitlements []string
+	flags := pflag.NewFlagSet("buildkitd", pflag.ContinueOnError)
+	flags.Usage = func() {}
+	flags.StringArrayVar(&allowInsecureEntitlements, "allow-insecure-entitlement", nil, "")
+	_ = flags.Parse(res)
+
+	var hasNetworkHostEntitlement bool
+	for _, e := range allowInsecureEntitlements {
+		if e == "network.host" {
+			hasNetworkHostEntitlement = 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") {
+		// 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")
+	}
+
+	return res, nil
+}

builder/builder_test.go

@@ -3,6 +3,7 @@ package builder
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -24,3 +25,115 @@ func TestCsvToMap(t *testing.T) {
 	require.Contains(t, r, "namespace")
 	require.Equal(t, r["namespace"], "default")
 }
+
+func TestParseBuildkitdFlags(t *testing.T) {
+	testCases := []struct {
+		name       string
+		flags      string
+		driver     string
+		driverOpts map[string]string
+		expected   []string
+		wantErr    bool
+	}{
+		{
+			"docker-container no flags",
+			"",
+			"docker-container",
+			nil,
+			[]string{
+				"--allow-insecure-entitlement=network.host",
+			},
+			false,
+		},
+		{
+			"kubernetes no flags",
+			"",
+			"kubernetes",
+			nil,
+			[]string{
+				"--allow-insecure-entitlement=network.host",
+			},
+			false,
+		},
+		{
+			"remote no flags",
+			"",
+			"remote",
+			nil,
+			nil,
+			false,
+		},
+		{
+			"docker-container with insecure flag",
+			"--allow-insecure-entitlement=security.insecure",
+			"docker-container",
+			nil,
+			[]string{
+				"--allow-insecure-entitlement=security.insecure",
+			},
+			false,
+		},
+		{
+			"docker-container with insecure and host flag",
+			"--allow-insecure-entitlement=network.host --allow-insecure-entitlement=security.insecure",
+			"docker-container",
+			nil,
+			[]string{
+				"--allow-insecure-entitlement=network.host",
+				"--allow-insecure-entitlement=security.insecure",
+			},
+			false,
+		},
+		{
+			"docker-container with network host opt",
+			"",
+			"docker-container",
+			map[string]string{"network": "host"},
+			[]string{
+				"--allow-insecure-entitlement=network.host",
+			},
+			false,
+		},
+		{
+			"docker-container with host flag and network host opt",
+			"--allow-insecure-entitlement=network.host",
+			"docker-container",
+			map[string]string{"network": "host"},
+			[]string{
+				"--allow-insecure-entitlement=network.host",
+			},
+			false,
+		},
+		{
+			"docker-container with insecure, host flag and network host opt",
+			"--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,
+		},
+		{
+			"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)
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, flags)
+		})
+	}
+}

driver/docker-container/factory.go

@@ -55,9 +55,6 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 		switch {
 		case k == "network":
 			d.netMode = v
-			if v == "host" {
-				d.InitConfig.BuildkitdFlags = append(d.InitConfig.BuildkitdFlags, "--allow-insecure-entitlement=network.host")
-			}
 		case k == "image":
 			d.image = v
 		case k == "memory":

tests/inspect.go

@@ -18,6 +18,7 @@ func inspectCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
 var inspectTests = []func(t *testing.T, sb integration.Sandbox){
 	testInspect,
 	testInspectBuildkitdFlags,
+	testInspectNetworkHostEntitlement,
 }
 
 func testInspect(t *testing.T, sb integration.Sandbox) {
@@ -78,3 +79,33 @@ func testInspectBuildkitdFlags(t *testing.T, sb integration.Sandbox) {
 	}
 	require.Fail(t, "--oci-worker-net=bridge not found in inspect output")
 }
+
+func testInspectNetworkHostEntitlement(t *testing.T, sb integration.Sandbox) {
+	if sb.Name() != "docker-container" {
+		t.Skip("only testing for docker-container driver")
+	}
+
+	var builderName string
+	t.Cleanup(func() {
+		if builderName == "" {
+			return
+		}
+		out, err := rmCmd(sb, withArgs(builderName))
+		require.NoError(t, err, out)
+	})
+
+	out, err := createCmd(sb, withArgs("--driver", "docker-container"))
+	require.NoError(t, err, out)
+	builderName = strings.TrimSpace(out)
+
+	out, err = inspectCmd(sb, withArgs(builderName))
+	require.NoError(t, err, out)
+
+	for _, line := range strings.Split(out, "\n") {
+		if v, ok := strings.CutPrefix(line, "BuildKit daemon flags:"); ok {
+			require.Contains(t, v, "--allow-insecure-entitlement=network.host")
+			return
+		}
+	}
+	require.Fail(t, "network.host insecure entitlement not found in inspect output")
+}