From 31715fa30e546282a6396b86a902cdfb7af0fd58 Mon Sep 17 00:00:00 2001 From: Sebastian Wicki Date: Tue, 25 Feb 2020 16:09:20 +0100 Subject: [PATCH 1/4] labels: Rename LabelArray.Same to LabelArray.Equals `Equals` is more idiomatic than `Same` and consistent with Label.Equals. Signed-off-by: Sebastian Wicki --- pkg/labels/array.go | 4 +- pkg/labels/array_test.go | 85 +++++++++++++++++++------------------ pkg/policy/selectorcache.go | 2 +- 3 files changed, 46 insertions(+), 45 deletions(-) diff --git a/pkg/labels/array.go b/pkg/labels/array.go index 88015ecc782a..3b92835a9d7e 100644 --- a/pkg/labels/array.go +++ b/pkg/labels/array.go @@ -213,8 +213,8 @@ func (ls LabelArray) StringMap() map[string]string { return o } -// Same returns true if the label arrays are the same, i.e., have the same labels in the same order. -func (ls LabelArray) Same(b LabelArray) bool { +// Equals returns true if the label arrays are the same, i.e., have the same labels in the same order. +func (ls LabelArray) Equals(b LabelArray) bool { if len(ls) != len(b) { return false } diff --git a/pkg/labels/array_test.go b/pkg/labels/array_test.go index 415d6d4af94c..fcb64d9d8471 100644 --- a/pkg/labels/array_test.go +++ b/pkg/labels/array_test.go @@ -84,7 +84,7 @@ func (s *LabelsSuite) TestHas(c *C) { } } -func (s *LabelsSuite) TestSame(c *C) { +func (s *LabelsSuite) TestEquals(c *C) { lbls1 := LabelArray{ NewLabel("env", "devel", LabelSourceAny), NewLabel("user", "bob", LabelSourceContainer), @@ -105,47 +105,48 @@ func (s *LabelsSuite) TestSame(c *C) { } lbls6 := LabelArray{} - c.Assert(lbls1.Same(lbls1), Equals, true) - c.Assert(lbls1.Same(lbls2), Equals, true) - c.Assert(lbls1.Same(lbls3), Equals, false) // inverted order - c.Assert(lbls1.Same(lbls4), Equals, false) // different count - c.Assert(lbls1.Same(lbls5), Equals, false) - c.Assert(lbls1.Same(lbls6), Equals, false) - - c.Assert(lbls2.Same(lbls1), Equals, true) - c.Assert(lbls2.Same(lbls2), Equals, true) - c.Assert(lbls2.Same(lbls3), Equals, false) // inverted order - c.Assert(lbls2.Same(lbls4), Equals, false) // different count - c.Assert(lbls2.Same(lbls5), Equals, false) - c.Assert(lbls2.Same(lbls6), Equals, false) - - c.Assert(lbls3.Same(lbls1), Equals, false) - c.Assert(lbls3.Same(lbls2), Equals, false) - c.Assert(lbls3.Same(lbls3), Equals, true) - c.Assert(lbls3.Same(lbls4), Equals, false) - c.Assert(lbls3.Same(lbls5), Equals, false) - c.Assert(lbls3.Same(lbls6), Equals, false) - - c.Assert(lbls4.Same(lbls1), Equals, false) - c.Assert(lbls4.Same(lbls2), Equals, false) - c.Assert(lbls4.Same(lbls3), Equals, false) - c.Assert(lbls4.Same(lbls4), Equals, true) - c.Assert(lbls4.Same(lbls5), Equals, false) - c.Assert(lbls4.Same(lbls6), Equals, false) - - c.Assert(lbls5.Same(lbls1), Equals, false) - c.Assert(lbls5.Same(lbls2), Equals, false) - c.Assert(lbls5.Same(lbls3), Equals, false) - c.Assert(lbls5.Same(lbls4), Equals, false) - c.Assert(lbls5.Same(lbls5), Equals, true) - c.Assert(lbls5.Same(lbls6), Equals, false) - - c.Assert(lbls6.Same(lbls1), Equals, false) - c.Assert(lbls6.Same(lbls2), Equals, false) - c.Assert(lbls6.Same(lbls3), Equals, false) - c.Assert(lbls6.Same(lbls4), Equals, false) - c.Assert(lbls6.Same(lbls5), Equals, false) - c.Assert(lbls6.Same(lbls6), Equals, true) + c.Assert(lbls1.Equals(lbls1), Equals, true) + c.Assert(lbls1.Equals(lbls2), Equals, true) + c.Assert(lbls1.Equals(lbls3), Equals, false) // inverted order + c.Assert(lbls1.Equals(lbls4), Equals, false) // different count + c.Assert(lbls1.Equals(lbls5), Equals, false) + c.Assert(lbls1.Equals(lbls6), Equals, false) + + c.Assert(lbls2.Equals(lbls1), Equals, true) + c.Assert(lbls2.Equals(lbls2), Equals, true) + c.Assert(lbls2.Equals(lbls3), Equals, false) // inverted order + c.Assert(lbls2.Equals(lbls4), Equals, false) // different count + c.Assert(lbls2.Equals(lbls5), Equals, false) + c.Assert(lbls2.Equals(lbls6), Equals, false) + + c.Assert(lbls3.Equals(lbls1), Equals, false) + c.Assert(lbls3.Equals(lbls2), Equals, false) + c.Assert(lbls3.Equals(lbls3), Equals, true) + c.Assert(lbls3.Equals(lbls4), Equals, false) + c.Assert(lbls3.Equals(lbls5), Equals, false) + c.Assert(lbls3.Equals(lbls6), Equals, false) + + c.Assert(lbls4.Equals(lbls1), Equals, false) + c.Assert(lbls4.Equals(lbls2), Equals, false) + c.Assert(lbls4.Equals(lbls3), Equals, false) + c.Assert(lbls4.Equals(lbls4), Equals, true) + c.Assert(lbls4.Equals(lbls5), Equals, false) + c.Assert(lbls4.Equals(lbls6), Equals, false) + + c.Assert(lbls5.Equals(lbls1), Equals, false) + c.Assert(lbls5.Equals(lbls2), Equals, false) + c.Assert(lbls5.Equals(lbls3), Equals, false) + c.Assert(lbls5.Equals(lbls4), Equals, false) + c.Assert(lbls5.Equals(lbls5), Equals, true) + c.Assert(lbls5.Equals(lbls6), Equals, false) + + c.Assert(lbls6.Equals(lbls1), Equals, false) + c.Assert(lbls6.Equals(lbls2), Equals, false) + c.Assert(lbls6.Equals(lbls3), Equals, false) + c.Assert(lbls6.Equals(lbls4), Equals, false) + c.Assert(lbls6.Equals(lbls5), Equals, false) + c.Assert(lbls6.Equals(lbls6), Equals, true) +} } // TestOutputConversions tests the various ways a LabelArray can be converted diff --git a/pkg/policy/selectorcache.go b/pkg/policy/selectorcache.go index bc13256f7b45..3a500046e298 100644 --- a/pkg/policy/selectorcache.go +++ b/pkg/policy/selectorcache.go @@ -791,7 +791,7 @@ func (sc *SelectorCache) UpdateIdentities(added, deleted cache.IdentityCache) { // order is different, but identity labels are // sorted for the kv-store, so there should // not be too many false negatives. - if lbls.Same(old.lbls) { + if lbls.Equals(old.lbls) { log.WithFields(logrus.Fields{logfields.Identity: numericID}).Debug("UpdateIdentities: Skipping add of an existing identical identity") delete(added, numericID) continue From 7c69d9d9e120bc79e039580d43da479dacd0b8a6 Mon Sep 17 00:00:00 2001 From: Sebastian Wicki Date: Tue, 25 Feb 2020 16:13:29 +0100 Subject: [PATCH 2/4] labels: Add Sort and Equals to LabelArrayList These functions are used to create canonical representations of the LabelArrayList that can be used to check if two LabelArrayList contain the same set of labels. Signed-off-by: Sebastian Wicki --- pkg/labels/array.go | 33 +++++++ pkg/labels/array_test.go | 101 ++++++++++++++++++++ pkg/labels/arraylist.go | 26 ++++++ pkg/labels/arraylist_test.go | 176 +++++++++++++++++++++++++++++++++++ 4 files changed, 336 insertions(+) create mode 100644 pkg/labels/arraylist_test.go diff --git a/pkg/labels/array.go b/pkg/labels/array.go index 3b92835a9d7e..d7a1622b9105 100644 --- a/pkg/labels/array.go +++ b/pkg/labels/array.go @@ -225,3 +225,36 @@ func (ls LabelArray) Equals(b LabelArray) bool { } return true } + +// Less returns true if ls comes before b in the lexicographical order. +// Assumes both ls and b are already sorted. +func (ls LabelArray) Less(b LabelArray) bool { + lsLen, bLen := len(ls), len(b) + + minLen := lsLen + if bLen < minLen { + minLen = bLen + } + + for i := 0; i < minLen; i++ { + switch { + // Key + case ls[i].Key < b[i].Key: + return true + case ls[i].Key > b[i].Key: + return false + // Value + case ls[i].Value < b[i].Value: + return true + case ls[i].Value > b[i].Value: + return false + // Source + case ls[i].Source < b[i].Source: + return true + case ls[i].Source > b[i].Source: + return false + } + } + + return lsLen < bLen +} diff --git a/pkg/labels/array_test.go b/pkg/labels/array_test.go index fcb64d9d8471..15418dc8b5da 100644 --- a/pkg/labels/array_test.go +++ b/pkg/labels/array_test.go @@ -147,6 +147,107 @@ func (s *LabelsSuite) TestEquals(c *C) { c.Assert(lbls6.Equals(lbls5), Equals, false) c.Assert(lbls6.Equals(lbls6), Equals, true) } + +func (s *LabelsSuite) TestLess(c *C) { + // lbls1-lbls8 are defined in lexical order + lbls1 := LabelArray(nil) + lbls2 := LabelArray{ + NewLabel("env", "devel", LabelSourceAny), + } + lbls3 := LabelArray{ + NewLabel("env", "devel", LabelSourceReserved), + } + lbls4 := LabelArray{ + NewLabel("env", "prod", LabelSourceAny), + } + lbls5 := LabelArray{ + NewLabel("env", "prod", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + } + lbls6 := LabelArray{ + NewLabel("env", "prod", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + } + lbls7 := LabelArray{ + NewLabel("env", "prod", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + NewLabel("xyz", "", LabelSourceAny), + } + lbls8 := LabelArray{ + NewLabel("foo", "bar", LabelSourceAny), + } + + c.Assert(lbls1.Less(lbls1), Equals, false) + c.Assert(lbls1.Less(lbls2), Equals, true) + c.Assert(lbls1.Less(lbls3), Equals, true) + c.Assert(lbls1.Less(lbls4), Equals, true) + c.Assert(lbls1.Less(lbls5), Equals, true) + c.Assert(lbls1.Less(lbls6), Equals, true) + c.Assert(lbls1.Less(lbls7), Equals, true) + c.Assert(lbls1.Less(lbls8), Equals, true) + + c.Assert(lbls2.Less(lbls1), Equals, false) + c.Assert(lbls2.Less(lbls2), Equals, false) + c.Assert(lbls2.Less(lbls3), Equals, true) + c.Assert(lbls2.Less(lbls4), Equals, true) + c.Assert(lbls2.Less(lbls5), Equals, true) + c.Assert(lbls2.Less(lbls6), Equals, true) + c.Assert(lbls2.Less(lbls7), Equals, true) + c.Assert(lbls2.Less(lbls8), Equals, true) + + c.Assert(lbls3.Less(lbls1), Equals, false) + c.Assert(lbls3.Less(lbls2), Equals, false) + c.Assert(lbls3.Less(lbls3), Equals, false) + c.Assert(lbls3.Less(lbls4), Equals, true) + c.Assert(lbls3.Less(lbls5), Equals, true) + c.Assert(lbls3.Less(lbls6), Equals, true) + c.Assert(lbls3.Less(lbls7), Equals, true) + c.Assert(lbls3.Less(lbls8), Equals, true) + + c.Assert(lbls4.Less(lbls1), Equals, false) + c.Assert(lbls4.Less(lbls2), Equals, false) + c.Assert(lbls4.Less(lbls3), Equals, false) + c.Assert(lbls4.Less(lbls4), Equals, false) + c.Assert(lbls4.Less(lbls5), Equals, true) + c.Assert(lbls4.Less(lbls6), Equals, true) + c.Assert(lbls4.Less(lbls7), Equals, true) + c.Assert(lbls4.Less(lbls8), Equals, true) + + c.Assert(lbls5.Less(lbls1), Equals, false) + c.Assert(lbls5.Less(lbls2), Equals, false) + c.Assert(lbls5.Less(lbls3), Equals, false) + c.Assert(lbls5.Less(lbls4), Equals, false) + c.Assert(lbls5.Less(lbls5), Equals, false) + c.Assert(lbls5.Less(lbls6), Equals, false) + c.Assert(lbls5.Less(lbls7), Equals, true) + c.Assert(lbls5.Less(lbls8), Equals, true) + + c.Assert(lbls6.Less(lbls1), Equals, false) + c.Assert(lbls6.Less(lbls2), Equals, false) + c.Assert(lbls6.Less(lbls3), Equals, false) + c.Assert(lbls6.Less(lbls4), Equals, false) + c.Assert(lbls6.Less(lbls5), Equals, false) + c.Assert(lbls6.Less(lbls6), Equals, false) + c.Assert(lbls6.Less(lbls7), Equals, true) + c.Assert(lbls6.Less(lbls8), Equals, true) + + c.Assert(lbls7.Less(lbls1), Equals, false) + c.Assert(lbls7.Less(lbls2), Equals, false) + c.Assert(lbls7.Less(lbls3), Equals, false) + c.Assert(lbls7.Less(lbls4), Equals, false) + c.Assert(lbls7.Less(lbls5), Equals, false) + c.Assert(lbls7.Less(lbls6), Equals, false) + c.Assert(lbls7.Less(lbls7), Equals, false) + c.Assert(lbls7.Less(lbls8), Equals, true) + + c.Assert(lbls8.Less(lbls1), Equals, false) + c.Assert(lbls8.Less(lbls2), Equals, false) + c.Assert(lbls8.Less(lbls3), Equals, false) + c.Assert(lbls8.Less(lbls4), Equals, false) + c.Assert(lbls8.Less(lbls5), Equals, false) + c.Assert(lbls8.Less(lbls6), Equals, false) + c.Assert(lbls8.Less(lbls7), Equals, false) + c.Assert(lbls8.Less(lbls8), Equals, false) } // TestOutputConversions tests the various ways a LabelArray can be converted diff --git a/pkg/labels/arraylist.go b/pkg/labels/arraylist.go index e48224e41cac..350bc1429e26 100644 --- a/pkg/labels/arraylist.go +++ b/pkg/labels/arraylist.go @@ -14,6 +14,8 @@ package labels +import "sort" + // LabelArrayList is an array of LabelArrays. It is primarily intended as a // simple collection type LabelArrayList []LabelArray @@ -40,3 +42,27 @@ func (ls LabelArrayList) GetModel() [][]string { } return res } + +// Equals returns true if the label arrays lists have the same label arrays in the same order. +func (ls LabelArrayList) Equals(b LabelArrayList) bool { + if len(ls) != len(b) { + return false + } + for l := range ls { + if !ls[l].Equals(b[l]) { + return false + } + } + return true +} + +// Sort sorts the LabelArrayList in-place, but also returns the sorted list +// for convenience. The LabelArrays themselves must already be sorted. This is +// true for all constructors of LabelArray. +func (ls LabelArrayList) Sort() LabelArrayList { + sort.Slice(ls, func(i, j int) bool { + return ls[i].Less(ls[j]) + }) + + return ls +} diff --git a/pkg/labels/arraylist_test.go b/pkg/labels/arraylist_test.go new file mode 100644 index 000000000000..34f9d579fc21 --- /dev/null +++ b/pkg/labels/arraylist_test.go @@ -0,0 +1,176 @@ +// Copyright 2020 Authors of Cilium +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !privileged_tests + +package labels + +import ( + "github.com/cilium/cilium/pkg/checker" + . "gopkg.in/check.v1" +) + +func (s *LabelsSuite) TestLabelArrayListEquals(c *C) { + list1 := LabelArrayList{ + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + }, + { + NewLabel("foo", "bar", LabelSourceAny), + }, + } + list2 := LabelArrayList{ + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + }, + { + NewLabel("foo", "bar", LabelSourceAny), + }, + } + list3 := LabelArrayList{ + { + NewLabel("foo", "bar", LabelSourceAny), + }, + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + }, + } + list4 := LabelArrayList{ + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + }, + } + list5 := LabelArrayList(nil) + list6 := LabelArrayList{} + + c.Assert(list1.Equals(list1), Equals, true) + c.Assert(list1.Equals(list2), Equals, true) + c.Assert(list1.Equals(list3), Equals, false) + c.Assert(list1.Equals(list4), Equals, false) + c.Assert(list1.Equals(list5), Equals, false) + c.Assert(list1.Equals(list6), Equals, false) + + c.Assert(list2.Equals(list1), Equals, true) + c.Assert(list2.Equals(list2), Equals, true) + c.Assert(list2.Equals(list3), Equals, false) + c.Assert(list2.Equals(list4), Equals, false) + c.Assert(list2.Equals(list5), Equals, false) + c.Assert(list2.Equals(list6), Equals, false) + + c.Assert(list3.Equals(list1), Equals, false) + c.Assert(list3.Equals(list2), Equals, false) + c.Assert(list3.Equals(list3), Equals, true) + c.Assert(list3.Equals(list4), Equals, false) + c.Assert(list3.Equals(list5), Equals, false) + c.Assert(list3.Equals(list6), Equals, false) + + c.Assert(list4.Equals(list1), Equals, false) + c.Assert(list4.Equals(list2), Equals, false) + c.Assert(list4.Equals(list3), Equals, false) + c.Assert(list4.Equals(list4), Equals, true) + c.Assert(list4.Equals(list5), Equals, false) + c.Assert(list4.Equals(list6), Equals, false) + + c.Assert(list5.Equals(list1), Equals, false) + c.Assert(list5.Equals(list2), Equals, false) + c.Assert(list5.Equals(list3), Equals, false) + c.Assert(list5.Equals(list4), Equals, false) + c.Assert(list5.Equals(list5), Equals, true) + c.Assert(list5.Equals(list6), Equals, true) + + c.Assert(list6.Equals(list1), Equals, false) + c.Assert(list6.Equals(list2), Equals, false) + c.Assert(list6.Equals(list3), Equals, false) + c.Assert(list6.Equals(list4), Equals, false) + c.Assert(list6.Equals(list5), Equals, true) + c.Assert(list6.Equals(list6), Equals, true) +} + +func (s *LabelsSuite) TestLabelArrayListSort(c *C) { + c.Assert(LabelArrayList(nil).Sort(), checker.DeepEquals, LabelArrayList(nil)) + c.Assert(LabelArrayList{}.Sort(), checker.DeepEquals, LabelArrayList{}) + + list1 := LabelArrayList{ + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + }, + { + NewLabel("aaa", "", LabelSourceReserved), + }, + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + NewLabel("xyz", "", LabelSourceAny), + }, + { + NewLabel("env", "devel", LabelSourceAny), + }, + nil, + { + NewLabel("foo", "bar", LabelSourceAny), + }, + } + expected1 := LabelArrayList{ + nil, + { + NewLabel("aaa", "", LabelSourceReserved), + }, + { + NewLabel("env", "devel", LabelSourceAny), + }, + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + }, + { + NewLabel("env", "devel", LabelSourceAny), + NewLabel("user", "bob", LabelSourceContainer), + NewLabel("xyz", "", LabelSourceAny), + }, + { + NewLabel("foo", "bar", LabelSourceAny), + }, + } + + c.Assert(list1.Sort(), checker.DeepEquals, expected1) + + list2 := LabelArrayList{ + { + NewLabel("aaa", "", LabelSourceReserved), + }, + { + NewLabel("env", "devel", LabelSourceAny), + }, + { + NewLabel("aaa", "", LabelSourceAny), + }, + } + expected2 := LabelArrayList{ + { + NewLabel("aaa", "", LabelSourceAny), + }, + { + NewLabel("aaa", "", LabelSourceReserved), + }, + { + NewLabel("env", "devel", LabelSourceAny), + }, + } + c.Assert(list2.Sort(), checker.DeepEquals, expected2) +} From d296f5f380756b79de4fd73f1c9589f7682d4094 Mon Sep 17 00:00:00 2001 From: Sebastian Wicki Date: Tue, 25 Feb 2020 16:24:26 +0100 Subject: [PATCH 3/4] policy: Track source policy rules in MapStateEntry This commit allows us to track the policies for which a certain policy map entry has been created. It is implemented by copying over the `DerivedFromRules` from the merged ingress/egress filters to the user-space representation of the policy map state. These entries are then moved over into the `realizedPolicy` of each endpoint when the policy maps are synced. Since the order of the `DerivedFromRules` rules is not deterministic, sort each LabelArrayList when the computed policy is attached. Default entries such as `AllowAnyIngress`, `AllowAnyEgress` and `AllowLocalHostIngress` are annotated with artificial labels (of label source `reserved`). Signed-off-by: Sebastian Wicki --- pkg/endpoint/bpf.go | 16 ++++-- pkg/policy/distillery_test.go | 103 +++++++++++++++++++++------------- pkg/policy/l4.go | 18 +++--- pkg/policy/mapstate.go | 84 +++++++++++++++++++++------ pkg/policy/resolve.go | 4 +- pkg/policy/resolve_test.go | 42 ++++++++++---- 6 files changed, 182 insertions(+), 85 deletions(-) diff --git a/pkg/endpoint/bpf.go b/pkg/endpoint/bpf.go index 461d750e1797..3a6c397bc09f 100644 --- a/pkg/endpoint/bpf.go +++ b/pkg/endpoint/bpf.go @@ -30,6 +30,7 @@ import ( "github.com/cilium/cilium/pkg/completion" "github.com/cilium/cilium/pkg/controller" "github.com/cilium/cilium/pkg/endpoint/regeneration" + "github.com/cilium/cilium/pkg/labels" "github.com/cilium/cilium/pkg/loadinfo" "github.com/cilium/cilium/pkg/logging/logfields" "github.com/cilium/cilium/pkg/maps/ctmap" @@ -224,7 +225,7 @@ func (e *Endpoint) addNewRedirectsFromDesiredPolicy(ingress bool, desiredRedirec } else { insertedDesiredMapState[keyFromFilter] = struct{}{} } - if entry != policy.NoRedirectEntry { + if entry.IsRedirectEntry() { entry.ProxyPort = redirectPort } e.desiredPolicy.PolicyMapState[keyFromFilter] = entry @@ -334,10 +335,15 @@ func (e *Endpoint) addVisibilityRedirects(ingress bool, desiredRedirects map[str TrafficDirection: direction.Uint8(), } - e.desiredPolicy.PolicyMapState[newKey] = policy.MapStateEntry{ - ProxyPort: redirectPort, + derivedFrom := labels.LabelArrayList{ + labels.LabelArray{ + labels.NewLabel(policy.LabelKeyPolicyDerivedFrom, policy.LabelVisibilityAnnotation, labels.LabelSourceReserved), + }, } + entry := policy.NewMapStateEntry(derivedFrom, true) + entry.ProxyPort = redirectPort + e.desiredPolicy.PolicyMapState[newKey] = entry insertedDesiredMapState[newKey] = struct{}{} } @@ -1068,7 +1074,7 @@ func (e *Endpoint) applyPolicyMapChanges() (proxyChanges bool, err error) { for keyToAdd, entry := range adds { // Keep the existing proxy port, if any - if entry != policy.NoRedirectEntry { + if entry.IsRedirectEntry() { entry.ProxyPort = e.realizedRedirects[policy.ProxyIDFromKey(e.ID, keyToAdd)] if entry.ProxyPort != 0 { proxyChanges = true @@ -1143,7 +1149,7 @@ func (e *Endpoint) addPolicyMapDelta() error { errors := 0 for keyToAdd, entry := range e.desiredPolicy.PolicyMapState { - if oldEntry, ok := e.realizedPolicy.PolicyMapState[keyToAdd]; !ok || oldEntry != entry { + if oldEntry, ok := e.realizedPolicy.PolicyMapState[keyToAdd]; !ok || !oldEntry.Equal(&entry) { if !e.addPolicyKey(keyToAdd, entry, false) { errors++ } diff --git a/pkg/policy/distillery_test.go b/pkg/policy/distillery_test.go index d388a6dcc996..a9b8888b52ce 100644 --- a/pkg/policy/distillery_test.go +++ b/pkg/policy/distillery_test.go @@ -195,45 +195,64 @@ var ( L7Proto: ParserTypeHTTP.String(), } // API rule definitions for default-deny, L3, L3L4, L3L4L7, L4, L4L7 + lbls____NoAllow = labels.ParseLabelArray("no-allow") rule____NoAllow = api.NewRule(). + WithLabels(lbls____NoAllow). WithIngressRules([]api.IngressRule{{}}) + lblsL3____Allow = labels.ParseLabelArray("l3-allow") ruleL3____Allow = api.NewRule(). + WithLabels(lblsL3____Allow). WithIngressRules([]api.IngressRule{{ FromEndpoints: []api.EndpointSelector{allowFooL3_}, ToPorts: allowAllL4_, }}) + lblsL3L4__Allow = labels.ParseLabelArray("l3l4-allow") ruleL3L4__Allow = api.NewRule(). + WithLabels(lblsL3L4__Allow). WithIngressRules([]api.IngressRule{{ FromEndpoints: []api.EndpointSelector{allowFooL3_}, ToPorts: allowPort80, }}) + lblsL3L4L7Allow = labels.ParseLabelArray("l3l4l7-allow") ruleL3L4L7Allow = api.NewRule(). + WithLabels(lblsL3L4L7Allow). WithIngressRules([]api.IngressRule{{ FromEndpoints: []api.EndpointSelector{allowFooL3_}, ToPorts: combineL4L7(allowPort80, allowHTTPRoot), }}) + lbls__L4__Allow = labels.ParseLabelArray("l4-allow") rule__L4__Allow = api.NewRule(). + WithLabels(lbls__L4__Allow). WithIngressRules([]api.IngressRule{{ ToPorts: allowPort80, }}) + lbls__L4L7Allow = labels.ParseLabelArray("l4l7-allow") rule__L4L7Allow = api.NewRule(). + WithLabels(lbls__L4L7Allow). WithIngressRules([]api.IngressRule{{ ToPorts: combineL4L7(allowPort80, allowHTTPRoot), }}) - + lbls__L3AllowFoo = labels.ParseLabelArray("l3-allow-foo") rule__L3AllowFoo = api.NewRule(). + WithLabels(lbls__L3AllowFoo). WithIngressRules([]api.IngressRule{{ FromEndpoints: []api.EndpointSelector{allowFooL3_}, }}) - + lbls__L3AllowBar = labels.ParseLabelArray("l3-allow-bar") rule__L3AllowBar = api.NewRule(). + WithLabels(lbls__L3AllowBar). WithIngressRules([]api.IngressRule{{ FromEndpoints: []api.EndpointSelector{allowBarL3_}, }}) + lbls____AllowAll = labels.ParseLabelArray("allow-all") rule____AllowAll = api.NewRule(). + WithLabels(lbls____AllowAll). WithIngressRules([]api.IngressRule{{ FromEndpoints: []api.EndpointSelector{api.WildcardEndpointSelector}, }}) + lblsAllowLocalHostIngress = labels.LabelArray{ + labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowLocalHostIngress, labels.LabelSourceReserved), + } // Misc other bpf key fields for convenience / readability. l7RedirectNone_ = uint16(0) @@ -246,8 +265,12 @@ var ( mapKeyAllow___L4 = Key{0, 80, 6, dirIngress} mapKeyAllowAll__ = Key{0, 0, 0, dirIngress} // Desired map entries for no L7 redirect / redirect to Proxy - mapEntryL7None_ = NoRedirectEntry - mapEntryL7Proxy = redirectEntry + mapEntryL7None_ = func(lbls ...labels.LabelArray) MapStateEntry { + return NewMapStateEntry(labels.LabelArrayList(lbls).Sort(), false) + } + mapEntryL7Proxy = func(lbls ...labels.LabelArray) MapStateEntry { + return NewMapStateEntry(labels.LabelArrayList(lbls).Sort(), true) + } ) // combineL4L7 returns a new PortRule that refers to the specified l4 ports and @@ -316,7 +339,7 @@ func (d *policyDistillery) distillPolicy(epLabels labels.LabelArray) (MapState, for _, l4 := range l4IngressPolicy { io.WriteString(d.log, fmt.Sprintf("[distill] Processing L4Filter (l4: %d/%s), (l3/7: %+v)\n", l4.Port, l4.Protocol, l4.L7RulesPerSelector)) for key, entry := range l4.ToMapState(0) { - io.WriteString(d.log, fmt.Sprintf("[distill] L4 ingress allow %+v (parser=%s, redirect=%t)\n", key, l4.L7Parser, entry != NoRedirectEntry)) + io.WriteString(d.log, fmt.Sprintf("[distill] L4 ingress allow %+v (parser=%s, redirect=%t)\n", key, l4.L7Parser, entry.IsRedirectEntry())) result[key] = entry } } @@ -336,8 +359,8 @@ func Test_MergeL3(t *testing.T) { rules api.Rules result MapState }{ - {0, api.Rules{rule__L3AllowFoo, rule__L3AllowBar}, MapState{mapKeyAllowFoo__: mapEntryL7None_, mapKeyAllowBar__: mapEntryL7None_}}, - {1, api.Rules{rule__L3AllowFoo, ruleL3L4__Allow}, MapState{mapKeyAllowFoo__: mapEntryL7None_, mapKeyAllowFooL4: mapEntryL7None_}}, + {0, api.Rules{rule__L3AllowFoo, rule__L3AllowBar}, MapState{mapKeyAllowFoo__: mapEntryL7None_(lbls__L3AllowFoo, lbls__L3AllowBar), mapKeyAllowBar__: mapEntryL7None_(lbls__L3AllowFoo, lbls__L3AllowBar)}}, + {1, api.Rules{rule__L3AllowFoo, ruleL3L4__Allow}, MapState{mapKeyAllowFoo__: mapEntryL7None_(lbls__L3AllowFoo), mapKeyAllowFooL4: mapEntryL7None_(lblsL3L4__Allow)}}, } for _, tt := range tests { @@ -380,37 +403,37 @@ func Test_MergeRules(t *testing.T) { // // Rule 0 | Rule 1 | Rule 2 | Rule 3 | Rule 4 | Desired BPF map state {0, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{}}, - {1, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFoo__: mapEntryL7None_}}, - {2, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7None_}}, - {3, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7None_, mapKeyAllowFoo__: mapEntryL7None_}}, - {4, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7None_}}, - {5, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7None_, mapKeyAllowFoo__: mapEntryL7None_}}, - {6, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7None_}}, // identical L3L4 entry suppressed - {7, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7None_, mapKeyAllowFoo__: mapEntryL7None_}}, // identical L3L4 entry suppressed - {8, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, - {9, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, - {10, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, - {11, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, - {12, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, // L3L4 entry suppressed to allow L4-only entry to redirect - {13, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, // L3L4 entry suppressed to allow L4-only entry to redirect - {14, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, // L3L4 entry suppressed to allow L4-only entry to redirect - {15, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, // L3L4 entry suppressed to allow L4-only entry to redirect - {16, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy}}, - {17, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, - {18, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy, mapKeyAllow___L4: mapEntryL7None_}}, - {19, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy, mapKeyAllow___L4: mapEntryL7None_, mapKeyAllowFoo__: mapEntryL7None_}}, - {20, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy}}, - {21, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, - {22, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy, mapKeyAllow___L4: mapEntryL7None_}}, - {23, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy, mapKeyAllow___L4: mapEntryL7None_, mapKeyAllowFoo__: mapEntryL7None_}}, - {24, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, // identical L3L4 entry suppressed - {25, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, // identical L3L4 entry suppressed - {26, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, // identical L3L4 entry suppressed - {27, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, // identical L3L4 entry suppressed - {28, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, // identical L3L4 entry suppressed - {29, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, // identical L3L4 entry suppressed - {30, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy}}, // identical L3L4 entry suppressed - {31, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy, mapKeyAllowFoo__: mapEntryL7None_}}, // identical L3L4 entry suppressed + {1, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {2, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7None_(lbls__L4__Allow)}}, + {3, api.Rules{rule____NoAllow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7None_(lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {4, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7None_(lblsL3L4__Allow)}}, + {5, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7None_(lblsL3L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {6, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7None_(lblsL3L4__Allow, lbls__L4__Allow)}}, // identical L3L4 entry suppressed + {7, api.Rules{rule____NoAllow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7None_(lblsL3L4__Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // identical L3L4 entry suppressed + {8, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow)}}, + {9, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {10, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow, lbls__L4__Allow)}}, + {11, api.Rules{rule____NoAllow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {12, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow, lblsL3L4__Allow)}}, // L3L4 entry suppressed to allow L4-only entry to redirect + {13, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow, lblsL3L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // L3L4 entry suppressed to allow L4-only entry to redirect + {14, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow)}}, // L3L4 entry suppressed to allow L4-only entry to redirect + {15, api.Rules{rule____NoAllow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lbls__L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // L3L4 entry suppressed to allow L4-only entry to redirect + {16, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow)}}, + {17, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {18, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4__Allow), mapKeyAllow___L4: mapEntryL7None_(lblsL3L4L7Allow, lbls__L4__Allow)}}, + {19, api.Rules{ruleL3L4L7Allow, rule____NoAllow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4__Allow), mapKeyAllow___L4: mapEntryL7None_(lblsL3L4L7Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {20, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow, lblsL3L4__Allow)}}, + {21, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow, lblsL3L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {22, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow), mapKeyAllow___L4: mapEntryL7None_(lblsL3L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow)}}, + {23, api.Rules{ruleL3L4L7Allow, rule____NoAllow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllowFooL4: mapEntryL7Proxy(lblsL3L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow), mapKeyAllow___L4: mapEntryL7None_(lblsL3L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, + {24, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow)}}, // identical L3L4 entry suppressed + {25, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // identical L3L4 entry suppressed + {26, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow, lbls__L4__Allow)}}, // identical L3L4 entry suppressed + {27, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, rule____NoAllow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // identical L3L4 entry suppressed + {28, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow, lblsL3L4__Allow)}}, // identical L3L4 entry suppressed + {29, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule____NoAllow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow, lblsL3L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // identical L3L4 entry suppressed + {30, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, rule____NoAllow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow)}}, // identical L3L4 entry suppressed + {31, api.Rules{ruleL3L4L7Allow, rule__L4L7Allow, ruleL3L4__Allow, rule__L4__Allow, ruleL3____Allow}, MapState{mapKeyAllow___L4: mapEntryL7Proxy(lblsL3L4L7Allow, lbls__L4L7Allow, lblsL3L4__Allow, lbls__L4__Allow), mapKeyAllowFoo__: mapEntryL7None_(lblsL3____Allow)}}, // identical L3L4 entry suppressed } for _, tt := range tests { repo := newPolicyDistillery(selectorCache) @@ -449,8 +472,8 @@ func Test_AllowAll(t *testing.T) { rules api.Rules result MapState }{ - {0, api.EndpointSelectorNone, api.Rules{rule____AllowAll}, MapState{mapKeyAllowAll__: mapEntryL7None_}}, - {1, api.WildcardEndpointSelector, api.Rules{rule____AllowAll}, MapState{mapKeyAllowAll__: mapEntryL7None_}}, + {0, api.EndpointSelectorNone, api.Rules{rule____AllowAll}, MapState{mapKeyAllowAll__: mapEntryL7None_(lblsAllowLocalHostIngress)}}, + {1, api.WildcardEndpointSelector, api.Rules{rule____AllowAll}, MapState{mapKeyAllowAll__: mapEntryL7None_(lbls____AllowAll)}}, } for _, tt := range tests { diff --git a/pkg/policy/l4.go b/pkg/policy/l4.go index e3f223a42bc7..64a172a05dd4 100644 --- a/pkg/policy/l4.go +++ b/pkg/policy/l4.go @@ -241,7 +241,7 @@ func (l4 *L4Filter) GetPort() uint16 { } // ToMapState converts filter into a MapState with two possible values: -// - NoRedirectEntry (ProxyPort = 0): No proxy redirection is needed for this key +// - Entry with ProxyPort = 0: No proxy redirection is needed for this key // - Entry with any other port #: Proxy redirection is required for this key, // caller must replace the ProxyPort with the actual // listening port number. @@ -291,11 +291,8 @@ func (l4 *L4Filter) ToMapState(direction trafficdirection.TrafficDirection) MapS }).Debug("ToMapState: Skipping L3/L4 key due to existing L4-only key") continue } - entry := NoRedirectEntry - if l7 != nil { - entry = redirectEntry - } + entry := NewMapStateEntry(l4.DerivedFromRules, l7 != nil) if cs.IsWildcard() { keyToAdd.Identity = 0 keysToAdd.RedirectPreferredInsert(keyToAdd, entry) @@ -364,7 +361,7 @@ func (l4 *L4Filter) IdentitySelectionUpdated(selector CachedSelector, selections direction = trafficdirection.Ingress } l4Policy.AccumulateMapChanges(added, deleted, uint16(l4.Port), uint8(l4.U8Proto), direction, - l4.L7RulesPerSelector[selector] != nil) + l4.L7RulesPerSelector[selector] != nil, l4.DerivedFromRules) } } @@ -546,6 +543,10 @@ func (l4 *L4Filter) detach(selectorCache *SelectorCache) { } func (l4 *L4Filter) attach(ctx PolicyContext, l4Policy *L4Policy) { + // All rules have been added to the L4Filter at this point. + // Sort the rules label array list for more efficient equality comparison. + l4.DerivedFromRules.Sort() + // Compute Envoy policies when a policy is ready to be used if ctx != nil { for _, l7policy := range l4.L7RulesPerSelector { @@ -802,10 +803,11 @@ func (l4 *L4Policy) insertUser(user *EndpointPolicy) { // The caller is responsible for making sure the same identity is not // present in both 'adds' and 'deletes'. func (l4 *L4Policy) AccumulateMapChanges(adds, deletes []identity.NumericIdentity, - port uint16, proto uint8, direction trafficdirection.TrafficDirection, redirect bool) { + port uint16, proto uint8, direction trafficdirection.TrafficDirection, + redirect bool, derivedFrom labels.LabelArrayList) { l4.mutex.RLock() for epPolicy := range l4.users { - epPolicy.policyMapChanges.AccumulateMapChanges(adds, deletes, port, proto, direction, redirect) + epPolicy.policyMapChanges.AccumulateMapChanges(adds, deletes, port, proto, direction, redirect, derivedFrom) } l4.mutex.RUnlock() } diff --git a/pkg/policy/mapstate.go b/pkg/policy/mapstate.go index 4c6478668eb4..fc416616e928 100644 --- a/pkg/policy/mapstate.go +++ b/pkg/policy/mapstate.go @@ -16,6 +16,7 @@ package policy import ( "github.com/cilium/cilium/pkg/identity" + "github.com/cilium/cilium/pkg/labels" "github.com/cilium/cilium/pkg/lock" "github.com/cilium/cilium/pkg/logging/logfields" "github.com/cilium/cilium/pkg/option" @@ -32,6 +33,14 @@ var ( } ) +const ( + LabelKeyPolicyDerivedFrom = "io.cilium.policy.derived-from" + LabelAllowLocalHostIngress = "allow-localhost-ingress" + LabelAllowAnyIngress = "allow-any-ingress" + LabelAllowAnyEgress = "allow-any-egress" + LabelVisibilityAnnotation = "visibility-annotation" +) + // MapState is a state of a policy map. type MapState map[Key]MapStateEntry @@ -68,22 +77,47 @@ type MapStateEntry struct { // If 0 (default), there is no proxy redirection for the corresponding // Key. Any other value signifies proxy redirection. ProxyPort uint16 + + // DerivedFromRules tracks the policy rules this entry derives from + DerivedFromRules labels.LabelArrayList } -// NoRedirectEntry is a special MapStateEntry used to signify that the -// entry is not redirecting to a proxy. -var NoRedirectEntry = MapStateEntry{ProxyPort: 0} +// NewMapStateEntry creates a map state entry. If redirect is true, the +// caller is expected to replace the ProxyPort field before it is added to +// the actual BPF map. +func NewMapStateEntry(derivedFrom labels.LabelArrayList, redirect bool) MapStateEntry { + var proxyPort uint16 + if redirect { + // Any non-zero value will do, as the callers replace this with the + // actual proxy listening port number before the entry is added to the + // actual bpf map. + proxyPort = 1 + } + + return MapStateEntry{ + ProxyPort: proxyPort, + DerivedFromRules: derivedFrom, + } +} -// redirectEntry is a MapStateEntry used to signify that the entry -// must redirect to a proxy. Any non-zero value would do, as the -// callers replace this with the actual proxy listening port number -// before the entry is added to the actual bpf map. -var redirectEntry = MapStateEntry{ProxyPort: 1} +// IsRedirectEntry returns true if e contains a redirect +func (e *MapStateEntry) IsRedirectEntry() bool { + return e.ProxyPort != 0 +} + +// Equal returns true of two entries are equal +func (e *MapStateEntry) Equal(o *MapStateEntry) bool { + if e == nil || o == nil { + return e == o + } + + return e.ProxyPort == o.ProxyPort && e.DerivedFromRules.Equals(o.DerivedFromRules) +} // RedirectPreferredInsert inserts a new entry giving priority to L7-redirects by // not overwriting a L7-redirect entry with a non-redirect entry func (keys MapState) RedirectPreferredInsert(key Key, entry MapStateEntry) { - if entry == NoRedirectEntry { + if !entry.IsRedirectEntry() { if _, ok := keys[key]; ok { // Key already exist, keep the existing entry so that // a redirect entry is never overwritten by a non-redirect @@ -100,7 +134,12 @@ func (keys MapState) RedirectPreferredInsert(key Key, entry MapStateEntry) { // endpoint. func (keys MapState) DetermineAllowLocalhostIngress(l4Policy *L4Policy) { if option.Config.AlwaysAllowLocalhost() { - keys[localHostKey] = MapStateEntry{} + derivedFrom := labels.LabelArrayList{ + labels.LabelArray{ + labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowLocalHostIngress, labels.LabelSourceReserved), + }, + } + keys[localHostKey] = NewMapStateEntry(derivedFrom, false) } } @@ -115,7 +154,12 @@ func (keys MapState) AllowAllIdentities(ingress, egress bool) { Nexthdr: 0, TrafficDirection: trafficdirection.Ingress.Uint8(), } - keys[keyToAdd] = MapStateEntry{} + derivedFrom := labels.LabelArrayList{ + labels.LabelArray{ + labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowLocalHostIngress, labels.LabelSourceReserved), + }, + } + keys[keyToAdd] = NewMapStateEntry(derivedFrom, false) } if egress { keyToAdd := Key{ @@ -124,7 +168,12 @@ func (keys MapState) AllowAllIdentities(ingress, egress bool) { Nexthdr: 0, TrafficDirection: trafficdirection.Egress.Uint8(), } - keys[keyToAdd] = MapStateEntry{} + derivedFrom := labels.LabelArrayList{ + labels.LabelArray{ + labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowAnyEgress, labels.LabelSourceReserved), + }, + } + keys[keyToAdd] = NewMapStateEntry(derivedFrom, false) } } @@ -147,7 +196,8 @@ type MapChanges struct { // cases where an identity is first added and then deleted, or first // deleted and then added. func (mc *MapChanges) AccumulateMapChanges(adds, deletes []identity.NumericIdentity, - port uint16, proto uint8, direction trafficdirection.TrafficDirection, redirect bool) { + port uint16, proto uint8, direction trafficdirection.TrafficDirection, + redirect bool, derivedFrom labels.LabelArrayList) { key := Key{ // The actual identity is set in the loops below Identity: 0, @@ -156,10 +206,8 @@ func (mc *MapChanges) AccumulateMapChanges(adds, deletes []identity.NumericIdent Nexthdr: proto, TrafficDirection: direction.Uint8(), } - var value MapStateEntry // defaults to ProxyPort 0 == no redirect - if redirect { - value = redirectEntry // Special entry to mark the need for redirection - } + + value := NewMapStateEntry(derivedFrom, redirect) if option.Config.Debug { log.WithFields(logrus.Fields{ @@ -179,7 +227,7 @@ func (mc *MapChanges) AccumulateMapChanges(adds, deletes []identity.NumericIdent } for _, id := range adds { key.Identity = id.Uint32() - // insert but do not allow NoRedirectEntry to overwrite a redirect entry + // insert but do not allow non-redirect entries to overwrite a redirect entry mc.adds.RedirectPreferredInsert(key, value) // Remove a potential previously deleted key diff --git a/pkg/policy/resolve.go b/pkg/policy/resolve.go index f6d33a59da89..ce6e5b768e1b 100644 --- a/pkg/policy/resolve.go +++ b/pkg/policy/resolve.go @@ -157,13 +157,13 @@ func (p *EndpointPolicy) computeDirectionL4PolicyMapEntries(l4PolicyMap L4Policy keysFromFilter := filter.ToMapState(direction) for keyFromFilter, entry := range keysFromFilter { // Fix up the proxy port for entries that need proxy redirection - if entry != NoRedirectEntry { + if entry.IsRedirectEntry() { entry.ProxyPort = p.PolicyOwner.LookupRedirectPort(filter) // If the currently allocated proxy port is 0, this is a new // redirect, for which no port has been allocated yet. Ignore // it for now. This will be configured by // e.addNewRedirectsFromDesiredPolicy() once the port has been allocated. - if entry == NoRedirectEntry { + if !entry.IsRedirectEntry() { continue } } diff --git a/pkg/policy/resolve_test.go b/pkg/policy/resolve_test.go index ad6a4cf95b1a..c0327f681a4d 100644 --- a/pkg/policy/resolve_test.go +++ b/pkg/policy/resolve_test.go @@ -415,6 +415,11 @@ func (ds *PolicyTestSuite) TestL7WithLocalHostWildcardd(c *C) { func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) { repo := bootstrapRepo(GenerateL3IngressRules, 1000, c) + ruleLabel := labels.ParseLabelArray("rule-foo-allow-port-80") + ruleLabelAllowAnyEgress := labels.LabelArray{ + labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowAnyEgress, labels.LabelSourceReserved), + } + idFooSelectLabelArray := labels.ParseSelectLabelArray("id=foo") idFooSelectLabels := labels.Labels{} for _, lbl := range idFooSelectLabelArray { @@ -425,6 +430,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) { selFoo := api.NewESFromLabels(labels.ParseSelectLabel("id=foo")) rule1 := api.Rule{ EndpointSelector: selFoo, + Labels: ruleLabel, Ingress: []api.IngressRule{ { ToPorts: []api.PortRule{{ @@ -447,6 +453,9 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) { c.Assert(err, IsNil) policy := selPolicy.DistillPolicy(DummyOwner{}) + rule1MapStateEntry := NewMapStateEntry(labels.LabelArrayList{ruleLabel}, false) + allowEgressMapStateEntry := NewMapStateEntry(labels.LabelArrayList{ruleLabelAllowAnyEgress}, false) + expectedEndpointPolicy := EndpointPolicy{ selectorPolicy: &selectorPolicy{ Revision: repo.GetRevision(), @@ -464,7 +473,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) { L7RulesPerSelector: L7DataMap{ wildcardCachedSelector: nil, }, - DerivedFromRules: labels.LabelArrayList{nil}, + DerivedFromRules: labels.LabelArrayList{ruleLabel}, }, }, Egress: L4PolicyMap{}, @@ -475,8 +484,8 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) { }, PolicyOwner: DummyOwner{}, PolicyMapState: MapState{ - {TrafficDirection: trafficdirection.Egress.Uint8()}: {}, - {DestPort: 80, Nexthdr: 6}: {}, + {TrafficDirection: trafficdirection.Egress.Uint8()}: allowEgressMapStateEntry, + {DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, }, } @@ -497,6 +506,11 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) { func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) { repo := bootstrapRepo(GenerateL3IngressRules, 1000, c) + ruleLabel := labels.ParseLabelArray("rule-world-allow-port-80") + ruleLabelAllowAnyEgress := labels.LabelArray{ + labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowAnyEgress, labels.LabelSourceReserved), + } + idFooSelectLabelArray := labels.ParseSelectLabelArray("id=foo") idFooSelectLabels := labels.Labels{} for _, lbl := range idFooSelectLabelArray { @@ -509,6 +523,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) { selFoo := api.NewESFromLabels(labels.ParseSelectLabel("id=foo")) rule1 := api.Rule{ EndpointSelector: selFoo, + Labels: ruleLabel, Ingress: []api.IngressRule{ { FromEntities: []api.Entity{api.EntityWorld}, @@ -566,6 +581,9 @@ func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) { cachedSelectorTest := testSelectorCache.FindCachedIdentitySelector(api.NewESFromLabels(lblTest)) c.Assert(cachedSelectorTest, Not(IsNil)) + rule1MapStateEntry := NewMapStateEntry(labels.LabelArrayList{ruleLabel, ruleLabel}, false) + allowEgressMapStateEntry := NewMapStateEntry(labels.LabelArrayList{ruleLabelAllowAnyEgress}, false) + expectedEndpointPolicy := EndpointPolicy{ selectorPolicy: &selectorPolicy{ Revision: repo.GetRevision(), @@ -583,7 +601,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) { cachedSelectorWorld: nil, cachedSelectorTest: nil, }, - DerivedFromRules: labels.LabelArrayList{nil, nil}, + DerivedFromRules: labels.LabelArrayList{ruleLabel, ruleLabel}, }, }, Egress: L4PolicyMap{}, @@ -594,16 +612,16 @@ func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) { }, PolicyOwner: DummyOwner{}, PolicyMapState: MapState{ - {TrafficDirection: trafficdirection.Egress.Uint8()}: {}, - {Identity: uint32(identity.ReservedIdentityWorld), DestPort: 80, Nexthdr: 6}: {}, + {TrafficDirection: trafficdirection.Egress.Uint8()}: allowEgressMapStateEntry, + {Identity: uint32(identity.ReservedIdentityWorld), DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, }, policyMapChanges: MapChanges{ adds: MapState{ - {Identity: 192, DestPort: 80, Nexthdr: 6}: {}, - {Identity: 194, DestPort: 80, Nexthdr: 6}: {}, + {Identity: 192, DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, + {Identity: 194, DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, }, deletes: MapState{ - {Identity: 193, DestPort: 80, Nexthdr: 6}: {}, + {Identity: 193, DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, }, }, } @@ -623,10 +641,10 @@ func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) { c.Assert(policy.policyMapChanges.deletes, IsNil) c.Assert(adds, checker.Equals, MapState{ - {Identity: 192, DestPort: 80, Nexthdr: 6}: {}, - {Identity: 194, DestPort: 80, Nexthdr: 6}: {}, + {Identity: 192, DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, + {Identity: 194, DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, }) c.Assert(deletes, checker.Equals, MapState{ - {Identity: 193, DestPort: 80, Nexthdr: 6}: {}, + {Identity: 193, DestPort: 80, Nexthdr: 6}: rule1MapStateEntry, }) } From 2442d148d60bfee789a1ce55e21134f86b900ffd Mon Sep 17 00:00:00 2001 From: Sebastian Wicki Date: Tue, 25 Feb 2020 16:27:20 +0100 Subject: [PATCH 4/4] endpoint: Add GetRealizedPolicyRuleLabelsForKey This function allows callers to get the list of policies which caused a certain policy map entry to be added for a given endpoint. Signed-off-by: Sebastian Wicki --- pkg/endpoint/policy.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/endpoint/policy.go b/pkg/endpoint/policy.go index 56425e4c4913..83f28b28c717 100644 --- a/pkg/endpoint/policy.go +++ b/pkg/endpoint/policy.go @@ -751,3 +751,23 @@ func (e *Endpoint) UpdateVisibilityPolicy(annoCB AnnotationsResolverCB) { } <-ch } + +// GetRealizedPolicyRuleLabelsForKey returns the list of policy rule labels +// which match a given flow key (in host byte-order). The returned +// LabelArrayList must not be modified. This function is exported to be +// accessed by code outside of the Cilium code base (e.g. Hubble). +func (e *Endpoint) GetRealizedPolicyRuleLabelsForKey(key policy.Key) ( + derivedFrom labels.LabelArrayList, + revision uint64, + ok bool, +) { + e.mutex.RLock() + defer e.mutex.RUnlock() + + entry, ok := e.realizedPolicy.PolicyMapState[key] + if !ok { + return nil, 0, false + } + + return entry.DerivedFromRules, e.policyRevision, true +}