diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go index 9dbff0d1e7..1abe0c47dc 100644 --- a/agent/consul/discoverychain/compile_test.go +++ b/agent/consul/discoverychain/compile_test.go @@ -23,10 +23,13 @@ func TestCompile_NoEntries_NoInferDefaults(t *testing.T) { } type compileTestCase struct { - entries *structs.DiscoveryChainConfigEntries - expect *structs.CompiledDiscoveryChain // the GroupResolverNodes map should have nil values - expectErr string - expectGraphErr bool + entries *structs.DiscoveryChainConfigEntries + // expect: the GroupResolverNodes map should have nil values + expect *structs.CompiledDiscoveryChain + // expectIsDefault tests behavior of CompiledDiscoveryChain.IsDefault() + expectIsDefault bool + expectErr string + expectGraphErr bool } func TestCompile(t *testing.T) { @@ -40,7 +43,7 @@ func TestCompile(t *testing.T) { "router with defaults and noop split and resolver": testcase_RouterWithDefaults_WithNoopSplit_WithResolver(), "route bypasses splitter": testcase_RouteBypassesSplit(), "noop split": testcase_NoopSplit_DefaultResolver(), - "noop split with protocol from proxy defaults": testcase_NoopSplit_DefaultResolver_ProcotolFromProxyDefaults(), + "noop split with protocol from proxy defaults": testcase_NoopSplit_DefaultResolver_ProtocolFromProxyDefaults(), "noop split with resolver": testcase_NoopSplit_WithResolver(), "subset split": testcase_SubsetSplit(), "service split": testcase_ServiceSplit(), @@ -55,6 +58,7 @@ func TestCompile(t *testing.T) { "resolver with default subset": testcase_Resolve_WithDefaultSubset(), "resolver with no entries and inferring defaults": testcase_DefaultResolver(), "default resolver with proxy defaults": testcase_DefaultResolver_WithProxyDefaults(), + "service redirect to service with default resolver is not a default chain": testcase_RedirectToDefaultResolverIsNotDefaultChain(), // TODO(rb): handle this case better: "circular split": testcase_CircularSplit(), "all the bells and whistles": testcase_AllBellsAndWhistles(), @@ -125,6 +129,7 @@ func TestCompile(t *testing.T) { } require.Equal(t, tc.expect, res) + require.Equal(t, tc.expectIsDefault, res.IsDefault()) } }) } @@ -298,7 +303,7 @@ func testcase_RouterWithDefaults_WithNoopSplit_DefaultResolver() compileTestCase return compileTestCase{entries: entries, expect: expect} } -func testcase_NoopSplit_DefaultResolver_ProcotolFromProxyDefaults() compileTestCase { +func testcase_NoopSplit_DefaultResolver_ProtocolFromProxyDefaults() compileTestCase { entries := newEntries() setGlobalProxyProtocol(entries, "http") @@ -1230,7 +1235,7 @@ func testcase_DefaultResolver() compileTestCase { newTarget("main", "", "default", "dc1"): nil, }, } - return compileTestCase{entries: entries, expect: expect} + return compileTestCase{entries: entries, expect: expect, expectIsDefault: true} } func testcase_DefaultResolver_WithProxyDefaults() compileTestCase { @@ -1273,7 +1278,47 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase { newTarget("main", "", "default", "dc1"): nil, }, } - return compileTestCase{entries: entries, expect: expect} + return compileTestCase{entries: entries, expect: expect, expectIsDefault: true} +} + +func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase { + entries := newEntries() + entries.AddResolvers( + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "main", + Redirect: &structs.ServiceResolverRedirect{ + Service: "other", + }, + }, + ) + + resolver := newDefaultServiceResolver("other") + + expect := &structs.CompiledDiscoveryChain{ + Protocol: "tcp", + Node: &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeGroupResolver, + Name: "other", + GroupResolver: &structs.DiscoveryGroupResolver{ + Definition: resolver, + Default: true, + ConnectTimeout: 5 * time.Second, + Target: newTarget("other", "", "default", "dc1"), + }, + }, + Resolvers: map[string]*structs.ServiceResolverConfigEntry{ + "other": resolver, + }, + Targets: []structs.DiscoveryTarget{ + newTarget("other", "", "default", "dc1"), + }, + GroupResolverNodes: map[structs.DiscoveryTarget]*structs.DiscoveryGraphNode{ + newTarget("other", "", "default", "dc1"): nil, + }, + } + + return compileTestCase{entries: entries, expect: expect, expectIsDefault: false /*being explicit here because this is the whole point of this test*/} } func testcase_Resolve_WithDefaultSubset() compileTestCase { diff --git a/agent/http_oss.go b/agent/http_oss.go index e9ae3a6fe9..fb4e10fdaf 100644 --- a/agent/http_oss.go +++ b/agent/http_oss.go @@ -88,6 +88,7 @@ func init() { registerEndpoint("/v1/health/state/", []string{"GET"}, (*HTTPServer).HealthChecksInState) registerEndpoint("/v1/health/service/", []string{"GET"}, (*HTTPServer).HealthServiceNodes) registerEndpoint("/v1/health/connect/", []string{"GET"}, (*HTTPServer).HealthConnectServiceNodes) + registerEndpoint("/v1/internal/discovery-chain/", []string{"GET"}, (*HTTPServer).InternalDiscoveryChain) registerEndpoint("/v1/internal/ui/nodes", []string{"GET"}, (*HTTPServer).UINodes) registerEndpoint("/v1/internal/ui/node/", []string{"GET"}, (*HTTPServer).UINodeInfo) registerEndpoint("/v1/internal/ui/services", []string{"GET"}, (*HTTPServer).UIServices) diff --git a/agent/internal_endpoint.go b/agent/internal_endpoint.go new file mode 100644 index 0000000000..da7d845264 --- /dev/null +++ b/agent/internal_endpoint.go @@ -0,0 +1,40 @@ +package agent + +import ( + "fmt" + "net/http" + "strings" + + "github.com/hashicorp/consul/agent/structs" +) + +// InternalDiscoveryChain is helpful for debugging. Eventually we should expose +// this data officially somehow. +func (s *HTTPServer) InternalDiscoveryChain(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + var args structs.DiscoveryChainRequest + if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { + return nil, nil + } + + args.Name = strings.TrimPrefix(req.URL.Path, "/v1/internal/discovery-chain/") + if args.Name == "" { + resp.WriteHeader(http.StatusBadRequest) + fmt.Fprint(resp, "Missing chain name") + return nil, nil + } + + // Make the RPC request + var out structs.DiscoveryChainResponse + defer setMeta(resp, &out.QueryMeta) + + if err := s.agent.RPC("ConfigEntry.ReadDiscoveryChain", &args, &out); err != nil { + return nil, err + } + + if out.Chain == nil { + resp.WriteHeader(http.StatusNotFound) + return nil, nil + } + + return out.Chain, nil +} diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index f6ee92d947..cf95314a36 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -40,11 +40,18 @@ type CompiledDiscoveryChain struct { Targets []DiscoveryTarget `json:",omitempty"` } +// IsDefault returns true if the compiled chain represents no routing, no +// splitting, and only the default resolution. We have to be careful here to +// avoid returning "yep this is default" when the only resolver action being +// applied is redirection to another resolver that is default, so we double +// check the resolver matches the requested resolver. func (c *CompiledDiscoveryChain) IsDefault() bool { if c.Node == nil { return true } - return c.Node.Type == DiscoveryGraphNodeTypeGroupResolver && c.Node.GroupResolver.Default + return c.Node.Name == c.ServiceName && + c.Node.Type == DiscoveryGraphNodeTypeGroupResolver && + c.Node.GroupResolver.Default } const ( diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/config_entries.hcl b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/config_entries.hcl new file mode 100644 index 0000000000..f5f3312d2b --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/config_entries.hcl @@ -0,0 +1,21 @@ +enable_central_service_config = true + +config_entries { + bootstrap { + kind = "proxy-defaults" + name = "global" + + config { + protocol = "http" + } + } + + bootstrap { + kind = "service-resolver" + name = "s2" + + redirect { + service = "s3" + } + } +} diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/s3.hcl b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/s3.hcl new file mode 100644 index 0000000000..c8761365b7 --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/s3.hcl @@ -0,0 +1,5 @@ +services { + name = "s3" + port = 8282 + connect { sidecar_service {} } +} \ No newline at end of file diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/setup.sh b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/setup.sh new file mode 100644 index 0000000000..03ba66933a --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/setup.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +set -euo pipefail + +# retry because resolving the central config might race +retry_default gen_envoy_bootstrap s1 19000 +retry_default gen_envoy_bootstrap s2 19001 +retry_default gen_envoy_bootstrap s3 19002 + +export REQUIRED_SERVICES="s1 s1-sidecar-proxy s2 s2-sidecar-proxy s3 s3-sidecar-proxy" diff --git a/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/verify.bats b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/verify.bats new file mode 100644 index 0000000000..260cc7f6a3 --- /dev/null +++ b/test/integration/connect/envoy/case-cfg-resolver-svc-redirect/verify.bats @@ -0,0 +1,46 @@ +#!/usr/bin/env bats + +load helpers + +@test "s1 proxy admin is up on :19000" { + retry_default curl -f -s localhost:19000/stats -o /dev/null +} + +@test "s2 proxy admin is up on :19001" { + retry_default curl -f -s localhost:19001/stats -o /dev/null +} + +@test "s3 proxy admin is up on :19002" { + retry_default curl -f -s localhost:19002/stats -o /dev/null +} + +@test "s1 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21000 s1 +} + +@test "s2 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21001 s2 +} + +@test "s3 proxy listener should be up and have right cert" { + assert_proxy_presents_cert_uri localhost:21002 s3 +} + +@test "s3 proxy should be healthy" { + assert_service_has_healthy_instances s3 1 +} + +@test "s1 upstream should have healthy endpoints for s3" { + assert_upstream_has_healthy_endpoints 127.0.0.1:19000 s3 1 +} + +@test "s1 upstream should be able to connect to its upstream simply" { + run retry_default curl -s -f -d hello localhost:5000 + [ "$status" -eq 0 ] + [ "$output" = "hello" ] +} + +@test "s1 upstream should be able to connect to s3 via upstream s2" { + assert_expected_fortio_name s3 +} + diff --git a/test/integration/connect/envoy/docker-compose.yml b/test/integration/connect/envoy/docker-compose.yml index 35d0f58634..0de581695d 100644 --- a/test/integration/connect/envoy/docker-compose.yml +++ b/test/integration/connect/envoy/docker-compose.yml @@ -46,24 +46,48 @@ services: depends_on: - consul image: "fortio/fortio" + environment: + - "FORTIO_NAME=s1" command: - "server" - "-http-port" - ":8080" - "-grpc-port" - ":8079" + - "-redirect-port" + - "disabled" network_mode: service:consul s2: depends_on: - consul image: "fortio/fortio" + environment: + - "FORTIO_NAME=s2" command: - "server" - "-http-port" - ":8181" - "-grpc-port" - ":8179" + - "-redirect-port" + - "disabled" + network_mode: service:consul + + s3: + depends_on: + - consul + image: "fortio/fortio" + environment: + - "FORTIO_NAME=s3" + command: + - "server" + - "-http-port" + - ":8282" + - "-grpc-port" + - ":8279" + - "-redirect-port" + - "disabled" network_mode: service:consul s1-sidecar-proxy: @@ -108,6 +132,27 @@ services: - *workdir-volume network_mode: service:consul + s3-sidecar-proxy: + depends_on: + - consul + image: "envoyproxy/envoy:v${ENVOY_VERSION:-1.8.0}" + command: + - "envoy" + - "-c" + - "/workdir/envoy/s3-bootstrap.json" + - "-l" + - "debug" + # Hot restart breaks since both envoys seem to interact with each other + # despite separate containers that don't share IPC namespace. Not quite + # sure how this happens but may be due to unix socket being in some shared + # location? + - "--disable-hot-restart" + - "--drain-time-s" + - "1" + volumes: + - *workdir-volume + network_mode: service:consul + verify: depends_on: - consul diff --git a/test/integration/connect/envoy/helpers.bash b/test/integration/connect/envoy/helpers.bash index 9b933df12e..a051228d16 100755 --- a/test/integration/connect/envoy/helpers.bash +++ b/test/integration/connect/envoy/helpers.bash @@ -253,3 +253,18 @@ function gen_envoy_bootstrap { return $status fi } + +function get_upstream_fortio_name { + run retry_default curl -v -s -f localhost:5000/debug?env=dump + [ "$status" == 0 ] + echo "$output" | grep -E "^FORTIO_NAME=" +} + +function assert_expected_fortio_name { + local EXPECT_NAME=$1 + + GOT=$(get_upstream_fortio_name) + echo "GOT $GOT" + + [ "$GOT" == "FORTIO_NAME=${EXPECT_NAME}" ] +}