From 5ec3274ae5d5f6352fd59a0001f248002ec1b3f2 Mon Sep 17 00:00:00 2001 From: Dhia Ayachi Date: Tue, 7 Jun 2022 15:03:59 -0400 Subject: [PATCH] Egress gtw/connect destination intentions (#13341) * update gateway-services table with endpoints * fix failing test * remove unneeded config in test * rename "endpoint" to "destination" * more endpoint renaming to destination in tests * update isDestination based on service-defaults config entry creation * use a 3 state kind to be able to set the kind to unknown (when neither a service or a destination exist) * set unknown state to empty to avoid modifying alot of tests * fix logic to set the kind correctly on CRUD * fix failing tests * add missing tests and fix service delete * fix failing test * Apply suggestions from code review Co-authored-by: Dan Stough * fix a bug with kind and add relevant test * fix compile error * fix failing tests * add kind to clone * fix failing tests * fix failing tests in catalog endpoint * fix service dump test * Apply suggestions from code review Co-authored-by: Dan Stough * remove duplicate tests * first draft of destinations intention in connect proxy * remove ServiceDestinationList * fix failing tests * fix agent/consul failing tests * change to filter intentions in the state store instead of adding a field. * fix failing tests * fix comment * fix comments * store service kind destination and add relevant tests * changes based on review * filter on destinations when querying source match * Apply suggestions from code review Co-authored-by: alex <8968914+acpana@users.noreply.github.com> * fix style * Apply suggestions from code review Co-authored-by: Dan Stough * rename destinationType to targetType. Co-authored-by: Dan Stough Co-authored-by: alex <8968914+acpana@users.noreply.github.com> Co-authored-by: github-team-consul-core --- agent/consul/intention_endpoint.go | 2 +- agent/consul/internal_endpoint.go | 2 +- agent/consul/state/catalog.go | 18 +------ agent/consul/state/config_entry.go | 12 ++++- agent/consul/state/config_entry_intention.go | 49 +++++++++++++++----- agent/consul/state/config_entry_test.go | 39 ++++++++++++++++ agent/consul/state/intention.go | 39 ++++++++++++---- agent/consul/state/intention_test.go | 4 +- agent/structs/intention.go | 9 ++++ agent/structs/structs.go | 5 ++ 10 files changed, 137 insertions(+), 42 deletions(-) diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index fc6db87db9..b2c2d49c04 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -764,7 +764,7 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In Partition: query.SourcePartition, Name: query.SourceName, } - _, intentions, err := store.IntentionMatchOne(nil, entry, structs.IntentionMatchSource) + _, intentions, err := store.IntentionMatchOne(nil, entry, structs.IntentionMatchSource, structs.IntentionTargetService) if err != nil { return fmt.Errorf("failed to query intentions for %s/%s", query.SourceNS, query.SourceName) } diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index 20169562d7..113abd2dd6 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -405,7 +405,7 @@ func (m *Internal) GatewayIntentions(args *structs.IntentionQueryRequest, reply Partition: gs.Service.PartitionOrDefault(), Name: gs.Service.Name, } - idx, intentions, err := state.IntentionMatchOne(ws, entry, structs.IntentionMatchDestination) + idx, intentions, err := state.IntentionMatchOne(ws, entry, structs.IntentionMatchDestination, structs.IntentionTargetService) if err != nil { return err } diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index ab4797fda8..ee1623956a 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -4009,14 +4009,7 @@ func (s *Store) ServiceTopology( Partition: entMeta.PartitionOrDefault(), Name: service, } - _, srcIntentions, err := compatIntentionMatchOneTxn( - tx, - ws, - matchEntry, - - // The given service is a source relative to its upstreams - structs.IntentionMatchSource, - ) + _, srcIntentions, err := compatIntentionMatchOneTxn(tx, ws, matchEntry, structs.IntentionMatchSource, structs.IntentionTargetService) if err != nil { return 0, nil, fmt.Errorf("failed to query intentions for %s", sn.String()) } @@ -4128,14 +4121,7 @@ func (s *Store) ServiceTopology( downstreamSources[svc.Name.String()] = source } - _, dstIntentions, err := compatIntentionMatchOneTxn( - tx, - ws, - matchEntry, - - // The given service is a destination relative to its downstreams - structs.IntentionMatchDestination, - ) + _, dstIntentions, err := compatIntentionMatchOneTxn(tx, ws, matchEntry, structs.IntentionMatchDestination, structs.IntentionTargetService) if err != nil { return 0, nil, fmt.Errorf("failed to query intentions for %s", sn.String()) } diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index e2cd8600f1..f0e4c069e8 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -356,12 +356,16 @@ func deleteConfigEntryTxn(tx WriteTxn, idx uint64, kind, name string, entMeta *a if gsKind == structs.GatewayServiceKindDestination { gsKind = structs.GatewayServiceKindUnknown } - if err := checkGatewayWildcardsAndUpdate(tx, idx, &structs.ServiceName{Name: c.GetName(), EnterpriseMeta: *c.GetEnterpriseMeta()}, gsKind); err != nil { + serviceName := structs.NewServiceName(c.GetName(), c.GetEnterpriseMeta()) + if err := checkGatewayWildcardsAndUpdate(tx, idx, &serviceName, gsKind); err != nil { return fmt.Errorf("failed updating gateway mapping: %s", err) } - if err := checkGatewayAndUpdate(tx, idx, &structs.ServiceName{Name: c.GetName(), EnterpriseMeta: *c.GetEnterpriseMeta()}, gsKind); err != nil { + if err := checkGatewayAndUpdate(tx, idx, &serviceName, gsKind); err != nil { return fmt.Errorf("failed updating gateway mapping: %s", err) } + if err := cleanupKindServiceName(tx, idx, serviceName, structs.ServiceKindDestination); err != nil { + return fmt.Errorf("failed to cleanup service name: \"%s\"; err: %v", serviceName, err) + } } } @@ -422,6 +426,10 @@ func insertConfigEntryWithTxn(tx WriteTxn, idx uint64, conf structs.ConfigEntry) if err := checkGatewayAndUpdate(tx, idx, &sn, gsKind); err != nil { return fmt.Errorf("failed updating gateway mapping: %s", err) } + + if err := upsertKindServiceName(tx, idx, structs.ServiceKindDestination, sn); err != nil { + return fmt.Errorf("failed to persist service name: %v", err) + } } } diff --git a/agent/consul/state/config_entry_intention.go b/agent/consul/state/config_entry_intention.go index 27c4912e6e..bd539489a5 100644 --- a/agent/consul/state/config_entry_intention.go +++ b/agent/consul/state/config_entry_intention.go @@ -208,7 +208,7 @@ func (s *Store) configIntentionMatchTxn(tx ReadTxn, ws memdb.WatchSet, args *str // improving that in the future, the test cases shouldn't have to // change for that. - index, ixns, err := configIntentionMatchOneTxn(tx, ws, entry, args.Type) + index, ixns, err := configIntentionMatchOneTxn(tx, ws, entry, args.Type, structs.IntentionTargetService) if err != nil { return 0, nil, err } @@ -224,14 +224,15 @@ func (s *Store) configIntentionMatchTxn(tx ReadTxn, ws memdb.WatchSet, args *str } func configIntentionMatchOneTxn( - tx ReadTxn, - ws memdb.WatchSet, + tx ReadTxn, ws memdb.WatchSet, matchEntry structs.IntentionMatchEntry, matchType structs.IntentionMatchType, + targetType structs.IntentionTargetType, ) (uint64, structs.Intentions, error) { switch matchType { + // targetType is only relevant for Source matches as egress Destinations can only be Intention Destinations in the mesh case structs.IntentionMatchSource: - return readSourceIntentionsFromConfigEntriesTxn(tx, ws, matchEntry.Name, matchEntry.GetEnterpriseMeta()) + return readSourceIntentionsFromConfigEntriesTxn(tx, ws, matchEntry.Name, matchEntry.GetEnterpriseMeta(), targetType) case structs.IntentionMatchDestination: return readDestinationIntentionsFromConfigEntriesTxn(tx, ws, matchEntry.Name, matchEntry.GetEnterpriseMeta()) default: @@ -239,7 +240,13 @@ func configIntentionMatchOneTxn( } } -func readSourceIntentionsFromConfigEntriesTxn(tx ReadTxn, ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta) (uint64, structs.Intentions, error) { +func readSourceIntentionsFromConfigEntriesTxn( + tx ReadTxn, + ws memdb.WatchSet, + serviceName string, + entMeta *acl.EnterpriseMeta, + targetType structs.IntentionTargetType, +) (uint64, structs.Intentions, error) { idx := maxIndexTxn(tx, tableConfigEntries) var ( @@ -249,9 +256,7 @@ func readSourceIntentionsFromConfigEntriesTxn(tx ReadTxn, ws memdb.WatchSet, ser names := getIntentionPrecedenceMatchServiceNames(serviceName, entMeta) for _, sn := range names { - results, err = readSourceIntentionsFromConfigEntriesForServiceTxn( - tx, ws, sn.Name, &sn.EnterpriseMeta, results, - ) + results, err = readSourceIntentionsFromConfigEntriesForServiceTxn(tx, ws, sn.Name, &sn.EnterpriseMeta, results, targetType) if err != nil { return 0, nil, err } @@ -263,7 +268,14 @@ func readSourceIntentionsFromConfigEntriesTxn(tx ReadTxn, ws memdb.WatchSet, ser return idx, results, nil } -func readSourceIntentionsFromConfigEntriesForServiceTxn(tx ReadTxn, ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta, results structs.Intentions) (structs.Intentions, error) { +func readSourceIntentionsFromConfigEntriesForServiceTxn( + tx ReadTxn, + ws memdb.WatchSet, + serviceName string, + entMeta *acl.EnterpriseMeta, + results structs.Intentions, + targetType structs.IntentionTargetType, +) (structs.Intentions, error) { sn := structs.NewServiceName(serviceName, entMeta) iter, err := tx.Get(tableConfigEntries, indexSource, sn) @@ -276,7 +288,23 @@ func readSourceIntentionsFromConfigEntriesForServiceTxn(tx ReadTxn, ws memdb.Wat entry := v.(*structs.ServiceIntentionsConfigEntry) for _, src := range entry.Sources { if src.SourceServiceName() == sn { - results = append(results, entry.ToIntention(src)) + entMeta := entry.DestinationServiceName().EnterpriseMeta + kind, err := GatewayServiceKind(tx, entry.DestinationServiceName().Name, &entMeta) + if err != nil { + return nil, err + } + switch targetType { + case structs.IntentionTargetService: + if kind == structs.GatewayServiceKindService || kind == structs.GatewayServiceKindUnknown { + results = append(results, entry.ToIntention(src)) + } + case structs.IntentionTargetDestination: + if kind == structs.GatewayServiceKindDestination { + results = append(results, entry.ToIntention(src)) + } + default: + return nil, fmt.Errorf("invalid target type") + } } } } @@ -298,7 +326,6 @@ func readDestinationIntentionsFromConfigEntriesTxn(tx ReadTxn, ws memdb.WatchSet results = append(results, entry.ToIntentions()...) } } - // Sort the results by precedence sort.Sort(structs.IntentionPrecedenceSorter(results)) diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go index 5d2cdf9fd2..b874f41af5 100644 --- a/agent/consul/state/config_entry_test.go +++ b/agent/consul/state/config_entry_test.go @@ -355,6 +355,11 @@ func TestStore_ServiceDefaults_Kind_Destination(t *testing.T) { require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindDestination) + _, kindServices, err := s.ServiceNamesOfKind(ws, structs.ServiceKindDestination) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindDestination) + ws = memdb.NewWatchSet() _, _, err = s.GatewayServices(ws, "Gtwy1", nil) require.NoError(t, err) @@ -369,6 +374,10 @@ func TestStore_ServiceDefaults_Kind_Destination(t *testing.T) { require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindUnknown) + _, kindServices, err = s.ServiceNamesOfKind(ws, structs.ServiceKindDestination) + require.NoError(t, err) + require.Len(t, kindServices, 0) + } func TestStore_ServiceDefaults_Kind_NotDestination(t *testing.T) { @@ -475,6 +484,11 @@ func TestStore_Service_TerminatingGateway_Kind_Service_Destination(t *testing.T) require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindService) + _, kindServices, err := s.ServiceNamesOfKind(ws, structs.ServiceKindTypical) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindTypical) + require.NoError(t, s.EnsureConfigEntry(0, destination)) _, gatewayServices, err = s.GatewayServices(nil, "Gtwy1", nil) @@ -482,6 +496,11 @@ func TestStore_Service_TerminatingGateway_Kind_Service_Destination(t *testing.T) require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindService) + _, kindServices, err = s.ServiceNamesOfKind(ws, structs.ServiceKindTypical) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindTypical) + ws = memdb.NewWatchSet() _, _, err = s.GatewayServices(ws, "Gtwy1", nil) require.NoError(t, err) @@ -496,6 +515,11 @@ func TestStore_Service_TerminatingGateway_Kind_Service_Destination(t *testing.T) require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindDestination) + _, kindServices, err = s.ServiceNamesOfKind(ws, structs.ServiceKindDestination) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindDestination) + } func TestStore_Service_TerminatingGateway_Kind_Destination_Service(t *testing.T) { @@ -541,6 +565,11 @@ func TestStore_Service_TerminatingGateway_Kind_Destination_Service(t *testing.T) require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindDestination) + _, kindServices, err := s.ServiceNamesOfKind(ws, structs.ServiceKindDestination) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindDestination) + require.NoError(t, s.EnsureNode(0, &structs.Node{Node: "node1"})) require.NoError(t, s.EnsureService(0, "node1", service)) @@ -552,6 +581,11 @@ func TestStore_Service_TerminatingGateway_Kind_Destination_Service(t *testing.T) require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindService) + _, kindServices, err = s.ServiceNamesOfKind(ws, structs.ServiceKindTypical) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindTypical) + ws = memdb.NewWatchSet() _, _, err = s.GatewayServices(ws, "Gtwy1", nil) require.NoError(t, err) @@ -566,6 +600,11 @@ func TestStore_Service_TerminatingGateway_Kind_Destination_Service(t *testing.T) require.Len(t, gatewayServices, 1) require.Equal(t, gatewayServices[0].ServiceKind, structs.GatewayServiceKindDestination) + _, kindServices, err = s.ServiceNamesOfKind(ws, structs.ServiceKindDestination) + require.NoError(t, err) + require.Len(t, kindServices, 1) + require.Equal(t, kindServices[0].Kind, structs.ServiceKindDestination) + } func TestStore_Service_TerminatingGateway_Kind_Service(t *testing.T) { diff --git a/agent/consul/state/intention.go b/agent/consul/state/intention.go index 821288f3bc..89f7304f60 100644 --- a/agent/consul/state/intention.go +++ b/agent/consul/state/intention.go @@ -600,6 +600,7 @@ func legacyIntentionGetTxn(tx ReadTxn, ws memdb.WatchSet, id string) (uint64, *s // Convert the interface{} if it is non-nil var result *structs.Intention + if intention != nil { result = intention.(*structs.Intention) } @@ -842,11 +843,12 @@ func (s *Store) IntentionMatchOne( ws memdb.WatchSet, entry structs.IntentionMatchEntry, matchType structs.IntentionMatchType, + destinationType structs.IntentionTargetType, ) (uint64, structs.Intentions, error) { tx := s.db.Txn(false) defer tx.Abort() - return compatIntentionMatchOneTxn(tx, ws, entry, matchType) + return compatIntentionMatchOneTxn(tx, ws, entry, matchType, destinationType) } func compatIntentionMatchOneTxn( @@ -854,6 +856,7 @@ func compatIntentionMatchOneTxn( ws memdb.WatchSet, entry structs.IntentionMatchEntry, matchType structs.IntentionMatchType, + destinationType structs.IntentionTargetType, ) (uint64, structs.Intentions, error) { usingConfigEntries, err := areIntentionsInConfigEntries(tx, ws) @@ -863,7 +866,7 @@ func compatIntentionMatchOneTxn( if !usingConfigEntries { return legacyIntentionMatchOneTxn(tx, ws, entry, matchType) } - return configIntentionMatchOneTxn(tx, ws, entry, matchType) + return configIntentionMatchOneTxn(tx, ws, entry, matchType, destinationType) } func legacyIntentionMatchOneTxn( @@ -949,8 +952,12 @@ type ServiceWithDecision struct { // IntentionTopology returns the upstreams or downstreams of a service. Upstreams and downstreams are inferred from // intentions. If intentions allow a connection from the target to some candidate service, the candidate service is considered // an upstream of the target. -func (s *Store) IntentionTopology(ws memdb.WatchSet, - target structs.ServiceName, downstreams bool, defaultDecision acl.EnforcementDecision) (uint64, structs.ServiceList, error) { +func (s *Store) IntentionTopology( + ws memdb.WatchSet, + target structs.ServiceName, + downstreams bool, + defaultDecision acl.EnforcementDecision, +) (uint64, structs.ServiceList, error) { tx := s.db.ReadTxn() defer tx.Abort() @@ -965,13 +972,17 @@ func (s *Store) IntentionTopology(ws memdb.WatchSet, resp := make(structs.ServiceList, 0) for _, svc := range services { - resp = append(resp, svc.Name) + resp = append(resp, structs.ServiceName{Name: svc.Name.Name, EnterpriseMeta: svc.Name.EnterpriseMeta}) } return idx, resp, nil } -func (s *Store) intentionTopologyTxn(tx ReadTxn, ws memdb.WatchSet, - target structs.ServiceName, downstreams bool, defaultDecision acl.EnforcementDecision) (uint64, []ServiceWithDecision, error) { +func (s *Store) intentionTopologyTxn( + tx ReadTxn, ws memdb.WatchSet, + target structs.ServiceName, + downstreams bool, + defaultDecision acl.EnforcementDecision, +) (uint64, []ServiceWithDecision, error) { var maxIdx uint64 @@ -987,7 +998,7 @@ func (s *Store) intentionTopologyTxn(tx ReadTxn, ws memdb.WatchSet, Partition: target.PartitionOrDefault(), Name: target.Name, } - index, intentions, err := compatIntentionMatchOneTxn(tx, ws, entry, intentionMatchType) + index, intentions, err := compatIntentionMatchOneTxn(tx, ws, entry, intentionMatchType, structs.IntentionTargetService) if err != nil { return 0, nil, fmt.Errorf("failed to query intentions for %s", target.String()) } @@ -1017,6 +1028,16 @@ func (s *Store) intentionTopologyTxn(tx ReadTxn, ws memdb.WatchSet, maxIdx = index } services = append(services, ingress...) + } else { + // destinations can only ever be upstream, since they are only allowed as intention destination. + index, destinations, err := serviceNamesOfKindTxn(tx, ws, structs.ServiceKindDestination, *wildcardMeta) + if err != nil { + return index, nil, fmt.Errorf("failed to list destination names: %v", err) + } + if index > maxIdx { + maxIdx = index + } + services = append(services, destinations...) } // When checking authorization to upstreams, the match type for the decision is `destination` because we are deciding @@ -1056,7 +1077,7 @@ func (s *Store) intentionTopologyTxn(tx ReadTxn, ws memdb.WatchSet, } result = append(result, ServiceWithDecision{ - Name: candidate, + Name: structs.ServiceName{Name: candidate.Name, EnterpriseMeta: candidate.EnterpriseMeta}, Decision: decision, }) } diff --git a/agent/consul/state/intention_test.go b/agent/consul/state/intention_test.go index fde26d1d9a..4a369d166a 100644 --- a/agent/consul/state/intention_test.go +++ b/agent/consul/state/intention_test.go @@ -1533,7 +1533,7 @@ func TestStore_IntentionMatchOne_table(t *testing.T) { Namespace: "default", Name: query, } - _, matches, err := s.IntentionMatchOne(nil, entry, typ) + _, matches, err := s.IntentionMatchOne(nil, entry, typ, structs.IntentionTargetService) require.NoError(t, err) // Verify matches @@ -1873,7 +1873,7 @@ func TestStore_IntentionDecision(t *testing.T) { Partition: acl.DefaultPartitionName, Name: tc.src, } - _, intentions, err := s.IntentionMatchOne(nil, entry, structs.IntentionMatchSource) + _, intentions, err := s.IntentionMatchOne(nil, entry, structs.IntentionMatchSource, structs.IntentionTargetService) if err != nil { require.NoError(t, err) } diff --git a/agent/structs/intention.go b/agent/structs/intention.go index feefc56726..ee1f89c0ab 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -494,6 +494,15 @@ const ( IntentionSourceConsul IntentionSourceType = "consul" ) +type IntentionTargetType string + +const ( + // IntentionTargetService is a service within the Consul catalog. + IntentionTargetService IntentionTargetType = "service" + // IntentionTargetDestination is a destination defined through a service-default config entry. + IntentionTargetDestination IntentionTargetType = "destination" +) + // Intentions is a list of intentions. type Intentions []*Intention diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 94c4256104..a7b4dcd732 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -1178,6 +1178,11 @@ const ( // This service allows external traffic to enter the mesh based on // centralized configuration. ServiceKindIngressGateway ServiceKind = "ingress-gateway" + + // ServiceKindDestination is a Destination for the Connect feature. + // This service allows external traffic to exit the mesh through a terminating gateway + //based on centralized configuration. + ServiceKindDestination ServiceKind = "destination" ) // Type to hold a address and port of a service