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

Save exposed ports in agent's store and expose them via API #10173

Merged
merged 6 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changelog/10173.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
```release-note:improvement
agent: Save exposed Envoy ports to the agent's state when `Expose.Checks` is true in proxy's configuration.
```
```release-note:improvement
api: Add `ExposedPort` to the health check API resource.
```
```release-note:improvement
command: Exclude exposed Envoy ports from traffic redirection when providing `-proxy-id` and `Expose.Checks` is set.
```
10 changes: 10 additions & 0 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -2555,6 +2555,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
return err
}
http.ProxyHTTP = httpInjectAddr(http.HTTP, proxy.Address, port)
check.ExposedPort = port
}

http.Start()
Expand Down Expand Up @@ -2624,6 +2625,7 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
return err
}
grpc.ProxyGRPC = grpcInjectAddr(grpc.GRPC, proxy.Address, port)
check.ExposedPort = port
}

grpc.Start()
Expand Down Expand Up @@ -3809,6 +3811,8 @@ func (a *Agent) rerouteExposedChecks(serviceID structs.ServiceID, proxyAddr stri
return err
}
c.ProxyHTTP = httpInjectAddr(c.HTTP, proxyAddr, port)
hc := a.State.Check(cid)
hc.ExposedPort = port
}
for cid, c := range a.checkGRPCs {
if c.ServiceID != serviceID {
Expand All @@ -3819,6 +3823,8 @@ func (a *Agent) rerouteExposedChecks(serviceID structs.ServiceID, proxyAddr stri
return err
}
c.ProxyGRPC = grpcInjectAddr(c.GRPC, proxyAddr, port)
hc := a.State.Check(cid)
hc.ExposedPort = port
}
return nil
}
Expand All @@ -3831,12 +3837,16 @@ func (a *Agent) resetExposedChecks(serviceID structs.ServiceID) {
for cid, c := range a.checkHTTPs {
if c.ServiceID == serviceID {
c.ProxyHTTP = ""
hc := a.State.Check(cid)
hc.ExposedPort = 0
ids = append(ids, cid)
}
}
for cid, c := range a.checkGRPCs {
if c.ServiceID == serviceID {
c.ProxyGRPC = ""
hc := a.State.Check(cid)
hc.ExposedPort = 0
ids = append(ids, cid)
}
}
Expand Down
89 changes: 40 additions & 49 deletions agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,7 @@ func TestAgent_IndexChurn(t *testing.T) {
// verifyIndexChurn registers some things and runs anti-entropy a bunch of times
// in a row to make sure there are no index bumps.
func verifyIndexChurn(t *testing.T, tags []string) {
t.Helper()
a := NewTestAgent(t, "")
defer a.Shutdown()

Expand Down Expand Up @@ -4299,8 +4300,8 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {
t.Fatalf("failed to add svc: %v", err)
}

// Register a proxy and expose HTTP checks
// This should trigger setting ProxyHTTP and ProxyGRPC in the checks
// Register a proxy and expose HTTP checks.
// This should trigger setting ProxyHTTP and ProxyGRPC in the checks.
proxy := &structs.NodeService{
Kind: "connect-proxy",
ID: "web-proxy",
Expand All @@ -4324,36 +4325,30 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
require.Equal(r, chks[0].ProxyHTTP, "http://localhost:21500/mypath?query")
})

got := chks[0].ProxyHTTP
if got == "" {
r.Fatal("proxyHTTP addr not set in check")
}

want := "http://localhost:21500/mypath?query"
if got != want {
r.Fatalf("unexpected proxy addr in check, want: %s, got: %s", want, got)
}
retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("http", nil))
require.Equal(r, hc.ExposedPort, 21500)
})

retry.Run(t, func(r *retry.R) {
chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))

// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks
got := chks[1].ProxyGRPC
if got == "" {
r.Fatal("ProxyGRPC addr not set in check")
}
// GRPC check will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks.
// Note that this relies on listener ports auto-incrementing in a.listenerPortLocked.
require.Equal(r, chks[1].ProxyGRPC, "localhost:21501/myservice")
})

// Node that this relies on listener ports auto-incrementing in a.listenerPortLocked
want := "localhost:21501/myservice"
if got != want {
r.Fatalf("unexpected proxy addr in check, want: %s, got: %s", want, got)
}
retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("grpc", nil))
require.Equal(r, hc.ExposedPort, 21501)
})

// Re-register a proxy and disable exposing HTTP checks
// Re-register a proxy and disable exposing HTTP checks.
// This should trigger resetting ProxyHTTP and ProxyGRPC to empty strings
// and reset saved exposed ports in the agent's state.
proxy = &structs.NodeService{
Kind: "connect-proxy",
ID: "web-proxy",
Expand All @@ -4377,21 +4372,24 @@ func TestAgent_RerouteExistingHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
require.Empty(r, chks[0].ProxyHTTP, "ProxyHTTP addr was not reset")
})

