Skip to content

Commit

Permalink
Allow additional CIDRs to be excluded from SNAT (#520)
Browse files Browse the repository at this point in the history
* Allow additional CIDRs to be excluded from SNAT

Introduces a mechanism to provide an exclusion list of CIDRs that should
not be SNATed.

Adds an environment variable `AWS_VPC_K8S_CNI_EXCLUDE_SNAT_CIDRS` to
provide a comma separated list of ipv4 CIDRs to exclude from SNAT.
The value of this environment variable will only be use when
`AWS_VPC_K8S_CNI_EXTERNALSNAT` is false.

The SNAT exclusion CIDRs will be used to:
- Generate `iptable` rules to prevent SNATing for packets directed to
the excluded CIDRs
- Generate `ip rule` entries to route packets directed to the excluded
CIDRs through the pod's `eni`. These configuration is done both at the
`cni` plugin level via the `rpc_handler`, and the runtime `network` api.

Given that the list of excluded CIDRs may vary by configuration it was
necessary to include a mechanism to clean up stale SNAT rules. If a
CIDR is removed from the exclusion list, the corresponding `iptable`
rule will be removed as well, and the chain will be adjusted.

Connects #469

Co-authored-by: @rewiko
Co-authored-by: @yorg1st

* Add formatting targets to Makefile

`format` applies formatting to the project's go files.
`check-format` checks that no files require formatting; if they do it
will fail the command.

It adds the `check-format` target to the travis build so that CI fails
if files are not properly formatted.

* Fix typo in SNAT chain rules' comment
  • Loading branch information
totahuanocotl authored and Claes Mogren committed Jul 11, 2019
1 parent e638b1b commit a4b14eb
Show file tree
Hide file tree
Showing 17 changed files with 534 additions and 72 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ install:

# Tests to run
script:
- GO111MODULE=on make check-format
- GO111MODULE=on make build-linux
- GO111MODULE=on make lint
- GO111MODULE=on make vet
Expand Down
18 changes: 17 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# language governing permissions and limitations under the License.
#

.PHONY: all build-linux clean docker docker-build lint unit-test vet download-portmap build-docker-test build-metrics docker-metrics metrics-unit-test docker-metrics-test docker-vet
.PHONY: all build-linux clean format check-format docker docker-build lint unit-test vet download-portmap build-docker-test build-metrics docker-metrics metrics-unit-test docker-metrics-test docker-vet

IMAGE ?= amazon/amazon-k8s-cni
VERSION ?= $(shell git describe --tags --always --dirty)
Expand Down Expand Up @@ -123,3 +123,19 @@ clean:
rm -f aws-cni
rm -f cni-metrics-helper/cni-metrics-helper
rm -f portmap

files := $(shell find . -path ./vendor -prune -or -not -name 'mock_publisher.go' -name '*.go' -print)
unformatted = $(shell goimports -l $(files))

format :
@echo "== format"
@goimports -w $(files)
@sync

check-format :
@echo "== check formatting"
ifneq "$(unformatted)" ""
@echo "needs formatting: $(unformatted)"
@echo "run 'make format'"
@exit 1
endif
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ Disable (`none`) this functionality if you rely on sequential port allocation fo

*Note*: Any options other than `none` will cause outbound connections to be assigned a source port that's not necessarily part of the ephemeral port range set at the OS level (/proc/sys/net/ipv4/ip_local_port_range). This is relevant for any customers that might have NACLs restricting traffic based on the port range found in ip_local_port_range

`AWS_VPC_K8S_CNI_EXCLUDE_SNAT_CIDRS`
Type: String
Default: empty
Specify a comma separated list of IPv4 CIDRs to exclude from SNAT. For every item in the list an `iptables` rule and off\-VPC
IP rule will be applied. If an item is not a valid ipv4 range it will be skipped. This should be used when `AWS_VPC_K8S_CNI_EXTERNALSNAT=false`.

`WARM_ENI_TARGET`
Type: Integer
Default: `1`
Expand Down
4 changes: 2 additions & 2 deletions ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
log.Debugf("Found ENI %s that has less than the maximum number of IP addresses allocated: cur=%d, max=%d", eni.ID, currentNumberOfAllocatedIPs, c.maxIPsPerENI)
// Try to allocate all available IPs for this ENI
// TODO: Retry with back-off, trying with half the number of IPs each time
err = c.awsClient.AllocIPAddresses(eni.ID, c.maxIPsPerENI- currentNumberOfAllocatedIPs)
err = c.awsClient.AllocIPAddresses(eni.ID, c.maxIPsPerENI-currentNumberOfAllocatedIPs)
if err != nil {
log.Warnf("failed to allocate all available IP addresses on ENI %s, err: %v", eni.ID, err)
// Try to just get one more IP
Expand All @@ -692,7 +692,7 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
return true, nil
}
return false, nil
}
}

