diff --git a/cmd/egress-cni-plugin/egressContext.go b/cmd/egress-cni-plugin/egressContext.go index fce7d3480e..97411bf1da 100644 --- a/cmd/egress-cni-plugin/egressContext.go +++ b/cmd/egress-cni-plugin/egressContext.go @@ -29,6 +29,7 @@ import ( "github.com/aws/amazon-vpc-cni-k8s/pkg/hostipamwrapper" "github.com/aws/amazon-vpc-cni-k8s/pkg/iptableswrapper" "github.com/aws/amazon-vpc-cni-k8s/pkg/netlinkwrapper" + "github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils" "github.com/aws/amazon-vpc-cni-k8s/pkg/nswrapper" "github.com/aws/amazon-vpc-cni-k8s/pkg/procsyswrapper" "github.com/aws/amazon-vpc-cni-k8s/pkg/utils/cniutils" @@ -82,7 +83,7 @@ func NewEgressAddContext(nsPath, ifName string) egressContext { ArgsIfName: ifName, Veth: vethwrapper.NewSetupVeth(), IptCreator: func(protocol iptables.Protocol) (iptableswrapper.IPTablesIface, error) { - return iptableswrapper.NewIPTables(protocol) + return networkutils.NewIPTables(protocol) }, } } @@ -95,7 +96,7 @@ func NewEgressDelContext(nsPath string) egressContext { Ns: nswrapper.NewNS(), NsPath: nsPath, IptCreator: func(protocol iptables.Protocol) (iptableswrapper.IPTablesIface, error) { - return iptableswrapper.NewIPTables(protocol) + return networkutils.NewIPTables(protocol) }, } } diff --git a/go.mod b/go.mod index 9b4d4f648f..a9356048b8 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/aws/amazon-vpc-cni-k8s/test/agent v0.0.0-20231212223725-21c4bd73015b github.com/aws/amazon-vpc-resource-controller-k8s v1.5.0 github.com/aws/aws-sdk-go v1.55.5 + github.com/cenkalti/backoff/v4 v4.3.0 github.com/containernetworking/cni v1.2.3 github.com/containernetworking/plugins v1.5.1 github.com/coreos/go-iptables v0.8.0 diff --git a/go.sum b/go.sum index fc1fb23151..c46494e1f5 100644 --- a/go.sum +++ b/go.sum @@ -51,6 +51,8 @@ github.com/bugsnag/osext v0.0.0-20130617224835-0dd3f918b21b h1:otBG+dV+YK+Soembj github.com/bugsnag/osext v0.0.0-20130617224835-0dd3f918b21b/go.mod h1:obH5gd0BsqsP2LwDJ9aOkm/6J86V6lyAXCoQWGw3K50= github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0 h1:nvj0OLI3YqYXer/kZD8Ri1aaunCxIEsOst1BVJswV0o= github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0/go.mod h1:D/8v3kj0zr8ZAKg1AQ6crr+5VwKN5eIywRkfhyM/+dE= +github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= +github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= diff --git a/pkg/networkutils/iptables.go b/pkg/networkutils/iptables.go new file mode 100644 index 0000000000..2fde1c02f8 --- /dev/null +++ b/pkg/networkutils/iptables.go @@ -0,0 +1,141 @@ +// Copyright 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 networkutils is a collection of iptables and netlink functions +package networkutils + +import ( + "time" + + "github.com/aws/amazon-vpc-cni-k8s/pkg/iptableswrapper" + "github.com/cenkalti/backoff/v4" + "github.com/coreos/go-iptables/iptables" +) + +type ipTables struct { + ipt iptableswrapper.IPTablesIface + backoff *backoff.ExponentialBackOff +} + +// NewIPTables return a ipTables struct that implements IPTablesIface +func NewIPTables(protocol iptables.Protocol) (iptableswrapper.IPTablesIface, error) { + ipt, err := iptables.New(iptables.IPFamily(protocol), iptables.Timeout(1)) + if err != nil { + return nil, err + } + return &ipTables{ + ipt: ipt, + backoff: backoff.NewExponentialBackOff(backoff.WithMaxElapsedTime(0)), // Never stop retrying as backward compatibility + }, nil +} + +func (i ipTables) Exists(table, chain string, rulespec ...string) (bool, error) { + operation := func() (bool, error) { + return i.ipt.Exists(table, chain, rulespec...) + } + result, err := backoff.RetryNotifyWithData(operation, i.backoff, logRetryError) + if err != nil { + return true, err + } + return result, nil +} + +func (i ipTables) Insert(table, chain string, pos int, rulespec ...string) error { + operation := func() error { + return i.ipt.Insert(table, chain, pos, rulespec...) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) Append(table, chain string, rulespec ...string) error { + operation := func() error { + return i.ipt.Append(table, chain, rulespec...) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) AppendUnique(table, chain string, rulespec ...string) error { + operation := func() error { + return i.ipt.AppendUnique(table, chain, rulespec...) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) Delete(table, chain string, rulespec ...string) error { + operation := func() error { + return i.ipt.Delete(table, chain, rulespec...) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) List(table, chain string) ([]string, error) { + operation := func() ([]string, error) { + return i.ipt.List(table, chain) + } + result, err := backoff.RetryNotifyWithData(operation, i.backoff, logRetryError) + if err != nil { + return nil, err + } + return result, nil +} + +func (i ipTables) NewChain(table, chain string) error { + operation := func() error { + return i.ipt.NewChain(table, chain) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) ClearChain(table, chain string) error { + operation := func() error { + return i.ipt.ClearChain(table, chain) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) DeleteChain(table, chain string) error { + operation := func() error { + return i.ipt.DeleteChain(table, chain) + } + return backoff.RetryNotify(operation, i.backoff, logRetryError) +} + +func (i ipTables) ListChains(table string) ([]string, error) { + operation := func() ([]string, error) { + return i.ipt.ListChains(table) + } + result, err := backoff.RetryNotifyWithData(operation, i.backoff, logRetryError) + if err != nil { + return nil, err + } + return result, nil +} + +func (i ipTables) ChainExists(table, chain string) (bool, error) { + operation := func() (bool, error) { + return i.ipt.ChainExists(table, chain) + } + result, err := backoff.RetryNotifyWithData(operation, i.backoff, logRetryError) + if err != nil { + return true, err + } + return result, nil +} + +func (i ipTables) HasRandomFully() bool { + return i.ipt.HasRandomFully() +} + +func logRetryError(err error, t time.Duration) { + log.Errorf("Another app is currently holding the xtables lock. Retrying in %f seconds", t.Seconds()) +} diff --git a/pkg/networkutils/iptables_test.go b/pkg/networkutils/iptables_test.go new file mode 100644 index 0000000000..46a08d6996 --- /dev/null +++ b/pkg/networkutils/iptables_test.go @@ -0,0 +1,277 @@ +// Copyright 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 networkutils is a collection of iptables and netlink functions +package networkutils + +import ( + "crypto/rand" + "math/big" + "testing" + + mock_iptables "github.com/aws/amazon-vpc-cni-k8s/pkg/iptableswrapper/mocks" + "github.com/cenkalti/backoff/v4" + "github.com/golang/mock/gomock" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" +) + +const ( + address1 = "203.0.113.1/32" + address2 = "203.0.113.2/32" + subnet1 = "192.0.2.0/24" +) + +var ( + timeoutError = errors.New("timeout") + permissionDeniedError = errors.New("permission denied") +) + +func randChain(t *testing.T) string { + n, err := rand.Int(rand.Reader, big.NewInt(1000000)) + if err != nil { + t.Fatalf("Failed to generate random chain name: %v", err) + } + + return "TEST-" + n.String() +} + +func setupIPTTest(t *testing.T) (*gomock.Controller, + string, + string, + *backoff.ExponentialBackOff) { + return gomock.NewController(t), + "filter", + randChain(t), + backoff.NewExponentialBackOff(backoff.WithMaxElapsedTime(0)) +} + +func TestExists(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().Exists(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(false, timeoutError), + mockIptable.EXPECT().Exists(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(false, nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + exists, _ := ipt.Exists(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT") + assert.Equal(t, exists, false) +} + +func TestInsert(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().Insert(table, chain, 2, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(timeoutError), + mockIptable.EXPECT().Insert(table, chain, 2, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(permissionDeniedError), + mockIptable.EXPECT().Insert(table, chain, 2, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(timeoutError), + mockIptable.EXPECT().Insert(table, chain, 2, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.Insert(table, chain, 2, "-s", subnet1, "-d", address2, "-j", "ACCEPT") + assert.NoError(t, err) +} + +func TestAppend(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().Append(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(timeoutError), + mockIptable.EXPECT().Append(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(permissionDeniedError), + mockIptable.EXPECT().Append(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(timeoutError), + mockIptable.EXPECT().Append(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.Append(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT") + assert.NoError(t, err) +} + +func TestAppendUnique(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().AppendUnique(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(timeoutError), + mockIptable.EXPECT().AppendUnique(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.AppendUnique(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT") + assert.NoError(t, err) +} + +func TestDelete(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().Delete(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(permissionDeniedError), + mockIptable.EXPECT().Delete(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT").Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.Delete(table, chain, "-s", subnet1, "-d", address2, "-j", "ACCEPT") + assert.NoError(t, err) +} + +func TestList(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + expected := []string{ + "-N " + chain, + "-A " + chain + " -s " + address1 + " -d " + subnet1 + " -j ACCEPT", + "-A " + chain + " -s " + address2 + " -d " + subnet1 + " -j ACCEPT", + } + + gomock.InOrder( + mockIptable.EXPECT().List(table, chain).Return(nil, timeoutError), + mockIptable.EXPECT().List(table, chain).Return(nil, timeoutError), + mockIptable.EXPECT().List(table, chain).Return(expected, nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + result, _ := ipt.List(table, chain) + assert.Equal(t, result, expected) +} + +func TestNewChain(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().NewChain(table, chain).Return(permissionDeniedError), + mockIptable.EXPECT().NewChain(table, chain).Return(timeoutError), + mockIptable.EXPECT().NewChain(table, chain).Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.NewChain(table, chain) + assert.NoError(t, err) +} + +func TestClearChain(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().ClearChain(table, chain).Return(permissionDeniedError), + mockIptable.EXPECT().ClearChain(table, chain).Return(timeoutError), + mockIptable.EXPECT().ClearChain(table, chain).Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.ClearChain(table, chain) + assert.NoError(t, err) +} + +func TestDeleteChain(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().DeleteChain(table, chain).Return(permissionDeniedError), + mockIptable.EXPECT().DeleteChain(table, chain).Return(timeoutError), + mockIptable.EXPECT().DeleteChain(table, chain).Return(permissionDeniedError), + mockIptable.EXPECT().DeleteChain(table, chain).Return(nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + err := ipt.DeleteChain(table, chain) + assert.NoError(t, err) +} + +func TestListChains(t *testing.T) { + ctrl, table, _, expBackoff := setupIPTTest(t) + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + expected := []string{ + "filter", + "input", + } + + gomock.InOrder( + mockIptable.EXPECT().ListChains(table).Return(nil, timeoutError), + mockIptable.EXPECT().ListChains(table).Return(expected, nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + result, _ := ipt.ListChains(table) + assert.Equal(t, expected, result) +} + +func TestChainExists(t *testing.T) { + ctrl, table, chain, expBackoff := setupIPTTest(t) + + mockIptable := mock_iptables.NewMockIPTablesIface(ctrl) + + gomock.InOrder( + mockIptable.EXPECT().ChainExists(table, chain).Return(false, timeoutError), + mockIptable.EXPECT().ChainExists(table, chain).Return(false, timeoutError), + mockIptable.EXPECT().ChainExists(table, chain).Return(true, nil), + ) + + ipt := &ipTables{ + ipt: mockIptable, + backoff: expBackoff, + } + + exists, _ := ipt.ChainExists(table, chain) + assert.Equal(t, exists, true) +} diff --git a/pkg/networkutils/network.go b/pkg/networkutils/network.go index c7a7f0a110..6ccf1198c9 100644 --- a/pkg/networkutils/network.go +++ b/pkg/networkutils/network.go @@ -204,7 +204,7 @@ func New() NetworkAPIs { netLink: netlinkwrapper.NewNetLink(), ns: nswrapper.NewNS(), newIptables: func(IPProtocol iptables.Protocol) (iptableswrapper.IPTablesIface, error) { - ipt, err := iptables.NewWithProtocol(IPProtocol) + ipt, err := NewIPTables(IPProtocol) return ipt, err }, }