From 91876a3664d2bd7c96c70a46f4ff8a9580f45977 Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Wed, 1 Nov 2017 15:18:09 -0700 Subject: [PATCH 1/3] Implement visitors to allow selector modification --- lib/selector/parser/ast.go | 129 ++++++++++++++++++++++++----- lib/selector/parser/parser.go | 22 ++--- lib/selector/parser/parser_test.go | 38 +++++++++ lib/selector/selector.go | 3 + 4 files changed, 159 insertions(+), 33 deletions(-) diff --git a/lib/selector/parser/ast.go b/lib/selector/parser/ast.go index 1e493e614..8f618babf 100644 --- a/lib/selector/parser/ast.go +++ b/lib/selector/parser/ast.go @@ -16,8 +16,11 @@ package parser import ( _ "crypto/sha256" // register hash func + "fmt" "strings" + log "github.com/sirupsen/logrus" + "github.com/projectcalico/libcalico-go/lib/hash" ) @@ -40,13 +43,47 @@ func (l MapAsLabels) Get(labelName string) (value string, present bool) { type Selector interface { // Evaluate evaluates the selector against the given labels expressed as a concrete map. Evaluate(labels map[string]string) bool + // EvaluateLabels evaluates the selector against the given labels expressed as an interface. // This allows for labels that are calculated on the fly. EvaluateLabels(labels Labels) bool + // String returns a string that represents this selector. String() string + // UniqueID returns the unique ID that represents this selector. UniqueID() string + + // AcceptVisitor allows an external visitor to modify this selector. + AcceptVisitor(v Visitor) +} + +type Visitor interface { + Visit(n interface{}) +} + +// PrefixVisitor implements the Visitor interface to allow prefixing of +// label names within a selector. +type PrefixVisitor struct { + Prefix string +} + +func (v PrefixVisitor) Visit(n interface{}) { + log.Debugf("PrefixVisitor visiting node %#v", n) + switch np := n.(type) { + case *LabelEqValueNode: + np.LabelName = fmt.Sprintf("%s%s", v.Prefix, np.LabelName) + case *LabelNeValueNode: + np.LabelName = fmt.Sprintf("%s%s", v.Prefix, np.LabelName) + case *HasNode: + np.LabelName = fmt.Sprintf("%s%s", v.Prefix, np.LabelName) + case *LabelInSetNode: + np.LabelName = fmt.Sprintf("%s%s", v.Prefix, np.LabelName) + case *LabelNotInSetNode: + np.LabelName = fmt.Sprintf("%s%s", v.Prefix, np.LabelName) + default: + log.Debug("Node is a no-op") + } } type selectorRoot struct { @@ -55,15 +92,19 @@ type selectorRoot struct { cachedHash *string } -func (sel selectorRoot) Evaluate(labels map[string]string) bool { +func (sel *selectorRoot) Evaluate(labels map[string]string) bool { return sel.EvaluateLabels(MapAsLabels(labels)) } -func (sel selectorRoot) EvaluateLabels(labels Labels) bool { +func (sel *selectorRoot) EvaluateLabels(labels Labels) bool { return sel.root.Evaluate(labels) } -func (sel selectorRoot) String() string { +func (sel *selectorRoot) AcceptVisitor(v Visitor) { + sel.root.AcceptVisitor(v) +} + +func (sel *selectorRoot) String() string { if sel.cachedString == nil { fragments := sel.root.collectFragments([]string{}) joined := strings.Join(fragments, "") @@ -72,7 +113,7 @@ func (sel selectorRoot) String() string { return *sel.cachedString } -func (sel selectorRoot) UniqueID() string { +func (sel *selectorRoot) UniqueID() string { if sel.cachedHash == nil { hash := hash.MakeUniqueID("s", sel.String()) sel.cachedHash = &hash @@ -84,6 +125,7 @@ var _ Selector = (*selectorRoot)(nil) type node interface { Evaluate(labels Labels) bool + AcceptVisitor(v Visitor) collectFragments(fragments []string) []string } @@ -92,7 +134,7 @@ type LabelEqValueNode struct { Value string } -func (node LabelEqValueNode) Evaluate(labels Labels) bool { +func (node *LabelEqValueNode) Evaluate(labels Labels) bool { val, ok := labels.Get(node.LabelName) if ok { return val == node.Value @@ -100,7 +142,11 @@ func (node LabelEqValueNode) Evaluate(labels Labels) bool { return false } -func (node LabelEqValueNode) collectFragments(fragments []string) []string { +func (node *LabelEqValueNode) AcceptVisitor(v Visitor) { + v.Visit(node) +} + +func (node *LabelEqValueNode) collectFragments(fragments []string) []string { var quote string if strings.Contains(node.Value, `"`) { quote = `'` @@ -115,7 +161,7 @@ type LabelInSetNode struct { Value StringSet } -func (node LabelInSetNode) Evaluate(labels Labels) bool { +func (node *LabelInSetNode) Evaluate(labels Labels) bool { val, ok := labels.Get(node.LabelName) if ok { return node.Value.Contains(val) @@ -123,7 +169,11 @@ func (node LabelInSetNode) Evaluate(labels Labels) bool { return false } -func (node LabelInSetNode) collectFragments(fragments []string) []string { +func (node *LabelInSetNode) AcceptVisitor(v Visitor) { + v.Visit(node) +} + +func (node *LabelInSetNode) collectFragments(fragments []string) []string { return collectInOpFragments(fragments, node.LabelName, "in", node.Value) } @@ -132,7 +182,11 @@ type LabelNotInSetNode struct { Value StringSet } -func (node LabelNotInSetNode) Evaluate(labels Labels) bool { +func (node *LabelNotInSetNode) AcceptVisitor(v Visitor) { + v.Visit(node) +} + +func (node *LabelNotInSetNode) Evaluate(labels Labels) bool { val, ok := labels.Get(node.LabelName) if ok { return !node.Value.Contains(val) @@ -140,7 +194,7 @@ func (node LabelNotInSetNode) Evaluate(labels Labels) bool { return true } -func (node LabelNotInSetNode) collectFragments(fragments []string) []string { +func (node *LabelNotInSetNode) collectFragments(fragments []string) []string { return collectInOpFragments(fragments, node.LabelName, "not in", node.Value) } @@ -172,7 +226,7 @@ type LabelNeValueNode struct { Value string } -func (node LabelNeValueNode) Evaluate(labels Labels) bool { +func (node *LabelNeValueNode) Evaluate(labels Labels) bool { val, ok := labels.Get(node.LabelName) if ok { return val != node.Value @@ -180,7 +234,11 @@ func (node LabelNeValueNode) Evaluate(labels Labels) bool { return true } -func (node LabelNeValueNode) collectFragments(fragments []string) []string { +func (node *LabelNeValueNode) AcceptVisitor(v Visitor) { + v.Visit(node) +} + +func (node *LabelNeValueNode) collectFragments(fragments []string) []string { var quote string if strings.Contains(node.Value, `"`) { quote = `'` @@ -194,7 +252,7 @@ type HasNode struct { LabelName string } -func (node HasNode) Evaluate(labels Labels) bool { +func (node *HasNode) Evaluate(labels Labels) bool { _, ok := labels.Get(node.LabelName) if ok { return true @@ -202,7 +260,11 @@ func (node HasNode) Evaluate(labels Labels) bool { return false } -func (node HasNode) collectFragments(fragments []string) []string { +func (node *HasNode) AcceptVisitor(v Visitor) { + v.Visit(node) +} + +func (node *HasNode) collectFragments(fragments []string) []string { return append(fragments, "has(", node.LabelName, ")") } @@ -210,11 +272,16 @@ type NotNode struct { Operand node } -func (node NotNode) Evaluate(labels Labels) bool { +func (node *NotNode) Evaluate(labels Labels) bool { return !node.Operand.Evaluate(labels) } -func (node NotNode) collectFragments(fragments []string) []string { +func (node *NotNode) AcceptVisitor(v Visitor) { + v.Visit(node) + node.Operand.AcceptVisitor(v) +} + +func (node *NotNode) collectFragments(fragments []string) []string { fragments = append(fragments, "!") return node.Operand.collectFragments(fragments) } @@ -223,7 +290,7 @@ type AndNode struct { Operands []node } -func (node AndNode) Evaluate(labels Labels) bool { +func (node *AndNode) Evaluate(labels Labels) bool { for _, operand := range node.Operands { if !operand.Evaluate(labels) { return false @@ -232,7 +299,14 @@ func (node AndNode) Evaluate(labels Labels) bool { return true } -func (node AndNode) collectFragments(fragments []string) []string { +func (node *AndNode) AcceptVisitor(v Visitor) { + v.Visit(node) + for _, op := range node.Operands { + op.AcceptVisitor(v) + } +} + +func (node *AndNode) collectFragments(fragments []string) []string { fragments = append(fragments, "(") fragments = node.Operands[0].collectFragments(fragments) for _, op := range node.Operands[1:] { @@ -247,7 +321,7 @@ type OrNode struct { Operands []node } -func (node OrNode) Evaluate(labels Labels) bool { +func (node *OrNode) Evaluate(labels Labels) bool { for _, operand := range node.Operands { if operand.Evaluate(labels) { return true @@ -256,7 +330,14 @@ func (node OrNode) Evaluate(labels Labels) bool { return false } -func (node OrNode) collectFragments(fragments []string) []string { +func (node *OrNode) AcceptVisitor(v Visitor) { + v.Visit(node) + for _, op := range node.Operands { + op.AcceptVisitor(v) + } +} + +func (node *OrNode) collectFragments(fragments []string) []string { fragments = append(fragments, "(") fragments = node.Operands[0].collectFragments(fragments) for _, op := range node.Operands[1:] { @@ -270,10 +351,14 @@ func (node OrNode) collectFragments(fragments []string) []string { type AllNode struct { } -func (node AllNode) Evaluate(labels Labels) bool { +func (node *AllNode) Evaluate(labels Labels) bool { return true } -func (node AllNode) collectFragments(fragments []string) []string { +func (node *AllNode) AcceptVisitor(v Visitor) { + v.Visit(node) +} + +func (node *AllNode) collectFragments(fragments []string) []string { return append(fragments, "all()") } diff --git a/lib/selector/parser/parser.go b/lib/selector/parser/parser.go index c4b36c8b5..dfe72f0e6 100644 --- a/lib/selector/parser/parser.go +++ b/lib/selector/parser/parser.go @@ -32,7 +32,7 @@ func Parse(selector string) (sel Selector, err error) { return } if tokens[0].Kind == tokenizer.TokEOF { - return selectorRoot{root: AllNode{}}, nil + return &selectorRoot{root: &AllNode{}}, nil } log.Debugf("Tokens %v", tokens) // The "||" operator has the lowest precedence so we start with that. @@ -44,7 +44,7 @@ func Parse(selector string) (sel Selector, err error) { err = errors.New(fmt.Sprint("unexpected content at end of selector ", remTokens)) return } - sel = selectorRoot{root: node} + sel = &selectorRoot{root: node} return } @@ -75,7 +75,7 @@ func parseOrExpression(tokens []tokenizer.Token) (sel node, remTokens []tokenize if len(andNodes) == 1 { sel = andNodes[0] } else { - sel = OrNode{andNodes} + sel = &OrNode{andNodes} } return } @@ -109,7 +109,7 @@ func parseAndExpression(tokens []tokenizer.Token) (sel node, remTokens []tokeniz if len(opNodes) == 1 { sel = opNodes[0] } else { - sel = AndNode{opNodes} + sel = &AndNode{opNodes} } return } @@ -141,10 +141,10 @@ func parseOperation(tokens []tokenizer.Token) (sel node, remTokens []tokenizer.T // Then, look for the various types of operator. switch tokens[0].Kind { case tokenizer.TokHas: - sel = HasNode{tokens[0].Value.(string)} + sel = &HasNode{tokens[0].Value.(string)} remTokens = tokens[1:] case tokenizer.TokAll: - sel = AllNode{} + sel = &AllNode{} remTokens = tokens[1:] case tokenizer.TokLabel: // should have an operator and a literal. @@ -155,14 +155,14 @@ func parseOperation(tokens []tokenizer.Token) (sel node, remTokens []tokenizer.T switch tokens[1].Kind { case tokenizer.TokEq: if tokens[2].Kind == tokenizer.TokStringLiteral { - sel = LabelEqValueNode{tokens[0].Value.(string), tokens[2].Value.(string)} + sel = &LabelEqValueNode{tokens[0].Value.(string), tokens[2].Value.(string)} remTokens = tokens[3:] } else { err = errors.New("Expected string") } case tokenizer.TokNe: if tokens[2].Kind == tokenizer.TokStringLiteral { - sel = LabelNeValueNode{tokens[0].Value.(string), tokens[2].Value.(string)} + sel = &LabelNeValueNode{tokens[0].Value.(string), tokens[2].Value.(string)} remTokens = tokens[3:] } else { err = errors.New("Expected string") @@ -194,9 +194,9 @@ func parseOperation(tokens []tokenizer.Token) (sel node, remTokens []tokenizer.T labelName := tokens[0].Value.(string) set := ConvertToStringSetInPlace(values) // Mutates values. if tokens[1].Kind == tokenizer.TokIn { - sel = LabelInSetNode{labelName, set} + sel = &LabelInSetNode{labelName, set} } else { - sel = LabelNotInSetNode{labelName, set} + sel = &LabelNotInSetNode{labelName, set} } } } else { @@ -224,7 +224,7 @@ func parseOperation(tokens []tokenizer.Token) (sel node, remTokens []tokenizer.T return } if negated && err == nil { - sel = NotNode{sel} + sel = &NotNode{sel} } return } diff --git a/lib/selector/parser/parser_test.go b/lib/selector/parser/parser_test.go index 7111c5835..8b0db3cff 100644 --- a/lib/selector/parser/parser_test.go +++ b/lib/selector/parser/parser_test.go @@ -19,7 +19,10 @@ import ( "fmt" + log "github.com/sirupsen/logrus" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" ) @@ -265,3 +268,38 @@ var _ = Describe("Parser", func() { }) } }) + +var _ = Describe("Visitor", func() { + + testVisitor := parser.PrefixVisitor{Prefix: "visited/"} + + DescribeTable("Visitor tests", + func(inSelector, outSelector string, visitor parser.Visitor) { + s, err := parser.Parse(inSelector) + By(fmt.Sprintf("parsing the selector %s", inSelector), func() { + Expect(err).To(BeNil()) + }) + + // Run the visitor against the selector. + s.AcceptVisitor(visitor) + + By("generating the correct output selector", func() { + log.Infof("[test] out: %s", s.String()) + Expect(s.String()).To(Equal(outSelector)) + }) + }, + + Entry("should visit a LabelEqValueNode", "k == 'v'", "visited/k == \"v\"", testVisitor), + Entry("should visit a LabelNeValueNode", "k != 'v'", "visited/k != \"v\"", testVisitor), + Entry("should visit an AndNode", "k == 'v' && x == 'y'", "(visited/k == \"v\" && visited/x == \"y\")", testVisitor), + Entry("should visit an OrNode", "k == 'v' || has(x)", "(visited/k == \"v\" || has(visited/x))", testVisitor), + Entry("should visit a NotNode", "!(k == 'v')", "!visited/k == \"v\"", testVisitor), + Entry("should visit a LabelInSetNode", "k in {'v'}", "visited/k in {\"v\"}", testVisitor), + Entry("should visit a LabelNotInSetNode", "k not in {'v'}", "visited/k not in {\"v\"}", testVisitor), + Entry("should visit a big complex selector", + "!(!(k == 'v' && has(t) || all()) && (a in {'b', 'c'}))", + "!(!((visited/k == \"v\" && has(visited/t)) || all()) && visited/a in {\"b\", \"c\"})", + testVisitor, + ), + ) +}) diff --git a/lib/selector/selector.go b/lib/selector/selector.go index 5a4b3ba83..7fe094a31 100644 --- a/lib/selector/selector.go +++ b/lib/selector/selector.go @@ -20,11 +20,14 @@ import "github.com/projectcalico/libcalico-go/lib/selector/parser" type Selector interface { // Evaluate evaluates the selector against the given labels expressed as a concrete map. Evaluate(labels map[string]string) bool + // EvaluateLabels evaluates the selector against the given labels expressed as an interface. // This allows for labels that are calculated on the fly. EvaluateLabels(labels parser.Labels) bool + // String returns a string that represents this selector. String() string + // UniqueID returns the unique ID that represents this selector. UniqueID() string } From 72bd24ef4f9f6e81347977eaf492df639dcfa44b Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Tue, 31 Oct 2017 16:42:49 -0700 Subject: [PATCH 2/3] Add NamespaceSelector --- lib/apis/v2/policy.go | 19 +++- lib/backend/k8s/conversion/conversion.go | 45 +++++---- lib/backend/k8s/conversion/conversion_test.go | 50 ++++++++++ .../syncersv1/updateprocessors/rules.go | 97 ++++++++++++------- .../syncersv1/updateprocessors/rules_test.go | 89 +++++++++++++---- 5 files changed, 225 insertions(+), 75 deletions(-) diff --git a/lib/apis/v2/policy.go b/lib/apis/v2/policy.go index 05a75f1fe..0a681b83b 100644 --- a/lib/apis/v2/policy.go +++ b/lib/apis/v2/policy.go @@ -79,6 +79,7 @@ type EntityRule struct { // Nets is an optional field that restricts the rule to only apply to traffic that // originates from (or terminates at) IP addresses in any of the given subnets. Nets []string `json:"nets,omitempty" validate:"omitempty,dive,cidr"` + // Selector is an optional field that contains a selector expression (see Policy for // sample syntax). Only traffic that originates from (terminates at) endpoints matching // the selector will be matched. @@ -96,6 +97,19 @@ type EntityRule struct { // The effect is that the latter will accept packets from non-Calico sources whereas the // former is limited to packets from Calico-controlled endpoints. Selector string `json:"selector,omitempty" validate:"omitempty,selector"` + + // NamespaceSelector is an optional field that contains a selector expression. Only traffic + // that originates from (or terminates at) endpoints within the selected namespaces will be + // matched. When both NamespaceSelector and Selector are defined on the same rule, then only + // workload endpoints that are matched by both selectors will be selected by the rule. + // + // For NetworkPolicy, an empty NamespaceSelector implies that the Selector is limited to selecting + // only workload endpoints in the same namespace as the NetworkPolicy. + // + // For GlobalNetworkPolicy, an empty NamespaceSelector implies the Selector applies to workload + // endpoints across all namespaces. + NamespaceSelector string `json:"namespaceSelector,omitempty" validate:"omitempty,selector"` + // Ports is an optional field that restricts the rule to only apply to traffic that has a // source (destination) port that matches one of these ranges/values. This value is a // list of integers or strings that represent ranges of ports. @@ -103,11 +117,14 @@ type EntityRule struct { // Since only some protocols have ports, if any ports are specified it requires the // Protocol match in the Rule to be set to "tcp" or "udp". Ports []numorstring.Port `json:"ports,omitempty" validate:"omitempty,dive"` - // NotTag is the negated version of the Tag field. + + // NotNets is the negated version of the Nets field. NotNets []string `json:"notNets,omitempty" validate:"omitempty,dive,cidr"` + // NotSelector is the negated version of the Selector field. See Selector field for // subtleties with negated selectors. NotSelector string `json:"notSelector,omitempty" validate:"omitempty,selector"` + // NotPorts is the negated version of the Ports field. // Since only some protocols have ports, if any ports are specified it requires the // Protocol match in the Rule to be set to "tcp" or "udp". diff --git a/lib/backend/k8s/conversion/conversion.go b/lib/backend/k8s/conversion/conversion.go index c8f77132c..732a76749 100644 --- a/lib/backend/k8s/conversion/conversion.go +++ b/lib/backend/k8s/conversion/conversion.go @@ -319,14 +319,10 @@ func (c Converter) K8sNetworkPolicyToCalico(np *extensions.NetworkPolicy) (*mode // k8sSelectorToCalico takes a namespaced k8s label selector and returns the Calico // equivalent. func (c Converter) k8sSelectorToCalico(s *metav1.LabelSelector, selectorType selectorType) string { - // All selectors should be limited in scope to Kubernetes pods. - selectors := []string{fmt.Sprintf("%s == 'k8s'", apiv2.LabelOrchestrator)} - - // If this is a namespace selector, the labels need to be prefixed with the profile namespace - // prefix. - var prefix string - if selectorType == SelectorNamespace { - prefix = NamespaceLabelPrefix + // Only prefix pod selectors - this won't work for namespace selectors. + selectors := []string{} + if selectorType == SelectorPod { + selectors = append(selectors, fmt.Sprintf("%s == 'k8s'", apiv2.LabelOrchestrator)) } // matchLabels is a map key => value, it means match if (label[key] == @@ -338,7 +334,7 @@ func (c Converter) k8sSelectorToCalico(s *metav1.LabelSelector, selectorType sel sort.Strings(keys) for _, k := range keys { v := s.MatchLabels[k] - selectors = append(selectors, fmt.Sprintf("%s%s == '%s'", prefix, k, v)) + selectors = append(selectors, fmt.Sprintf("%s == '%s'", k, v)) } // matchExpressions is a list of in/notin/exists/doesnotexist tests. @@ -348,13 +344,13 @@ func (c Converter) k8sSelectorToCalico(s *metav1.LabelSelector, selectorType sel // Each selector is formatted differently based on the operator. switch e.Operator { case metav1.LabelSelectorOpIn: - selectors = append(selectors, fmt.Sprintf("%s%s in { '%s' }", prefix, e.Key, valueList)) + selectors = append(selectors, fmt.Sprintf("%s in { '%s' }", e.Key, valueList)) case metav1.LabelSelectorOpNotIn: - selectors = append(selectors, fmt.Sprintf("%s%s not in { '%s' }", prefix, e.Key, valueList)) + selectors = append(selectors, fmt.Sprintf("%s not in { '%s' }", e.Key, valueList)) case metav1.LabelSelectorOpExists: - selectors = append(selectors, fmt.Sprintf("has(%s%s)", prefix, e.Key)) + selectors = append(selectors, fmt.Sprintf("has(%s)", e.Key)) case metav1.LabelSelectorOpDoesNotExist: - selectors = append(selectors, fmt.Sprintf("! has(%s%s)", prefix, e.Key)) + selectors = append(selectors, fmt.Sprintf("! has(%s%s)", e.Key)) } } @@ -410,16 +406,17 @@ func (c Converter) k8sRuleToCalico(rPeers []extensions.NetworkPolicyPeer, rPorts for _, port := range ports { for _, peer := range peers { protocol, calicoPorts := c.k8sPortToCalicoFields(port) - selector, nets, notNets := c.k8sPeerToCalicoFields(peer, ns) + selector, nsSelector, nets, notNets := c.k8sPeerToCalicoFields(peer, ns) if ingress { // Build inbound rule and append to list. rules = append(rules, apiv2.Rule{ Action: "allow", Protocol: protocol, Source: apiv2.EntityRule{ - Selector: selector, - Nets: nets, - NotNets: notNets, + Selector: selector, + NamespaceSelector: nsSelector, + Nets: nets, + NotNets: notNets, }, Destination: apiv2.EntityRule{ Ports: calicoPorts, @@ -431,10 +428,11 @@ func (c Converter) k8sRuleToCalico(rPeers []extensions.NetworkPolicyPeer, rPorts Action: "allow", Protocol: protocol, Destination: apiv2.EntityRule{ - Ports: calicoPorts, - Selector: selector, - Nets: nets, - NotNets: notNets, + Ports: calicoPorts, + Selector: selector, + NamespaceSelector: nsSelector, + Nets: nets, + NotNets: notNets, }, }) } @@ -462,7 +460,7 @@ func (c Converter) k8sProtocolToCalico(protocol *kapiv1.Protocol) *numorstring.P return nil } -func (c Converter) k8sPeerToCalicoFields(peer *extensions.NetworkPolicyPeer, ns string) (selector string, nets []string, notNets []string) { +func (c Converter) k8sPeerToCalicoFields(peer *extensions.NetworkPolicyPeer, ns string) (selector, nsSelector string, nets []string, notNets []string) { // If no peer, return zero values for all fields (selector, nets and !nets). if peer == nil { return @@ -475,7 +473,8 @@ func (c Converter) k8sPeerToCalicoFields(peer *extensions.NetworkPolicyPeer, ns return } if peer.NamespaceSelector != nil { - selector = c.k8sSelectorToCalico(peer.NamespaceSelector, SelectorNamespace) + nsSelector = c.k8sSelectorToCalico(peer.NamespaceSelector, SelectorNamespace) + selector = fmt.Sprintf("%s == 'k8s'", apiv2.LabelOrchestrator) return } if peer.IPBlock != nil { diff --git a/lib/backend/k8s/conversion/conversion_test.go b/lib/backend/k8s/conversion/conversion_test.go index 3bd345a54..6aaa31f73 100644 --- a/lib/backend/k8s/conversion/conversion_test.go +++ b/lib/backend/k8s/conversion/conversion_test.go @@ -600,6 +600,56 @@ var _ = Describe("Test NetworkPolicy conversion", func() { Expect(pol.Value.(*apiv2.NetworkPolicy).Spec.Types[0]).To(Equal(apiv2.PolicyTypeIngress)) }) + It("should parse a NetworkPolicy with a namespaceSelector", func() { + np := extensions.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test.policy", + Namespace: "default", + }, + Spec: extensions.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"label": "value"}, + }, + Ingress: []extensions.NetworkPolicyIngressRule{ + extensions.NetworkPolicyIngressRule{ + From: []extensions.NetworkPolicyPeer{ + extensions.NetworkPolicyPeer{ + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "namespaceRole": "dev", + "namespaceFoo": "bar", + }, + }, + }, + }, + }, + }, + PolicyTypes: []extensions.PolicyType{extensions.PolicyTypeIngress}, + }, + } + + // Parse the policy. + pol, err := c.K8sNetworkPolicyToCalico(&np) + Expect(err).NotTo(HaveOccurred()) + + // Assert key fields are correct. + Expect(pol.Key.(model.ResourceKey).Name).To(Equal("knp.default.test.policy")) + + // Assert value fields are correct. + Expect(int(*pol.Value.(*apiv2.NetworkPolicy).Spec.Order)).To(Equal(1000)) + Expect(pol.Value.(*apiv2.NetworkPolicy).Spec.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s' && label == 'value'")) + Expect(len(pol.Value.(*apiv2.NetworkPolicy).Spec.IngressRules)).To(Equal(1)) + Expect(pol.Value.(*apiv2.NetworkPolicy).Spec.IngressRules[0].Source.Selector).To(Equal("projectcalico.org/orchestrator == 'k8s'")) + Expect(pol.Value.(*apiv2.NetworkPolicy).Spec.IngressRules[0].Source.NamespaceSelector).To(Equal("namespaceFoo == 'bar' && namespaceRole == 'dev'")) + + // There should be no EgressRules + Expect(len(pol.Value.(*apiv2.NetworkPolicy).Spec.EgressRules)).To(Equal(0)) + + // Check that Types field exists and has only 'ingress' + Expect(len(pol.Value.(*apiv2.NetworkPolicy).Spec.Types)).To(Equal(1)) + Expect(pol.Value.(*apiv2.NetworkPolicy).Spec.Types[0]).To(Equal(apiv2.PolicyTypeIngress)) + }) + It("should parse a NetworkPolicy with an empty namespaceSelector", func() { np := extensions.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ diff --git a/lib/backend/syncersv1/updateprocessors/rules.go b/lib/backend/syncersv1/updateprocessors/rules.go index 73c496437..74f0c8c40 100644 --- a/lib/backend/syncersv1/updateprocessors/rules.go +++ b/lib/backend/syncersv1/updateprocessors/rules.go @@ -24,6 +24,7 @@ import ( "github.com/projectcalico/libcalico-go/lib/backend/k8s/conversion" "github.com/projectcalico/libcalico-go/lib/backend/model" cnet "github.com/projectcalico/libcalico-go/lib/net" + "github.com/projectcalico/libcalico-go/lib/selector/parser" ) func RulesAPIV2ToBackend(ars []apiv2.Rule, ns string) []model.Rule { @@ -51,45 +52,61 @@ func RuleAPIV2ToBackend(ar apiv2.Rule, ns string) model.Rule { notICMPType = ar.NotICMP.Type } - // If we have any selector specified, then we may need to add the namespace selector. - // We do this if this policy is namespaced AND if the Selector does not have any other - // k8s namespace (profile label) selector in it. - // TODO this is TEMPORARY CODE: We currently do a simple regex to search for pcns. in the - // selector to see if we are performing k8s namespace queries. - nsSelector := fmt.Sprintf("%s == '%s'", apiv2.LabelNamespace, ns) - if ns != "" && (ar.Source.Selector != "" || ar.Source.NotSelector != "") { + // Determine which namespaces are impacted by this rule. + var sourceNSSelector string + if ar.Source.NamespaceSelector != "" { + // A namespace selector was given - the rule applies to all namespaces + // which match this selector. + sourceNSSelector = parseNamespaceSelector(ar.Source.NamespaceSelector) + } else if ns != "" { + // No namespace selector was given and this is a namespaced network policy, + // so the rule applies only to its own namespace. + sourceNSSelector = fmt.Sprintf("%s == '%s'", apiv2.LabelNamespace, ns) + } + + var destNSSelector string + if ar.Destination.NamespaceSelector != "" { + // A namespace selector was given - the rule applies to all namespaces + // which match this selector. + destNSSelector = parseNamespaceSelector(ar.Destination.NamespaceSelector) + } else if ns != "" { + // No namespace selector was given and this is a namespaced network policy, + // so the rule applies only to its own namespace. + destNSSelector = fmt.Sprintf("%s == '%s'", apiv2.LabelNamespace, ns) + } + + var srcSelector, dstSelector string + if sourceNSSelector != "" { + // We need to namespace the rule's selector when converting to a v1 object. logCxt := log.WithFields(log.Fields{ - "Namespace": ns, - "Selector": ar.Source.Selector, - "NotSelector": ar.Source.NotSelector, + "Namespace": ns, + "Selector": ar.Source.Selector, + "NamespaceSelector": ar.Source.NamespaceSelector, + "NotSelector": ar.Source.NotSelector, }) - logCxt.Debug("Maybe update source Selector to include namespace") - if !strings.Contains(ar.Source.Selector, conversion.NamespaceLabelPrefix) { - logCxt.Debug("Updating source selector") - if ar.Source.Selector == "" { - ar.Source.Selector = nsSelector - } else { - ar.Source.Selector = fmt.Sprintf("(%s) && %s", ar.Source.Selector, nsSelector) - } + logCxt.Debug("Update source Selector to include namespace") + if ar.Source.Selector != "" { + srcSelector = fmt.Sprintf("(%s) && (%s)", sourceNSSelector, ar.Source.Selector) + } else { + srcSelector = sourceNSSelector } } - if ns != "" && (ar.Destination.Selector != "" || ar.Destination.NotSelector != "") { + + if destNSSelector != "" { + // We need to namespace the rule's selector when converting to a v1 object. logCxt := log.WithFields(log.Fields{ - "Namespace": ns, - "Selector": ar.Destination.Selector, - "NotSelector": ar.Destination.NotSelector, + "Namespace": ns, + "Selector": ar.Destination.Selector, + "NamespaceSelector": ar.Destination.NamespaceSelector, + "NotSelector": ar.Destination.NotSelector, }) - logCxt.Debug("Maybe update Destination Selector to include namespace") - if !strings.Contains(ar.Destination.Selector, conversion.NamespaceLabelPrefix) { - logCxt.Debug("Updating Destination selector") - if ar.Destination.Selector == "" { - ar.Destination.Selector = nsSelector - } else { - ar.Destination.Selector = fmt.Sprintf("(%s) && %s", ar.Destination.Selector, nsSelector) - } + logCxt.Debug("Update Destination Selector to include namespace") + if ar.Destination.Selector != "" { + dstSelector = fmt.Sprintf("(%s) && (%s)", destNSSelector, ar.Destination.Selector) + } else { + dstSelector = destNSSelector } } - return model.Rule{ Action: ruleActionAPIV2ToBackend(ar.Action), IPVersion: ar.IPVersion, @@ -101,10 +118,10 @@ func RuleAPIV2ToBackend(ar apiv2.Rule, ns string) model.Rule { NotICMPType: notICMPType, SrcNets: convertStringsToNets(ar.Source.Nets), - SrcSelector: ar.Source.Selector, + SrcSelector: srcSelector, SrcPorts: ar.Source.Ports, DstNets: normalizeIPNets(ar.Destination.Nets), - DstSelector: ar.Destination.Selector, + DstSelector: dstSelector, DstPorts: ar.Destination.Ports, NotSrcNets: convertStringsToNets(ar.Source.NotNets), @@ -116,6 +133,20 @@ func RuleAPIV2ToBackend(ar apiv2.Rule, ns string) model.Rule { } } +// parseNamespaceSelector takes a v2 namespace selector and returns the appropriate v1 representation +// by prefixing the keys with the `pcns.` prefix. For example, `k == 'v'` becomes `pcns.k == 'v'`. +func parseNamespaceSelector(s string) string { + parsedSelector, err := parser.Parse(s) + if err != nil { + log.WithError(err).Errorf("Failed to parse namespace selector: %s", s) + return "" + } + parsedSelector.AcceptVisitor(parser.PrefixVisitor{Prefix: conversion.NamespaceLabelPrefix}) + updated := parsedSelector.String() + log.WithFields(log.Fields{"original": s, "updated": updated}).Debug("Updated namespace selector") + return updated +} + // normalizeIPNet converts an IPNet to a network by ensuring the IP address is correctly masked. func normalizeIPNet(n string) *cnet.IPNet { if n == "" { diff --git a/lib/backend/syncersv1/updateprocessors/rules_test.go b/lib/backend/syncersv1/updateprocessors/rules_test.go index ae6465260..28f7c0b57 100644 --- a/lib/backend/syncersv1/updateprocessors/rules_test.go +++ b/lib/backend/syncersv1/updateprocessors/rules_test.go @@ -64,7 +64,7 @@ var _ = Describe("Test the Rules Conversion Functions", func() { Expect(rulev1.NotICMPType).To(Equal(&intype)) Expect(rulev1.SrcNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.10.1/32")})) - Expect(rulev1.SrcSelector).To(Equal("(mylabel = value1) && projectcalico.org/namespace == 'namespace2'")) + Expect(rulev1.SrcSelector).To(Equal("(projectcalico.org/namespace == 'namespace2') && (mylabel = value1)")) Expect(rulev1.SrcPorts).To(Equal([]numorstring.Port{port80})) Expect(rulev1.DstNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.1.1/32")})) Expect(rulev1.DstSelector).To(Equal("projectcalico.org/namespace == 'namespace2'")) @@ -97,20 +97,20 @@ var _ = Describe("Test the Rules Conversion Functions", func() { Code: &encode, }, Source: apiv2.EntityRule{ - Nets: []string{"10.100.1.1"}, - Selector: "pcns.namespacelabel1 == 'value1'", - Ports: []numorstring.Port{port443}, - NotNets: []string{"192.168.80.1"}, - NotSelector: "has(label2)", - NotPorts: []numorstring.Port{port80}, + Nets: []string{"10.100.1.1"}, + NamespaceSelector: "namespacelabel1 == 'value1'", + Ports: []numorstring.Port{port443}, + NotNets: []string{"192.168.80.1"}, + NotSelector: "has(label2)", + NotPorts: []numorstring.Port{port80}, }, Destination: apiv2.EntityRule{ - Nets: []string{"10.100.10.1"}, - Selector: "pcns.namespacelabel2 == 'value2'", - Ports: []numorstring.Port{port80}, - NotNets: []string{"192.168.40.1"}, - NotSelector: "has(label1)", - NotPorts: []numorstring.Port{port443}, + Nets: []string{"10.100.10.1"}, + NamespaceSelector: "namespacelabel2 == 'value2'", + Ports: []numorstring.Port{port80}, + NotNets: []string{"192.168.40.1"}, + NotSelector: "has(label1)", + NotPorts: []numorstring.Port{port443}, }, } // Correct outbound rule @@ -124,10 +124,10 @@ var _ = Describe("Test the Rules Conversion Functions", func() { Expect(rulev1.NotICMPType).To(Equal(&entype)) Expect(rulev1.SrcNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.1.1/32")})) - Expect(rulev1.SrcSelector).To(Equal("pcns.namespacelabel1 == 'value1'")) + Expect(rulev1.SrcSelector).To(Equal("pcns.namespacelabel1 == \"value1\"")) Expect(rulev1.SrcPorts).To(Equal([]numorstring.Port{port443})) Expect(rulev1.DstNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.10.1/32")})) - Expect(rulev1.DstSelector).To(Equal("pcns.namespacelabel2 == 'value2'")) + Expect(rulev1.DstSelector).To(Equal("pcns.namespacelabel2 == \"value2\"")) Expect(rulev1.DstPorts).To(Equal([]numorstring.Port{port80})) Expect(rulev1.NotSrcNets).To(Equal([]*cnet.IPNet{mustParseCIDR("192.168.80.1/32")})) @@ -150,7 +150,7 @@ var _ = Describe("Test the Rules Conversion Functions", func() { Expect(rulev1.NotICMPType).To(Equal(&intype)) Expect(rulev1.SrcNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.10.1/32")})) - Expect(rulev1.SrcSelector).To(Equal("(mylabel = value1) && projectcalico.org/namespace == 'namespace1'")) + Expect(rulev1.SrcSelector).To(Equal("(projectcalico.org/namespace == 'namespace1') && (mylabel = value1)")) Expect(rulev1.SrcPorts).To(Equal([]numorstring.Port{port80})) Expect(rulev1.DstNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.1.1/32")})) Expect(rulev1.DstSelector).To(Equal("projectcalico.org/namespace == 'namespace1'")) @@ -174,10 +174,10 @@ var _ = Describe("Test the Rules Conversion Functions", func() { Expect(rulev1.SrcNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.1.1/32")})) // Make sure that the pcns prefix prevented the namespace from making it into the selector. - Expect(rulev1.SrcSelector).To(Equal("pcns.namespacelabel1 == 'value1'")) + Expect(rulev1.SrcSelector).To(Equal("pcns.namespacelabel1 == \"value1\"")) Expect(rulev1.SrcPorts).To(Equal([]numorstring.Port{port443})) Expect(rulev1.DstNets).To(Equal([]*cnet.IPNet{mustParseCIDR("10.100.10.1/32")})) - Expect(rulev1.DstSelector).To(Equal("pcns.namespacelabel2 == 'value2'")) + Expect(rulev1.DstSelector).To(Equal("pcns.namespacelabel2 == \"value2\"")) Expect(rulev1.DstPorts).To(Equal([]numorstring.Port{port80})) Expect(rulev1.NotSrcNets).To(Equal([]*cnet.IPNet{mustParseCIDR("192.168.80.1/32")})) @@ -187,4 +187,57 @@ var _ = Describe("Test the Rules Conversion Functions", func() { Expect(rulev1.NotDstSelector).To(Equal("has(label1)")) Expect(rulev1.NotDstPorts).To(Equal([]numorstring.Port{port443})) }) + + It("should parse a rule with both a selector and namespace selector", func() { + r := apiv2.Rule{ + Action: apiv2.Allow, + Source: apiv2.EntityRule{ + Selector: "projectcalico.org/orchestrator == 'k8s'", + NamespaceSelector: "key == 'value'", + }, + Destination: apiv2.EntityRule{ + Selector: "projectcalico.org/orchestrator == 'k8s'", + NamespaceSelector: "key == 'value'", + }, + } + + // Process the rule and get the corresponding v1 representation. + rulev1 := updateprocessors.RuleAPIV2ToBackend(r, "namespace") + + expected := "(pcns.key == \"value\") && (projectcalico.org/orchestrator == 'k8s')" + By("generating the correct source selector", func() { + Expect(rulev1.SrcSelector).To(Equal(expected)) + }) + + By("generating the correct destination selector", func() { + Expect(rulev1.SrcSelector).To(Equal(expected)) + }) + }) + + It("should parse a complex namespace selector", func() { + s := "!has(key) || (has(key) && !key in {'value'})" + e := "(!has(pcns.key) || (has(pcns.key) && !pcns.key in {\"value\"}))" + + r := apiv2.Rule{ + Action: apiv2.Allow, + Source: apiv2.EntityRule{ + NamespaceSelector: s, + }, + Destination: apiv2.EntityRule{ + NamespaceSelector: s, + }, + } + + // Process the rule and get the corresponding v1 representation. + rulev1 := updateprocessors.RuleAPIV2ToBackend(r, "namespace") + + By("generating the correct source selector", func() { + Expect(rulev1.SrcSelector).To(Equal(e)) + }) + + By("generating the correct destination selector", func() { + Expect(rulev1.SrcSelector).To(Equal(e)) + }) + }) + }) From 372a8eb6432bad44589734d149c08c262d080c9e Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Wed, 1 Nov 2017 16:20:24 -0700 Subject: [PATCH 3/3] Don't pass pcns labels on weps to Felix --- .../updateprocessors/workloadendpointprocessor.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/backend/syncersv1/updateprocessors/workloadendpointprocessor.go b/lib/backend/syncersv1/updateprocessors/workloadendpointprocessor.go index 0ab6bad1d..454f3d203 100644 --- a/lib/backend/syncersv1/updateprocessors/workloadendpointprocessor.go +++ b/lib/backend/syncersv1/updateprocessors/workloadendpointprocessor.go @@ -17,8 +17,10 @@ package updateprocessors import ( "errors" "net" + "strings" apiv2 "github.com/projectcalico/libcalico-go/lib/apis/v2" + "github.com/projectcalico/libcalico-go/lib/backend/k8s/conversion" "github.com/projectcalico/libcalico-go/lib/backend/model" "github.com/projectcalico/libcalico-go/lib/backend/watchersyncer" "github.com/projectcalico/libcalico-go/lib/names" @@ -114,6 +116,16 @@ func convertWorkloadEndpointV2ToV1Value(val interface{}) (interface{}, error) { }) } + // Make sure there are no "namespace" labels on the wep + // we pass to felix. This prevents a wep from pretending it is + // in another namespace. + labels := map[string]string{} + for k, v := range v2res.GetLabels() { + if !strings.HasPrefix(k, conversion.NamespaceLabelPrefix) { + labels[k] = v + } + } + v1value := &model.WorkloadEndpoint{ State: "active", Name: v2res.Spec.InterfaceName, @@ -123,7 +135,7 @@ func convertWorkloadEndpointV2ToV1Value(val interface{}) (interface{}, error) { IPv6Nets: ipv6Nets, IPv4NAT: ipv4NAT, IPv6NAT: ipv6NAT, - Labels: v2res.GetLabels(), + Labels: labels, IPv4Gateway: ipv4Gateway, IPv6Gateway: ipv6Gateway, Ports: ports,