diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index a874bb8a58c..50694cc4658 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -177,7 +177,7 @@ func newInternal(exec utilexec.Interface, dbus utildbus.Interface, protocol Prot hasListener: false, hasRandomFully: version.AtLeast(RandomFullyMinVersion), waitFlag: getIPTablesWaitFlag(version), - restoreWaitFlag: getIPTablesRestoreWaitFlag(version), + restoreWaitFlag: getIPTablesRestoreWaitFlag(version, exec, protocol), lockfilePath: lockfilePath, } return runner @@ -578,12 +578,46 @@ func getIPTablesWaitFlag(version *utilversion.Version) []string { } // Checks if iptables-restore has a "wait" flag -func getIPTablesRestoreWaitFlag(version *utilversion.Version) []string { +func getIPTablesRestoreWaitFlag(version *utilversion.Version, exec utilexec.Interface, protocol Protocol) []string { if version.AtLeast(WaitRestoreMinVersion) { return []string{WaitString, WaitSecondsValue} - } else { + } + + // Older versions may have backported features; if iptables-restore supports + // --version, assume it also supports --wait + vstring, err := getIPTablesRestoreVersionString(exec, protocol) + if err != nil || vstring == "" { + klog.V(3).Infof("couldn't get iptables-restore version; assuming it doesn't support --wait") return nil } + if _, err := utilversion.ParseGeneric(vstring); err != nil { + klog.V(3).Infof("couldn't parse iptables-restore version; assuming it doesn't support --wait") + return nil + } + return []string{WaitString} +} + +// getIPTablesRestoreVersionString runs "iptables-restore --version" to get the version string +// in the form "X.X.X" +func getIPTablesRestoreVersionString(exec utilexec.Interface, protocol Protocol) (string, error) { + // this doesn't access mutable state so we don't need to use the interface / runner + + // iptables-restore hasn't always had --version, and worse complains + // about unrecognized commands but doesn't exit when it gets them. + // Work around that by setting stdin to nothing so it exits immediately. + iptablesRestoreCmd := iptablesRestoreCommand(protocol) + cmd := exec.Command(iptablesRestoreCmd, "--version") + cmd.SetStdin(bytes.NewReader([]byte{})) + bytes, err := cmd.CombinedOutput() + if err != nil { + return "", err + } + versionMatcher := regexp.MustCompile("v([0-9]+(\\.[0-9]+)+)") + match := versionMatcher.FindStringSubmatch(string(bytes)) + if match == nil { + return "", fmt.Errorf("no iptables version found in string: %s", bytes) + } + return match[1], nil } // goroutine to listen for D-Bus signals diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index f263c1d2eef..fb34f017f02 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -45,19 +45,23 @@ func protocolStr(protocol Protocol) string { } func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { - version := " v1.9.22" + version := " v1.4.22" iptablesCmd := iptablesCommand(protocol) + iptablesRestoreCmd := iptablesRestoreCommand(protocol) protoStr := protocolStr(protocol) fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version response (for runner instantiation) func() ([]byte, error) { return []byte(iptablesCmd + version), nil }, + // iptables-restore version response (for runner instantiation) + func() ([]byte, error) { return []byte(iptablesRestoreCmd + version), nil }, }, } fexec := fakeexec.FakeExec{ CommandScript: []fakeexec.FakeCommandAction{ func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), protocol) @@ -67,6 +71,11 @@ func testIPTablesVersionCmds(t *testing.T, protocol Protocol) { 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]) } + + // 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]) + } } func TestIPTablesVersionCmdsIPv4(t *testing.T) { @@ -486,11 +495,13 @@ func TestGetIPTablesHasCheckCommand(t *testing.T) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ func() ([]byte, error) { return []byte(testCase.Version), nil }, + func() ([]byte, error) { return []byte(testCase.Version), nil }, }, } fexec := fakeexec.FakeExec{ CommandScript: []fakeexec.FakeCommandAction{ func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } ipt := New(&fexec, nil, ProtocolIpv4) @@ -639,6 +650,8 @@ func TestWaitFlagUnavailable(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.19"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte{}, nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -647,6 +660,8 @@ func TestWaitFlagUnavailable(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ // iptables version check func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + // iptables-restore version check + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } @@ -656,10 +671,10 @@ func TestWaitFlagUnavailable(t *testing.T) { if err != nil { t.Errorf("expected success, got %v", err) } - if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if sets.NewString(fcmd.CombinedOutputLog[1]...).Has(WaitString) { + if sets.NewString(fcmd.CombinedOutputLog[2]...).Has(WaitString) { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -669,6 +684,8 @@ func TestWaitFlagOld(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.20"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte{}, nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -677,6 +694,7 @@ func TestWaitFlagOld(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -685,14 +703,14 @@ func TestWaitFlagOld(t *testing.T) { if err != nil { t.Errorf("expected success, got %v", err) } - if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", WaitString) { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) + if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString) { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } - if sets.NewString(fcmd.CombinedOutputLog[1]...).Has(WaitSecondsValue) { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) + if sets.NewString(fcmd.CombinedOutputLog[2]...).Has(WaitSecondsValue) { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -701,6 +719,8 @@ func TestWaitFlagNew(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte{}, nil }, // Success. func() ([]byte, error) { return []byte{}, nil }, }, @@ -709,6 +729,7 @@ func TestWaitFlagNew(t *testing.T) { CommandScript: []fakeexec.FakeCommandAction{ func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := New(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4) @@ -717,11 +738,11 @@ func TestWaitFlagNew(t *testing.T) { if err != nil { t.Errorf("expected success, got %v", err) } - if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } - if !sets.NewString(fcmd.CombinedOutputLog[1]...).HasAll("iptables", WaitString, WaitSecondsValue) { - t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[1]) + if !sets.NewString(fcmd.CombinedOutputLog[2]...).HasAll("iptables", WaitString, WaitSecondsValue) { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } } @@ -736,7 +757,7 @@ func TestReload(t *testing.T) { fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check - func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + func() ([]byte, error) { return []byte("iptables v1.6.4"), nil }, // first reload // EnsureChain @@ -1096,6 +1117,8 @@ func TestRestoreAllWaitOldIptablesRestore(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return []byte{}, nil }, func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, }, @@ -1105,6 +1128,7 @@ func TestRestoreAllWaitOldIptablesRestore(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } runner := newInternal(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4, TestLockfilePath) @@ -1116,16 +1140,16 @@ func TestRestoreAllWaitOldIptablesRestore(t *testing.T) { t.Fatalf("expected success, got %v", err) } - commandSet := sets.NewString(fcmd.CombinedOutputLog[1]...) + commandSet := sets.NewString(fcmd.CombinedOutputLog[2]...) if !commandSet.HasAll("iptables-restore", "--counters", "--noflush") { t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) } - if commandSet.HasAll(WaitString, WaitSecondsValue) { + if commandSet.HasAll(WaitString) { t.Errorf("wrong CombinedOutput() log (unexpected %s option), got %s", WaitString, fcmd.CombinedOutputLog[1]) } - if fcmd.CombinedOutputCalls != 2 { - t.Errorf("expected 2 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) } // Failure. @@ -1143,11 +1167,14 @@ func TestRestoreAllGrabNewLock(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte{}, nil }, }, } fexec := fakeexec.FakeExec{ CommandScript: []fakeexec.FakeCommandAction{ func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } @@ -1183,11 +1210,14 @@ func TestRestoreAllGrabOldLock(t *testing.T) { CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte{}, nil }, }, } fexec := fakeexec.FakeExec{ CommandScript: []fakeexec.FakeCommandAction{ func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, } @@ -1210,3 +1240,52 @@ func TestRestoreAllGrabOldLock(t *testing.T) { t.Errorf("expected timeout error, got %v", err) } } + +// TestRestoreAllWaitBackportedIptablesRestore tests that the "wait" flag is passed +// to a seemingly-old-but-actually-new iptables-restore +func TestRestoreAllWaitBackportedIptablesRestore(t *testing.T) { + fcmd := fakeexec.FakeCmd{ + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + // iptables version check + func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + // iptables-restore version check + func() ([]byte, error) { return []byte("iptables v1.4.22"), nil }, + func() ([]byte, error) { return []byte{}, nil }, + func() ([]byte, error) { return nil, &fakeexec.FakeExitError{Status: 1} }, + }, + } + fexec := fakeexec.FakeExec{ + CommandScript: []fakeexec.FakeCommandAction{ + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, + }, + } + runner := newInternal(&fexec, dbus.NewFake(nil, nil), ProtocolIpv4, TestLockfilePath) + defer os.Remove(TestLockfilePath) + defer runner.Destroy() + + err := runner.RestoreAll([]byte{}, NoFlushTables, RestoreCounters) + if err != nil { + t.Fatalf("expected success, got %v", err) + } + + commandSet := sets.NewString(fcmd.CombinedOutputLog[2]...) + if !commandSet.HasAll("iptables-restore", "--counters", "--noflush") { + t.Errorf("wrong CombinedOutput() log, got %s", fcmd.CombinedOutputLog[2]) + } + if !commandSet.HasAll(WaitString) { + t.Errorf("wrong CombinedOutput() log (expected %s option), got %s", WaitString, fcmd.CombinedOutputLog[1]) + } + + if fcmd.CombinedOutputCalls != 3 { + t.Errorf("expected 3 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls) + } + + // Failure. + err = runner.Restore(TableNAT, []byte{}, FlushTables, RestoreCounters) + if err == nil { + t.Errorf("expected failure") + } +}