From db8ecb2dd17fb17aca62da24ef19342e62e7f5d9 Mon Sep 17 00:00:00 2001 From: Paul Glass Date: Fri, 4 Nov 2022 16:00:39 -0500 Subject: [PATCH 1/2] Use tempfile instead of mkfifo for Windows --- Makefile | 4 ++-- pkg/envoy/proxy.go | 31 ++++--------------------------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 1293e7d1..814dee10 100644 --- a/Makefile +++ b/Makefile @@ -8,8 +8,8 @@ GOBIN ?= $(GOPATH)/bin # Get local ARCH; on Intel Mac, 'uname -m' returns x86_64 which we turn into amd64. # Not using 'go env GOOS/GOARCH' here so 'make docker' will work without local Go install. -ARCH = $(shell A=$$(uname -m); [ $$A = x86_64 ] && A=amd64; echo $$A) -OS = $(shell uname | tr [[:upper:]] [[:lower:]]) +ARCH ?= $(shell A=$$(uname -m); [ $$A = x86_64 ] && A=amd64; echo $$A) +OS ?= $(shell uname | tr [[:upper:]] [[:lower:]]) PLATFORM = $(OS)/$(ARCH) DIST = dist/$(PLATFORM) BIN = $(DIST)/$(BIN_NAME) diff --git a/pkg/envoy/proxy.go b/pkg/envoy/proxy.go index 86916ddd..9a03cb04 100644 --- a/pkg/envoy/proxy.go +++ b/pkg/envoy/proxy.go @@ -5,12 +5,12 @@ import ( "errors" "fmt" "io" + "log" "os" "os/exec" "path/filepath" "strings" "sync/atomic" - "syscall" "time" "github.com/hashicorp/go-hclog" @@ -179,32 +179,9 @@ func writeBootstrapConfig(cfg []byte) (string, func() error, error) { 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) - } - }() + log.Printf("bootstrap config path: %s", path) + err := os.WriteFile(path, cfg, 0600) return path, func() error { err := os.Remove(path) @@ -212,7 +189,7 @@ func writeBootstrapConfig(cfg []byte) (string, func() error, error) { return nil } return err - }, nil + }, err } // buildCommand builds the exec.Cmd to run Envoy with the relevant arguments From 234930cdb54c7223241efd201fb2fb5115763151 Mon Sep 17 00:00:00 2001 From: Paul Glass Date: Fri, 4 Nov 2022 16:16:06 -0500 Subject: [PATCH 2/2] Remove x-consul-token from bootstrap config file --- internal/bootstrap/bootstrap_tpl.go | 13 +------------ .../testdata/TestBootstrapConfig/basic.golden | 7 +------ .../central-telemetry-config.golden | 7 +------ .../custom-prometheus-scrape-path.golden | 7 +------ .../TestBootstrapConfig/ready-listener.golden | 7 +------ .../unix-socket-xds-server.golden | 7 +------ 6 files changed, 6 insertions(+), 42 deletions(-) diff --git a/internal/bootstrap/bootstrap_tpl.go b/internal/bootstrap/bootstrap_tpl.go index ec872b1e..e157b905 100644 --- a/internal/bootstrap/bootstrap_tpl.go +++ b/internal/bootstrap/bootstrap_tpl.go @@ -43,12 +43,6 @@ type BootstrapTplArgs struct { // service and is expected to be used for that purpose. LocalAgentClusterName string - // Token is the Consul ACL token provided which is required to make gRPC - // discovery requests. If non-empty, this must be configured as the gRPC - // service "initial_metadata" with the key "x-consul-token" in order to - // authorize the discovery streaming RPCs. - Token string - // StaticClustersJSON is JSON string, each is expected to be a valid Cluster // definition. They are appended to the "static_resources.clusters" list. Note // that cluster names should be chosen in such a way that they won't collide @@ -272,12 +266,7 @@ const bootstrapTemplate = `{ "api_type": "DELTA_GRPC", "transport_api_version": "V3", "grpc_services": { - "initial_metadata": [ - { - "key": "x-consul-token", - "value": "{{ .Token }}" - } - ], + "initial_metadata": [], "envoy_grpc": { "cluster_name": "{{ .LocalAgentClusterName }}" } diff --git a/pkg/consuldp/testdata/TestBootstrapConfig/basic.golden b/pkg/consuldp/testdata/TestBootstrapConfig/basic.golden index ca0741a3..396fba7c 100644 --- a/pkg/consuldp/testdata/TestBootstrapConfig/basic.golden +++ b/pkg/consuldp/testdata/TestBootstrapConfig/basic.golden @@ -155,12 +155,7 @@ "api_type": "DELTA_GRPC", "transport_api_version": "V3", "grpc_services": { - "initial_metadata": [ - { - "key": "x-consul-token", - "value": "" - } - ], + "initial_metadata": [], "envoy_grpc": { "cluster_name": "consul-dataplane" } diff --git a/pkg/consuldp/testdata/TestBootstrapConfig/central-telemetry-config.golden b/pkg/consuldp/testdata/TestBootstrapConfig/central-telemetry-config.golden index 8ed91715..f87a207d 100644 --- a/pkg/consuldp/testdata/TestBootstrapConfig/central-telemetry-config.golden +++ b/pkg/consuldp/testdata/TestBootstrapConfig/central-telemetry-config.golden @@ -169,12 +169,7 @@ "api_type": "DELTA_GRPC", "transport_api_version": "V3", "grpc_services": { - "initial_metadata": [ - { - "key": "x-consul-token", - "value": "" - } - ], + "initial_metadata": [], "envoy_grpc": { "cluster_name": "consul-dataplane" } diff --git a/pkg/consuldp/testdata/TestBootstrapConfig/custom-prometheus-scrape-path.golden b/pkg/consuldp/testdata/TestBootstrapConfig/custom-prometheus-scrape-path.golden index f21bdabe..932d87e0 100644 --- a/pkg/consuldp/testdata/TestBootstrapConfig/custom-prometheus-scrape-path.golden +++ b/pkg/consuldp/testdata/TestBootstrapConfig/custom-prometheus-scrape-path.golden @@ -244,12 +244,7 @@ "api_type": "DELTA_GRPC", "transport_api_version": "V3", "grpc_services": { - "initial_metadata": [ - { - "key": "x-consul-token", - "value": "" - } - ], + "initial_metadata": [], "envoy_grpc": { "cluster_name": "consul-dataplane" } diff --git a/pkg/consuldp/testdata/TestBootstrapConfig/ready-listener.golden b/pkg/consuldp/testdata/TestBootstrapConfig/ready-listener.golden index 1d45d37d..2a2b0815 100644 --- a/pkg/consuldp/testdata/TestBootstrapConfig/ready-listener.golden +++ b/pkg/consuldp/testdata/TestBootstrapConfig/ready-listener.golden @@ -244,12 +244,7 @@ "api_type": "DELTA_GRPC", "transport_api_version": "V3", "grpc_services": { - "initial_metadata": [ - { - "key": "x-consul-token", - "value": "" - } - ], + "initial_metadata": [], "envoy_grpc": { "cluster_name": "consul-dataplane" } diff --git a/pkg/consuldp/testdata/TestBootstrapConfig/unix-socket-xds-server.golden b/pkg/consuldp/testdata/TestBootstrapConfig/unix-socket-xds-server.golden index 4affbca5..0ed7b495 100644 --- a/pkg/consuldp/testdata/TestBootstrapConfig/unix-socket-xds-server.golden +++ b/pkg/consuldp/testdata/TestBootstrapConfig/unix-socket-xds-server.golden @@ -154,12 +154,7 @@ "api_type": "DELTA_GRPC", "transport_api_version": "V3", "grpc_services": { - "initial_metadata": [ - { - "key": "x-consul-token", - "value": "" - } - ], + "initial_metadata": [], "envoy_grpc": { "cluster_name": "consul-dataplane" }