Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Nets field, deprecate the Net field. #470

Merged
merged 1 commit into from
Jul 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions lib/api/rule.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spurred by the conversation in slack - is there a good place to add a warning log when users use Nets, so we can indicate that it's deprecated?

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"`
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a requirement to support both nets and net on the same object?

Maybe we could simplify this by just validating that only one of net and nets is provided - might also be useful as a way to get users to drop using the deprecated field (they need to stop using it in order to use the new feature) so that there's less pain when that field is dropped some day in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup I agree - I think I'd suggested similar on the other PR :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robbrockbank That was the approach I took, the validation function for the front-end does do that so a normal user shouldn't be able to hit this. I thought it was better to have felix tolerate having both in the backend though; no point dropping a policy on the floor if it has a fairly sensible interpretation.

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)
}
14 changes: 14 additions & 0 deletions lib/backend/backend_suite_test.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down
193 changes: 122 additions & 71 deletions lib/backend/model/rule.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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, " ")
Expand Down
Loading