diff --git a/Dockerfile b/Dockerfile index 67b725e9..e0b5bc31 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,8 +8,8 @@ # prebuilt binaries in any other form. FROM envoyproxy/envoy-distroless:v1.24.10 as envoy-binary -# Modify the envoy binary to be able to bind to privileged ports (< 1024) -FROM alpine:latest AS setcap +# Modify the envoy binary to be able to bind to privileged ports (< 1024). +FROM debian:bullseye-slim AS setcap-envoy-binary ARG BIN_NAME=consul-dataplane ARG TARGETARCH @@ -18,7 +18,7 @@ ARG TARGETOS COPY --from=envoy-binary /usr/local/bin/envoy /usr/local/bin/ COPY dist/$TARGETOS/$TARGETARCH/$BIN_NAME /usr/local/bin/ -RUN apk add libcap +RUN apt-get update && apt install -y libcap2-bin RUN setcap CAP_NET_BIND_SERVICE=+ep /usr/local/bin/envoy RUN setcap CAP_NET_BIND_SERVICE=+ep /usr/local/bin/$BIN_NAME @@ -56,8 +56,9 @@ LABEL name=${BIN_NAME}\ COPY --from=dumb-init /usr/bin/dumb-init /usr/local/bin/ COPY --from=go-discover /go/bin/discover /usr/local/bin/ -COPY --from=setcap /usr/local/bin/envoy /usr/local/bin/ -COPY --from=setcap /usr/local/bin/$BIN_NAME /usr/local/bin/ +COPY --from=setcap-envoy-binary /usr/local/bin/envoy /usr/local/bin/ +COPY --from=setcap-envoy-binary /usr/local/bin/$BIN_NAME /usr/local/bin/ +COPY LICENSE /licenses/copyright.txt USER 100 @@ -95,8 +96,8 @@ RUN groupadd --gid 1000 $PRODUCT_NAME && \ COPY --from=dumb-init /usr/bin/dumb-init /usr/local/bin/ COPY --from=go-discover /go/bin/discover /usr/local/bin/ -COPY --from=setcap /usr/local/bin/envoy /usr/local/bin/ -COPY --from=setcap /usr/local/bin/$BIN_NAME /usr/local/bin/ +COPY --from=setcap-envoy-binary /usr/local/bin/envoy /usr/local/bin/ +COPY --from=setcap-envoy-binary /usr/local/bin/$BIN_NAME /usr/local/bin/ COPY LICENSE /licenses/copyright.txt USER 100 diff --git a/pkg/envoy/proxy.go b/pkg/envoy/proxy.go index 43c111e8..642d1414 100644 --- a/pkg/envoy/proxy.go +++ b/pkg/envoy/proxy.go @@ -6,9 +6,7 @@ import ( "fmt" "io" "net/http" - "os" "os/exec" - "path/filepath" "strings" "sync/atomic" "syscall" @@ -134,14 +132,8 @@ func (p *Proxy) Run(ctx context.Context) error { return errors.New("proxy may only be run once") } - // Write the bootstrap config to a pipe. - configPath, cleanup, err := writeBootstrapConfig(p.cfg.BootstrapConfig) - if err != nil { - return err - } - // Run the Envoy process. - p.cmd = p.buildCommand(ctx, configPath) + p.cmd = p.buildCommand(ctx, string(p.cfg.BootstrapConfig)) // Start Envoy in its own process group to avoid directly receiving // SIGTERM intended for consul-dataplane, let proxy manager handle @@ -152,10 +144,6 @@ func (p *Proxy) Run(ctx context.Context) error { p.cfg.Logger.Debug("running envoy proxy", "command", strings.Join(p.cmd.Args, " ")) if err := p.cmd.Start(); err != nil { - // Clean up the pipe if we weren't able to run Envoy. - if err := cleanup(); err != nil { - p.cfg.Logger.Error("failed to cleanup boostrap config", "error", err) - } return err } @@ -166,9 +154,6 @@ func (p *Proxy) Run(ctx context.Context) error { err := p.cmd.Wait() p.cfg.Logger.Info("envoy process exited", "error", err) p.transitionState(stateRunning, stateExited) - if err := cleanup(); err != nil { - p.cfg.Logger.Error("failed to cleanup boostrap config", "error", err) - } p.exitedCh <- err close(p.exitedCh) }() @@ -281,56 +266,9 @@ func (p *Proxy) transitionState(before, after state) bool { return atomic.CompareAndSwapUint32((*uint32)(&p.state), uint32(before), uint32(after)) } -// writeBootstrapConfig writes the given Envoy bootstrap config to a named pipe -// and returns the path. It also returns a cleanup function that must be called -// when Envoy is done with it. -// -// We use a named pipe rather than a tempfile because it prevents writing any -// secrets to disk. See: https://github.com/hashicorp/consul/pull/5964 -func writeBootstrapConfig(cfg []byte) (string, func() error, error) { - path := filepath.Join( - os.TempDir(), - fmt.Sprintf("envoy-%x-bootstrap.json", time.Now().UnixNano()+int64(os.Getpid())), - ) - if err := syscall.Mkfifo(path, 0600); err != nil { - return "", nil, err - } - - // O_WRONLY causes OpenFile to block until there's a reader (Envoy). Opening - // the pipe with O_RDWR wouldn't block but would result in just sending stuff - // to ourself. - // - // TODO(boxofrad): We don't have a way to cancel this goroutine. If the Envoy - // process never opens the other end of the pipe this will hang forever. The - // workaround we use in `consul connect envoy` is to write to the pipe in a - // subprocess that self-destructs after 10 minutes. - go func() { - file, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND, 0600) - if err != nil { - os.Remove(path) - return - } - - _, err = file.Write(cfg) - file.Close() - - if err != nil { - os.Remove(path) - } - }() - - return path, func() error { - err := os.Remove(path) - if err == nil || errors.Is(err, os.ErrNotExist) { - return nil - } - return err - }, nil -} - // buildCommand builds the exec.Cmd to run Envoy with the relevant arguments // (e.g. config path) and its logs redirected to the logger. -func (p *Proxy) buildCommand(ctx context.Context, cfgPath string) *exec.Cmd { +func (p *Proxy) buildCommand(ctx context.Context, cfgYaml string) *exec.Cmd { var logFormat string if p.cfg.LogJSON { logFormat = logFormatJSON @@ -368,7 +306,7 @@ func (p *Proxy) buildCommand(ctx context.Context, cfgPath string) *exec.Cmd { args := append( []string{ - "--config-path", cfgPath, + "--config-yaml", cfgYaml, "--log-format", logFormat, "--log-level", logLevel, diff --git a/pkg/envoy/proxy_test.go b/pkg/envoy/proxy_test.go index 54a90a54..689a978b 100644 --- a/pkg/envoy/proxy_test.go +++ b/pkg/envoy/proxy_test.go @@ -1,8 +1,10 @@ package envoy import ( + "bytes" "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -12,11 +14,26 @@ import ( "time" "github.com/hashicorp/go-hclog" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestProxy(t *testing.T) { - bootstrapConfig := []byte(`hello world`) + bootstrapConfig := []byte(` + { + "dynamic_resources": { + "cds_config": { + "ads": {}, + "initial_fetch_timeout": "0s", + "resource_api_version": "V3" + }, + "lds_config": { + "ads": {}, + "initial_fetch_timeout": "0s", + "resource_api_version": "V3" + }, + } + }`) // This test checks that we're starting the Envoy process with the correct // arguments and that it is able to read the config we provide. It does so @@ -25,9 +42,12 @@ func TestProxy(t *testing.T) { outputPath := testOutputPath() t.Cleanup(func() { _ = os.Remove(outputPath) }) + // Capture fake-envoy output so we know if we encounter an error case below + envoyOut := bytes.NewBuffer([]byte{}) + p, err := NewProxy(ProxyConfig{ Logger: hclog.New(&hclog.LoggerOptions{Level: hclog.Warn, Output: io.Discard}), - EnvoyLogOutput: io.Discard, + EnvoyLogOutput: envoyOut, ExecutablePath: "testdata/fake-envoy", ExtraArgs: []string{"--test-output", outputPath}, BootstrapConfig: bootstrapConfig, @@ -42,7 +62,10 @@ func TestProxy(t *testing.T) { Args []byte ConfigData []byte } - require.Eventually(t, func() bool { + + // Wait for output file to be generated by fake-envoy. + // Use assert so that we can check for error output below. + assert.Eventually(t, func() bool { outputBytes, err := os.ReadFile(outputPath) if err != nil { t.Logf("failed to read output file: %v", err) @@ -55,23 +78,26 @@ func TestProxy(t *testing.T) { return true }, 2*time.Second, 50*time.Millisecond) + // Check the output from fake-envoy to make sure we didn't hit an error case + require.Empty(t, envoyOut.String()) + // Check that fake-envoy was able to read the config from the pipe. - require.Equal(t, bootstrapConfig, output.ConfigData) + assert.Equal(t, string(bootstrapConfig), string(output.ConfigData)) // Check that we're correctly configuring the log level. - require.Contains(t, string(output.Args), "--log-level warn") + assert.Contains(t, string(output.Args), "--log-level warn") // Check that we're disabling hot restarts. - require.Contains(t, string(output.Args), "--disable-hot-restart") + assert.Contains(t, string(output.Args), "--disable-hot-restart") // Check the process is still running. - require.NoError(t, p.cmd.Process.Signal(syscall.Signal(0))) + assert.NoError(t, p.cmd.Process.Signal(syscall.Signal(0))) // Ensure Kill kills and reaps the process. require.NoError(t, p.Kill()) - require.Eventually(t, func() bool { - return p.cmd.Process.Signal(syscall.Signal(0)) == os.ErrProcessDone + err := p.cmd.Process.Signal(syscall.Signal(0)) + return errors.Is(err, os.ErrProcessDone) }, 2*time.Second, 50*time.Millisecond) } diff --git a/pkg/envoy/testdata/fake-envoy b/pkg/envoy/testdata/fake-envoy index f927ee24..145d8822 100755 --- a/pkg/envoy/testdata/fake-envoy +++ b/pkg/envoy/testdata/fake-envoy @@ -1,20 +1,20 @@ #!/bin/bash # This script pretends to be Envoy in unit tests. It captures the flags and the -# bootstrap config from the named pipe specified via `--config-path`, and writes -# them to the file at `--test-output` (which is read and checked in the test). +# bootstrap config specified via `--config-yaml`, and writes them to the file at +# `--test-output` (which is read and checked in the test). # It then sleeps for 10 minutes to check we're correctly killing the process. set -e -config_path="" +config_yaml="" test_output="" prev_arg="" for arg in "$@"; do case "$prev_arg" in - --config-path) - config_path="$arg" + --config-yaml) + config_yaml="$arg" ;; --test-output) test_output="$arg" @@ -23,8 +23,8 @@ for arg in "$@"; do prev_arg="$arg" done -if [ -z "$config_path" ]; then - >&2 echo "--config-path is required" +if [ -z "$config_yaml" ]; then + >&2 echo "--config-yaml is required" exit 1 fi @@ -35,7 +35,7 @@ fi # Base64 encode the data to avoid having to escape it in the JSON output. args=$(echo "$@" | base64 | tr -d \\n) -config_data=$(base64 -i $config_path | tr -d \\n) +config_data=$(echo -n "$config_yaml" | base64 | tr -d \\n) cat < "$test_output" {