From bb832e2bba8e7fd5cbd572cb09da65ebc0e585e6 Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Wed, 8 Jun 2022 13:24:10 -0400 Subject: [PATCH] Add SourcePeer fields to relevant Intentions types (#13390) --- acl/enterprisemeta_oss.go | 4 +- agent/structs/config_entry_intentions.go | 42 +++++-- agent/structs/intention.go | 44 ++++++-- agent/structs/intention_test.go | 134 +++++++++++++++-------- agent/structs/structs.go | 8 +- agent/structs/structs_filtering_test.go | 5 + api/config_entry_intentions.go | 1 + api/connect_intention.go | 5 + 8 files changed, 172 insertions(+), 71 deletions(-) diff --git a/acl/enterprisemeta_oss.go b/acl/enterprisemeta_oss.go index 7623709a93..97d01c10a6 100644 --- a/acl/enterprisemeta_oss.go +++ b/acl/enterprisemeta_oss.go @@ -82,7 +82,9 @@ func (m *EnterpriseMeta) MergeNoWildcard(_ *EnterpriseMeta) { // do nothing } -func (_ *EnterpriseMeta) Normalize() {} +func (_ *EnterpriseMeta) Normalize() {} +func (_ *EnterpriseMeta) NormalizePartition() {} +func (_ *EnterpriseMeta) NormalizeNamespace() {} func (m *EnterpriseMeta) Matches(_ *EnterpriseMeta) bool { return true diff --git a/agent/structs/config_entry_intentions.go b/agent/structs/config_entry_intentions.go index 8f7cd81233..04a7b2277f 100644 --- a/agent/structs/config_entry_intentions.go +++ b/agent/structs/config_entry_intentions.go @@ -123,6 +123,7 @@ func (e *ServiceIntentionsConfigEntry) ToIntention(src *SourceIntention) *Intent ixn := &Intention{ ID: src.LegacyID, Description: src.Description, + SourcePeer: src.Peer, SourcePartition: src.PartitionOrEmpty(), SourceNS: src.NamespaceOrDefault(), SourceName: src.Name, @@ -259,6 +260,9 @@ type SourceIntention struct { // formerly Intention.SourceNS acl.EnterpriseMeta `hcl:",squash" mapstructure:",squash"` + + // Peer is the name of the remote peer of the source service, if applicable. + Peer string `json:",omitempty"` } type IntentionPermission struct { @@ -361,11 +365,11 @@ func (e *ServiceIntentionsConfigEntry) UpdateOver(rawPrev ConfigEntry) error { } var ( - prevSourceByName = make(map[ServiceName]*SourceIntention) + prevSourceByName = make(map[PeeredServiceName]*SourceIntention) prevSourceByLegacyID = make(map[string]*SourceIntention) ) for _, src := range prev.Sources { - prevSourceByName[src.SourceServiceName()] = src + prevSourceByName[PeeredServiceName{Peer: src.Peer, ServiceName: src.SourceServiceName()}] = src if src.LegacyID != "" { prevSourceByLegacyID[src.LegacyID] = src } @@ -377,7 +381,7 @@ func (e *ServiceIntentionsConfigEntry) UpdateOver(rawPrev ConfigEntry) error { } // Check that the LegacyID fields are handled correctly during updates. - if prevSrc, ok := prevSourceByName[src.SourceServiceName()]; ok { + if prevSrc, ok := prevSourceByName[PeeredServiceName{Peer: src.Peer, ServiceName: src.SourceServiceName()}]; ok { if prevSrc.LegacyID == "" { return fmt.Errorf("Sources[%d].LegacyID: cannot set this field", i) } else if src.LegacyID != prevSrc.LegacyID { @@ -423,10 +427,17 @@ func (e *ServiceIntentionsConfigEntry) normalize(legacyWrite bool) error { src.Type = IntentionSourceConsul } - // If the source namespace is omitted it inherits that of the - // destination. - src.EnterpriseMeta.MergeNoWildcard(&e.EnterpriseMeta) - src.EnterpriseMeta.Normalize() + // Normalize the source's namespace and partition. + // If the source is not peered, it inherits the destination's + // EnterpriseMeta. + if src.Peer == "" { + src.EnterpriseMeta.MergeNoWildcard(&e.EnterpriseMeta) + src.EnterpriseMeta.Normalize() + } else { + // If the source is peered, normalize the namespace only, + // since peer is mutually exclusive with partition. + src.EnterpriseMeta.NormalizeNamespace() + } // Compute the precedence only AFTER normalizing namespaces since the // namespaces are factored into the calculation. @@ -542,7 +553,7 @@ func (e *ServiceIntentionsConfigEntry) validate(legacyWrite bool) error { return fmt.Errorf("Name is required") } - if err := validateIntentionWildcards(e.Name, &e.EnterpriseMeta); err != nil { + if err := validateIntentionWildcards(e.Name, &e.EnterpriseMeta, ""); err != nil { return err } @@ -568,7 +579,7 @@ func (e *ServiceIntentionsConfigEntry) validate(legacyWrite bool) error { return fmt.Errorf("Sources[%d].Name is required", i) } - if err := validateIntentionWildcards(src.Name, &src.EnterpriseMeta); err != nil { + if err := validateIntentionWildcards(src.Name, &src.EnterpriseMeta, src.Peer); err != nil { return fmt.Errorf("Sources[%d].%v", i, err) } @@ -576,6 +587,10 @@ func (e *ServiceIntentionsConfigEntry) validate(legacyWrite bool) error { return fmt.Errorf("Sources[%d].%v", i, err) } + if src.Peer != "" && src.PartitionOrEmpty() != "" { + return fmt.Errorf("Sources[%d].Peer: cannot set Peer and Partition at the same time.", i) + } + // Length of opaque values if len(src.Description) > metaValueMaxLength { return fmt.Errorf( @@ -583,6 +598,10 @@ func (e *ServiceIntentionsConfigEntry) validate(legacyWrite bool) error { } if legacyWrite { + if src.Peer != "" { + return fmt.Errorf("Sources[%d].Peer cannot be set by legacy intentions", i) + } + if len(src.LegacyMeta) > metaMaxKeyPairs { return fmt.Errorf( "Sources[%d].Meta exceeds maximum element count %d", i, metaMaxKeyPairs) @@ -753,7 +772,7 @@ func (e *ServiceIntentionsConfigEntry) validate(legacyWrite bool) error { } // Wildcard usage verification -func validateIntentionWildcards(name string, entMeta *acl.EnterpriseMeta) error { +func validateIntentionWildcards(name string, entMeta *acl.EnterpriseMeta, peerName string) error { ns := entMeta.NamespaceOrDefault() if ns != WildcardSpecifier { if strings.Contains(ns, WildcardSpecifier) { @@ -772,6 +791,9 @@ func validateIntentionWildcards(name string, entMeta *acl.EnterpriseMeta) error if strings.Contains(entMeta.PartitionOrDefault(), WildcardSpecifier) { return fmt.Errorf("Partition: cannot use wildcard '*' in partition") } + if strings.Contains(peerName, WildcardSpecifier) { + return fmt.Errorf("Peer: cannot use wildcard '*' in peer") + } return nil } diff --git a/agent/structs/intention.go b/agent/structs/intention.go index ee1f89c0ab..eb70ba1ee8 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -57,6 +57,11 @@ type Intention struct { SourcePartition string `json:",omitempty"` DestinationPartition string `json:",omitempty"` + // SourcePeer cannot be a wildcard "*" and is not compatible with legacy + // intentions. Cannot be used with SourcePartition, as both represent the + // same level of tenancy (partition is local to cluster, peer is remote). + SourcePeer string `json:",omitempty"` + // SourceType is the type of the value for the source. SourceType IntentionSourceType @@ -311,7 +316,9 @@ func (ixn *Intention) CanRead(authz acl.Authorizer) bool { // complete intention. This is so that both ends can be aware of why // something does or does not work. - if ixn.SourceName != "" { + // If SourcePeer is set, tenancy is irrelevant in the context of the local cluster + // so we skip authorizing on the Source end. + if ixn.SourceName != "" && ixn.SourcePeer == "" { ixn.FillAuthzContext(&authzContext, false) if authz.IntentionRead(ixn.SourceName, &authzContext) == acl.Allow { return true @@ -394,9 +401,13 @@ func (x *Intention) String() string { idPart = "ID: " + x.ID + ", " } - var srcPartitionPart string + // Cluster may be either partition (local) or peer (remote) + var srcClusterPart string if x.SourcePartition != "" { - srcPartitionPart = x.SourcePartition + "/" + srcClusterPart = x.SourcePartition + "/" + } + if x.SourcePeer != "" { + srcClusterPart = "peer(" + x.SourcePeer + ")/" } var dstPartitionPart string @@ -412,7 +423,7 @@ func (x *Intention) String() string { } return fmt.Sprintf("%s%s/%s => %s%s/%s (%sPrecedence: %d, %s)", - srcPartitionPart, x.SourceNS, x.SourceName, + srcClusterPart, x.SourceNS, x.SourceName, dstPartitionPart, x.DestinationNS, x.DestinationName, idPart, x.Precedence, @@ -461,6 +472,7 @@ func (x *Intention) ToSourceIntention(legacy bool) *SourceIntention { src := &SourceIntention{ Name: x.SourceName, EnterpriseMeta: *x.SourceEnterpriseMeta(), + Peer: x.SourcePeer, Action: x.Action, Permissions: nil, // explicitly not symmetric with the old APIs Precedence: 0, // Ignore, let it be computed. @@ -570,7 +582,8 @@ type IntentionMutation struct { ID string Destination ServiceName Source ServiceName - Value *SourceIntention + // TODO(peering): check if this needs peer field + Value *SourceIntention } // RequestDatacenter returns the datacenter for a given request. @@ -716,6 +729,8 @@ type IntentionQueryExact struct { // TODO(partitions): check query works with partitions SourcePartition string `json:",omitempty"` DestinationPartition string `json:",omitempty"` + + SourcePeer string `json:",omitempty"` } // Validate is used to ensure all 4 required parameters are specified. @@ -736,6 +751,7 @@ func (q *IntentionQueryExact) Validate() error { return err } +// TODO(peering): add support for listing peer type IntentionListRequest struct { Datacenter string Legacy bool `json:"-"` @@ -764,12 +780,18 @@ func (s IntentionPrecedenceSorter) Less(i, j int) bool { return a.Precedence > b.Precedence } - // Tie break on lexicographic order of the tuple in canonical form (SrcPxn, - // SrcNS, Src, DstPxn, DstNS, Dst). This is arbitrary but it keeps sorting - // deterministic which is a nice property for consistency. It is arguably - // open to abuse if implementations rely on this however by definition the - // order among same-precedence rules is arbitrary and doesn't affect whether - // an allow or deny rule is acted on since all applicable rules are checked. + // Tie break on lexicographic order of the tuple in canonical form: + // + // (SrcPeer, SrcPxn, SrcNS, Src, DstPxn, DstNS, Dst) + // + // This is arbitrary but it keeps sorting deterministic which is a nice + // property for consistency. It is arguably open to abuse if implementations + // rely on this however by definition the order among same-precedence rules + // is arbitrary and doesn't affect whether an allow or deny rule is acted on + // since all applicable rules are checked. + if a.SourcePeer != b.SourcePeer { + return a.SourcePeer < b.SourcePeer + } if a.SourcePartition != b.SourcePartition { return a.SourcePartition < b.SourcePartition } diff --git a/agent/structs/intention_test.go b/agent/structs/intention_test.go index 247bbe284f..f5aac91ca7 100644 --- a/agent/structs/intention_test.go +++ b/agent/structs/intention_test.go @@ -242,58 +242,85 @@ func TestIntentionValidate(t *testing.T) { } func TestIntentionPrecedenceSorter(t *testing.T) { + type fields struct { + SrcPeer string + SrcNS string + SrcN string + DstNS string + DstN string + } cases := []struct { Name string - Input [][]string // SrcNS, SrcN, DstNS, DstN - Expected [][]string // Same structure as Input + Input []fields + Expected []fields }{ { "exhaustive list", - [][]string{ - {"*", "*", "exact", "*"}, - {"*", "*", "*", "*"}, - {"exact", "*", "exact", "exact"}, - {"*", "*", "exact", "exact"}, - {"exact", "exact", "*", "*"}, - {"exact", "exact", "exact", "exact"}, - {"exact", "exact", "exact", "*"}, - {"exact", "*", "exact", "*"}, - {"exact", "*", "*", "*"}, + []fields{ + // Peer fields + {SrcPeer: "peer", SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "*", SrcN: "*", DstNS: "*", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "peer", SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "exact", DstNS: "*", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "*", DstNS: "*", DstN: "*"}, + + {SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcNS: "*", SrcN: "*", DstNS: "*", DstN: "*"}, + {SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcNS: "exact", SrcN: "exact", DstNS: "*", DstN: "*"}, + {SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "exact"}, + {SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "*"}, + {SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcNS: "exact", SrcN: "*", DstNS: "*", DstN: "*"}, }, - [][]string{ - {"exact", "exact", "exact", "exact"}, - {"exact", "*", "exact", "exact"}, - {"*", "*", "exact", "exact"}, - {"exact", "exact", "exact", "*"}, - {"exact", "*", "exact", "*"}, - {"*", "*", "exact", "*"}, - {"exact", "exact", "*", "*"}, - {"exact", "*", "*", "*"}, - {"*", "*", "*", "*"}, + []fields{ + {SrcPeer: "", SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "", SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "", SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "peer", SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "exact"}, + {SrcPeer: "", SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "exact", DstNS: "exact", DstN: "*"}, + {SrcPeer: "", SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcPeer: "", SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "*", SrcN: "*", DstNS: "exact", DstN: "*"}, + {SrcPeer: "", SrcNS: "exact", SrcN: "exact", DstNS: "*", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "exact", DstNS: "*", DstN: "*"}, + {SrcPeer: "", SrcNS: "exact", SrcN: "*", DstNS: "*", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "exact", SrcN: "*", DstNS: "*", DstN: "*"}, + {SrcPeer: "", SrcNS: "*", SrcN: "*", DstNS: "*", DstN: "*"}, + {SrcPeer: "peer", SrcNS: "*", SrcN: "*", DstNS: "*", DstN: "*"}, }, }, { "tiebreak deterministically", - [][]string{ - {"a", "*", "a", "b"}, - {"a", "*", "a", "a"}, - {"b", "a", "a", "a"}, - {"a", "b", "a", "a"}, - {"a", "a", "b", "a"}, - {"a", "a", "a", "b"}, - {"a", "a", "a", "a"}, + []fields{ + {SrcNS: "a", SrcN: "*", DstNS: "a", DstN: "b"}, + {SrcNS: "a", SrcN: "*", DstNS: "a", DstN: "a"}, + {SrcNS: "b", SrcN: "a", DstNS: "a", DstN: "a"}, + {SrcNS: "a", SrcN: "b", DstNS: "a", DstN: "a"}, + {SrcNS: "a", SrcN: "a", DstNS: "b", DstN: "a"}, + {SrcNS: "a", SrcN: "a", DstNS: "a", DstN: "b"}, + {SrcNS: "a", SrcN: "a", DstNS: "a", DstN: "a"}, }, - [][]string{ + []fields{ // Exact matches first in lexicographical order (arbitrary but // deterministic) - {"a", "a", "a", "a"}, - {"a", "a", "a", "b"}, - {"a", "a", "b", "a"}, - {"a", "b", "a", "a"}, - {"b", "a", "a", "a"}, + {SrcNS: "a", SrcN: "a", DstNS: "a", DstN: "a"}, + {SrcNS: "a", SrcN: "a", DstNS: "a", DstN: "b"}, + {SrcNS: "a", SrcN: "a", DstNS: "b", DstN: "a"}, + {SrcNS: "a", SrcN: "b", DstNS: "a", DstN: "a"}, + {SrcNS: "b", SrcN: "a", DstNS: "a", DstN: "a"}, // Wildcards next, lexicographical - {"a", "*", "a", "a"}, - {"a", "*", "a", "b"}, + {SrcNS: "a", SrcN: "*", DstNS: "a", DstN: "a"}, + {SrcNS: "a", SrcN: "*", DstNS: "a", DstN: "b"}, }, }, } @@ -304,10 +331,11 @@ func TestIntentionPrecedenceSorter(t *testing.T) { var input Intentions for _, v := range tc.Input { input = append(input, &Intention{ - SourceNS: v[0], - SourceName: v[1], - DestinationNS: v[2], - DestinationName: v[3], + SourcePeer: v.SrcPeer, + SourceNS: v.SrcNS, + SourceName: v.SrcN, + DestinationNS: v.DstNS, + DestinationName: v.DstN, }) } @@ -320,13 +348,14 @@ func TestIntentionPrecedenceSorter(t *testing.T) { sort.Sort(IntentionPrecedenceSorter(input)) // Get back into a comparable form - var actual [][]string + var actual []fields for _, v := range input { - actual = append(actual, []string{ - v.SourceNS, - v.SourceName, - v.DestinationNS, - v.DestinationName, + actual = append(actual, fields{ + SrcPeer: v.SourcePeer, + SrcNS: v.SourceNS, + SrcN: v.SourceName, + DstNS: v.DestinationNS, + DstN: v.DestinationName, }) } assert.Equal(t, tc.Expected, actual) @@ -443,6 +472,15 @@ func TestIntention_String(t *testing.T) { }, partitionPrefix + `default/foo => ` + partitionPrefix + `default/bar (Precedence: 9, Permissions: 2)`, }, + "L4 allow with source peer": { + &Intention{ + SourceName: "foo", + SourcePeer: "billing", + DestinationName: "bar", + Action: IntentionActionAllow, + }, + `peer(billing)/default/foo => ` + partitionPrefix + `default/bar (Precedence: 9, Action: ALLOW)`, + }, } for name, tc := range cases { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index a7b4dcd732..0bccf8a419 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -1181,7 +1181,7 @@ const ( // 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. + // based on centralized configuration. ServiceKindDestination ServiceKind = "destination" ) @@ -2154,6 +2154,12 @@ type IndexedServices struct { QueryMeta } +// PeeredServiceName is a basic tuple of ServiceName and peer +type PeeredServiceName struct { + ServiceName ServiceName + Peer string +} + type ServiceName struct { Name string acl.EnterpriseMeta diff --git a/agent/structs/structs_filtering_test.go b/agent/structs/structs_filtering_test.go index 6b111ea2cc..6b0541e57d 100644 --- a/agent/structs/structs_filtering_test.go +++ b/agent/structs/structs_filtering_test.go @@ -682,6 +682,11 @@ var expectedFieldConfigIntention bexpr.FieldConfigurations = bexpr.FieldConfigur CoerceFn: bexpr.CoerceString, SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual, bexpr.MatchIn, bexpr.MatchNotIn, bexpr.MatchMatches, bexpr.MatchNotMatches}, }, + "SourcePeer": &bexpr.FieldConfiguration{ + StructFieldName: "SourcePeer", + CoerceFn: bexpr.CoerceString, + SupportedOperations: []bexpr.MatchOperator{bexpr.MatchEqual, bexpr.MatchNotEqual, bexpr.MatchIn, bexpr.MatchNotIn, bexpr.MatchMatches, bexpr.MatchNotMatches}, + }, "SourcePartition": &bexpr.FieldConfiguration{ StructFieldName: "SourcePartition", CoerceFn: bexpr.CoerceString, diff --git a/api/config_entry_intentions.go b/api/config_entry_intentions.go index 3741e0a590..0bff5e8e39 100644 --- a/api/config_entry_intentions.go +++ b/api/config_entry_intentions.go @@ -18,6 +18,7 @@ type ServiceIntentionsConfigEntry struct { type SourceIntention struct { Name string + Peer string `json:",omitempty"` Partition string `json:",omitempty"` Namespace string `json:",omitempty"` Action IntentionAction `json:",omitempty"` diff --git a/api/connect_intention.go b/api/connect_intention.go index 734d4ab0fd..0c2500fd06 100644 --- a/api/connect_intention.go +++ b/api/connect_intention.go @@ -35,6 +35,11 @@ type Intention struct { SourcePartition string `json:",omitempty"` DestinationPartition string `json:",omitempty"` + // SourcePeer cannot be a wildcard "*" and is not compatible with legacy + // intentions. Cannot be used with SourcePartition, as both represent the + // same level of tenancy (partition is local to cluster, peer is remote). + SourcePeer string `json:",omitempty"` + // SourceType is the type of the value for the source. SourceType IntentionSourceType