From 391db7e58b501b3ed7561fec352f2f3f5004a29f Mon Sep 17 00:00:00 2001 From: Chris Thain Date: Tue, 20 Jun 2023 11:32:26 -0700 Subject: [PATCH 1/2] fix localhost target URI issue --- agent/envoyextensions/builtin/ext-authz/structs.go | 5 ++++- agent/xds/delta_envoy_extender_test.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/agent/envoyextensions/builtin/ext-authz/structs.go b/agent/envoyextensions/builtin/ext-authz/structs.go index b64011a991e..daef2f62f76 100644 --- a/agent/envoyextensions/builtin/ext-authz/structs.go +++ b/agent/envoyextensions/builtin/ext-authz/structs.go @@ -652,7 +652,10 @@ func (t *Target) validate() error { addr := strings.Split(t.URI, ":") if len(addr) == 2 { t.host = addr[0] - if t.host != "localhost" && t.host != "127.0.0.1" { + if t.host == "localhost" || t.host == "127.0.0.1" { + // Always use the IP address form. + t.host = "127.0.0.1" + } else { resultErr = multierror.Append(resultErr, fmt.Errorf("invalid host for Target.URI %q: expected 'localhost' or '127.0.0.1'", t.URI)) } if t.port, err = strconv.Atoi(addr[1]); err != nil { diff --git a/agent/xds/delta_envoy_extender_test.go b/agent/xds/delta_envoy_extender_test.go index 0a76d622195..285dc335768 100644 --- a/agent/xds/delta_envoy_extender_test.go +++ b/agent/xds/delta_envoy_extender_test.go @@ -12,7 +12,7 @@ import ( ) func makeExtAuthzEnvoyExtension(svc string, opts ...string) []structs.EnvoyExtension { - target := map[string]any{"URI": "127.0.0.1:9191"} + target := map[string]any{"URI": "localhost:9191"} insertOptions := map[string]any{} required := false ent := false From f204d5b52ab80836128882a65d7d7c5e53b2fa3d Mon Sep 17 00:00:00 2001 From: Chris Thain Date: Wed, 21 Jun 2023 08:39:06 -0700 Subject: [PATCH 2/2] allow localhost hostname and configure DNS --- .../builtin/ext-authz/ext_authz_test.go | 31 ++++++++++- .../builtin/ext-authz/structs.go | 52 +++++++++++++------ agent/xds/delta_envoy_extender_oss_test.go | 1 + agent/xds/delta_envoy_extender_test.go | 9 +++- ...uthz-http-local-http-service.latest.golden | 4 +- .../configuration/ext-authz.mdx | 4 +- .../envoy-extensions/usage/ext-authz.mdx | 2 +- 7 files changed, 79 insertions(+), 24 deletions(-) diff --git a/agent/envoyextensions/builtin/ext-authz/ext_authz_test.go b/agent/envoyextensions/builtin/ext-authz/ext_authz_test.go index e0b4245edda..88e87d7e9a8 100644 --- a/agent/envoyextensions/builtin/ext-authz/ext_authz_test.go +++ b/agent/envoyextensions/builtin/ext-authz/ext_authz_test.go @@ -59,7 +59,7 @@ func TestConstructor(t *testing.T) { }, }, }, - errMsg: `invalid host for Target.URI "foo.bar.com:9191": expected 'localhost' or '127.0.0.1'`, + errMsg: `invalid host for Target.URI "foo.bar.com:9191": expected "localhost", "127.0.0.1", or "::1"`, }, "non-loopback address": { args: map[string]any{ @@ -72,7 +72,34 @@ func TestConstructor(t *testing.T) { }, }, }, - errMsg: `invalid host for Target.URI "10.0.0.1:9191": expected 'localhost' or '127.0.0.1'`, + errMsg: `invalid host for Target.URI "10.0.0.1:9191": expected "localhost", "127.0.0.1", or "::1"`, + }, + "invalid target port": { + args: map[string]any{ + "ProxyType": "connect-proxy", + "Config": map[string]any{ + "GrpcService": map[string]any{ + "Target": map[string]any{ + "URI": "localhost:zero", + }, + }, + }, + }, + errMsg: `invalid format for Target.URI "localhost:zero": expected host:port`, + }, + "invalid target timeout": { + args: map[string]any{ + "ProxyType": "connect-proxy", + "Config": map[string]any{ + "GrpcService": map[string]any{ + "Target": map[string]any{ + "URI": "localhost:9191", + "Timeout": "one", + }, + }, + }, + }, + errMsg: `failed to parse Target.Timeout "one" as a duration`, }, "no uri or service target": { args: map[string]any{ diff --git a/agent/envoyextensions/builtin/ext-authz/structs.go b/agent/envoyextensions/builtin/ext-authz/structs.go index daef2f62f76..a14cedd63a7 100644 --- a/agent/envoyextensions/builtin/ext-authz/structs.go +++ b/agent/envoyextensions/builtin/ext-authz/structs.go @@ -34,6 +34,9 @@ const ( defaultMetadataNS = "consul" defaultStatPrefix = "response" defaultStatusOnError = 403 + localhost = "localhost" + localhostIPv4 = "127.0.0.1" + localhostIPv6 = "::1" ) type extAuthzConfig struct { @@ -185,6 +188,12 @@ func (c *extAuthzConfig) toEnvoyCluster(_ *cmn.RuntimeConfig) (*envoy_cluster_v3 return nil, err } + clusterType := &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_STATIC} + if host == localhost { + // If the host is "localhost" use a STRICT_DNS cluster type to perform DNS lookup. + clusterType = &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_STRICT_DNS} + } + var typedExtProtoOpts map[string]*anypb.Any if c.isGRPC() { // By default HTTP/1.1 is used for the transport protocol. gRPC requires that we explicitly configure HTTP/2 @@ -205,7 +214,7 @@ func (c *extAuthzConfig) toEnvoyCluster(_ *cmn.RuntimeConfig) (*envoy_cluster_v3 return &envoy_cluster_v3.Cluster{ Name: LocalExtAuthzClusterName, - ClusterDiscoveryType: &envoy_cluster_v3.Cluster_Type{Type: envoy_cluster_v3.Cluster_STATIC}, + ClusterDiscoveryType: clusterType, ConnectTimeout: target.timeoutDurationPB(), LoadAssignment: &envoy_endpoint_v3.ClusterLoadAssignment{ ClusterName: LocalExtAuthzClusterName, @@ -645,21 +654,13 @@ func (t *Target) validate() error { } if t.isURI() { - // Strip the protocol if one was provided - if _, addr, hasProto := strings.Cut(t.URI, "://"); hasProto { - t.URI = addr - } - addr := strings.Split(t.URI, ":") - if len(addr) == 2 { - t.host = addr[0] - if t.host == "localhost" || t.host == "127.0.0.1" { - // Always use the IP address form. - t.host = "127.0.0.1" - } else { - resultErr = multierror.Append(resultErr, fmt.Errorf("invalid host for Target.URI %q: expected 'localhost' or '127.0.0.1'", t.URI)) - } - if t.port, err = strconv.Atoi(addr[1]); err != nil { - resultErr = multierror.Append(resultErr, fmt.Errorf("invalid port for Target.URI %q", addr[1])) + t.host, t.port, err = parseAddr(t.URI) + if err == nil { + switch t.host { + case localhost, localhostIPv4, localhostIPv6: + default: + resultErr = multierror.Append(resultErr, + fmt.Errorf("invalid host for Target.URI %q: expected %q, %q, or %q", t.URI, localhost, localhostIPv4, localhostIPv6)) } } else { resultErr = multierror.Append(resultErr, fmt.Errorf("invalid format for Target.URI %q: expected host:port", t.URI)) @@ -675,3 +676,22 @@ func (t *Target) validate() error { } return resultErr } + +func parseAddr(s string) (host string, port int, err error) { + // Strip the protocol if one was provided + if _, addr, hasProto := strings.Cut(s, "://"); hasProto { + s = addr + } + idx := strings.LastIndex(s, ":") + switch idx { + case -1, len(s) - 1: + err = fmt.Errorf("invalid input format %q: expected host:port", s) + case 0: + host = localhost + port, err = strconv.Atoi(s[idx+1:]) + default: + host = s[:idx] + port, err = strconv.Atoi(s[idx+1:]) + } + return +} diff --git a/agent/xds/delta_envoy_extender_oss_test.go b/agent/xds/delta_envoy_extender_oss_test.go index 3d92b6d25de..2f18809b193 100644 --- a/agent/xds/delta_envoy_extender_oss_test.go +++ b/agent/xds/delta_envoy_extender_oss_test.go @@ -676,6 +676,7 @@ end`, ns.Proxy.EnvoyExtensions = makeExtAuthzEnvoyExtension( "http", "dest=local", + "target-uri=localhost:9191", "insert=AfterLastMatch:envoy.filters.http.header_to_metadata", ) }, nil) diff --git a/agent/xds/delta_envoy_extender_test.go b/agent/xds/delta_envoy_extender_test.go index 285dc335768..6cd57fa53a0 100644 --- a/agent/xds/delta_envoy_extender_test.go +++ b/agent/xds/delta_envoy_extender_test.go @@ -12,7 +12,7 @@ import ( ) func makeExtAuthzEnvoyExtension(svc string, opts ...string) []structs.EnvoyExtension { - target := map[string]any{"URI": "localhost:9191"} + target := map[string]any{"URI": "127.0.0.1:9191"} insertOptions := map[string]any{} required := false ent := false @@ -50,6 +50,13 @@ func makeExtAuthzEnvoyExtension(svc string, opts ...string) []structs.EnvoyExten "FilterName": filterName, } } + case "target-uri": + target = map[string]any{"URI": v} + configMap = map[string]any{ + serviceKey: map[string]any{ + "Target": target, + }, + } case "config-type": if v == "full" { target["Timeout"] = "2s" diff --git a/agent/xds/testdata/builtin_extension/clusters/ext-authz-http-local-http-service.latest.golden b/agent/xds/testdata/builtin_extension/clusters/ext-authz-http-local-http-service.latest.golden index 3b0f2da69ce..992da1ae684 100644 --- a/agent/xds/testdata/builtin_extension/clusters/ext-authz-http-local-http-service.latest.golden +++ b/agent/xds/testdata/builtin_extension/clusters/ext-authz-http-local-http-service.latest.golden @@ -142,7 +142,7 @@ { "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", "name": "local_ext_authz", - "type": "STATIC", + "type": "STRICT_DNS", "loadAssignment": { "clusterName": "local_ext_authz", "endpoints": [ @@ -152,7 +152,7 @@ "endpoint": { "address": { "socketAddress": { - "address": "127.0.0.1", + "address": "localhost", "portValue": 9191 } } diff --git a/website/content/docs/connect/proxies/envoy-extensions/configuration/ext-authz.mdx b/website/content/docs/connect/proxies/envoy-extensions/configuration/ext-authz.mdx index 6b5d8cc272a..2d3c48789a9 100644 --- a/website/content/docs/connect/proxies/envoy-extensions/configuration/ext-authz.mdx +++ b/website/content/docs/connect/proxies/envoy-extensions/configuration/ext-authz.mdx @@ -348,7 +348,7 @@ The following table describes how to configure parameters for the `Service` fiel ### `Arguments.Config.GrpcService.Target.Uri` -Specifies the URI of the external authorization service. Configure this field when you must provide an explicit URI to the external authorization service, such as cases in which the authorization service is running on the same host or pod. If set, the value of this field must be either `localhost:` or `127.0.0.1:` +Specifies the URI of the external authorization service. Configure this field when you must provide an explicit URI to the external authorization service, such as cases in which the authorization service is running on the same host or pod. If set, the value of this field must be one of `localhost:`, `127.0.0.1:`, or `::1:`. Configure either the `Uri` field or the [`Service`](#arguments-config-grpcservice-target-service) field, but not both. @@ -434,7 +434,7 @@ The following table describes how to configure parameters for the `Service` fiel ### `Arguments{}.Config{}.HttpService{}.Target{}.Uri` -Specifies the URI of the external authorization service. Configure this field when you must provide an explicit URI to the external authorization service, such as cases in which the authorization service is running on the same host or pod. +Specifies the URI of the external authorization service. Configure this field when you must provide an explicit URI to the external authorization service, such as cases in which the authorization service is running on the same host or pod. If set, the value of this field must be one of `localhost:`, `127.0.0.1:`, or `::1:`. Configure either the `Uri` field or the [`Service`](#arguments-config-httpservice-target-service) field, but not both. diff --git a/website/content/docs/connect/proxies/envoy-extensions/usage/ext-authz.mdx b/website/content/docs/connect/proxies/envoy-extensions/usage/ext-authz.mdx index f3cc5432846..3062879d472 100644 --- a/website/content/docs/connect/proxies/envoy-extensions/usage/ext-authz.mdx +++ b/website/content/docs/connect/proxies/envoy-extensions/usage/ext-authz.mdx @@ -115,7 +115,7 @@ The following Envoy configurations are not supported: | `failure_mode_allow` | Set the `EnvoyExtension.Required` field to `true` in the [service defaults configuration entry](/consul/docs/connect/config-entries/service-defaults#envoyextensions) or [proxy defaults configuration entry](/consul/docs/connect/config-entries/proxy-defaults#envoyextensions). | | `filter_enabled` | Set the `EnvoyExtension.Required` field to `true` in the [service defaults configuration entry](/consul/docs/connect/config-entries/service-defaults#envoyextensions) or [proxy defaults configuration entry](/consul/docs/connect/config-entries/proxy-defaults#envoyextensions). | | `filter_enabled_metadata` | Set the `EnvoyExtension.Required` field to `true` in the [service defaults configuration entry](/consul/docs/connect/config-entries/service-defaults#envoyextensions) or [proxy defaults configuration entry](/consul/docs/connect/config-entries/proxy-defaults#envoyextensions). | -| `transport_api_version` | Consul only supports v3 of the transport API. As a result, there is no workaround for implement the behavior of this field. | +| `transport_api_version` | Consul only supports v3 of the transport API. As a result, there is no workaround for implementing the behavior of this field. | ## Apply the configuration entry