diff --git a/lib/api/rule.go b/lib/api/rule.go index fe1ad9a5a..807d2cb63 100644 --- a/lib/api/rule.go +++ b/lib/api/rule.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016 Tigera, Inc. All rights reserved. +// Copyright (c) 2016-2017 Tigera, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -75,7 +75,7 @@ type ICMPFields struct { // to a particular entity (that is either the source or destination). // // A source EntityRule matches the source endpoint and originating traffic. -// A desination EntityRule matches the destination endpoint and terminating traffic. +// A destination EntityRule matches the destination endpoint and terminating traffic. type EntityRule struct { // Tag is an optional field that restricts the rule to only apply to traffic that // originates from (or terminates at) endpoints that have profiles with the given tag @@ -84,8 +84,13 @@ type EntityRule struct { // Net is an optional field that restricts the rule to only apply to traffic that // originates from (or terminates at) IP addresses in the given subnet. + // Deprecated: superseded by the Nets field. Net *net.IPNet `json:"net,omitempty" validate:"omitempty"` + // 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 []*net.IPNet `json:"nets,omitempty" validate:"omitempty"` + // 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. @@ -115,9 +120,15 @@ type EntityRule struct { // NotTag is the negated version of the Tag field. NotTag string `json:"notTag,omitempty" validate:"omitempty,tag"` - // NotNet is the negated version of the Net field. + // NotNet is an optional field that restricts the rule to only apply to traffic that + // does not originate from (or terminate at) an IP address in the given subnet. + // Deprecated: superseded by NotNets. NotNet *net.IPNet `json:"notNet,omitempty" validate:"omitempty"` + // NotNets is an optional field that restricts the rule to only apply to traffic that + // does not originate from (or terminate at) an IP address in any of the given subnets. + NotNets []*net.IPNet `json:"nets,omitempty" validate:"omitempty"` + // 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"` @@ -128,3 +139,28 @@ type EntityRule struct { // Protocol match in the Rule to be set to "tcp" or "udp". NotPorts []numorstring.Port `json:"notPorts,omitempty" validate:"omitempty,dive"` } + +func combineNets(n *net.IPNet, nets []*net.IPNet) []*net.IPNet { + if n == nil { + return nets + } + if len(nets) == 0 { + return []*net.IPNet{n} + } + combined := make([]*net.IPNet, len(nets)+1) + copy(combined, nets) + combined[len(combined)-1] = n + return combined +} + +// GetNets returns either r.Nets or a slice containing r.Net. It is useful for unifying the +// two representations. +func (r EntityRule) GetNets() []*net.IPNet { + return combineNets(r.Net, r.Nets) +} + +// GetNets returns either r.NotNets or a slice containing NotNet. It is useful for unifying the +// two representations. +func (r EntityRule) GetNotNets() []*net.IPNet { + return combineNets(r.NotNet, r.NotNets) +} diff --git a/lib/backend/backend_suite_test.go b/lib/backend/backend_suite_test.go index dcda2a747..d2aca4bf5 100644 --- a/lib/backend/backend_suite_test.go +++ b/lib/backend/backend_suite_test.go @@ -1,3 +1,17 @@ +// Copyright (c) 2017 Tigera, Inc. All rights reserved. + +// 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. + package backend_test import ( diff --git a/lib/backend/model/rule.go b/lib/backend/model/rule.go index 5691670df..dd4ebf478 100644 --- a/lib/backend/model/rule.go +++ b/lib/backend/model/rule.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016 Tigera, Inc. All rights reserved. +// Copyright (c) 2016-2017 Tigera, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -38,25 +38,66 @@ type Rule struct { SrcTag string `json:"src_tag,omitempty" validate:"omitempty,tag"` SrcNet *net.IPNet `json:"src_net,omitempty" validate:"omitempty"` + SrcNets []*net.IPNet `json:"src_nets,omitempty" validate:"omitempty"` SrcSelector string `json:"src_selector,omitempty" validate:"omitempty,selector"` SrcPorts []numorstring.Port `json:"src_ports,omitempty" validate:"omitempty"` DstTag string `json:"dst_tag,omitempty" validate:"omitempty,tag"` DstSelector string `json:"dst_selector,omitempty" validate:"omitempty,selector"` DstNet *net.IPNet `json:"dst_net,omitempty" validate:"omitempty"` + DstNets []*net.IPNet `json:"dst_nets,omitempty" validate:"omitempty"` DstPorts []numorstring.Port `json:"dst_ports,omitempty" validate:"omitempty"` NotSrcTag string `json:"!src_tag,omitempty" validate:"omitempty,tag"` NotSrcNet *net.IPNet `json:"!src_net,omitempty" validate:"omitempty"` + NotSrcNets []*net.IPNet `json:"!src_nets,omitempty" validate:"omitempty"` NotSrcSelector string `json:"!src_selector,omitempty" validate:"omitempty,selector"` NotSrcPorts []numorstring.Port `json:"!src_ports,omitempty" validate:"omitempty"` NotDstTag string `json:"!dst_tag,omitempty" validate:"omitempty"` NotDstSelector string `json:"!dst_selector,omitempty" validate:"omitempty,selector"` NotDstNet *net.IPNet `json:"!dst_net,omitempty" validate:"omitempty"` + NotDstNets []*net.IPNet `json:"!dst_nets,omitempty" validate:"omitempty"` NotDstPorts []numorstring.Port `json:"!dst_ports,omitempty" validate:"omitempty"` LogPrefix string `json:"log_prefix,omitempty" validate:"omitempty"` } +func combineNets(n *net.IPNet, nets []*net.IPNet) []*net.IPNet { + if n == nil { + return nets + } + if len(nets) == 0 { + return []*net.IPNet{n} + } + var combination = make([]*net.IPNet, len(nets)+1) + copy(combination, nets) + combination[len(nets)] = n + return combination +} + +func (r Rule) AllSrcNets() []*net.IPNet { + return combineNets(r.SrcNet, r.SrcNets) +} + +func (r Rule) AllDstNets() []*net.IPNet { + return combineNets(r.DstNet, r.DstNets) +} + +func (r Rule) AllNotSrcNets() []*net.IPNet { + return combineNets(r.NotSrcNet, r.NotSrcNets) +} + +func (r Rule) AllNotDstNets() []*net.IPNet { + return combineNets(r.NotDstNet, r.NotDstNets) +} + +func joinNets(nets []*net.IPNet) string { + parts := make([]string, len(nets)) + for i, n := range nets { + parts[i] = n.String() + } + return strings.Join(parts, ",") +} + func (r Rule) String() string { parts := make([]string, 0) // Action. @@ -87,83 +128,93 @@ func (r Rule) String() string { parts = append(parts, "!code", strconv.Itoa(*r.NotICMPCode)) } - // Source attributes. - fromParts := make([]string, 0) - if len(r.SrcPorts) > 0 { - srcPorts := make([]string, len(r.SrcPorts)) - for ii, port := range r.SrcPorts { - srcPorts[ii] = port.String() + { + // Source attributes. New block ensures that fromParts goes out-of-scope before + // we calculate toParts. This prevents copy/paste errors. + fromParts := make([]string, 0) + if len(r.SrcPorts) > 0 { + srcPorts := make([]string, len(r.SrcPorts)) + for ii, port := range r.SrcPorts { + srcPorts[ii] = port.String() + } + fromParts = append(fromParts, "ports", strings.Join(srcPorts, ",")) } - fromParts = append(fromParts, "ports", strings.Join(srcPorts, ",")) - } - if r.SrcTag != "" { - fromParts = append(fromParts, "tag", r.SrcTag) - } - if r.SrcSelector != "" { - fromParts = append(fromParts, "selector", fmt.Sprintf("%#v", r.SrcSelector)) - } - if r.SrcNet != nil { - fromParts = append(fromParts, "cidr", r.SrcNet.String()) - } - if len(r.NotSrcPorts) > 0 { - notSrcPorts := make([]string, len(r.NotSrcPorts)) - for ii, port := range r.NotSrcPorts { - notSrcPorts[ii] = port.String() + if r.SrcTag != "" { + fromParts = append(fromParts, "tag", r.SrcTag) + } + if r.SrcSelector != "" { + fromParts = append(fromParts, "selector", fmt.Sprintf("%#v", r.SrcSelector)) + } + srcNets := r.AllSrcNets() + if len(srcNets) != 0 { + fromParts = append(fromParts, "cidr", joinNets(srcNets)) + } + if len(r.NotSrcPorts) > 0 { + notSrcPorts := make([]string, len(r.NotSrcPorts)) + for ii, port := range r.NotSrcPorts { + notSrcPorts[ii] = port.String() + } + fromParts = append(fromParts, "!ports", strings.Join(notSrcPorts, ",")) + } + if r.NotSrcTag != "" { + fromParts = append(fromParts, "!tag", r.NotSrcTag) + } + if r.NotSrcSelector != "" { + fromParts = append(fromParts, "!selector", fmt.Sprintf("%#v", r.NotSrcSelector)) + } + notSrcNets := r.AllNotSrcNets() + if len(notSrcNets) != 0 { + fromParts = append(fromParts, "!cidr", joinNets(notSrcNets)) } - fromParts = append(fromParts, "!ports", strings.Join(notSrcPorts, ",")) - } - if r.NotSrcTag != "" { - fromParts = append(fromParts, "!tag", r.NotSrcTag) - } - if r.NotSrcSelector != "" { - fromParts = append(fromParts, "!selector", fmt.Sprintf("%#v", r.NotSrcSelector)) - } - if r.NotSrcNet != nil { - fromParts = append(fromParts, "!cidr", r.NotSrcNet.String()) - } - // Destination attributes. - toParts := make([]string, 0) - if len(r.DstPorts) > 0 { - DstPorts := make([]string, len(r.DstPorts)) - for ii, port := range r.DstPorts { - DstPorts[ii] = port.String() + if len(fromParts) > 0 { + parts = append(parts, "from") + parts = append(parts, fromParts...) } - toParts = append(toParts, "ports", strings.Join(DstPorts, ",")) } - if r.DstTag != "" { - toParts = append(toParts, "tag", r.DstTag) - } - if r.DstSelector != "" { - toParts = append(toParts, "selector", fmt.Sprintf("%#v", r.DstSelector)) - } - if r.DstNet != nil { - toParts = append(toParts, "cidr", r.DstNet.String()) - } - if len(r.NotDstPorts) > 0 { - NotDstPorts := make([]string, len(r.NotDstPorts)) - for ii, port := range r.NotDstPorts { - NotDstPorts[ii] = port.String() + + { + // Destination attributes. + toParts := make([]string, 0) + if len(r.DstPorts) > 0 { + DstPorts := make([]string, len(r.DstPorts)) + for ii, port := range r.DstPorts { + DstPorts[ii] = port.String() + } + toParts = append(toParts, "ports", strings.Join(DstPorts, ",")) + } + if r.DstTag != "" { + toParts = append(toParts, "tag", r.DstTag) + } + if r.DstSelector != "" { + toParts = append(toParts, "selector", fmt.Sprintf("%#v", r.DstSelector)) + } + dstNets := r.AllDstNets() + if len(dstNets) != 0 { + toParts = append(toParts, "cidr", joinNets(dstNets)) + } + if len(r.NotDstPorts) > 0 { + notDstPorts := make([]string, len(r.NotDstPorts)) + for ii, port := range r.NotDstPorts { + notDstPorts[ii] = port.String() + } + toParts = append(toParts, "!ports", strings.Join(notDstPorts, ",")) + } + if r.NotDstTag != "" { + toParts = append(toParts, "!tag", r.NotDstTag) + } + if r.NotDstSelector != "" { + toParts = append(toParts, "!selector", fmt.Sprintf("%#v", r.NotDstSelector)) + } + notDstNets := r.AllNotDstNets() + if len(notDstNets) != 0 { + toParts = append(toParts, "!cidr", joinNets(notDstNets)) } - toParts = append(toParts, "!ports", strings.Join(NotDstPorts, ",")) - } - if r.NotDstTag != "" { - toParts = append(toParts, "!tag", r.NotDstTag) - } - if r.NotDstSelector != "" { - toParts = append(toParts, "!selector", fmt.Sprintf("%#v", r.NotDstSelector)) - } - if r.NotDstNet != nil { - toParts = append(toParts, "!cidr", r.NotDstNet.String()) - } - if len(fromParts) > 0 { - parts = append(parts, "from") - parts = append(parts, fromParts...) - } - if len(toParts) > 0 { - parts = append(parts, "to") - parts = append(parts, toParts...) + if len(toParts) > 0 { + parts = append(parts, "to") + parts = append(parts, toParts...) + } } return strings.Join(parts, " ") diff --git a/lib/client/policy_e2e_test.go b/lib/client/policy_e2e_test.go index 15986a1e4..11bf3d6d2 100644 --- a/lib/client/policy_e2e_test.go +++ b/lib/client/policy_e2e_test.go @@ -55,6 +55,14 @@ var policySpec1 = api.PolicySpec{ Selector: "thing == 'value'", } +// When reading back, the rules should have been updated to the newer format. +var policySpec1AfterRead = api.PolicySpec{ + Order: &order1, + IngressRules: []api.Rule{testutils.InRule1AfterRead, testutils.InRule2AfterRead}, + EgressRules: []api.Rule{testutils.EgressRule1AfterRead, testutils.EgressRule2AfterRead}, + Selector: "thing == 'value'", +} + var policySpec2 = api.PolicySpec{ Order: &order2, IngressRules: []api.Rule{testutils.InRule2, testutils.InRule1}, @@ -63,10 +71,19 @@ var policySpec2 = api.PolicySpec{ DoNotTrack: true, } +// When reading back, the rules should have been updated to the newer format. +var policySpec2AfterRead = api.PolicySpec{ + Order: &order2, + IngressRules: []api.Rule{testutils.InRule2AfterRead, testutils.InRule1AfterRead}, + EgressRules: []api.Rule{testutils.EgressRule2AfterRead, testutils.EgressRule1AfterRead}, + Selector: "thing2 == 'value2'", + DoNotTrack: true, +} + var _ = testutils.E2eDatastoreDescribe("Policy tests", testutils.DatastoreEtcdV2, func(config api.CalicoAPIConfig) { DescribeTable("Policy e2e tests", - func(meta1, meta2 api.PolicyMetadata, spec1, spec2 api.PolicySpec) { + func(meta1, meta2 api.PolicyMetadata, spec1, spec2, spec1AfterRead, spec2AfterRead api.PolicySpec) { c := testutils.CreateCleanClient(config) By("Updating the policy before it is created") _, outError := c.Policies().Update(&api.Policy{Metadata: meta1, Spec: spec1}) @@ -95,8 +112,8 @@ var _ = testutils.E2eDatastoreDescribe("Policy tests", testutils.DatastoreEtcdV2 // Should match spec1 & outPolicy1 and outPolicy2 & spec2 and errors to be nil. Expect(outError1).NotTo(HaveOccurred()) Expect(outError2).NotTo(HaveOccurred()) - Expect(outPolicy1.Spec).To(Equal(spec1)) - Expect(outPolicy2.Spec).To(Equal(spec2)) + Expect(outPolicy1.Spec).To(Equal(spec1AfterRead)) + Expect(outPolicy2.Spec).To(Equal(spec2AfterRead)) By("Update, Get and compare") @@ -109,7 +126,7 @@ var _ = testutils.E2eDatastoreDescribe("Policy tests", testutils.DatastoreEtcdV2 // Assert the Spec for policy with meta1 matches spec2 and no error. Expect(outError1).NotTo(HaveOccurred()) - Expect(outPolicy1.Spec).To(Equal(spec2)) + Expect(outPolicy1.Spec).To(Equal(spec2AfterRead)) // Assert the Metadata for policy with meta1 matches meta1. Expect(outPolicy1.Metadata).To(Equal(meta1)) @@ -186,6 +203,8 @@ var _ = testutils.E2eDatastoreDescribe("Policy tests", testutils.DatastoreEtcdV2 api.PolicyMetadata{Name: "policy.1"}, policySpec1, policySpec2, + policySpec1AfterRead, + policySpec2AfterRead, ), // Test 2: Pass one fully populated PolicySpec and another empty PolicySpec and expect the series of operations to succeed. @@ -194,6 +213,8 @@ var _ = testutils.E2eDatastoreDescribe("Policy tests", testutils.DatastoreEtcdV2 api.PolicyMetadata{Name: "policy.1"}, policySpec1, api.PolicySpec{}, + policySpec1AfterRead, + api.PolicySpec{}, ), // Test 3: Pass one partially populated PolicySpec and another fully populated PolicySpec and expect the series of operations to succeed. @@ -204,6 +225,10 @@ var _ = testutils.E2eDatastoreDescribe("Policy tests", testutils.DatastoreEtcdV2 Selector: "has(myLabel-8.9/88-._9)", }, policySpec2, + api.PolicySpec{ + Selector: "has(myLabel-8.9/88-._9)", + }, + policySpec2AfterRead, ), ) }) diff --git a/lib/client/profile_e2e_test.go b/lib/client/profile_e2e_test.go index 52fa632da..dc5615778 100644 --- a/lib/client/profile_e2e_test.go +++ b/lib/client/profile_e2e_test.go @@ -49,18 +49,30 @@ var profileSpec1 = api.ProfileSpec{ IngressRules: []api.Rule{testutils.InRule1, testutils.InRule2}, EgressRules: []api.Rule{testutils.EgressRule1, testutils.EgressRule2}, } + +// When reading back, the rules should have been updated to the newer format. +var profileSpec1AfterRead = api.ProfileSpec{ + IngressRules: []api.Rule{testutils.InRule1AfterRead, testutils.InRule2AfterRead}, + EgressRules: []api.Rule{testutils.EgressRule1AfterRead, testutils.EgressRule2AfterRead}, +} var tags1 = []string{"profile1-tag1", "profile1-tag2"} var profileSpec2 = api.ProfileSpec{ IngressRules: []api.Rule{testutils.InRule2, testutils.InRule1}, EgressRules: []api.Rule{testutils.EgressRule2, testutils.EgressRule1}, } + +// When reading back, the rules should have been updated to the newer format. +var profileSpec2AfterRead = api.ProfileSpec{ + IngressRules: []api.Rule{testutils.InRule2AfterRead, testutils.InRule1AfterRead}, + EgressRules: []api.Rule{testutils.EgressRule2AfterRead, testutils.EgressRule1AfterRead}, +} var tags2 = []string{"profile2-tag1", "profile2-tag2"} var _ = testutils.E2eDatastoreDescribe("Profile tests", testutils.DatastoreEtcdV2, func(config api.CalicoAPIConfig) { DescribeTable("Profile e2e tests", - func(meta1, meta2 api.ProfileMetadata, spec1, spec2 api.ProfileSpec) { + func(meta1, meta2 api.ProfileMetadata, spec1, spec2, spec1AfterRead, spec2AfterRead api.ProfileSpec) { c := testutils.CreateCleanClient(config) By("Updating the profile before it is created") @@ -90,8 +102,8 @@ var _ = testutils.E2eDatastoreDescribe("Profile tests", testutils.DatastoreEtcdV // Should match spec1 & outProfile1 and outProfile2 & spec2 and errors to be nil. Expect(outError1).NotTo(HaveOccurred()) Expect(outError2).NotTo(HaveOccurred()) - Expect(outProfile1.Spec).To(Equal(spec1)) - Expect(outProfile2.Spec).To(Equal(spec2)) + Expect(outProfile1.Spec).To(Equal(spec1AfterRead)) + Expect(outProfile2.Spec).To(Equal(spec2AfterRead)) By("Update, Get and compare") @@ -104,7 +116,7 @@ var _ = testutils.E2eDatastoreDescribe("Profile tests", testutils.DatastoreEtcdV // Assert the Spec for profile with meta1 matches spec2 and no error. Expect(outError1).NotTo(HaveOccurred()) - Expect(outProfile1.Spec).To(Equal(spec2)) + Expect(outProfile1.Spec).To(Equal(spec2AfterRead)) By("List all the profiles and compare") @@ -192,6 +204,8 @@ var _ = testutils.E2eDatastoreDescribe("Profile tests", testutils.DatastoreEtcdV }, profileSpec1, profileSpec2, + profileSpec1AfterRead, + profileSpec2AfterRead, ), // Test 2: Pass one fully populated ProfileSpec and another empty ProfileSpec and expect the series of operations to succeed. @@ -213,6 +227,8 @@ var _ = testutils.E2eDatastoreDescribe("Profile tests", testutils.DatastoreEtcdV }, profileSpec1, api.ProfileSpec{}, + profileSpec1AfterRead, + api.ProfileSpec{}, ), // Test 3: Pass one partially populated ProfileSpec and another fully populated ProfileSpec and expect the series of operations to succeed. @@ -235,6 +251,10 @@ var _ = testutils.E2eDatastoreDescribe("Profile tests", testutils.DatastoreEtcdV IngressRules: []api.Rule{testutils.InRule1}, }, profileSpec2, + api.ProfileSpec{ + IngressRules: []api.Rule{testutils.InRule1AfterRead}, + }, + profileSpec2AfterRead, ), ) }) diff --git a/lib/converter/converter_suite_test.go b/lib/converter/converter_suite_test.go new file mode 100644 index 000000000..e9d9076fe --- /dev/null +++ b/lib/converter/converter_suite_test.go @@ -0,0 +1,27 @@ +// Copyright (c) 2017 Tigera, Inc. All rights reserved. + +// 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. + +package converter_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "testing" +) + +func TestConverter(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Converter Suite") +} diff --git a/lib/converter/rule.go b/lib/converter/rule.go index c7aa0fa0d..c9b64908b 100644 --- a/lib/converter/rule.go +++ b/lib/converter/rule.go @@ -15,6 +15,10 @@ package converter import ( + "sync" + + log "github.com/Sirupsen/logrus" + "github.com/projectcalico/libcalico-go/lib/api" "github.com/projectcalico/libcalico-go/lib/backend/model" "github.com/projectcalico/libcalico-go/lib/net" @@ -46,6 +50,8 @@ func RulesBackendToAPI(brs []model.Rule) []api.Rule { return ars } +var logDeprecationOnce sync.Once + // ruleAPIToBackend converts an API Rule structure to a Backend Rule structure. func ruleAPIToBackend(ar api.Rule) model.Rule { var icmpCode, icmpType, notICMPCode, notICMPType *int @@ -59,6 +65,14 @@ func ruleAPIToBackend(ar api.Rule) model.Rule { notICMPType = ar.NotICMP.Type } + if ar.Source.Net != nil || ar.Source.NotNet != nil || + ar.Destination.Net != nil || ar.Destination.NotNet != nil { + logDeprecationOnce.Do(func() { + log.Warning("The Net and NotNet fields in Source/Destination " + + "EntityRules are deprecated. Please use Nets or NotNets.") + }) + } + return model.Rule{ Action: ruleActionAPIToBackend(ar.Action), IPVersion: ar.IPVersion, @@ -71,26 +85,29 @@ func ruleAPIToBackend(ar api.Rule) model.Rule { SrcTag: ar.Source.Tag, SrcNet: ar.Source.Net, + SrcNets: ar.Source.Nets, SrcSelector: ar.Source.Selector, SrcPorts: ar.Source.Ports, DstTag: ar.Destination.Tag, DstNet: normalizeIPNet(ar.Destination.Net), + DstNets: normalizeIPNets(ar.Destination.Nets), DstSelector: ar.Destination.Selector, DstPorts: ar.Destination.Ports, NotSrcTag: ar.Source.NotTag, NotSrcNet: ar.Source.NotNet, + NotSrcNets: ar.Source.NotNets, NotSrcSelector: ar.Source.NotSelector, NotSrcPorts: ar.Source.NotPorts, NotDstTag: ar.Destination.NotTag, NotDstNet: normalizeIPNet(ar.Destination.NotNet), + NotDstNets: normalizeIPNets(ar.Destination.NotNets), NotDstSelector: ar.Destination.NotSelector, NotDstPorts: ar.Destination.NotPorts, } } -// normalizeIPNet converts an IPNet to a network by ensuring the IP address -// is correctly masked. +// normalizeIPNet converts an IPNet to a network by ensuring the IP address is correctly masked. func normalizeIPNet(n *net.IPNet) *net.IPNet { if n == nil { return nil @@ -98,6 +115,19 @@ func normalizeIPNet(n *net.IPNet) *net.IPNet { return n.Network() } +// normalizeIPNets converts an []*IPNet to a slice of networks by ensuring the IP addresses +// are correctly masked. +func normalizeIPNets(nets []*net.IPNet) []*net.IPNet { + if nets == nil { + return nil + } + out := make([]*net.IPNet, len(nets)) + for i, n := range nets { + out[i] = normalizeIPNet(n) + } + return out +} + // ruleBackendToAPI convert a Backend Rule structure to an API Rule structure. func ruleBackendToAPI(br model.Rule) api.Rule { var icmp, notICMP *api.ICMPFields @@ -122,22 +152,22 @@ func ruleBackendToAPI(br model.Rule) api.Rule { NotICMP: notICMP, Source: api.EntityRule{ Tag: br.SrcTag, - Net: br.SrcNet, + Nets: br.AllSrcNets(), Selector: br.SrcSelector, Ports: br.SrcPorts, NotTag: br.NotSrcTag, - NotNet: br.NotSrcNet, + NotNets: br.AllNotSrcNets(), NotSelector: br.NotSrcSelector, NotPorts: br.NotSrcPorts, }, Destination: api.EntityRule{ Tag: br.DstTag, - Net: br.DstNet, + Nets: br.AllDstNets(), Selector: br.DstSelector, Ports: br.DstPorts, NotTag: br.NotDstTag, - NotNet: br.NotDstNet, + NotNets: br.AllNotDstNets(), NotSelector: br.NotDstSelector, NotPorts: br.NotDstPorts, }, diff --git a/lib/converter/rule_test.go b/lib/converter/rule_test.go new file mode 100644 index 000000000..dd1fbbb95 --- /dev/null +++ b/lib/converter/rule_test.go @@ -0,0 +1,127 @@ +// Copyright (c) 2017 Tigera, Inc. All rights reserved. + +// 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. + +package converter_test + +import ( + . "github.com/projectcalico/libcalico-go/lib/converter" + + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "github.com/projectcalico/libcalico-go/lib/api" + "github.com/projectcalico/libcalico-go/lib/backend/model" + "github.com/projectcalico/libcalico-go/lib/net" +) + +var ( + cidr1 = net.MustParseCIDR("10.0.0.1/24") + cidr2 = net.MustParseCIDR("11.0.0.1/24") + cidr3 = net.MustParseCIDR("12.0.0.1/24") + cidr4 = net.MustParseCIDR("13.0.0.1/24") + cidr3Net = net.MustParseCIDR("12.0.0.0/24") + cidr4Net = net.MustParseCIDR("13.0.0.0/24") + cidr3Norm = (&cidr3Net).Network() + cidr4Norm = (&cidr4Net).Network() +) + +var _ = DescribeTable("RulesAPIToBackend", + func(input api.Rule, expected model.Rule) { + output := RulesAPIToBackend([]api.Rule{input}) + Expect(output).To(ConsistOf(expected)) + }, + Entry("empty rule", api.Rule{}, model.Rule{}), + Entry("should normalise destination net fields", + api.Rule{ + Source: api.EntityRule{ + Net: &cidr1, + NotNet: &cidr2, + }, + Destination: api.EntityRule{ + Net: &cidr3, + NotNet: &cidr4, + }, + }, + model.Rule{ + SrcNet: &cidr1, + NotSrcNet: &cidr2, + DstNet: cidr3Norm, + NotDstNet: cidr4Norm, + }, + ), + Entry("should normalise destination nets fields", + api.Rule{ + Source: api.EntityRule{ + Nets: []*net.IPNet{&cidr1}, + NotNets: []*net.IPNet{&cidr2}, + }, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&cidr3}, + NotNets: []*net.IPNet{&cidr4}, + }, + }, + model.Rule{ + SrcNets: []*net.IPNet{&cidr1}, + NotSrcNets: []*net.IPNet{&cidr2}, + DstNets: []*net.IPNet{cidr3Norm}, + NotDstNets: []*net.IPNet{cidr4Norm}, + }, + ), +) + +var _ = DescribeTable("RulesBackendToAPI", + func(input model.Rule, expected api.Rule) { + output := RulesBackendToAPI([]model.Rule{input}) + Expect(output).To(ConsistOf(expected)) + }, + Entry("empty rule should get explicit action", model.Rule{}, api.Rule{Action: "allow"}), + Entry("should convert net to nets fields", + model.Rule{ + SrcNet: &cidr1, + NotSrcNet: &cidr2, + DstNet: &cidr3, + NotDstNet: &cidr4, + }, + api.Rule{ + Action: "allow", + Source: api.EntityRule{ + Nets: []*net.IPNet{&cidr1}, + NotNets: []*net.IPNet{&cidr2}, + }, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&cidr3}, + NotNets: []*net.IPNet{&cidr4}, + }, + }, + ), + Entry("should pass through nets fields", + model.Rule{ + SrcNets: []*net.IPNet{&cidr1}, + NotSrcNets: []*net.IPNet{&cidr2}, + DstNets: []*net.IPNet{&cidr3}, + NotDstNets: []*net.IPNet{&cidr4}, + }, + api.Rule{ + Action: "allow", + Source: api.EntityRule{ + Nets: []*net.IPNet{&cidr1}, + NotNets: []*net.IPNet{&cidr2}, + }, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&cidr3}, + NotNets: []*net.IPNet{&cidr4}, + }, + }, + ), +) diff --git a/lib/testutils/rules.go b/lib/testutils/rules.go index b2dd7625e..6200fc5de 100644 --- a/lib/testutils/rules.go +++ b/lib/testutils/rules.go @@ -51,6 +51,18 @@ var InRule1 = api.Rule{ }, } +var InRule1AfterRead = api.Rule{ + Action: "allow", + IPVersion: &ipv4, + Protocol: &strProtocol1, + ICMP: &icmp1, + Source: api.EntityRule{ + Tag: "tag1", + Nets: []*net.IPNet{&cidr1}, + Selector: "label1 == 'value1'", + }, +} + var InRule2 = api.Rule{ Action: "deny", IPVersion: &ipv6, @@ -63,6 +75,18 @@ var InRule2 = api.Rule{ }, } +var InRule2AfterRead = api.Rule{ + Action: "deny", + IPVersion: &ipv6, + Protocol: &numProtocol1, + ICMP: &icmp1, + Source: api.EntityRule{ + Tag: "tag2", + Nets: []*net.IPNet{&cidrv61}, + Selector: "has(label2)", + }, +} + var EgressRule1 = api.Rule{ Action: "pass", IPVersion: &ipv4, @@ -75,6 +99,18 @@ var EgressRule1 = api.Rule{ }, } +var EgressRule1AfterRead = api.Rule{ + Action: "pass", + IPVersion: &ipv4, + Protocol: &numProtocol1, + ICMP: &icmp1, + Source: api.EntityRule{ + Tag: "tag3", + Nets: []*net.IPNet{&cidr2}, + Selector: "all()", + }, +} + var EgressRule2 = api.Rule{ Action: "allow", IPVersion: &ipv6, @@ -86,3 +122,15 @@ var EgressRule2 = api.Rule{ Selector: "label2 == '1234'", }, } + +var EgressRule2AfterRead = api.Rule{ + Action: "allow", + IPVersion: &ipv6, + Protocol: &strProtocol2, + ICMP: &icmp1, + Source: api.EntityRule{ + Tag: "tag4", + Nets: []*net.IPNet{&cidrv62}, + Selector: "label2 == '1234'", + }, +} diff --git a/lib/validator/validator.go b/lib/validator/validator.go index 3af20d76d..355c693c0 100644 --- a/lib/validator/validator.go +++ b/lib/validator/validator.go @@ -23,6 +23,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/projectcalico/libcalico-go/lib/api" "github.com/projectcalico/libcalico-go/lib/errors" + calinet "github.com/projectcalico/libcalico-go/lib/net" "github.com/projectcalico/libcalico-go/lib/numorstring" "github.com/projectcalico/libcalico-go/lib/scope" "github.com/projectcalico/libcalico-go/lib/selector" @@ -406,25 +407,47 @@ func validateRule(v *validator.Validate, structLevel *validator.StructLevel) { } } - // Check for mismatch in IP versions - if rule.Source.Net != nil && rule.IPVersion != nil { - if rule.Source.Net.Version() != *(rule.IPVersion) { - structLevel.ReportError(reflect.ValueOf(rule.Source.Net), "Source.Net", - "", reason("rule contains an IP version that does not match src CIDR version")) - } + // Check that only one of the net or nets fields is specified. + hasNetField := rule.Source.Net != nil || + rule.Destination.Net != nil || + rule.Source.NotNet != nil || + rule.Destination.NotNet != nil + hasNetsField := len(rule.Source.Nets) != 0 || + len(rule.Destination.Nets) != 0 || + len(rule.Source.NotNets) != 0 || + len(rule.Destination.NotNets) != 0 + if hasNetField && hasNetsField { + structLevel.ReportError(reflect.ValueOf(rule.Source.Nets), + "Source/Destination.Net/Nets", + "Source/Destination.Net/Nets", + reason("only one of Net and Nets fields allowed")) } - if rule.Destination.Net != nil && rule.IPVersion != nil { - if rule.Destination.Net.Version() != *(rule.IPVersion) { - structLevel.ReportError(reflect.ValueOf(rule.Destination.Net), "Destination.Net", - "", reason("rule contains an IP version that does not match dst CIDR version")) + + var seenV4, seenV6 bool + + scanNets := func(nets []*calinet.IPNet, fieldName string) { + var v4, v6 bool + for _, n := range nets { + v4 = v4 || n.Version() == 4 + v6 = v6 || n.Version() == 6 } - } - if rule.Source.Net != nil && rule.Destination.Net != nil { - if rule.Source.Net.Version() != rule.Destination.Net.Version() { - structLevel.ReportError(reflect.ValueOf(rule.Destination.Net), "Destination.Net", - "", reason("rule does not support mixing of IPv4/v6 CIDRs")) + if rule.IPVersion != nil && ((v4 && *rule.IPVersion != 4) || (v6 && *rule.IPVersion != 6)) { + structLevel.ReportError(reflect.ValueOf(rule.Source.Net), fieldName, + "", reason("rule IP version doesn't match CIDR version")) + } + if v4 && seenV6 || v6 && seenV4 || v4 && v6 { + // This field makes the rule inconsistent. + structLevel.ReportError(reflect.ValueOf(nets), fieldName, + "", reason("rule contains both IPv4 and IPv6 CIDRs")) } + seenV4 = seenV4 || v4 + seenV6 = seenV6 || v6 } + + scanNets(rule.Source.GetNets(), "Source.Net(s)") + scanNets(rule.Source.GetNotNets(), "Source.NotNet(s)") + scanNets(rule.Destination.GetNets(), "Destination.Net(s)") + scanNets(rule.Destination.GetNotNets(), "Destination.NotNet(s)") } func validateBackendRule(v *validator.Validate, structLevel *validator.StructLevel) { diff --git a/lib/validator/validator_test.go b/lib/validator/validator_test.go index 260edb075..851562a07 100644 --- a/lib/validator/validator_test.go +++ b/lib/validator/validator_test.go @@ -59,10 +59,10 @@ func init() { DescribeTable("Validator", func(input interface{}, valid bool) { if valid { - Expect(validator.Validate(input)).To(BeNil(), + Expect(validator.Validate(input)).NotTo(HaveOccurred(), "expected value to be valid") } else { - Expect(validator.Validate(input)).ToNot(BeNil(), + Expect(validator.Validate(input)).To(HaveOccurred(), "expected value to be invalid") } }, @@ -529,6 +529,110 @@ func init() { Net: &netv6_1, }, }, false), + Entry("net list: should reject rule with net and nets", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V4, + Source: api.EntityRule{ + Net: &netv4_3, + Nets: []*net.IPNet{&netv4_3}, + }, + }, false), + Entry("net list: should reject rule with not net and not nets", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V4, + Source: api.EntityRule{ + NotNet: &netv4_3, + NotNets: []*net.IPNet{&netv4_3}, + }, + }, false), + Entry("net list: should reject rule with net and nets", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V4, + Destination: api.EntityRule{ + Net: &netv4_3, + Nets: []*net.IPNet{&netv4_3}, + }, + }, false), + Entry("net list: should reject rule with not net and not nets", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V4, + Destination: api.EntityRule{ + NotNet: &netv4_3, + NotNets: []*net.IPNet{&netv4_3}, + }, + }, false), + Entry("net list: should reject rule mixed IPv4 (src) and IPv6 (dest)", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + Source: api.EntityRule{ + Nets: []*net.IPNet{&netv4_3}, + }, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&netv6_3}, + }, + }, false), + Entry("net list: should reject rule mixed IPv6 (src) and IPv4 (dest)", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + Source: api.EntityRule{ + Nets: []*net.IPNet{&netv6_2}, + }, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&netv4_2}, + }, + }, false), + Entry("net list: should reject rule mixed IPv6 version and IPv4 Net", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V6, + Source: api.EntityRule{ + Nets: []*net.IPNet{&netv4_4}, + }, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&netv4_2}, + }, + }, false), + Entry("net list: should reject rule mixed IPv6 version and IPv4 Net", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V6, + Source: api.EntityRule{ + Net: &netv4_4, + }, + Destination: api.EntityRule{ + NotNets: []*net.IPNet{&netv4_2}, + }, + }, false), + Entry("net list: should reject rule mixed IPVersion and Source Net IP version", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V6, + Source: api.EntityRule{ + Nets: []*net.IPNet{&netv4_1}, + }, + }, false), + Entry("net list: should reject rule mixed IPVersion and Dest Net IP version", + api.Rule{ + Action: "allow", + Protocol: protocolFromString("tcp"), + IPVersion: &V4, + Destination: api.EntityRule{ + Nets: []*net.IPNet{&netv6_1}, + }, + }, false), // (API) NodeSpec Entry("should accept node with IPv4 BGP", api.NodeSpec{BGP: &api.NodeBGPSpec{IPv4Address: &netv4_1}}, true),