From ef934a2c5ecd02d5ab9eba4f2e1992a1a36ce37d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 9 Apr 2020 16:26:29 -0700 Subject: [PATCH] Add Protocol() method to iptables Enables simpler printing of which IP family the iptables interface is managing. --- .../network/hostport/fake_iptables.go | 10 ++- .../network/hostport/hostport_manager_test.go | 2 +- .../network/hostport/hostport_syncer_test.go | 2 +- pkg/proxy/iptables/proxier.go | 15 ++--- pkg/proxy/ipvs/proxier.go | 9 +-- pkg/util/iptables/iptables.go | 14 ++-- pkg/util/iptables/iptables_test.go | 64 ++++++++----------- pkg/util/iptables/testing/fake.go | 13 ++-- 8 files changed, 59 insertions(+), 70 deletions(-) diff --git a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go index 0c96e7a78b8..1f15170bdcb 100644 --- a/pkg/kubelet/dockershim/network/hostport/fake_iptables.go +++ b/pkg/kubelet/dockershim/network/hostport/fake_iptables.go @@ -40,7 +40,7 @@ type fakeTable struct { type fakeIPTables struct { tables map[string]*fakeTable builtinChains map[string]sets.String - ipv6 bool + protocol utiliptables.Protocol } func NewFakeIPTables() *fakeIPTables { @@ -51,7 +51,7 @@ func NewFakeIPTables() *fakeIPTables { string(utiliptables.TableNAT): sets.NewString("PREROUTING", "INPUT", "OUTPUT", "POSTROUTING"), string(utiliptables.TableMangle): sets.NewString("PREROUTING", "INPUT", "FORWARD", "OUTPUT", "POSTROUTING"), }, - ipv6: false, + protocol: utiliptables.ProtocolIpv4, } } @@ -224,7 +224,11 @@ func (f *fakeIPTables) DeleteRule(tableName utiliptables.Table, chainName utilip } func (f *fakeIPTables) IsIpv6() bool { - return f.ipv6 + return f.protocol == utiliptables.ProtocolIpv6 +} + +func (f *fakeIPTables) Protocol() utiliptables.Protocol { + return f.protocol } func saveChain(chain *fakeChain, data *bytes.Buffer) { diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go b/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go index b0bafe322c8..cb18e1d9180 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_manager_test.go @@ -387,7 +387,7 @@ func TestGetHostportChain(t *testing.T) { func TestHostportManagerIPv6(t *testing.T) { iptables := NewFakeIPTables() - iptables.ipv6 = true + iptables.protocol = utiliptables.ProtocolIpv6 portOpener := NewFakeSocketManager() manager := &hostportManager{ hostPortMap: make(map[hostport]closeable), diff --git a/pkg/kubelet/dockershim/network/hostport/hostport_syncer_test.go b/pkg/kubelet/dockershim/network/hostport/hostport_syncer_test.go index 6af21e806b2..3179b3782e8 100644 --- a/pkg/kubelet/dockershim/network/hostport/hostport_syncer_test.go +++ b/pkg/kubelet/dockershim/network/hostport/hostport_syncer_test.go @@ -244,7 +244,7 @@ func matchRule(chain *fakeChain, match string) bool { func TestOpenPodHostportsIPv6(t *testing.T) { fakeIPTables := NewFakeIPTables() - fakeIPTables.ipv6 = true + fakeIPTables.protocol = utiliptables.ProtocolIpv6 fakeOpener := NewFakeSocketManager() h := &hostportSyncer{ diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 1aafe9e1ce7..67c22a4c3f2 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -283,7 +283,7 @@ func NewProxier(ipt utiliptables.Interface, // Generate the masquerade mark to use for SNAT rules. masqueradeValue := 1 << uint(masqueradeBit) masqueradeMark := fmt.Sprintf("%#08x/%#08x", masqueradeValue, masqueradeValue) - klog.V(2).Infof("iptables(%s) masquerade mark: %s", ipVersion(ipt.IsIpv6()), masqueradeMark) + klog.V(2).Infof("iptables(%s) masquerade mark: %s", ipt.Protocol(), masqueradeMark) endpointSlicesEnabled := utilfeature.DefaultFeatureGate.Enabled(features.EndpointSliceProxying) @@ -321,7 +321,7 @@ func NewProxier(ipt utiliptables.Interface, burstSyncs := 2 klog.V(2).Infof("iptables(%s) sync params: minSyncPeriod=%v, syncPeriod=%v, burstSyncs=%d", - ipVersion(ipt.IsIpv6()), minSyncPeriod, syncPeriod, burstSyncs) + ipt.Protocol(), minSyncPeriod, syncPeriod, burstSyncs) // We pass syncPeriod to ipt.Monitor, which will call us only if it needs to. // We need to pass *some* maxInterval to NewBoundedFrequencyRunner anyway though. // time.Hour is arbitrary. @@ -332,21 +332,14 @@ func NewProxier(ipt utiliptables.Interface, proxier.syncProxyRules, syncPeriod, wait.NeverStop) if ipt.HasRandomFully() { - klog.V(2).Infof("iptables(%s) supports --random-fully", ipVersion(ipt.IsIpv6())) + klog.V(2).Infof("iptables(%s) supports --random-fully", ipt.Protocol()) } else { - klog.V(2).Infof("iptables(%s) does not support --random-fully", ipVersion(ipt.IsIpv6())) + klog.V(2).Infof("iptables(%s) does not support --random-fully", ipt.Protocol()) } return proxier, nil } -func ipVersion(isIPv6 bool) string { - if isIPv6 { - return "ipv6" - } - return "ipv4" -} - // NewDualStackProxier creates a MetaProxier instance, with IPv4 and IPv6 proxies. func NewDualStackProxier( ipt [2]utiliptables.Interface, diff --git a/pkg/proxy/ipvs/proxier.go b/pkg/proxy/ipvs/proxier.go index 1b22f8b0cda..b2b5c6bb5fb 100644 --- a/pkg/proxy/ipvs/proxier.go +++ b/pkg/proxy/ipvs/proxier.go @@ -475,19 +475,12 @@ func NewProxier(ipt utiliptables.Interface, } burstSyncs := 2 klog.V(2).Infof("ipvs(%s) sync params: minSyncPeriod=%v, syncPeriod=%v, burstSyncs=%d", - ipVersion(ipt.IsIpv6()), minSyncPeriod, syncPeriod, burstSyncs) + ipt.Protocol(), minSyncPeriod, syncPeriod, burstSyncs) proxier.syncRunner = async.NewBoundedFrequencyRunner("sync-runner", proxier.syncProxyRules, minSyncPeriod, syncPeriod, burstSyncs) proxier.gracefuldeleteManager.Run() return proxier, nil } -func ipVersion(isIPv6 bool) string { - if isIPv6 { - return "ipv6" - } - return "ipv4" -} - // NewDualStackProxier returns a new Proxier for dual-stack operation func NewDualStackProxier( ipt [2]utiliptables.Interface, diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index e1f68be5938..5ae88b9e42c 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -55,8 +55,10 @@ type Interface interface { EnsureRule(position RulePosition, table Table, chain Chain, args ...string) (bool, error) // DeleteRule checks if the specified rule is present and, if so, deletes it. DeleteRule(table Table, chain Chain, args ...string) error - // IsIpv6 returns true if this is managing ipv6 tables + // IsIpv6 returns true if this is managing ipv6 tables. IsIpv6() bool + // Protocol returns the IP family this instance is managing, + Protocol() Protocol // SaveInto calls `iptables-save` for table and stores result in a given buffer. SaveInto(table Table, buffer *bytes.Buffer) error // Restore runs `iptables-restore` passing data through []byte. @@ -87,13 +89,13 @@ type Interface interface { } // Protocol defines the ip protocol either ipv4 or ipv6 -type Protocol byte +type Protocol string const ( // ProtocolIpv4 represents ipv4 protocol in iptables - ProtocolIpv4 Protocol = iota + 1 + ProtocolIpv4 Protocol = "IPv4" // ProtocolIpv6 represents ipv6 protocol in iptables - ProtocolIpv6 + ProtocolIpv6 Protocol = "IPv6" ) // Table represents different iptable like filter,nat, mangle and raw @@ -323,6 +325,10 @@ func (runner *runner) IsIpv6() bool { return runner.protocol == ProtocolIpv6 } +func (runner *runner) Protocol() Protocol { + return runner.protocol +} + // SaveInto is part of Interface. func (runner *runner) SaveInto(table Table, buffer *bytes.Buffer) error { runner.mu.Lock() diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index 1022ba85d65..85855caced5 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -35,18 +35,10 @@ import ( const TestLockfilePath = "xtables.lock" -func protocolStr(protocol Protocol) string { - if protocol == ProtocolIpv4 { - return "IPv4" - } - return "IPv6" -} - func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { version := " v1.4.22" iptablesCmd := iptablesCommand(protocol) iptablesRestoreCmd := iptablesRestoreCommand(protocol) - protoStr := protocolStr(protocol) fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeAction{ @@ -66,12 +58,12 @@ func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { // Check that proper iptables version command was used during runner instantiation if !sets.NewString(fcmd.CombinedOutputLog[0]...).HasAll(iptablesCmd, "--version") { - t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protoStr, iptablesCmd, fcmd.CombinedOutputLog[0]) + t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protocol, iptablesCmd, fcmd.CombinedOutputLog[0]) } // Check that proper iptables restore version command was used during runner instantiation if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll(iptablesRestoreCmd, "--version") { - t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protoStr, iptablesRestoreCmd, fcmd.CombinedOutputLog[1]) + t.Errorf("%s runner instantiate: Expected cmd '%s --version', Got '%s'", protocol, iptablesRestoreCmd, fcmd.CombinedOutputLog[1]) } } @@ -84,8 +76,6 @@ func TestIPTablesVersionCmdsIPv6(t *testing.T) { } func testEnsureChain(t *testing.T, protocol Protocol) { - protoStr := protocolStr(protocol) - fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeAction{ // iptables version check @@ -110,30 +100,30 @@ func testEnsureChain(t *testing.T, protocol Protocol) { // Success. exists, err := runner.EnsureChain(TableNAT, Chain("FOOBAR")) if err != nil { - t.Errorf("%s new chain: Expected success, got %v", protoStr, err) + t.Errorf("%s new chain: Expected success, got %v", protocol, err) } if exists { - t.Errorf("%s new chain: Expected exists = false", protoStr) + t.Errorf("%s new chain: Expected exists = false", protocol) } if fcmd.CombinedOutputCalls != 2 { - t.Errorf("%s new chain: Expected 2 CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) + t.Errorf("%s new chain: Expected 2 CombinedOutput() calls, got %d", protocol, fcmd.CombinedOutputCalls) } cmd := iptablesCommand(protocol) if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll(cmd, "-t", "nat", "-N", "FOOBAR") { - t.Errorf("%s new chain: Expected cmd containing '%s -t nat -N FOOBAR', got %s", protoStr, cmd, fcmd.CombinedOutputLog[2]) + t.Errorf("%s new chain: Expected cmd containing '%s -t nat -N FOOBAR', got %s", protocol, cmd, fcmd.CombinedOutputLog[2]) } // Exists. exists, err = runner.EnsureChain(TableNAT, Chain("FOOBAR")) if err != nil { - t.Errorf("%s existing chain: Expected success, got %v", protoStr, err) + t.Errorf("%s existing chain: Expected success, got %v", protocol, err) } if !exists { - t.Errorf("%s existing chain: Expected exists = true", protoStr) + t.Errorf("%s existing chain: Expected exists = true", protocol) } // Simulate failure. _, err = runner.EnsureChain(TableNAT, Chain("FOOBAR")) if err == nil { - t.Errorf("%s: Expected failure", protoStr) + t.Errorf("%s: Expected failure", protocol) } } @@ -764,7 +754,6 @@ func testSaveInto(t *testing.T, protocol Protocol) { version := " v1.9.22" iptablesCmd := iptablesCommand(protocol) iptablesSaveCmd := iptablesSaveCommand(protocol) - protoStr := protocolStr(protocol) output := fmt.Sprintf(`# Generated by %s on Thu Jan 19 11:38:09 2017 *filter @@ -799,31 +788,31 @@ COMMIT // Success. err := runner.SaveInto(TableNAT, buffer) if err != nil { - t.Fatalf("%s: Expected success, got %v", protoStr, err) + t.Fatalf("%s: Expected success, got %v", protocol, err) } if string(buffer.Bytes()) != output { - t.Errorf("%s: Expected output '%s', got '%v'", protoStr, output, string(buffer.Bytes())) + t.Errorf("%s: Expected output '%s', got '%v'", protocol, output, string(buffer.Bytes())) } if fcmd.CombinedOutputCalls != 1 { - t.Errorf("%s: Expected 1 CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) + t.Errorf("%s: Expected 1 CombinedOutput() calls, got %d", protocol, fcmd.CombinedOutputCalls) } if fcmd.RunCalls != 1 { - t.Errorf("%s: Expected 1 Run() call, got %d", protoStr, fcmd.RunCalls) + t.Errorf("%s: Expected 1 Run() call, got %d", protocol, fcmd.RunCalls) } if !sets.NewString(fcmd.RunLog[0]...).HasAll(iptablesSaveCmd, "-t", "nat") { - t.Errorf("%s: Expected cmd containing '%s -t nat', got '%s'", protoStr, iptablesSaveCmd, fcmd.RunLog[0]) + t.Errorf("%s: Expected cmd containing '%s -t nat', got '%s'", protocol, iptablesSaveCmd, fcmd.RunLog[0]) } // Failure. buffer.Reset() err = runner.SaveInto(TableNAT, buffer) if err == nil { - t.Errorf("%s: Expected failure", protoStr) + t.Errorf("%s: Expected failure", protocol) } if string(buffer.Bytes()) != stderrOutput { - t.Errorf("%s: Expected output '%s', got '%v'", protoStr, stderrOutput, string(buffer.Bytes())) + t.Errorf("%s: Expected output '%s', got '%v'", protocol, stderrOutput, string(buffer.Bytes())) } } @@ -839,7 +828,6 @@ func testRestore(t *testing.T, protocol Protocol) { version := " v1.9.22" iptablesCmd := iptablesCommand(protocol) iptablesRestoreCmd := iptablesRestoreCommand(protocol) - protoStr := protocolStr(protocol) fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeAction{ @@ -867,55 +855,55 @@ func testRestore(t *testing.T, protocol Protocol) { // both flags true err := runner.Restore(TableNAT, []byte{}, FlushTables, RestoreCounters) if err != nil { - t.Errorf("%s flush,restore: Expected success, got %v", protoStr, err) + t.Errorf("%s flush,restore: Expected success, got %v", protocol, err) } commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT), "--counters") || commandSet.HasAny("--noflush") { - t.Errorf("%s flush, restore: Expected cmd containing '%s -T %s --counters', got '%s'", protoStr, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[1]) + t.Errorf("%s flush, restore: Expected cmd containing '%s -T %s --counters', got '%s'", protocol, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[1]) } // FlushTables, NoRestoreCounters err = runner.Restore(TableNAT, []byte{}, FlushTables, NoRestoreCounters) if err != nil { - t.Errorf("%s flush, no restore: Expected success, got %v", protoStr, err) + t.Errorf("%s flush, no restore: Expected success, got %v", protocol, err) } commandSet = sets.NewString(fcmd.CombinedOutputLog[2]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT)) || commandSet.HasAny("--noflush", "--counters") { - t.Errorf("%s flush, no restore: Expected cmd containing '--noflush' or '--counters', got '%s'", protoStr, fcmd.CombinedOutputLog[2]) + t.Errorf("%s flush, no restore: Expected cmd containing '--noflush' or '--counters', got '%s'", protocol, fcmd.CombinedOutputLog[2]) } // NoFlushTables, RestoreCounters err = runner.Restore(TableNAT, []byte{}, NoFlushTables, RestoreCounters) if err != nil { - t.Errorf("%s no flush, restore: Expected success, got %v", protoStr, err) + t.Errorf("%s no flush, restore: Expected success, got %v", protocol, err) } commandSet = sets.NewString(fcmd.CombinedOutputLog[3]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT), "--noflush", "--counters") { - t.Errorf("%s no flush, restore: Expected cmd containing '--noflush' and '--counters', got '%s'", protoStr, fcmd.CombinedOutputLog[3]) + t.Errorf("%s no flush, restore: Expected cmd containing '--noflush' and '--counters', got '%s'", protocol, fcmd.CombinedOutputLog[3]) } // NoFlushTables, NoRestoreCounters err = runner.Restore(TableNAT, []byte{}, NoFlushTables, NoRestoreCounters) if err != nil { - t.Errorf("%s no flush, no restore: Expected success, got %v", protoStr, err) + t.Errorf("%s no flush, no restore: Expected success, got %v", protocol, err) } commandSet = sets.NewString(fcmd.CombinedOutputLog[4]...) if !commandSet.HasAll(iptablesRestoreCmd, "-T", string(TableNAT), "--noflush") || commandSet.HasAny("--counters") { - t.Errorf("%s no flush, no restore: Expected cmd containing '%s -T %s --noflush', got '%s'", protoStr, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[4]) + t.Errorf("%s no flush, no restore: Expected cmd containing '%s -T %s --noflush', got '%s'", protocol, iptablesRestoreCmd, string(TableNAT), fcmd.CombinedOutputLog[4]) } if fcmd.CombinedOutputCalls != 5 { - t.Errorf("%s: Expected 5 total CombinedOutput() calls, got %d", protoStr, fcmd.CombinedOutputCalls) + t.Errorf("%s: Expected 5 total CombinedOutput() calls, got %d", protocol, fcmd.CombinedOutputCalls) } // Failure. err = runner.Restore(TableNAT, []byte{}, FlushTables, RestoreCounters) if err == nil { - t.Errorf("%s Expected a failure", protoStr) + t.Errorf("%s Expected a failure", protocol) } } diff --git a/pkg/util/iptables/testing/fake.go b/pkg/util/iptables/testing/fake.go index 199bbb36b9f..9bfdb9e9b32 100644 --- a/pkg/util/iptables/testing/fake.go +++ b/pkg/util/iptables/testing/fake.go @@ -57,17 +57,17 @@ type Rule map[string]string type FakeIPTables struct { hasRandomFully bool Lines []byte - ipv6 bool + protocol iptables.Protocol } // NewFake returns a no-op iptables.Interface func NewFake() *FakeIPTables { - return &FakeIPTables{} + return &FakeIPTables{protocol: iptables.ProtocolIpv4} } // NewIpv6Fake returns a no-op iptables.Interface with IsIPv6() == true func NewIpv6Fake() *FakeIPTables { - return &FakeIPTables{ipv6: true} + return &FakeIPTables{protocol: iptables.ProtocolIpv6} } // SetHasRandomFully is part of iptables.Interface @@ -103,7 +103,12 @@ func (*FakeIPTables) DeleteRule(table iptables.Table, chain iptables.Chain, args // IsIpv6 is part of iptables.Interface func (f *FakeIPTables) IsIpv6() bool { - return f.ipv6 + return f.protocol == iptables.ProtocolIpv6 +} + +// Protocol is part of iptables.Interface +func (f *FakeIPTables) Protocol() iptables.Protocol { + return f.protocol } // Save is part of iptables.Interface