Add subnet to per-node BGP config (#308)

Ensure using valid IP and CIDR validator

Add tests for node and node validation

lib/api/bgppeer.go

@@ -48,7 +48,7 @@ type BGPPeerMetadata struct {
 	Node string `json:"node,omitempty" validate:"omitempty,name"`
 
 	// The IP address of the peer.
-	PeerIP net.IP `json:"peerIP" validate:"omitempty,ip"`
+	PeerIP net.IP `json:"peerIP" validate:"omitempty"`
 }
 
 // BGPPeerSpec contains the specification for a BGPPeer resource.

lib/api/hostendpoint.go

@@ -59,7 +59,7 @@ type HostEndpointSpec struct {
 	// 	endpoints, the ExpectedIPs field is used for that purpose. (If only the interface
 	// 	name is specified, Calico does not learn the IPs of the interface for use in match
 	// 	criteria.)
-	ExpectedIPs []net.IP `json:"expectedIPs,omitempty" validate:"omitempty,dive,ip"`
+	ExpectedIPs []net.IP `json:"expectedIPs,omitempty" validate:"omitempty"`
 
 	// A list of identifiers of security Profile objects that apply to this endpoint. Each
 	// profile is applied in the order that they appear in this list.  Profile rules are applied

lib/api/ippool.go

@@ -16,6 +16,7 @@ package api
 
 import (
 	"github.com/projectcalico/libcalico-go/lib/api/unversioned"
+	"github.com/projectcalico/libcalico-go/lib/ipip"
 	"github.com/projectcalico/libcalico-go/lib/net"
 )
 
@@ -56,6 +57,13 @@ type IPIPConfiguration struct {
 	// When enabled is true, ipip tunneling will be used to deliver packets to
 	// destinations within this pool.
 	Enabled bool `json:"enabled,omitempty"`
+
+	// The IPIP mode.  This can be one of "always" or "cross-subnet".  A mode
+	// of "always" will also use IPIP tunneling for routing to destination IP
+	// addresses within this pool.  A mode of "cross-subnet" will only use IPIP
+	// tunneling when the destination node is on a different subnet to the
+	// originating node.  The default value (if not specified) is "always".
+	Mode ipip.Mode `json:"mode,omitempty"`
 }
 
 // NewIPPool creates a new (zeroed) Pool struct with the TypeMetadata initialised to the current

lib/api/node.go

@@ -61,13 +61,13 @@ type NodeBGPSpec struct {
 	// default value will be used.
 	ASNumber *numorstring.ASNumber `json:"asNumber,omitempty"`
 
-	// IPv4Address is the IPv4 address of this node.  At least one of the
-	// IPv4 and IPv6 addresses should be specified.
-	IPv4Address *net.IP `json:"ipv4Address,omitempty" validate:"omitempty,ipv4"`
+	// IPv4Address is the IPv4 address and network of this node.  At least
+	// one of the IPv4 and IPv6 addresses should be specified.
+	IPv4Address *net.IPNet `json:"ipv4Address,omitempty" validate:"omitempty"`
 
-	// IPv6Address is the IPv6 address of this node.  At least one of the
-	// IPv4 and IPv6 addresses should be specified.
-	IPv6Address *net.IP `json:"ipv6Address,omitempty" validate:"omitempty,ipv6"`
+	// IPv6Address is the IPv6 address and network of this node.  At least
+	// one of the IPv4 and IPv6 addresses should be specified.
+	IPv6Address *net.IPNet `json:"ipv6Address,omitempty" validate:"omitempty"`
 }
 
 // NewNode creates a new (zeroed) NodeList struct with the TypeMetadata initialised to the current

lib/api/workloadendpoint.go

@@ -56,7 +56,7 @@ type WorkloadEndpointSpec struct {
 	// allowed to leave this interface if they come from an address in one of these subnets.
 	//
 	// Currently only /32 for IPv4 and /128 for IPv6 networks are supported.
-	IPNetworks []net.IPNet `json:"ipNetworks,omitempty" validate:"omitempty,dive,cidr"`
+	IPNetworks []net.IPNet `json:"ipNetworks,omitempty" validate:"omitempty"`
 
 	// IPNATs is a list of 1:1 NAT mappings to apply to the endpoint. Inbound connections
 	// to the external IP will be forwarded to the internal IP. Connections initiated from the
@@ -66,10 +66,10 @@ type WorkloadEndpointSpec struct {
 	IPNATs []IPNAT `json:"ipNATs,omitempty" validate:"omitempty,dive"`
 
 	// IPv4Gateway is the gateway IPv4 address for traffic from the workload.
-	IPv4Gateway *net.IP `json:"ipv4Gateway,omitempty" validate:"omitempty,ipv4"`
+	IPv4Gateway *net.IP `json:"ipv4Gateway,omitempty" validate:"omitempty"`
 
 	// IPv6Gateway is the gateway IPv6 address for traffic from the workload.
-	IPv6Gateway *net.IP `json:"ipv6Gateway,omitempty" validate:"omitempty,ipv6"`
+	IPv6Gateway *net.IP `json:"ipv6Gateway,omitempty" validate:"omitempty"`
 
 	// A list of security Profile resources that apply to this endpoint. Each profile is
 	// applied in the order that they appear in this list.  Profile rules are applied
@@ -87,10 +87,10 @@ type WorkloadEndpointSpec struct {
 type IPNAT struct {
 	// The internal IP address which must be associated with the owning endpoint via the
 	// configured IPNetworks for the endpoint.
-	InternalIP net.IP `json:"internalIP" validate:"ip"`
+	InternalIP net.IP `json:"internalIP"`
 
 	// The external IP address.
-	ExternalIP net.IP `json:"externalIP" validate:"ip"`
+	ExternalIP net.IP `json:"externalIP"`
 }
 
 // String returns a friendly form of an IPNAT.

lib/backend/backend_test.go

@@ -41,10 +41,10 @@ var _ = Describe("Backend tests", func() {
 
 		block = model.KVPair{
 			Key: model.BlockKey{
-				CIDR: testutils.MustParseCIDR("10.0.0.0/26"),
+				CIDR: testutils.MustParseNetwork("10.0.0.0/26"),
 			},
 			Value: model.AllocationBlock{
-				CIDR: testutils.MustParseCIDR("10.0.0.0/26"),
+				CIDR: testutils.MustParseNetwork("10.0.0.0/26"),
 			},
 		}
 
@@ -77,7 +77,7 @@ var _ = Describe("Backend tests", func() {
 
 		It("updates a kv pair", func() {
 			block.Value = model.AllocationBlock{
-				CIDR: testutils.MustParseCIDR("192.168.0.0/26"),
+				CIDR: testutils.MustParseNetwork("192.168.0.0/26"),
 			}
 
 			kv, err := etcdClient.Update(&block)
@@ -108,7 +108,7 @@ var _ = Describe("Backend tests", func() {
 
 		It("updates a kv pair", func() {
 			block.Value = model.AllocationBlock{
-				CIDR: testutils.MustParseCIDR("192.168.0.0/26"),
+				CIDR: testutils.MustParseNetwork("192.168.0.0/26"),
 			}
 
 			kv, err := etcdClient.Apply(&block)
@@ -122,7 +122,7 @@ var _ = Describe("Backend tests", func() {
 
 		It("creates a kv pair", func() {
 			block.Key = model.BlockKey{
-				CIDR: testutils.MustParseCIDR("192.168.0.0/26"),
+				CIDR: testutils.MustParseNetwork("192.168.0.0/26"),
 			}
 
 			kv, err := etcdClient.Apply(&block)
@@ -136,7 +136,7 @@ var _ = Describe("Backend tests", func() {
 
 		It("sets revision field", func() {
 			block.Value = model.AllocationBlock{
-				CIDR: testutils.MustParseCIDR("192.168.0.0/26"),
+				CIDR: testutils.MustParseNetwork("192.168.0.0/26"),
 			}
 
 			kv, err := etcdClient.Apply(&block)

lib/backend/compat/compat.go

@@ -374,11 +374,24 @@ func (c *ModelAdaptor) getNodeSubcomponents(nk model.NodeKey, nv *model.Node) er
 	if component, err = c.client.Get(model.HostBGPConfigKey{Hostname: nk.Hostname, Name: "ip_addr_v4"}); err == nil {
 		strval = component.Value.(string)
 		if strval != "" {
-			nv.BGPIPv4 = &net.IP{}
-			err = nv.BGPIPv4.UnmarshalText([]byte(strval))
+			nv.BGPIPv4Addr = &net.IP{}
+			err = nv.BGPIPv4Addr.UnmarshalText([]byte(strval))
 			if err != nil {
 				log.WithError(err).Warning("Error unmarshalling IPv4")
-				nv.BGPIPv4 = nil
+				nv.BGPIPv4Addr = nil
+			}
+		}
+	} else if _, ok := err.(errors.ErrorResourceDoesNotExist); !ok {
+		return err
+	}
+
+	if component, err = c.client.Get(model.HostBGPConfigKey{Hostname: nk.Hostname, Name: "network_v4"}); err == nil {
+		strval = component.Value.(string)
+		if strval != "" {
+			_, nv.BGPIPv4Net, err = net.ParseCIDR(strval)
+			if err != nil {
+				log.WithError(err).Warning("Error unmarshalling IPv4Net")
+				nv.BGPIPv4Net = nil
 			}
 		}
 	} else if _, ok := err.(errors.ErrorResourceDoesNotExist); !ok {
@@ -388,11 +401,24 @@ func (c *ModelAdaptor) getNodeSubcomponents(nk model.NodeKey, nv *model.Node) er
 	if component, err = c.client.Get(model.HostBGPConfigKey{Hostname: nk.Hostname, Name: "ip_addr_v6"}); err == nil {
 		strval = component.Value.(string)
 		if strval != "" {
-			nv.BGPIPv6 = &net.IP{}
-			err = nv.BGPIPv6.UnmarshalText([]byte(strval))
+			nv.BGPIPv6Addr = &net.IP{}
+			err = nv.BGPIPv6Addr.UnmarshalText([]byte(strval))
 			if err != nil {
 				log.WithError(err).Warning("Error unmarshalling IPv6")
-				nv.BGPIPv6 = nil
+				nv.BGPIPv6Addr = nil
+			}
+		}
+	} else if _, ok := err.(errors.ErrorResourceDoesNotExist); !ok {
+		return err
+	}
+
+	if component, err = c.client.Get(model.HostBGPConfigKey{Hostname: nk.Hostname, Name: "network_v6"}); err == nil {
+		strval = component.Value.(string)
+		if strval != "" {
+			_, nv.BGPIPv6Net, err = net.ParseCIDR(strval)
+			if err != nil {
+				log.WithError(err).Warning("Error unmarshalling IPv6Net")
+				nv.BGPIPv6Net = nil
 			}
 		}
 	} else if _, ok := err.(errors.ErrorResourceDoesNotExist); !ok {
@@ -484,12 +510,12 @@ func toNodeComponents(d *model.KVPair) (primary *model.KVPair, optional []*model
 	// case should be a blank string).  Felix on the other hand deals
 	// with values not existing.
 	ipv4Str := ""
-	if n.BGPIPv4 != nil {
-		ipv4Str = n.BGPIPv4.String()
+	if n.BGPIPv4Addr != nil {
+		ipv4Str = n.BGPIPv4Addr.String()
 	}
 	ipv6Str := ""
-	if n.BGPIPv6 != nil {
-		ipv6Str = n.BGPIPv6.String()
+	if n.BGPIPv6Addr != nil {
+		ipv6Str = n.BGPIPv6Addr.String()
 	}
 
 	// Add the BGP IPv4 and IPv6 values - these are always present.
@@ -514,10 +540,10 @@ func toNodeComponents(d *model.KVPair) (primary *model.KVPair, optional []*model
 	// address and the host ASN.  If either config is not specified, set
 	// the value to be nil to indicate to our default processing to delete
 	// the entry rather than set it.
-	if n.BGPIPv4 != nil {
+	if n.BGPIPv4Addr != nil {
 		optional = append(optional, &model.KVPair{
 			Key:   model.HostIPKey{nk.Hostname},
-			Value: n.BGPIPv4,
+			Value: n.BGPIPv4Addr,
 		})
 	} else {
 		optional = append(optional, &model.KVPair{
@@ -541,6 +567,38 @@ func toNodeComponents(d *model.KVPair) (primary *model.KVPair, optional []*model
 			},
 		})
 	}
+	if n.BGPIPv4Net != nil {
+		optional = append(optional, &model.KVPair{
+			Key: model.HostBGPConfigKey{
+				Hostname: nk.Hostname,
+				Name:     "network_v4",
+			},
+			Value: n.BGPIPv4Net.String(),
+		})
+	} else {
+		optional = append(optional, &model.KVPair{
+			Key: model.HostBGPConfigKey{
+				Hostname: nk.Hostname,
+				Name:     "network_v4",
+			},
+		})
+	}
+	if n.BGPIPv6Net != nil {
+		optional = append(optional, &model.KVPair{
+			Key: model.HostBGPConfigKey{
+				Hostname: nk.Hostname,
+				Name:     "network_v6",
+			},
+			Value: n.BGPIPv6Net.String(),
+		})
+	} else {
+		optional = append(optional, &model.KVPair{
+			Key: model.HostBGPConfigKey{
+				Hostname: nk.Hostname,
+				Name:     "network_v6",
+			},
+		})
+	}
 
 	return primary, optional
 }
@@ -576,6 +634,18 @@ func toNodeDeleteComponents(d *model.KVPair) (primary *model.KVPair, optional []
 				Name:     "as_num",
 			},
 		},
+		&model.KVPair{
+			Key: model.HostBGPConfigKey{
+				Hostname: nk.Hostname,
+				Name:     "network_v4",
+			},
+		},
+		&model.KVPair{
+			Key: model.HostBGPConfigKey{
+				Hostname: nk.Hostname,
+				Name:     "network_v6",
+			},
+		},
 	}
 
 	return primary, optional

lib/backend/k8s/resources/ippools_conversion.go

@@ -16,16 +16,16 @@ package resources
 
 import (
 	"encoding/json"
-	log "github.com/Sirupsen/logrus"
 	"strings"
 
+	log "github.com/Sirupsen/logrus"
 	"github.com/projectcalico/libcalico-go/lib/backend/k8s/thirdparty"
 	"github.com/projectcalico/libcalico-go/lib/backend/model"
 	kapi "k8s.io/client-go/pkg/api"
 )
 
-// ThirdPartyToIPPool takes a Kubrnetes ThirdPartyResource representation
-// of a Calico IP Pool and returns the equivalen IPPool object.
+// ThirdPartyToIPPool takes a Kubernetes ThirdPartyResource representation
+// of a Calico IP Pool and returns the equivalent IPPool object.
 func ThirdPartyToIPPool(t *thirdparty.IpPool) *model.KVPair {
 	v := model.IPPool{}
 	err := json.Unmarshal([]byte(t.Spec.Value), &v)

lib/backend/model/ippool.go

@@ -23,6 +23,7 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/projectcalico/libcalico-go/lib/errors"
+	"github.com/projectcalico/libcalico-go/lib/ipip"
 	"github.com/projectcalico/libcalico-go/lib/net"
 )
 
@@ -93,6 +94,7 @@ func (options IPPoolListOptions) KeyFromDefaultPath(path string) Key {
 type IPPool struct {
 	CIDR          net.IPNet `json:"cidr"`
 	IPIPInterface string    `json:"ipip"`
+	IPIPMode      ipip.Mode `json:"ipip_mode"`
 	Masquerade    bool      `json:"masquerade"`
 	IPAM          bool      `json:"ipam"`
 	Disabled      bool      `json:"disabled"`

lib/backend/model/node.go

@@ -43,8 +43,10 @@ type Node struct {
 	Labels map[string]string `json:"labels,omitempty"`
 
 	// BGP specific configuration
-	BGPIPv4     *net.IP
-	BGPIPv6     *net.IP
+	BGPIPv4Addr *net.IP
+	BGPIPv6Addr *net.IP
+	BGPIPv4Net  *net.IPNet
+	BGPIPv6Net  *net.IPNet
 	BGPASNumber *numorstring.ASNumber
 }
 

lib/client/ipam_test.go

@@ -125,7 +125,7 @@ var _ = Describe("IPAM tests", func() {
 		ic := setupIPAMClient(c, true)
 
 		host := "host-A"
-		pool1 := testutils.MustParseCIDR("10.0.0.0/24")
+		pool1 := testutils.MustParseNetwork("10.0.0.0/24")
 		var block cnet.IPNet
 
 		testutils.CreateNewIPPool(*c, "10.0.0.0/24", false, false, true)
@@ -183,7 +183,7 @@ var _ = Describe("IPAM tests", func() {
 		})
 
 		// Step-4: Create a new IP Pool.
-		pool2 := testutils.MustParseCIDR("20.0.0.0/24")
+		pool2 := testutils.MustParseNetwork("20.0.0.0/24")
 		testutils.CreateNewIPPool(*c, "20.0.0.0/24", false, false, true)
 
 		// Step-5: AutoAssign 1 IP without specifying a pool - expect the assigned IP is from pool2.
@@ -241,8 +241,8 @@ var _ = Describe("IPAM tests", func() {
 		ic := setupIPAMClient(c, true)
 
 		host := "host-A"
-		pool1 := testutils.MustParseCIDR("10.0.0.0/24")
-		pool2 := testutils.MustParseCIDR("20.0.0.0/24")
+		pool1 := testutils.MustParseNetwork("10.0.0.0/24")
+		pool2 := testutils.MustParseNetwork("20.0.0.0/24")
 		var block1, block2 cnet.IPNet
 
 		testutils.CreateNewIPPool(*c, "10.0.0.0/24", false, false, true)
@@ -341,8 +341,8 @@ var _ = Describe("IPAM tests", func() {
 		ic := setupIPAMClient(c, true)
 
 		host := "host-A"
-		pool1 := testutils.MustParseCIDR("10.0.0.0/24")
-		pool2 := testutils.MustParseCIDR("20.0.0.0/24")
+		pool1 := testutils.MustParseNetwork("10.0.0.0/24")
+		pool2 := testutils.MustParseNetwork("20.0.0.0/24")
 
 		testutils.CreateNewIPPool(*c, "10.0.0.0/24", false, false, true)
 		testutils.CreateNewIPPool(*c, "20.0.0.0/24", false, false, true)
@@ -485,7 +485,7 @@ var _ = Describe("IPAM tests", func() {
 
 	DescribeTable("ClaimAffinity: claim IPNet vs actual number of blocks claimed",
 		func(args testArgsClaimAff) {
-			inIPNet := testutils.MustParseCIDR(args.inNet)
+			inIPNet := testutils.MustParseNetwork(args.inNet)
 			c, _ := testutils.NewClient("")
 
 			// Wipe clean etcd, create a new client, and pools when cleanEnv flag is true.
@@ -609,7 +609,7 @@ func testIPAMAssignIP(inIP net.IP, host string, poolSubnet []string, cleanEnv bo
 // testIPAMAutoAssign takes number of requested IPv4 and IPv6, and hostname, and setus up/cleans up client and etcd,
 // then it calls AutoAssign (function under test) and returns the number of returned IPv4 and IPv6 addresses and returned error.
 func testIPAMAutoAssign(inv4, inv6 int, host string, cleanEnv bool, poolSubnet []string, usePool string) (int, int, error) {
-	fromPool := testutils.MustParseCIDR(usePool)
+	fromPool := testutils.MustParseNetwork(usePool)
 	args := client.AutoAssignArgs{
 		Num4:      inv4,
 		Num6:      inv6,