// setupENI does following:
// 1) add ENI to datastore
Expand Down
22 changes: 11 additions & 11 deletions ipamd/ipamd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ import (
"github.com/aws/amazon-vpc-cni-k8s/ipamd/datastore"
"github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1"
"github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils"
"github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils/mocks"
mock_awsutils "github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils/mocks"
"github.com/aws/amazon-vpc-cni-k8s/pkg/docker"
"github.com/aws/amazon-vpc-cni-k8s/pkg/docker/mocks"
"github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig/mocks"
mock_docker "github.com/aws/amazon-vpc-cni-k8s/pkg/docker/mocks"
mock_eniconfig "github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig/mocks"
"github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi"
"github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi/mocks"
"github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils/mocks"
mock_k8sapi "github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi/mocks"
mock_networkutils "github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils/mocks"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -237,9 +237,9 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) {

mockAWS.EXPECT().DescribeENI(eni2).Return(
[]*ec2.NetworkInterfacePrivateIpAddress{
{ PrivateIpAddress: &testAddr11, Primary: &primary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
{PrivateIpAddress: &testAddr11, Primary: &primary},
{PrivateIpAddress: &testAddr12, Primary: &notPrimary},
{PrivateIpAddress: &testAddr12, Primary: &notPrimary},
},
&attachmentID, nil)

Expand Down Expand Up @@ -302,9 +302,9 @@ func TestTryAddIPToENI(t *testing.T) {
mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
mockAWS.EXPECT().DescribeENI(secENIid).Return(
[]*ec2.NetworkInterfacePrivateIpAddress{
{ PrivateIpAddress: &testAddr11, Primary: &primary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
{ PrivateIpAddress: &testAddr12, Primary: &notPrimary },
{PrivateIpAddress: &testAddr11, Primary: &primary},
{PrivateIpAddress: &testAddr12, Primary: &notPrimary},
{PrivateIpAddress: &testAddr12, Primary: &notPrimary},
},
&attachmentID, nil)

Expand Down
10 changes: 9 additions & 1 deletion ipamd/rpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,20 @@ func (s *server) AddNetwork(ctx context.Context, in *pb.AddNetworkRequest) (*pb.
pbVPCcidrs = append(pbVPCcidrs, *cidr)
}

useExternalSNAT := s.ipamContext.networkClient.UseExternalSNAT()
if !useExternalSNAT {
for _, cidr := range s.ipamContext.networkClient.GetExcludeSNATCIDRs() {
log.Debugf("CIDR SNAT Exclusion %s", cidr)
pbVPCcidrs = append(pbVPCcidrs, cidr)
}
}

resp := pb.AddNetworkReply{
Success: err == nil,
IPv4Addr: addr,
IPv4Subnet: "",
DeviceNumber: int32(deviceNumber),
UseExternalSNAT: s.ipamContext.networkClient.UseExternalSNAT(),
UseExternalSNAT: useExternalSNAT,
VPCcidrs: pbVPCcidrs,
}

Expand Down
93 changes: 93 additions & 0 deletions ipamd/rpc_handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright 2017 Amazon.com, Inc. or its affiliates. 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. A copy of the
// License is located at
//
// http://aws.amazon.com/apache2.0/
//
// or in the "license" file accompanying this file. This file 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 ipamd

import (
"context"
"testing"

"github.com/aws/amazon-vpc-cni-k8s/ipamd/datastore"
"github.com/aws/aws-sdk-go/aws"

pb "github.com/aws/amazon-vpc-cni-k8s/rpc"

"github.com/stretchr/testify/assert"
)

func TestServer_AddNetwork(t *testing.T) {
ctrl, mockAWS, mockK8S, mockDocker, mockNetwork, _ := setup(t)
defer ctrl.Finish()

mockContext := &IPAMContext{
awsClient: mockAWS,
k8sClient: mockK8S,
maxIPsPerENI: 14,
maxENI: 4,
warmENITarget: 1,
warmIPTarget: 3,
dockerClient: mockDocker,
networkClient: mockNetwork,
dataStore: datastore.NewDataStore(),
}

rpcServer := server{ipamContext: mockContext}

addNetworkRequest := &pb.AddNetworkRequest{
Netns: "netns",
K8S_POD_NAME: "pod",
K8S_POD_NAMESPACE: "ns",
K8S_POD_INFRA_CONTAINER_ID: "cid",
IfName: "eni",
}

vpcCIDRs := []*string{aws.String(vpcCIDR)}
testCases := []struct {
name string
useExternalSNAT bool
vpcCIDRs []*string
snatExclusionCIDRs []string
}{
{
"VPC CIDRs",
true,
vpcCIDRs,
nil,
},
{
"SNAT Exclusion CIDRs",
false,
vpcCIDRs,
[]string{"10.12.0.0/16", "10.13.0.0/16"},
},
}
for _, tc := range testCases {
mockAWS.EXPECT().GetVPCIPv4CIDRs().Return(tc.vpcCIDRs)
mockNetwork.EXPECT().UseExternalSNAT().Return(tc.useExternalSNAT)
if !tc.useExternalSNAT {
mockNetwork.EXPECT().GetExcludeSNATCIDRs().Return(tc.snatExclusionCIDRs)
}

addNetworkReply, err := rpcServer.AddNetwork(context.TODO(), addNetworkRequest)
assert.NoError(t, err, tc.name)

assert.Equal(t, tc.useExternalSNAT, addNetworkReply.UseExternalSNAT, tc.name)

var expectedCIDRs []string
for _, cidr := range tc.vpcCIDRs {
expectedCIDRs = append(expectedCIDRs, *cidr)
}
expectedCIDRs = append([]string{vpcCIDR}, tc.snatExclusionCIDRs...)
assert.Equal(t, expectedCIDRs, addNetworkReply.VPCcidrs, tc.name)
}
}
4 changes: 2 additions & 2 deletions pkg/awsutils/awsutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"

"github.com/aws/amazon-vpc-cni-k8s/pkg/ec2metadata/mocks"
"github.com/aws/amazon-vpc-cni-k8s/pkg/ec2wrapper/mocks"
mock_ec2metadata "github.com/aws/amazon-vpc-cni-k8s/pkg/ec2metadata/mocks"
mock_ec2wrapper "github.com/aws/amazon-vpc-cni-k8s/pkg/ec2wrapper/mocks"
)

const (
Expand Down
3 changes: 2 additions & 1 deletion pkg/ec2metadatawrapper/ec2metadatawrapper_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package ec2metadatawrapper

import (
mockec2metadatawrapper "github.com/aws/amazon-vpc-cni-k8s/pkg/ec2metadatawrapper/mocks"
"testing"
"time"

mockec2metadatawrapper "github.com/aws/amazon-vpc-cni-k8s/pkg/ec2metadatawrapper/mocks"

"github.com/aws/aws-sdk-go/aws/ec2metadata"

"github.com/golang/mock/gomock"
Expand Down
2 changes: 1 addition & 1 deletion pkg/k8sapi/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
clientset "k8s.io/client-go/kubernetes"

"github.com/operator-framework/operator-sdk/pkg/k8sclient"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down
3 changes: 2 additions & 1 deletion pkg/netlinkwrapper/netlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
package netlinkwrapper

import (
"github.com/vishvananda/netlink"
"syscall"

"github.com/vishvananda/netlink"
)

// NetLink wraps methods used from the vishvananda/netlink package
Expand Down
12 changes: 12 additions & 0 deletions pkg/networkutils/mocks/network_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit a4b14eb

Please sign in to comment.