From 6d01d07cf88da478406b8202a1bef662bf65b3c1 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 21 Apr 2023 09:58:13 -0700 Subject: [PATCH] Include virtual services from discovery chain in intention topology (#16862) --- agent/consul/state/catalog.go | 24 ++++++++++ agent/consul/state/intention.go | 39 +++++++++++++--- agent/consul/state/intention_test.go | 67 ++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 6 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 2e1a7099ae..65bf8d7062 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -3039,6 +3039,30 @@ func (s *Store) VirtualIPForService(psn structs.PeeredServiceName) (string, erro return result.String(), nil } +func (s *Store) ServiceVirtualIPs() (uint64, []ServiceVirtualIP, error) { + tx := s.db.Txn(false) + defer tx.Abort() + + return servicesVirtualIPsTxn(tx) +} + +func servicesVirtualIPsTxn(tx ReadTxn) (uint64, []ServiceVirtualIP, error) { + iter, err := tx.Get(tableServiceVirtualIPs, indexID) + if err != nil { + return 0, nil, err + } + + var vips []ServiceVirtualIP + for raw := iter.Next(); raw != nil; raw = iter.Next() { + vip := raw.(ServiceVirtualIP) + vips = append(vips, vip) + } + + idx := maxIndexWatchTxn(tx, nil, tableServiceVirtualIPs) + + return idx, vips, nil +} + func (s *Store) ServiceManualVIPs(psn structs.PeeredServiceName) (*ServiceVirtualIP, error) { tx := s.db.Txn(false) defer tx.Abort() diff --git a/agent/consul/state/intention.go b/agent/consul/state/intention.go index 9548c4eb85..3212b20389 100644 --- a/agent/consul/state/intention.go +++ b/agent/consul/state/intention.go @@ -1063,12 +1063,40 @@ func (s *Store) intentionTopologyTxn( // Ideally those should be excluded as well, since they can't be upstreams/downstreams without a proxy. // Maybe narrow serviceNamesOfKindTxn to services represented by proxies? (ingress, sidecar- wildcardMeta := structs.WildcardEnterpriseMetaInPartition(structs.WildcardSpecifier) - var services []*KindServiceName + + services := make(map[structs.ServiceName]struct{}) + addSvcs := func(svcs []*KindServiceName) { + for _, s := range svcs { + services[s.Service] = struct{}{} + } + } + + var tempServices []*KindServiceName if intentionTarget == structs.IntentionTargetService { - index, services, err = serviceNamesOfKindTxn(tx, ws, structs.ServiceKindTypical, *wildcardMeta) + index, tempServices, err = serviceNamesOfKindTxn(tx, ws, structs.ServiceKindTypical, *wildcardMeta) + if err != nil { + return index, nil, fmt.Errorf("failed to list service names: %v", err) + } + addSvcs(tempServices) + + // Query the virtual ip table as well to include virtual services that don't have a registered instance yet. + vipIndex, vipServices, err := servicesVirtualIPsTxn(tx) + if err != nil { + return index, nil, fmt.Errorf("failed to list service virtual IPs: %v", err) + } + for _, svc := range vipServices { + services[svc.Service.ServiceName] = struct{}{} + } + if vipIndex > index { + index = vipIndex + } } else { // destinations can only ever be upstream, since they are only allowed as intention destination. - index, services, err = serviceNamesOfKindTxn(tx, ws, structs.ServiceKindDestination, *wildcardMeta) + index, tempServices, err = serviceNamesOfKindTxn(tx, ws, structs.ServiceKindDestination, *wildcardMeta) + if err != nil { + return index, nil, fmt.Errorf("failed to list destination service names: %v", err) + } + addSvcs(tempServices) } if err != nil { return index, nil, fmt.Errorf("failed to list ingress service names: %v", err) @@ -1086,7 +1114,7 @@ func (s *Store) intentionTopologyTxn( if index > maxIdx { maxIdx = index } - services = append(services, ingress...) + addSvcs(ingress) } // When checking authorization to upstreams, the match type for the decision is `destination` because we are deciding @@ -1097,8 +1125,7 @@ func (s *Store) intentionTopologyTxn( decisionMatchType = structs.IntentionMatchSource } result := make([]ServiceWithDecision, 0, len(services)) - for _, svc := range services { - candidate := svc.Service + for candidate := range services { if candidate.Name == structs.ConsulServiceName { continue } diff --git a/agent/consul/state/intention_test.go b/agent/consul/state/intention_test.go index 297fcd9502..34343b145a 100644 --- a/agent/consul/state/intention_test.go +++ b/agent/consul/state/intention_test.go @@ -2161,6 +2161,7 @@ func TestStore_IntentionTopology(t *testing.T) { name string defaultDecision acl.EnforcementDecision intentions []structs.ServiceIntentionsConfigEntry + discoveryChains []structs.ConfigEntry target structs.ServiceName downstreams bool expect expect @@ -2192,6 +2193,68 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, }, + { + name: "(upstream) acl allow includes virtual service", + defaultDecision: acl.Allow, + discoveryChains: []structs.ConfigEntry{ + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "backend", + }, + }, + target: structs.NewServiceName("web", nil), + downstreams: false, + expect: expect{ + idx: 10, + services: structs.ServiceList{ + { + Name: "api", + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + }, + { + Name: "backend", + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + }, + { + Name: "mysql", + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + }, + }, + }, + }, + { + name: "(upstream) acl deny all intentions allow virtual service", + defaultDecision: acl.Deny, + discoveryChains: []structs.ConfigEntry{ + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "backend", + }, + }, + intentions: []structs.ServiceIntentionsConfigEntry{ + { + Kind: structs.ServiceIntentions, + Name: "backend", + Sources: []*structs.SourceIntention{ + { + Name: "web", + Action: structs.IntentionActionAllow, + }, + }, + }, + }, + target: structs.NewServiceName("web", nil), + downstreams: false, + expect: expect{ + idx: 11, + services: structs.ServiceList{ + { + Name: "backend", + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + }, + }, + }, + }, { name: "(upstream) acl deny all intentions allow one", defaultDecision: acl.Deny, @@ -2378,6 +2441,10 @@ func TestStore_IntentionTopology(t *testing.T) { require.NoError(t, s.EnsureConfigEntry(idx, &ixn)) idx++ } + for _, entry := range tt.discoveryChains { + require.NoError(t, s.EnsureConfigEntry(idx, entry)) + idx++ + } idx, got, err := s.IntentionTopology(nil, tt.target, tt.downstreams, tt.defaultDecision, structs.IntentionTargetService) require.NoError(t, err)