Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use unix socket for Envoy admin webpage #3934

Merged
merged 12 commits into from
Aug 24, 2021
4 changes: 2 additions & 2 deletions cmd/contour/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ func registerBootstrap(app *kingpin.Application) (*kingpin.CmdClause, *envoy.Boo
bootstrap := app.Command("bootstrap", "Generate bootstrap configuration.")
bootstrap.Arg("path", "Configuration file ('-' for standard output).").Required().StringVar(&config.Path)
bootstrap.Flag("resources-dir", "Directory where configuration files will be written to.").StringVar(&config.ResourcesDir)
bootstrap.Flag("admin-address", "Envoy admin interface address.").StringVar(&config.AdminAddress)
bootstrap.Flag("admin-port", "Envoy admin interface port.").IntVar(&config.AdminPort)
bootstrap.Flag("admin-address", "Path to Envoy admin unix domain socket.").Default("/admin/admin.sock").StringVar(&config.AdminAddress)
bootstrap.Flag("admin-port", "DEPRECATED: Envoy admin interface port.").IntVar(&config.AdminPort)
bootstrap.Flag("xds-address", "xDS gRPC API address.").StringVar(&config.XDSAddress)
bootstrap.Flag("xds-port", "xDS gRPC API port.").IntVar(&config.XDSGRPCPort)
bootstrap.Flag("envoy-cafile", "CA Filename for Envoy secure xDS gRPC communication.").Envar("ENVOY_CAFILE").StringVar(&config.GrpcCABundle)
Expand Down
7 changes: 5 additions & 2 deletions cmd/contour/contour.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

resource_v3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3"
"github.com/projectcontour/contour/internal/build"
"github.com/projectcontour/contour/internal/envoy"
envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3"
"github.com/projectcontour/contour/internal/k8s"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -70,10 +71,12 @@ func main() {
case sdmShutdown.FullCommand():
sdmShutdownCtx.shutdownHandler()
case bootstrap.FullCommand():
err := bootstrapCtx.XDSResourceVersion.Validate()
if err != nil {
if err := bootstrapCtx.XDSResourceVersion.Validate(); err != nil {
log.WithError(err).Fatal("failed to parse bootstrap args")
}
if err := envoy.ValidAdminAddress(bootstrapCtx.AdminAddress); err != nil {
log.WithField("flag", "--admin-address").WithError(err).Fatal("failed to parse bootstrap args")
}
if err := envoy_v3.WriteBootstrap(bootstrapCtx); err != nil {
log.WithError(err).Fatal("failed to write bootstrap configuration")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/contour/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func doServe(log logrus.FieldLogger, ctx *serveContext) error {
endpointHandler := xdscache_v3.NewEndpointsTranslator(log.WithField("context", "endpointstranslator"))

resources := []xdscache.ResourceCache{
xdscache_v3.NewListenerCache(listenerConfig, ctx.statsAddr, ctx.statsPort),
xdscache_v3.NewListenerCache(listenerConfig, ctx.statsAddr, ctx.statsPort, ctx.Config.Network.EnvoyAdminPort),
&xdscache_v3.SecretCache{},
&xdscache_v3.RouteCache{},
&xdscache_v3.ClusterCache{},
Expand Down
50 changes: 35 additions & 15 deletions cmd/contour/shutdownmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
package main

import (
"context"
"fmt"
"io"
"log"
"net"
"net/http"
"os"
"time"
Expand All @@ -30,9 +32,9 @@ import (
)

const (
prometheusURLFormat = "http://localhost:%d/stats/prometheus"
healthcheckFailURLFormat = "http://localhost:%d/healthcheck/fail"
prometheusStat = "envoy_http_downstream_cx_active"
prometheusURL = "http://unix/stats/prometheus"
healthcheckFailURL = "http://unix/healthcheck/fail"
prometheusStat = "envoy_http_downstream_cx_active"
)

// shutdownReadyFile is the default file path used in the /shutdown endpoint.
Expand Down Expand Up @@ -70,9 +72,12 @@ type shutdownContext struct {
// that can be open when polling for active connections in Envoy
minOpenConnections int

// adminPort defines the port for our envoy pod, being configurable through --admin-port flag
// Deprecated: adminPort defines the port for the Envoy admin webpage, being configurable through --admin-port flag
adminPort int

// adminAddress defines the address for the Envoy admin webpage, being configurable through --admin-address flag
adminAddress string

logrus.FieldLogger
}

Expand All @@ -91,7 +96,6 @@ func newShutdownContext() *shutdownContext {
checkDelay: 60 * time.Second,
drainDelay: 0,
minOpenConnections: 0,
adminPort: 9001,
}
}

Expand Down Expand Up @@ -145,7 +149,7 @@ func (s *shutdownContext) shutdownHandler() {
// Send shutdown signal to Envoy to start draining connections
s.Infof("failing envoy healthchecks")

// Retry any failures to shutdownEnvoy(s.adminPort) in a Backoff time window
// Retry any failures to shutdownEnvoy(s.adminAddress) in a Backoff time window
// doing 4 total attempts, multiplying the Duration by the Factor
// for each iteration.
err := retry.OnError(wait.Backoff{
Expand All @@ -158,7 +162,7 @@ func (s *shutdownContext) shutdownHandler() {
return true
}, func() error {
s.Infof("attempting to shutdown")
return shutdownEnvoy(s.adminPort)
return shutdownEnvoy(s.adminAddress)
})
if err != nil {
// May be conflict if max retries were hit, or may be something unrelated
Expand All @@ -170,7 +174,7 @@ func (s *shutdownContext) shutdownHandler() {
time.Sleep(s.checkDelay)

for {
openConnections, err := getOpenConnections(s.adminPort)
openConnections, err := getOpenConnections(s.adminAddress)
if err != nil {
s.Error(err)
} else {
Expand All @@ -196,10 +200,17 @@ func (s *shutdownContext) shutdownHandler() {
}

// shutdownEnvoy sends a POST request to /healthcheck/fail to tell Envoy to start draining connections
func shutdownEnvoy(adminPort int) error {
healthcheckFailURL := fmt.Sprintf(healthcheckFailURLFormat, adminPort)
func shutdownEnvoy(adminAddress string) error {

httpClient := http.Client{
Transport: &http.Transport{
DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
return net.Dial("unix", adminAddress)
},
},
}
/* #nosec */
resp, err := http.Post(healthcheckFailURL, "", nil)
resp, err := httpClient.Post(healthcheckFailURL, "", nil)
if err != nil {
return fmt.Errorf("creating healthcheck fail POST request failed: %s", err)
}
Expand All @@ -212,11 +223,19 @@ func shutdownEnvoy(adminPort int) error {
}

// getOpenConnections parses a http request to a prometheus endpoint returning the sum of values found
func getOpenConnections(adminPort int) (int, error) {
prometheusURL := fmt.Sprintf(prometheusURLFormat, adminPort)
func getOpenConnections(adminAddress string) (int, error) {

httpClient := http.Client{
Transport: &http.Transport{
DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
return net.Dial("unix", adminAddress)
},
},
}

// Make request to Envoy Prometheus endpoint
/* #nosec */
resp, err := http.Get(prometheusURL)
resp, err := httpClient.Get(prometheusURL)
if err != nil {
return -1, fmt.Errorf("creating metrics GET request failed: %s", err)
}
Expand Down Expand Up @@ -289,7 +308,8 @@ func registerShutdown(cmd *kingpin.CmdClause, log logrus.FieldLogger) (*kingpin.
ctx.FieldLogger = log.WithField("context", "shutdown")

shutdown := cmd.Command("shutdown", "Initiate an shutdown sequence which configures Envoy to begin draining connections.")
shutdown.Flag("admin-port", "Envoy admin interface port.").IntVar(&ctx.adminPort)
shutdown.Flag("admin-port", "DEPRECATED: Envoy admin interface port.").IntVar(&ctx.adminPort)
shutdown.Flag("admin-address", "Envoy admin interface address.").Default("/admin/admin.sock").StringVar(&ctx.adminAddress)
shutdown.Flag("check-interval", "Time to poll Envoy for open connections.").DurationVar(&ctx.checkInterval)
shutdown.Flag("check-delay", "Time to wait before polling Envoy for open connections.").Default("60s").DurationVar(&ctx.checkDelay)
shutdown.Flag("drain-delay", "Time to wait before draining Envoy connections.").Default("0s").DurationVar(&ctx.drainDelay)
Expand Down
7 changes: 0 additions & 7 deletions cmd/contour/shutdownmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ func TestShutdownManager_HealthzHandler(t *testing.T) {
}
}

func TestShutdownManager_NewShutDownContextCheckDefaultAdminPort(t *testing.T) {
shutdownContext := newShutdownContext()
if (*shutdownContext).adminPort != 9001 {
t.Errorf("TestShutdownManager_NewShutDownContextWithoutAdminPort failed: expected shutdown port %d, got %d", 9001, (*shutdownContext).adminPort)
}
}

func TestShutdownManager_ShutdownReadyHandler_Success(t *testing.T) {
// Create a request to pass to our handler
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
Expand Down
2 changes: 2 additions & 0 deletions examples/contour/01-contour-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ data:
# Configure the number of additional ingress proxy hops from the
# right side of the x-forwarded-for HTTP header to trust.
# num-trusted-hops: 0
# Configure the port used to access the Envoy Admin interface.
# admin-port: 9001
#
# Configure an optional global rate limit service.
# rateLimitService:
Expand Down
7 changes: 7 additions & 0 deletions examples/contour/03-envoy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ spec:
initialDelaySeconds: 3
periodSeconds: 10
name: shutdown-manager
volumeMounts:
- name: envoy-admin
mountPath: /admin
- args:
- -c
- /config/envoy.json
Expand Down Expand Up @@ -89,6 +92,8 @@ spec:
- name: envoycert
mountPath: /certs
readOnly: true
- name: envoy-admin
mountPath: /admin
lifecycle:
preStop:
httpGet:
Expand Down Expand Up @@ -126,6 +131,8 @@ spec:
serviceAccountName: envoy
terminationGracePeriodSeconds: 300
volumes:
- name: envoy-admin
emptyDir: {}
- name: envoy-config
emptyDir: {}
- name: envoycert
Expand Down
9 changes: 9 additions & 0 deletions examples/render/contour-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ data:
# Configure the number of additional ingress proxy hops from the
# right side of the x-forwarded-for HTTP header to trust.
# num-trusted-hops: 0
# Configure the port used to access the Envoy Admin interface.
# admin-port: 9001
#
# Configure an optional global rate limit service.
# rateLimitService:
Expand Down Expand Up @@ -3097,6 +3099,9 @@ spec:
initialDelaySeconds: 3
periodSeconds: 10
name: shutdown-manager
volumeMounts:
- name: envoy-admin
mountPath: /admin
- args:
- -c
- /config/envoy.json
Expand Down Expand Up @@ -3141,6 +3146,8 @@ spec:
- name: envoycert
mountPath: /certs
readOnly: true
- name: envoy-admin
mountPath: /admin
lifecycle:
preStop:
httpGet:
Expand Down Expand Up @@ -3178,6 +3185,8 @@ spec:
serviceAccountName: envoy
terminationGracePeriodSeconds: 300
volumes:
- name: envoy-admin
emptyDir: {}
- name: envoy-config
emptyDir: {}
- name: envoycert
Expand Down
9 changes: 9 additions & 0 deletions examples/render/contour.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ data:
# Configure the number of additional ingress proxy hops from the
# right side of the x-forwarded-for HTTP header to trust.
# num-trusted-hops: 0
# Configure the port used to access the Envoy Admin interface.
# admin-port: 9001
#
# Configure an optional global rate limit service.
# rateLimitService:
Expand Down Expand Up @@ -3094,6 +3096,9 @@ spec:
initialDelaySeconds: 3
periodSeconds: 10
name: shutdown-manager
volumeMounts:
- name: envoy-admin
mountPath: /admin
- args:
- -c
- /config/envoy.json
Expand Down Expand Up @@ -3138,6 +3143,8 @@ spec:
- name: envoycert
mountPath: /certs
readOnly: true
- name: envoy-admin
mountPath: /admin
lifecycle:
preStop:
httpGet:
Expand Down Expand Up @@ -3175,6 +3182,8 @@ spec:
serviceAccountName: envoy
terminationGracePeriodSeconds: 300
volumes:
- name: envoy-admin
emptyDir: {}
- name: envoy-config
emptyDir: {}
- name: envoycert
Expand Down
22 changes: 18 additions & 4 deletions internal/envoy/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package envoy

import (
"fmt"
"net"
"os"

"github.com/golang/protobuf/jsonpb"
Expand All @@ -40,12 +42,12 @@ type BootstrapConfig struct {
// Defaults to /dev/null.
AdminAccessLogPath string

// AdminAddress is the TCP address that the administration server will listen on.
// Defaults to 127.0.0.1.
// AdminAddress is the Unix Socket address that the administration server will listen on.
// Defaults to /admin/admin.sock.
AdminAddress string
skriss marked this conversation as resolved.
Show resolved Hide resolved

// Deprecated
// AdminPort is the port that the administration server will listen on.
// Defaults to 9001.
AdminPort int

// XDSAddress is the TCP address of the gRPC XDS management server.
Expand Down Expand Up @@ -93,7 +95,7 @@ type BootstrapConfig struct {
func (c *BootstrapConfig) GetXdsAddress() string { return stringOrDefault(c.XDSAddress, "127.0.0.1") }
func (c *BootstrapConfig) GetXdsGRPCPort() int { return intOrDefault(c.XDSGRPCPort, 8001) }
func (c *BootstrapConfig) GetAdminAddress() string {
return stringOrDefault(c.AdminAddress, "127.0.0.1")
return stringOrDefault(c.AdminAddress, "/admin/admin.sock")
}
func (c *BootstrapConfig) GetAdminPort() int { return intOrDefault(c.AdminPort, 9001) }
func (c *BootstrapConfig) GetAdminAccessLogPath() string {
Expand All @@ -102,6 +104,18 @@ func (c *BootstrapConfig) GetAdminAccessLogPath() string {
func (c *BootstrapConfig) GetDNSLookupFamily() string {
return stringOrDefault(c.DNSLookupFamily, "auto")
}

// ValidAdminAddress checks if the address supplied is
// "localhost" or an IP address. Only a Unix Socket
// is supported for this address to mitigate security.
func ValidAdminAddress(address string) error {
sunjayBhatia marked this conversation as resolved.
Show resolved Hide resolved
// Value of "localhost" is invalid.
if address == "localhost" || net.ParseIP(address) != nil {
return fmt.Errorf("invalid value %q, cannot be `localhost` or an ip address", address)
}
return nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check that the unix socket exists, or at least that the path is a valid one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure of the timing of that type of check. This is the bootstrap code which executes in the initContainer. Then the pod would startup after that. Need to double check if the volumes are all ready in init phase or not, I don't remember off the top of my head.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So volumes should be there, but Envoy creates the actual socket, so we can't check for this in the bootstrap since Envoy's not yet started.

}

func stringOrDefault(s, def string) string {
if s == "" {
return def
Expand Down
42 changes: 42 additions & 0 deletions internal/envoy/bootstrap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright Project Contour Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package envoy

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

func TestValidAdminAddress(t *testing.T) {

tests := []struct {
name string
address string
want error
}{
{name: "valid socket name", address: "/admin/admin.sock", want: nil},
{name: "valid socket name", address: "admin.sock", want: nil},
{name: "ip address invalid", address: "127.0.0.1", want: fmt.Errorf("invalid value %q, cannot be `localhost` or an ip address", "127.0.0.1")},
{name: "localhost invalid", address: "localhost", want: fmt.Errorf("invalid value %q, cannot be `localhost` or an ip address", "localhost")},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := ValidAdminAddress(tc.address)
assert.Equal(t, tc.want, got)
})
}
}
Loading