Fix add host gateway behavior for app (#50020)

We don't only need to add this when the executor frontend URL points to
it, but also if the site config is set to host.docker.internal, hence I
made this an executor config flag instead. I think this should fix it,
but `sg start app` is acting up for me so would appreciate if someone
could verify that.

Closes https://github.com/sourcegraph/sourcegraph/issues/50011

enterprise/cmd/executor/internal/config/config.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/sourcegraph/sourcegraph/enterprise/internal/executor/types"
 	"github.com/sourcegraph/sourcegraph/internal/conf/confdefaults"
-	"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
 	"github.com/sourcegraph/sourcegraph/internal/env"
 	"github.com/sourcegraph/sourcegraph/internal/hostname"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
@@ -47,18 +46,23 @@ type Config struct {
 	DockerRegistryNodeExporterURL  string
 	WorkerHostname                 string
 	DockerRegistryMirrorURL        string
+	DockerAddHostGateway           bool
 	DockerAuthConfig               types.DockerAuthConfig
 	dockerAuthConfigStr            string
 	dockerAuthConfigUnmarshalError error
+
+	defaultFrontendPassword string
+}
+
+func NewAppConfig() *Config {
+	return &Config{
+		defaultFrontendPassword: confdefaults.AppInMemoryExecutorPassword,
+	}
 }
 
 func (c *Config) Load() {
 	c.FrontendURL = c.Get("EXECUTOR_FRONTEND_URL", "", "The external URL of the sourcegraph instance.")
-	c.FrontendAuthorizationToken = c.Get("EXECUTOR_FRONTEND_PASSWORD", "", "The authorization token supplied to the frontend.")
-	if deploy.IsApp() {
-		// In App deployments, we respect the in-memory executor password only.
-		c.FrontendAuthorizationToken = confdefaults.AppInMemoryExecutorPassword
-	}
+	c.FrontendAuthorizationToken = c.Get("EXECUTOR_FRONTEND_PASSWORD", c.defaultFrontendPassword, "The authorization token supplied to the frontend.")
 	c.QueueName = c.Get("EXECUTOR_QUEUE_NAME", "", "The name of the queue to listen to.")
 	c.QueuePollInterval = c.GetInterval("EXECUTOR_QUEUE_POLL_INTERVAL", "1s", "Interval between dequeue requests.")
 	c.MaximumNumJobs = c.GetInt("EXECUTOR_MAXIMUM_NUM_JOBS", "1", "Number of virtual machines or containers that can be running at once.")
@@ -82,6 +86,7 @@ func (c *Config) Load() {
 	c.DockerRegistryNodeExporterURL = c.GetOptional("DOCKER_REGISTRY_NODE_EXPORTER_URL", "The URL of the Docker Registry instance's node_exporter, without the /metrics path.")
 	c.MaxActiveTime = c.GetInterval("EXECUTOR_MAX_ACTIVE_TIME", "0", "The maximum time that can be spent by the worker dequeueing records to be handled.")
 	c.DockerRegistryMirrorURL = c.GetOptional("EXECUTOR_DOCKER_REGISTRY_MIRROR_URL", "The address of a docker registry mirror to use in firecracker VMs. Supports multiple values, separated with a comma.")
+	c.DockerAddHostGateway = c.GetBool("EXECUTOR_DOCKER_ADD_HOST_GATEWAY", "false", "If true, host.docker.internal will be exposed to the docker commands run by the runtime. Warn: Can be insecure. Only use this if you understand what you're doing. This is mostly used for running against a Sourcegraph on the same host.")
 	c.dockerAuthConfigStr = c.GetOptional("EXECUTOR_DOCKER_AUTH_CONFIG", "The content of the docker config file including auth for services. If using firecracker, only static credentials are supported, not credential stores nor credential helpers.")
 
 	if c.dockerAuthConfigStr != "" {
@@ -105,6 +110,9 @@ func (c *Config) Validate() error {
 	if u.Scheme == "" || u.Host == "" {
 		c.AddError(errors.New("EXECUTOR_FRONTEND_URL must be in the format scheme://host (and optionally :port)"))
 	}
+	if u.Hostname() == "host.docker.internal" && !c.DockerAddHostGateway {
+		c.AddError(errors.New("Making the executor talk to host.docker.internal but not allowing host gateway access using EXECUTOR_DOCKER_ADD_HOST_GATEWAY can cause connectivity problems"))
+	}
 
 	if c.dockerAuthConfigUnmarshalError != nil {
 		c.AddError(errors.Wrap(c.dockerAuthConfigUnmarshalError, "invalid EXECUTOR_DOCKER_AUTH_CONFIG, failed to parse"))

enterprise/cmd/executor/internal/run/util.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"net/url"
 	"os/exec"
 	"runtime"
 	"strings"
@@ -134,13 +133,9 @@ func workerOptions(c *config.Config) workerutil.WorkerOptions {
 }
 
 func dockerOptions(c *config.Config) command.DockerOptions {
-	u, _ := url.Parse(c.FrontendURL)
 	return command.DockerOptions{
 		DockerAuthConfig: c.DockerAuthConfig,
-		// If the configured Sourcegraph endpoint is host.docker.internal add a
-		// host entry and route to it to the containers. This is used for LSIF
-		// uploads and should not be required anymore once we support native uploads.
-		AddHostGateway: u.Hostname() == "host.docker.internal",
+		AddHostGateway:   c.DockerAddHostGateway,
 	}
 }
 

enterprise/cmd/executor/singlebinary/service.go

@@ -7,8 +7,6 @@ import (
 
 	"github.com/sourcegraph/sourcegraph/enterprise/cmd/executor/internal/config"
 	"github.com/sourcegraph/sourcegraph/enterprise/cmd/executor/internal/run"
-	"github.com/sourcegraph/sourcegraph/internal/conf/confdefaults"
-	"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
 	"github.com/sourcegraph/sourcegraph/internal/debugserver"
 	"github.com/sourcegraph/sourcegraph/internal/env"
 	"github.com/sourcegraph/sourcegraph/internal/observation"
@@ -20,30 +18,26 @@ type svc struct{}
 func (svc) Name() string { return "executor" }
 
 func (svc) Configure() (env.Config, []debugserver.Endpoint) {
-	var conf config.Config
+	conf := config.NewAppConfig()
 	conf.Load()
-	return &conf, nil
+	return conf, nil
 }
 
 func (svc) Start(ctx context.Context, observationCtx *observation.Context, ready service.ReadyFunc, cfg env.Config) error {
 	conf := cfg.(*config.Config)
-	// Always use the in-memory secret.
-	conf.FrontendAuthorizationToken = confdefaults.AppInMemoryExecutorPassword
 
 	// TODO(sqs) HACK(sqs): TODO(app): run executors for both queues
-	if deploy.IsApp() {
-		otherConfig := *conf
-		if conf.QueueName == "batches" {
-			otherConfig.QueueName = "codeintel"
-		} else {
-			otherConfig.QueueName = "batches"
-		}
-		go func() {
-			if err := run.StandaloneRunRun(ctx, observationCtx.Logger, &otherConfig, false); err != nil {
-				observationCtx.Logger.Fatal("executor for other queue failed", log.Error(err))
-			}
-		}()
+	otherConfig := *conf
+	if conf.QueueName == "batches" {
+		otherConfig.QueueName = "codeintel"
+	} else {
+		otherConfig.QueueName = "batches"
 	}
+	go func() {
+		if err := run.StandaloneRunRun(ctx, observationCtx.Logger, &otherConfig, false); err != nil {
+			observationCtx.Logger.Fatal("executor for other queue failed", log.Error(err))
+		}
+	}()
 
 	return run.StandaloneRunRun(ctx, observationCtx.Logger, conf, false)
 }

internal/singleprogram/singleprogram.go

@@ -123,6 +123,9 @@ func Init(logger log.Logger) {
 	// is safe to do.
 	setDefaultEnv(logger, "EXECUTOR_FRONTEND_URL", "http://localhost:3080")
 	setDefaultEnv(logger, "EXECUTOR_FRONTEND_PASSWORD", confdefaults.AppInMemoryExecutorPassword)
+	// Required because we set "executors.frontendURL": "http://host.docker.internal:3080" in site
+	// configuration.
+	setDefaultEnv(logger, "EXECUTOR_DOCKER_ADD_HOST_GATEWAY", "true")
 
 	// TODO(single-binary): HACK: This is a hack to workaround the fact that the 2nd time you run `sourcegraph`
 	// OOB migration validation fails:

sg.config.yaml

@@ -98,14 +98,6 @@ env:
   PRECISE_CODE_INTEL_UPLOAD_AWS_ENDPOINT: http://localhost:9000
   PRECISE_CODE_INTEL_UPLOAD_BACKEND: blobstore
 
-  # Required for frontend and executor to communicate
-  EXECUTOR_FRONTEND_URL: http://localhost:3080
-  # Must match the secret defined in the site config.
-  EXECUTOR_FRONTEND_PASSWORD: hunter2hunter2hunter2
-
-  # Disable firecracker inside executor in dev
-  EXECUTOR_USE_FIRECRACKER: false
-
   # Disable auto-indexing the CNCF repo group (this only works in Cloud)
   # This setting will be going away soon
   DISABLE_CNCF: notonmybox
@@ -562,6 +554,12 @@ commands:
       go build -gcflags="$GCFLAGS" -o .bin/executor github.com/sourcegraph/sourcegraph/enterprise/cmd/executor
     checkBinary: .bin/executor
     env:
+      # Required for frontend and executor to communicate
+      EXECUTOR_FRONTEND_URL: http://localhost:3080
+      # Must match the secret defined in the site config.
+      EXECUTOR_FRONTEND_PASSWORD: hunter2hunter2hunter2
+      # Disable firecracker inside executor in dev
+      EXECUTOR_USE_FIRECRACKER: false
       EXECUTOR_QUEUE_NAME: TEMPLATE
     watch:
       - lib
@@ -993,6 +991,12 @@ bazelCommands:
     env:
       EXECUTOR_QUEUE_NAME: TEMPLATE
       TMPDIR: $HOME/.sourcegraph/executor-temp
+      # Required for frontend and executor to communicate
+      EXECUTOR_FRONTEND_URL: http://localhost:3080
+      # Must match the secret defined in the site config.
+      EXECUTOR_FRONTEND_PASSWORD: hunter2hunter2hunter2
+      # Disable firecracker inside executor in dev
+      EXECUTOR_USE_FIRECRACKER: false
   codeintel-executor:
     <<: *executor_template_bazel
     env: