From da91e999f65332e4831cd9211166090e1ad4185f Mon Sep 17 00:00:00 2001 From: Freddy Date: Wed, 7 Oct 2020 18:35:34 -0600 Subject: [PATCH] Return intention info in svc topology endpoint (#8853) --- agent/connect_auth.go | 6 +- agent/consul/helper_test.go | 55 +++++ agent/consul/intention_endpoint.go | 59 +----- agent/consul/internal_endpoint.go | 7 +- agent/consul/internal_endpoint_test.go | 111 +++++++--- agent/consul/state/catalog.go | 40 +++- agent/consul/state/intention.go | 56 +++++ agent/consul/state/intention_test.go | 119 +++++++++++ agent/structs/intention.go | 11 + agent/structs/structs.go | 6 + agent/ui_endpoint.go | 67 ++++-- agent/ui_endpoint_test.go | 279 ++++++++++++++++--------- 12 files changed, 611 insertions(+), 205 deletions(-) diff --git a/agent/connect_auth.go b/agent/connect_auth.go index b3084e38b8..c498cb2e23 100644 --- a/agent/connect_auth.go +++ b/agent/connect_auth.go @@ -125,13 +125,9 @@ func (a *Agent) ConnectAuthorize(token string, } // No match, we need to determine the default behavior. We do this by - // specifying the anonymous token, which will get the default behavior. The + // fetching the default intention behavior from the resolved authorizer. The // default behavior if ACLs are disabled is to allow connections to mimic the // behavior of Consul itself: everything is allowed if ACLs are disabled. - authz, err = a.resolveToken("") - if err != nil { - return returnErr(err) - } if authz == nil { // ACLs not enabled at all, the default is allow all. return true, "ACLs disabled, access is allowed by default", &meta, nil diff --git a/agent/consul/helper_test.go b/agent/consul/helper_test.go index 7c28649bdd..c194ba37f9 100644 --- a/agent/consul/helper_test.go +++ b/agent/consul/helper_test.go @@ -844,4 +844,59 @@ func registerTestTopologyEntries(t *testing.T, codec rpc.ClientCodec, token stri }, } registerTestCatalogEntriesMap(t, codec, registrations) + + // Add intentions: deny all, web -> redis with L7 perms, but omit intention for api -> web + entries := []structs.ConfigEntryRequest{ + { + Datacenter: "dc1", + Entry: &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + Config: map[string]interface{}{ + "protocol": "http", + }, + }, + WriteRequest: structs.WriteRequest{Token: token}, + }, + { + Datacenter: "dc1", + Entry: &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "redis", + Sources: []*structs.SourceIntention{ + { + Name: "web", + Permissions: []*structs.IntentionPermission{ + { + Action: structs.IntentionActionAllow, + HTTP: &structs.IntentionHTTPPermission{ + Methods: []string{"GET"}, + }, + }, + }, + }, + }, + }, + WriteRequest: structs.WriteRequest{Token: token}, + }, + { + Datacenter: "dc1", + Entry: &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "*", + Meta: map[string]string{structs.MetaExternalSource: "nomad"}, + Sources: []*structs.SourceIntention{ + { + Name: "*", + Action: structs.IntentionActionDeny, + }, + }, + }, + WriteRequest: structs.WriteRequest{Token: token}, + }, + } + for _, req := range entries { + var out bool + require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.Apply", &req, &out)) + } } diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index 29b13be290..95cb5183d9 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -783,53 +783,11 @@ func (s *Intention) Check( } } - // Get the matches for this destination - state := s.srv.fsm.State() - _, matches, err := state.IntentionMatch(nil, &structs.IntentionQueryMatch{ - Type: structs.IntentionMatchDestination, - Entries: []structs.IntentionMatchEntry{ - { - Namespace: query.DestinationNS, - Name: query.DestinationName, - }, - }, - }) - if err != nil { - return err - } - if len(matches) != 1 { - // This should never happen since the documented behavior of the - // Match call is that it'll always return exactly the number of results - // as entries passed in. But we guard against misbehavior. - return errors.New("internal error loading matches") - } - - // Figure out which source matches this request. - var ixnMatch *structs.Intention - for _, ixn := range matches[0] { - if _, ok := uri.Authorize(ixn); ok { - ixnMatch = ixn - break - } - } - - if ixnMatch != nil { - if len(ixnMatch.Permissions) == 0 { - // This is an L4 intention. - reply.Allowed = ixnMatch.Action == structs.IntentionActionAllow - return nil - } - - // This is an L7 intention, so DENY. - reply.Allowed = false - return nil - } - // Note: the default intention policy is like an intention with a // wildcarded destination in that it is limited to L4-only. // No match, we need to determine the default behavior. We do this by - // specifying the anonymous token token, which will get that behavior. + // fetching the default intention behavior from the resolved authorizer. // The default behavior if ACLs are disabled is to allow connections // to mimic the behavior of Consul itself: everything is allowed if // ACLs are disabled. @@ -837,15 +795,18 @@ func (s *Intention) Check( // NOTE(mitchellh): This is the same behavior as the agent authorize // endpoint. If this behavior is incorrect, we should also change it there // which is much more important. - authz, err = s.srv.ResolveToken("") - if err != nil { - return err + defaultDecision := acl.Allow + if authz != nil { + defaultDecision = authz.IntentionDefaultAllow(nil) } - reply.Allowed = true - if authz != nil { - reply.Allowed = authz.IntentionDefaultAllow(nil) == acl.Allow + state := s.srv.fsm.State() + decision, err := state.IntentionDecision(uri, query.DestinationName, query.DestinationNS, defaultDecision) + if err != nil { + return fmt.Errorf("failed to get intention decision from (%s/%s) to (%s/%s): %v", + query.SourceNS, query.SourceName, query.DestinationNS, query.DestinationName, err) } + reply.Allowed = decision.Allowed return nil } diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index 1464ea06cf..d98f2ce71a 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -168,7 +168,12 @@ func (m *Internal) ServiceTopology(args *structs.ServiceSpecificRequest, reply * &args.QueryOptions, &reply.QueryMeta, func(ws memdb.WatchSet, state *state.Store) error { - index, topology, err := state.ServiceTopology(ws, args.Datacenter, args.ServiceName, &args.EnterpriseMeta) + defaultAllow := acl.Allow + if authz != nil { + defaultAllow = authz.IntentionDefaultAllow(nil) + } + + index, topology, err := state.ServiceTopology(ws, args.Datacenter, args.ServiceName, defaultAllow, &args.EnterpriseMeta) if err != nil { return err } diff --git a/agent/consul/internal_endpoint_test.go b/agent/consul/internal_endpoint_test.go index b14dc4a573..15d57e8bdc 100644 --- a/agent/consul/internal_endpoint_test.go +++ b/agent/consul/internal_endpoint_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib/stringslice" + "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/consul/types" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" @@ -1623,49 +1624,95 @@ func TestInternal_ServiceTopology(t *testing.T) { // redis and redis-proxy on node zip registerTestTopologyEntries(t, codec, "") - t.Run("api", func(t *testing.T) { - args := structs.ServiceSpecificRequest{ - Datacenter: "dc1", - ServiceName: "api", - } - var out structs.IndexedServiceTopology - require.NoError(t, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) - require.False(t, out.FilteredByACLs) + var ( + api = structs.NewServiceName("api", structs.DefaultEnterpriseMeta()) + web = structs.NewServiceName("web", structs.DefaultEnterpriseMeta()) + redis = structs.NewServiceName("redis", structs.DefaultEnterpriseMeta()) + ) - // bar/web, bar/web-proxy, baz/web, baz/web-proxy - require.Len(t, out.ServiceTopology.Upstreams, 4) - require.Len(t, out.ServiceTopology.Downstreams, 0) + t.Run("api", func(t *testing.T) { + retry.Run(t, func(r *retry.R) { + args := structs.ServiceSpecificRequest{ + Datacenter: "dc1", + ServiceName: "api", + } + var out structs.IndexedServiceTopology + require.NoError(r, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) + require.False(r, out.FilteredByACLs) + + // bar/web, bar/web-proxy, baz/web, baz/web-proxy + require.Len(r, out.ServiceTopology.Upstreams, 4) + require.Len(r, out.ServiceTopology.Downstreams, 0) + + expectUp := map[string]structs.IntentionDecisionSummary{ + web.String(): { + Allowed: false, + HasPermissions: false, + ExternalSource: "nomad", + }, + } + require.Equal(r, expectUp, out.ServiceTopology.UpstreamDecisions) + }) }) t.Run("web", func(t *testing.T) { - args := structs.ServiceSpecificRequest{ - Datacenter: "dc1", - ServiceName: "web", - } - var out structs.IndexedServiceTopology - require.NoError(t, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) - require.False(t, out.FilteredByACLs) + retry.Run(t, func(r *retry.R) { + args := structs.ServiceSpecificRequest{ + Datacenter: "dc1", + ServiceName: "web", + } + var out structs.IndexedServiceTopology + require.NoError(r, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) + require.False(r, out.FilteredByACLs) - // foo/api, foo/api-proxy - require.Len(t, out.ServiceTopology.Upstreams, 2) + // foo/api, foo/api-proxy + require.Len(r, out.ServiceTopology.Downstreams, 2) - // zip/redis, zip/redis-proxy - require.Len(t, out.ServiceTopology.Downstreams, 2) + expectDown := map[string]structs.IntentionDecisionSummary{ + api.String(): { + Allowed: false, + HasPermissions: false, + ExternalSource: "nomad", + }, + } + require.Equal(r, expectDown, out.ServiceTopology.DownstreamDecisions) + + // zip/redis, zip/redis-proxy + require.Len(r, out.ServiceTopology.Upstreams, 2) + + expectUp := map[string]structs.IntentionDecisionSummary{ + redis.String(): { + Allowed: false, + HasPermissions: true, + }, + } + require.Equal(r, expectUp, out.ServiceTopology.UpstreamDecisions) + }) }) t.Run("redis", func(t *testing.T) { - args := structs.ServiceSpecificRequest{ - Datacenter: "dc1", - ServiceName: "redis", - } - var out structs.IndexedServiceTopology - require.NoError(t, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) - require.False(t, out.FilteredByACLs) + retry.Run(t, func(r *retry.R) { + args := structs.ServiceSpecificRequest{ + Datacenter: "dc1", + ServiceName: "redis", + } + var out structs.IndexedServiceTopology + require.NoError(r, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) + require.False(r, out.FilteredByACLs) - require.Len(t, out.ServiceTopology.Upstreams, 0) + require.Len(r, out.ServiceTopology.Upstreams, 0) - // bar/web, bar/web-proxy, baz/web, baz/web-proxy - require.Len(t, out.ServiceTopology.Downstreams, 4) + // bar/web, bar/web-proxy, baz/web, baz/web-proxy + require.Len(r, out.ServiceTopology.Downstreams, 4) + + expectDown := map[string]structs.IntentionDecisionSummary{ + web.String(): { + Allowed: false, + HasPermissions: true, + }, + } + require.Equal(r, expectDown, out.ServiceTopology.DownstreamDecisions) + }) }) } diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 6c69a59e6f..fff8ba1556 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -6,6 +6,8 @@ import ( "reflect" "strings" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" @@ -2911,6 +2913,7 @@ func checkProtocolMatch(tx ReadTxn, ws memdb.WatchSet, svc *structs.GatewayServi func (s *Store) ServiceTopology( ws memdb.WatchSet, dc, service string, + defaultAllow acl.EnforcementDecision, entMeta *structs.EnterpriseMeta, ) (uint64, *structs.ServiceTopology, error) { tx := s.db.ReadTxn() @@ -2936,6 +2939,22 @@ func (s *Store) ServiceTopology( maxIdx = idx } + upstreamDecisions := make(map[string]structs.IntentionDecisionSummary) + + // The given service is the source relative to upstreams + sourceURI := connect.SpiffeIDService{ + Namespace: entMeta.NamespaceOrDefault(), + Service: service, + } + for _, un := range upstreamNames { + decision, err := s.IntentionDecision(&sourceURI, un.Name, un.NamespaceOrDefault(), defaultAllow) + if err != nil { + return 0, nil, fmt.Errorf("failed to get intention decision from (%s/%s) to (%s/%s): %v", + sourceURI.Namespace, sourceURI.Service, un.Name, un.NamespaceOrDefault(), err) + } + upstreamDecisions[un.String()] = decision + } + idx, downstreamNames, err := s.downstreamsForServiceTxn(tx, ws, dc, sn) if err != nil { return 0, nil, err @@ -2951,9 +2970,26 @@ func (s *Store) ServiceTopology( maxIdx = idx } + downstreamDecisions := make(map[string]structs.IntentionDecisionSummary) + for _, dn := range downstreamNames { + // Downstreams are the source relative to the given service + sourceURI := connect.SpiffeIDService{ + Namespace: dn.NamespaceOrDefault(), + Service: dn.Name, + } + decision, err := s.IntentionDecision(&sourceURI, service, entMeta.NamespaceOrDefault(), defaultAllow) + if err != nil { + return 0, nil, fmt.Errorf("failed to get intention decision from (%s/%s) to (%s/%s): %v", + sourceURI.Namespace, sourceURI.Service, service, dn.NamespaceOrDefault(), err) + } + downstreamDecisions[dn.String()] = decision + } + resp := &structs.ServiceTopology{ - Upstreams: upstreams, - Downstreams: downstreams, + Upstreams: upstreams, + Downstreams: downstreams, + UpstreamDecisions: upstreamDecisions, + DownstreamDecisions: downstreamDecisions, } return maxIdx, resp, nil } diff --git a/agent/consul/state/intention.go b/agent/consul/state/intention.go index 24e0359d8c..8381f048d7 100644 --- a/agent/consul/state/intention.go +++ b/agent/consul/state/intention.go @@ -5,6 +5,8 @@ import ( "fmt" "sort" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/go-memdb" ) @@ -447,6 +449,60 @@ func (s *Store) LegacyIntentionDeleteAll(idx uint64) error { return tx.Commit() } +// IntentionDecision returns whether a connection should be allowed from a source URI to some destination +// It returns true or false for the enforcement, and also a boolean for whether +func (s *Store) IntentionDecision( + srcURI connect.CertURI, dstName, dstNS string, defaultDecision acl.EnforcementDecision, +) (structs.IntentionDecisionSummary, error) { + + _, matches, err := s.IntentionMatch(nil, &structs.IntentionQueryMatch{ + Type: structs.IntentionMatchDestination, + Entries: []structs.IntentionMatchEntry{ + { + Namespace: dstNS, + Name: dstName, + }, + }, + }) + if err != nil { + return structs.IntentionDecisionSummary{}, err + } + if len(matches) != 1 { + // This should never happen since the documented behavior of the + // Match call is that it'll always return exactly the number of results + // as entries passed in. But we guard against misbehavior. + return structs.IntentionDecisionSummary{}, errors.New("internal error loading matches") + } + + // Figure out which source matches this request. + var ixnMatch *structs.Intention + for _, ixn := range matches[0] { + if _, ok := srcURI.Authorize(ixn); ok { + ixnMatch = ixn + break + } + } + + var resp structs.IntentionDecisionSummary + if ixnMatch == nil { + // No intention found, fall back to default + resp.Allowed = defaultDecision == acl.Allow + return resp, nil + } + + // Intention found, combine action + permissions + resp.Allowed = ixnMatch.Action == structs.IntentionActionAllow + if len(ixnMatch.Permissions) > 0 { + // If there are L7 permissions, DENY. + // We are only evaluating source and destination, not the request that will be sent. + resp.Allowed = false + resp.HasPermissions = true + } + resp.ExternalSource = ixnMatch.Meta[structs.MetaExternalSource] + + return resp, nil +} + // IntentionMatch returns the list of intentions that match the namespace and // name for either a source or destination. This applies the resolution rules // so wildcards will match any value. diff --git a/agent/consul/state/intention_test.go b/agent/consul/state/intention_test.go index ecab833132..1b14daecf8 100644 --- a/agent/consul/state/intention_test.go +++ b/agent/consul/state/intention_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/go-memdb" @@ -1083,6 +1085,123 @@ func TestStore_LegacyIntention_Snapshot_Restore(t *testing.T) { }() } +// Note: This test does not have an equivalent with legacy intentions as an input. +// That's because the config vs legacy split is handled by store.IntentionMatch +// which has its own tests +func TestStore_IntentionDecision(t *testing.T) { + // web to redis allowed and with permissions + // api to redis denied and without perms (so redis has multiple matches as destination) + // api to web without permissions and with meta + entries := []structs.ConfigEntry{ + &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + Config: map[string]interface{}{ + "protocol": "http", + }, + }, + &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "redis", + Sources: []*structs.SourceIntention{ + { + Name: "web", + Permissions: []*structs.IntentionPermission{ + { + Action: structs.IntentionActionAllow, + HTTP: &structs.IntentionHTTPPermission{ + Methods: []string{"GET"}, + }, + }, + }, + }, + { + Name: "api", + Action: structs.IntentionActionDeny, + }, + }, + }, + &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "web", + Meta: map[string]string{structs.MetaExternalSource: "nomad"}, + Sources: []*structs.SourceIntention{ + { + Name: "api", + Action: structs.IntentionActionAllow, + }, + }, + }, + } + + s := testConfigStateStore(t) + for _, entry := range entries { + require.NoError(t, s.EnsureConfigEntry(1, entry, nil)) + } + + tt := []struct { + name string + src string + dst string + defaultDecision acl.EnforcementDecision + expect structs.IntentionDecisionSummary + }{ + { + name: "no matching intention and default deny", + src: "does-not-exist", + dst: "ditto", + defaultDecision: acl.Deny, + expect: structs.IntentionDecisionSummary{Allowed: false}, + }, + { + name: "no matching intention and default allow", + src: "does-not-exist", + dst: "ditto", + defaultDecision: acl.Allow, + expect: structs.IntentionDecisionSummary{Allowed: true}, + }, + { + name: "denied with permissions", + src: "web", + dst: "redis", + expect: structs.IntentionDecisionSummary{ + Allowed: false, + HasPermissions: true, + }, + }, + { + name: "denied without permissions", + src: "api", + dst: "redis", + expect: structs.IntentionDecisionSummary{ + Allowed: false, + HasPermissions: false, + }, + }, + { + name: "allowed from external source", + src: "api", + dst: "web", + expect: structs.IntentionDecisionSummary{ + Allowed: true, + HasPermissions: false, + ExternalSource: "nomad", + }, + }, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + uri := connect.SpiffeIDService{ + Service: tc.src, + Namespace: structs.IntentionDefaultNamespace, + } + decision, err := s.IntentionDecision(&uri, tc.dst, structs.IntentionDefaultNamespace, tc.defaultDecision) + require.NoError(t, err) + require.Equal(t, tc.expect, decision) + }) + } +} + func disableLegacyIntentions(s *Store) error { return s.SystemMetadataSet(1, &structs.SystemMetadataEntry{ Key: structs.SystemMetadataIntentionFormatKey, diff --git a/agent/structs/intention.go b/agent/structs/intention.go index ef85743a60..800f7c1f27 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -641,6 +641,17 @@ type IntentionQueryCheckResponse struct { Allowed bool } +// IntentionDecisionSummary contains a summary of a set of intentions between two services +// Currently contains: +// - Whether all actions are allowed +// - Whether the matching intention has L7 permissions attached +// - Whether the intention is managed by an external source like k8s, +type IntentionDecisionSummary struct { + Allowed bool + HasPermissions bool + ExternalSource string +} + // IntentionQueryExact holds the parameters for performing a lookup of an // intention by its unique name instead of its ID. type IntentionQueryExact struct { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index ca730e7449..41520ba604 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -104,6 +104,9 @@ const ( // mesh gateway is usable for wan federation. MetaWANFederationKey = "consul-wan-federation" + // MetaExternalSource is the metadata key used when a resource is managed by a source outside Consul like nomad/k8s + MetaExternalSource = "external-source" + // MaxLockDelay provides a maximum LockDelay value for // a session. Any value above this will not be respected. MaxLockDelay = 60 * time.Second @@ -1867,6 +1870,9 @@ type IndexedServiceTopology struct { type ServiceTopology struct { Upstreams CheckServiceNodes Downstreams CheckServiceNodes + + UpstreamDecisions map[string]IntentionDecisionSummary + DownstreamDecisions map[string]IntentionDecisionSummary } // IndexedConfigEntries has its own encoding logic which differs from diff --git a/agent/ui_endpoint.go b/agent/ui_endpoint.go index e4ac0d0984..38d8325e06 100644 --- a/agent/ui_endpoint.go +++ b/agent/ui_endpoint.go @@ -11,11 +11,6 @@ import ( "github.com/hashicorp/consul/api" ) -// metaExternalSource is the key name for the service instance meta that -// defines the external syncing source. This is used by the UI APIs below -// to extract this. -const metaExternalSource = "external-source" - // ServiceSummary is used to summarize a service type ServiceSummary struct { Kind structs.ServiceKind `json:",omitempty"` @@ -42,13 +37,6 @@ func (s *ServiceSummary) LessThan(other *ServiceSummary) bool { return s.Name < other.Name } -type ServiceListingSummary struct { - ServiceSummary - - ConnectedWithProxy bool - ConnectedWithGateway bool -} - type GatewayConfig struct { AssociatedServiceCount int `json:",omitempty"` Addresses []string `json:",omitempty"` @@ -57,9 +45,22 @@ type GatewayConfig struct { addressesSet map[string]struct{} } +type ServiceListingSummary struct { + ServiceSummary + + ConnectedWithProxy bool + ConnectedWithGateway bool +} + +type ServiceTopologySummary struct { + ServiceSummary + + Intention structs.IntentionDecisionSummary +} + type ServiceTopology struct { - Upstreams []*ServiceSummary - Downstreams []*ServiceSummary + Upstreams []*ServiceTopologySummary + Downstreams []*ServiceTopologySummary FilteredByACLs bool } @@ -291,12 +292,38 @@ RPC: upstreams, _ := summarizeServices(out.ServiceTopology.Upstreams.ToServiceDump(), nil, "") downstreams, _ := summarizeServices(out.ServiceTopology.Downstreams.ToServiceDump(), nil, "") - sum := ServiceTopology{ - Upstreams: prepSummaryOutput(upstreams, true), - Downstreams: prepSummaryOutput(downstreams, true), + var ( + upstreamResp []*ServiceTopologySummary + downstreamResp []*ServiceTopologySummary + ) + + // Sort and attach intention data for upstreams and downstreams + sortedUpstreams := prepSummaryOutput(upstreams, true) + for _, svc := range sortedUpstreams { + sn := structs.NewServiceName(svc.Name, &svc.EnterpriseMeta) + sum := ServiceTopologySummary{ + ServiceSummary: *svc, + Intention: out.ServiceTopology.UpstreamDecisions[sn.String()], + } + upstreamResp = append(upstreamResp, &sum) + } + + sortedDownstreams := prepSummaryOutput(downstreams, true) + for _, svc := range sortedDownstreams { + sn := structs.NewServiceName(svc.Name, &svc.EnterpriseMeta) + sum := ServiceTopologySummary{ + ServiceSummary: *svc, + Intention: out.ServiceTopology.DownstreamDecisions[sn.String()], + } + downstreamResp = append(downstreamResp, &sum) + } + + topo := ServiceTopology{ + Upstreams: upstreamResp, + Downstreams: downstreamResp, FilteredByACLs: out.FilteredByACLs, } - return sum, nil + return topo, nil } func summarizeServices(dump structs.ServiceDump, cfg *config.RuntimeConfig, dc string) (map[structs.ServiceName]*ServiceSummary, map[structs.ServiceName]bool) { @@ -370,8 +397,8 @@ func summarizeServices(dump structs.ServiceDump, cfg *config.RuntimeConfig, dc s // sources. We only want to add unique sources so there is extra // accounting here with an unexported field to maintain the set // of sources. - if len(svc.Meta) > 0 && svc.Meta[metaExternalSource] != "" { - source := svc.Meta[metaExternalSource] + if len(svc.Meta) > 0 && svc.Meta[structs.MetaExternalSource] != "" { + source := svc.Meta[structs.MetaExternalSource] if sum.externalSourceSet == nil { sum.externalSourceSet = make(map[string]struct{}) } diff --git a/agent/ui_endpoint_test.go b/agent/ui_endpoint_test.go index 1f9ef262f2..1ca6f3147e 100644 --- a/agent/ui_endpoint_test.go +++ b/agent/ui_endpoint_test.go @@ -3,6 +3,7 @@ package agent import ( "bytes" "fmt" + "github.com/hashicorp/consul/sdk/testutil/retry" "io" "io/ioutil" "net/http" @@ -246,7 +247,7 @@ func TestUiServices(t *testing.T) { Service: "api-proxy", ID: "api-proxy-1", Tags: []string{}, - Meta: map[string]string{metaExternalSource: "k8s"}, + Meta: map[string]string{structs.MetaExternalSource: "k8s"}, Port: 1234, Proxy: structs.ConnectProxyConfig{ DestinationServiceName: "api", @@ -272,7 +273,7 @@ func TestUiServices(t *testing.T) { Service: "web", ID: "web-1", Tags: []string{}, - Meta: map[string]string{metaExternalSource: "k8s"}, + Meta: map[string]string{structs.MetaExternalSource: "k8s"}, Port: 1234, }, Checks: []*structs.HealthCheck{ @@ -936,7 +937,7 @@ func TestUIServiceTopology(t *testing.T) { a := NewTestAgent(t, "") defer a.Shutdown() - // Register terminating gateway and config entry linking it to postgres + redis + // Register api -> web -> redis { registrations := map[string]*structs.RegisterRequest{ "Node foo": { @@ -1204,115 +1205,201 @@ func TestUIServiceTopology(t *testing.T) { } } - t.Run("api", func(t *testing.T) { - // Request topology for api - req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/api", nil) - resp := httptest.NewRecorder() - obj, err := a.srv.UIServiceTopology(resp, req) - assert.Nil(t, err) - assertIndex(t, resp) - - expect := ServiceTopology{ - Upstreams: []*ServiceSummary{ - { - Name: "web", - Datacenter: "dc1", - Nodes: []string{"bar", "baz"}, - InstanceCount: 2, - ChecksPassing: 3, - ChecksWarning: 1, - ChecksCritical: 2, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + // Add intentions: deny all, web -> redis with L7 perms, but omit intention for api -> web + { + entries := []structs.ConfigEntryRequest{ + { + Datacenter: "dc1", + Entry: &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + Config: map[string]interface{}{ + "protocol": "http", + }, + }, + }, + { + Datacenter: "dc1", + Entry: &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "redis", + Sources: []*structs.SourceIntention{ + { + Name: "web", + Permissions: []*structs.IntentionPermission{ + { + Action: structs.IntentionActionAllow, + HTTP: &structs.IntentionHTTPPermission{ + Methods: []string{"GET"}, + }, + }, + }, + }, + }, + }, + }, + { + Datacenter: "dc1", + Entry: &structs.ServiceIntentionsConfigEntry{ + Kind: structs.ServiceIntentions, + Name: "*", + Meta: map[string]string{structs.MetaExternalSource: "nomad"}, + Sources: []*structs.SourceIntention{ + { + Name: "*", + Action: structs.IntentionActionDeny, + }, + }, }, }, - FilteredByACLs: false, } - result := obj.(ServiceTopology) + for _, req := range entries { + out := false + require.NoError(t, a.RPC("ConfigEntry.Apply", &req, &out)) + } + } - // Internal accounting that is not returned in JSON response - for _, u := range result.Upstreams { - u.externalSourceSet = nil - u.checks = nil - } - require.Equal(t, expect, result) + t.Run("api", func(t *testing.T) { + retry.Run(t, func(r *retry.R) { + // Request topology for api + req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/api", nil) + resp := httptest.NewRecorder() + obj, err := a.srv.UIServiceTopology(resp, req) + assert.Nil(r, err) + require.NoError(r, checkIndex(resp)) + + expect := ServiceTopology{ + Upstreams: []*ServiceTopologySummary{ + { + ServiceSummary: ServiceSummary{ + Name: "web", + Datacenter: "dc1", + Nodes: []string{"bar", "baz"}, + InstanceCount: 2, + ChecksPassing: 3, + ChecksWarning: 1, + ChecksCritical: 2, + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + }, + Intention: structs.IntentionDecisionSummary{ + Allowed: false, + HasPermissions: false, + ExternalSource: "nomad", + }, + }, + }, + FilteredByACLs: false, + } + result := obj.(ServiceTopology) + + // Internal accounting that is not returned in JSON response + for _, u := range result.Upstreams { + u.externalSourceSet = nil + u.checks = nil + } + require.Equal(r, expect, result) + }) }) t.Run("web", func(t *testing.T) { - // Request topology for web - req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/web", nil) - resp := httptest.NewRecorder() - obj, err := a.srv.UIServiceTopology(resp, req) - assert.Nil(t, err) - assertIndex(t, resp) + retry.Run(t, func(r *retry.R) { + // Request topology for web + req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/web", nil) + resp := httptest.NewRecorder() + obj, err := a.srv.UIServiceTopology(resp, req) + assert.Nil(r, err) + require.NoError(r, checkIndex(resp)) - expect := ServiceTopology{ - Upstreams: []*ServiceSummary{ - { - Name: "redis", - Datacenter: "dc1", - Nodes: []string{"zip"}, - InstanceCount: 1, - ChecksPassing: 2, - ChecksCritical: 1, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + expect := ServiceTopology{ + Upstreams: []*ServiceTopologySummary{ + { + ServiceSummary: ServiceSummary{ + Name: "redis", + Datacenter: "dc1", + Nodes: []string{"zip"}, + InstanceCount: 1, + ChecksPassing: 2, + ChecksCritical: 1, + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + }, + Intention: structs.IntentionDecisionSummary{ + Allowed: false, + HasPermissions: true, + }, + }, }, - }, - Downstreams: []*ServiceSummary{ - { - Name: "api", - Datacenter: "dc1", - Nodes: []string{"foo"}, - InstanceCount: 1, - ChecksPassing: 3, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + Downstreams: []*ServiceTopologySummary{ + { + ServiceSummary: ServiceSummary{ + Name: "api", + Datacenter: "dc1", + Nodes: []string{"foo"}, + InstanceCount: 1, + ChecksPassing: 3, + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + }, + Intention: structs.IntentionDecisionSummary{ + Allowed: false, + HasPermissions: false, + ExternalSource: "nomad", + }, + }, }, - }, - FilteredByACLs: false, - } - result := obj.(ServiceTopology) + FilteredByACLs: false, + } + result := obj.(ServiceTopology) - // Internal accounting that is not returned in JSON response - for _, u := range result.Upstreams { - u.externalSourceSet = nil - u.checks = nil - } - for _, d := range result.Downstreams { - d.externalSourceSet = nil - d.checks = nil - } - require.Equal(t, expect, result) + // Internal accounting that is not returned in JSON response + for _, u := range result.Upstreams { + u.externalSourceSet = nil + u.checks = nil + } + for _, d := range result.Downstreams { + d.externalSourceSet = nil + d.checks = nil + } + require.Equal(r, expect, result) + }) }) t.Run("redis", func(t *testing.T) { - // Request topology for redis - req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/redis", nil) - resp := httptest.NewRecorder() - obj, err := a.srv.UIServiceTopology(resp, req) - assert.Nil(t, err) - assertIndex(t, resp) + retry.Run(t, func(r *retry.R) { + // Request topology for redis + req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/redis", nil) + resp := httptest.NewRecorder() + obj, err := a.srv.UIServiceTopology(resp, req) + assert.Nil(r, err) + require.NoError(r, checkIndex(resp)) - expect := ServiceTopology{ - Downstreams: []*ServiceSummary{ - { - Name: "web", - Datacenter: "dc1", - Nodes: []string{"bar", "baz"}, - InstanceCount: 2, - ChecksPassing: 3, - ChecksWarning: 1, - ChecksCritical: 2, - EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + expect := ServiceTopology{ + Downstreams: []*ServiceTopologySummary{ + { + ServiceSummary: ServiceSummary{ + Name: "web", + Datacenter: "dc1", + Nodes: []string{"bar", "baz"}, + InstanceCount: 2, + ChecksPassing: 3, + ChecksWarning: 1, + ChecksCritical: 2, + EnterpriseMeta: *structs.DefaultEnterpriseMeta(), + }, + Intention: structs.IntentionDecisionSummary{ + Allowed: false, + HasPermissions: true, + }, + }, }, - }, - FilteredByACLs: false, - } - result := obj.(ServiceTopology) + FilteredByACLs: false, + } + result := obj.(ServiceTopology) - // Internal accounting that is not returned in JSON response - for _, d := range result.Downstreams { - d.externalSourceSet = nil - d.checks = nil - } - require.Equal(t, expect, result) + // Internal accounting that is not returned in JSON response + for _, d := range result.Downstreams { + d.externalSourceSet = nil + d.checks = nil + } + require.Equal(r, expect, result) + }) }) }