got := chks[0].ProxyHTTP
if got != "" {
r.Fatal("ProxyHTTP addr was not reset")
}
retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("http", nil))
require.Equal(r, hc.ExposedPort, 0)
})

retry.Run(t, func(r *retry.R) {
chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))

// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks
got := chks[1].ProxyGRPC
if got != "" {
r.Fatal("ProxyGRPC addr was not reset")
}
// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks.
require.Empty(r, chks[1].ProxyGRPC, "ProxyGRPC addr was not reset")
})

retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("grpc", nil))
require.Equal(r, hc.ExposedPort, 0)
})
}

Expand Down Expand Up @@ -4480,31 +4478,24 @@ func TestAgent_RerouteNewHTTPChecks(t *testing.T) {

retry.Run(t, func(r *retry.R) {
chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))
require.Equal(r, chks[0].ProxyHTTP, "http://localhost:21500/mypath?query")
})

got := chks[0].ProxyHTTP
if got == "" {
r.Fatal("ProxyHTTP addr not set in check")
}

want := "http://localhost:21500/mypath?query"
if got != want {
r.Fatalf("unexpected proxy addr in http check, want: %s, got: %s", want, got)
}
retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("http", nil))
require.Equal(r, hc.ExposedPort, 21500)
})

retry.Run(t, func(r *retry.R) {
chks := a.ServiceHTTPBasedChecks(structs.NewServiceID("web", nil))

// Will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks
got := chks[1].ProxyGRPC
if got == "" {
r.Fatal("ProxyGRPC addr not set in check")
}
// GRPC check will be at a later index than HTTP check because of the fetching order in ServiceHTTPBasedChecks.
require.Equal(r, chks[1].ProxyGRPC, "localhost:21501/myservice")
})

want := "localhost:21501/myservice"
if got != want {
r.Fatalf("unexpected proxy addr in grpc check, want: %s, got: %s", want, got)
}
retry.Run(t, func(r *retry.R) {
hc := a.State.Check(structs.NewCheckID("grpc", nil))
require.Equal(r, hc.ExposedPort, 21501)
})
}

Expand Down
6 changes: 5 additions & 1 deletion agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1400,7 +1400,7 @@ type NodeServiceList struct {
Services []*NodeService
}

// HealthCheck represents a single check on a given node
// HealthCheck represents a single check on a given node.
type HealthCheck struct {
Node string
CheckID types.CheckID // Unique per-node ID
Expand All @@ -1413,6 +1413,10 @@ type HealthCheck struct {
ServiceTags []string // optional service tags
Type string // Check type: http/ttl/tcp/etc

// ExposedPort is the port of the exposed Envoy listener representing the
// HTTP or GRPC health check of the service.
ExposedPort int

Definition HealthCheckDefinition `bexpr:"-"`

EnterpriseMeta `hcl:",squash" mapstructure:",squash" bexpr:"-"`
Expand Down
7 changes: 6 additions & 1 deletion agent/structs/structs_filtering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"sync"
"testing"

bexpr "github.com/hashicorp/go-bexpr"
"github.com/hashicorp/go-bexpr"
"github.com/mitchellh/pointerstructure"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -545,6 +545,11 @@ var expectedFieldConfigHealthCheck bexpr.FieldConfigurations = bexpr.FieldConfig
SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual, bexpr.MatchIn, bexpr.MatchNotIn, bexpr.MatchMatches, bexpr.MatchNotMatches},
StructFieldName: "Type",
},
"ExposedPort": &bexpr.FieldConfiguration{
CoerceFn: bexpr.CoerceInt,
SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual},
StructFieldName: "ExposedPort",
},
}

var expectedFieldConfigCheckServiceNode bexpr.FieldConfigurations = bexpr.FieldConfigurations{
Expand Down
1 change: 1 addition & 0 deletions api/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type AgentCheck struct {
ServiceID string
ServiceName string
Type string
ExposedPort int
Definition HealthCheckDefinition
Namespace string `json:",omitempty"`
}
Expand Down
15 changes: 15 additions & 0 deletions command/connect/redirecttraffic/redirect_traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,21 @@ func (c *cmd) generateConfigFromFlags() (iptables.Config, error) {
cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(exposePath.ListenerPort))
}
}

// Exclude any exposed health check ports when Proxy.Expose.Checks is true.
if svc.Proxy.Expose.Checks {
// Get the health checks of the destination service.
checks, err := c.client.Agent().ChecksWithFilter(fmt.Sprintf("ServiceName == %q", svc.Proxy.DestinationServiceName))
if err != nil {
return iptables.Config{}, err
}

for _, check := range checks {
if check.ExposedPort != 0 {
cfg.ExcludeInboundPorts = append(cfg.ExcludeInboundPorts, strconv.Itoa(check.ExposedPort))
}
}
}
}

for _, port := range c.excludeInboundPorts {
Expand Down