From f6a58a3320af5f0f2bfc6c0620464ef3a4de1bb2 Mon Sep 17 00:00:00 2001 From: Erik Seliger Date: Tue, 28 Mar 2023 23:18:56 +0200 Subject: [PATCH] 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 --- .../cmd/executor/internal/config/config.go | 20 +++++++++---- enterprise/cmd/executor/internal/run/util.go | 7 +---- .../cmd/executor/singlebinary/service.go | 30 ++++++++----------- internal/singleprogram/singleprogram.go | 3 ++ sg.config.yaml | 20 ++++++++----- 5 files changed, 42 insertions(+), 38 deletions(-) diff --git a/enterprise/cmd/executor/internal/config/config.go b/enterprise/cmd/executor/internal/config/config.go index 23869257b67..6cb5e46407a 100644 --- a/enterprise/cmd/executor/internal/config/config.go +++ b/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")) diff --git a/enterprise/cmd/executor/internal/run/util.go b/enterprise/cmd/executor/internal/run/util.go index d696f8b0744..317e69bff13 100644 --- a/enterprise/cmd/executor/internal/run/util.go +++ b/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, } } diff --git a/enterprise/cmd/executor/singlebinary/service.go b/enterprise/cmd/executor/singlebinary/service.go index 22e42f0f50d..00d1ef1650f 100644 --- a/enterprise/cmd/executor/singlebinary/service.go +++ b/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) } diff --git a/internal/singleprogram/singleprogram.go b/internal/singleprogram/singleprogram.go index 321da7f67b4..1398693351d 100644 --- a/internal/singleprogram/singleprogram.go +++ b/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: diff --git a/sg.config.yaml b/sg.config.yaml index efbf427cff4..f31cbec4264 100644 --- a/sg.config.yaml +++ b/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: