Skip to content

Commit

Permalink
Revert "Update rp_filter only when /proc is accessible"
Browse files Browse the repository at this point in the history 
This reverts commit 29474be.
  • Loading branch information
@SaranBalaji90 @mogren
SaranBalaji90 authored and mogren committed Apr 8, 2020
1 parent 44d0770 commit 070aa1f
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 89 deletions.
14 changes: 4 additions & 10 deletions pkg/networkutils/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,10 @@ func (n *linuxNetwork) SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []*string,
primaryIntfRPFilter := "net/ipv4/conf/" + primaryIntf + "/rp_filter"
const rpFilterLoose = "2"

if n.procSys.IsPathWriteAccessible(primaryIntfRPFilter) {
// Setting RPF will be removed from aws-node when we bump our minor version.
log.Debugf("Setting RPF for primary interface: %s", primaryIntfRPFilter)
err = n.procSys.Set(primaryIntfRPFilter, rpFilterLoose)
if err != nil {
return errors.Wrapf(err, "failed to configure %s RPF check", primaryIntf)
}
} else {
// when aws-node run as an un-privileged pod, /proc will be mounted as read only
log.Infof("Skip updating RPF for primary interface: %s", primaryIntfRPFilter)
log.Debugf("Setting RPF for primary interface: %s", primaryIntfRPFilter)
err = n.procSys.Set(primaryIntfRPFilter, rpFilterLoose)
if err != nil {
return errors.Wrapf(err, "failed to configure %s RPF check", primaryIntf)
}
}

Expand Down
117 changes: 60 additions & 57 deletions pkg/networkutils/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,18 @@ func TestSetupHostNetworkNodePortEnabled(t *testing.T) {
procSys: mockProcSys,
}

setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)
mockPrimaryInterfaceLookup(ctrl, mockNetLink)
mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)

var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

var vpcCIDRs []*string

Expand Down Expand Up @@ -353,8 +363,18 @@ func TestSetupHostNetworkWithExcludeSNATCIDRs(t *testing.T) {
procSys: mockProcSys,
}

setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

var vpcCIDRs []*string
vpcCIDRs = []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")}
Expand Down Expand Up @@ -396,8 +416,18 @@ func TestSetupHostNetworkCleansUpStaleSNATRules(t *testing.T) {
},
procSys: mockProcSys,
}
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

vpcCIDRs := []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")}
_ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-0", "!", "-d", "10.10.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAN", "-j", "AWS-SNAT-CHAIN-1") //AWS SNAT CHAN proves backwards compatibility
Expand Down Expand Up @@ -447,8 +477,18 @@ func TestSetupHostNetworkExcludedSNATCIDRsIdempotent(t *testing.T) {
},
procSys: mockProcSys,
}
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

_ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-0", "!", "-d", "10.10.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-1")
_ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-1", "!", "-d", "10.11.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-2")
Expand Down Expand Up @@ -497,8 +537,18 @@ func TestSetupHostNetworkMultipleCIDRs(t *testing.T) {
},
procSys: mockProcSys,
}
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, true)
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

var vpcCIDRs []*string
vpcCIDRs = []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")}
Expand Down Expand Up @@ -532,53 +582,6 @@ func TestIncrementIPv4Addr(t *testing.T) {
}
}

func TestSetupHostNetworkForUnprivilegedPod(t *testing.T) {
ctrl, mockNetLink, _, mockNS, mockIptables, mockProcSys := setup(t)
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: true,
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
newIptables: func() (iptablesIface, error) {
return mockIptables, nil
},
procSys: mockProcSys,
}
var vpcCIDRs []*string
setupNetLinkMocks(ctrl, mockNetLink)
setupProcSysMocks(mockProcSys, false)

err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP)
assert.NoError(t, err)
}

func setupProcSysMocks(mockProcSys *mock_procsyswrapper.MockProcSys, shouldHaveWriteAccess bool) {
rpFilterPath := "net/ipv4/conf/lo/rp_filter"

mockProcSys.EXPECT().IsPathWriteAccessible(rpFilterPath).Return(shouldHaveWriteAccess)
if shouldHaveWriteAccess {
mockProcSys.EXPECT().Set(rpFilterPath, "2").Return(nil)
}
}

func setupNetLinkMocks(ctrl *gomock.Controller, mockNetLink *mock_netlinkwrapper.MockNetLink) {
mockPrimaryInterfaceLookup(ctrl, mockNetLink)
mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)

var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
}

type mockIptables struct {
// dataplaneState is a map from table name to chain name to slice of rulespecs
dataplaneState map[string]map[string][][]string
Expand Down
14 changes: 0 additions & 14 deletions pkg/procsyswrapper/mocks/procsys_mocks.go
View file

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

8 changes: 0 additions & 8 deletions pkg/procsyswrapper/procsys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,11 @@ package procsyswrapper

import (
"io/ioutil"

"golang.org/x/sys/unix"
)

type ProcSys interface {
Get(key string) (string, error)
Set(key, value string) error
IsPathWriteAccessible(key string) bool
}

type procSys struct {
Expand All @@ -45,8 +42,3 @@ func (p *procSys) Get(key string) (string, error) {
func (p *procSys) Set(key, value string) error {
return ioutil.WriteFile(p.path(key), []byte(value), 0644)
}

// IsPathWriteAccessible verifies if aws-node pod can write to the specified path
func (p *procSys) IsPathWriteAccessible(key string) bool {
return unix.Access(p.path(key), unix.W_OK) != nil
}

0 comments on commit 070aa1f

Please sign in to comment.