Relax validation for expose.paths config (#10394)

Previously we would return an error if duplicate paths were specified.
This could lead to problems in cases where a user has the same path,
say /healthz, on two different ports.

This validation was added to signal a potential misconfiguration.
Instead we will only check for duplicate listener ports, since that is
what would lead to ambiguity issues when generating xDS config.

In the future we could look into using a single listener and creating
distinct filter chains for each path/port.
This commit is contained in:
Freddy 2021-06-14 14:04:11 -06:00 committed by GitHub
parent c3eacac764
commit f19b1f0058
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 24 deletions

3
.changelog/10394.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
connect: allow exposing duplicate HTTP paths through a proxy instance.
```

View File

@ -1230,18 +1230,13 @@ func (s *NodeService) Validate() error {
} }
bindAddrs[addr] = struct{}{} bindAddrs[addr] = struct{}{}
} }
var knownPaths = make(map[string]bool)
var knownListeners = make(map[int]bool) var knownListeners = make(map[int]bool)
for _, path := range s.Proxy.Expose.Paths { for _, path := range s.Proxy.Expose.Paths {
if path.Path == "" { if path.Path == "" {
result = multierror.Append(result, fmt.Errorf("expose.paths: empty path exposed")) result = multierror.Append(result, fmt.Errorf("expose.paths: empty path exposed"))
} }
if seen := knownPaths[path.Path]; seen {
result = multierror.Append(result, fmt.Errorf("expose.paths: duplicate paths exposed"))
}
knownPaths[path.Path] = true
if seen := knownListeners[path.ListenerPort]; seen { if seen := knownListeners[path.ListenerPort]; seen {
result = multierror.Append(result, fmt.Errorf("expose.paths: duplicate listener ports exposed")) result = multierror.Append(result, fmt.Errorf("expose.paths: duplicate listener ports exposed"))
} }

View File

@ -574,38 +574,38 @@ func TestStructs_NodeService_ValidateExposeConfig(t *testing.T) {
} }
cases := map[string]testCase{ cases := map[string]testCase{
"valid": { "valid": {
func(x *NodeService) {}, Modify: func(x *NodeService) {},
"", Err: "",
}, },
"empty path": { "empty path": {
func(x *NodeService) { x.Proxy.Expose.Paths[0].Path = "" }, Modify: func(x *NodeService) { x.Proxy.Expose.Paths[0].Path = "" },
"empty path exposed", Err: "empty path exposed",
}, },
"invalid port negative": { "invalid port negative": {
func(x *NodeService) { x.Proxy.Expose.Paths[0].ListenerPort = -1 }, Modify: func(x *NodeService) { x.Proxy.Expose.Paths[0].ListenerPort = -1 },
"invalid listener port", Err: "invalid listener port",
}, },
"invalid port too large": { "invalid port too large": {
func(x *NodeService) { x.Proxy.Expose.Paths[0].ListenerPort = 65536 }, Modify: func(x *NodeService) { x.Proxy.Expose.Paths[0].ListenerPort = 65536 },
"invalid listener port", Err: "invalid listener port",
}, },
"duplicate paths": { "duplicate paths are allowed": {
func(x *NodeService) { Modify: func(x *NodeService) {
x.Proxy.Expose.Paths[0].Path = "/metrics" x.Proxy.Expose.Paths[0].Path = "/healthz"
x.Proxy.Expose.Paths[1].Path = "/metrics" x.Proxy.Expose.Paths[1].Path = "/healthz"
}, },
"duplicate paths exposed", Err: "",
}, },
"duplicate ports": { "duplicate listener ports are not allowed": {
func(x *NodeService) { Modify: func(x *NodeService) {
x.Proxy.Expose.Paths[0].ListenerPort = 21600 x.Proxy.Expose.Paths[0].ListenerPort = 21600
x.Proxy.Expose.Paths[1].ListenerPort = 21600 x.Proxy.Expose.Paths[1].ListenerPort = 21600
}, },
"duplicate listener ports exposed", Err: "duplicate listener ports exposed",
}, },
"protocol not supported": { "protocol not supported": {
func(x *NodeService) { x.Proxy.Expose.Paths[0].Protocol = "foo" }, Modify: func(x *NodeService) { x.Proxy.Expose.Paths[0].Protocol = "foo" },
"protocol 'foo' not supported for path", Err: "protocol 'foo' not supported for path",
}, },
} }