Skip to content

Commit

Permalink
Merge pull request #82650 from danwinship/automated-cherry-pick-of-#8…
Browse files Browse the repository at this point in the history 
…2602-upstream-release-1.16

Automated cherry pick of #82602: Fix iptables version detection code to handle RHEL 7
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Sep 13, 2019
2 parents 01a5d09 + 6ac83fc commit 3151d32
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 22 deletions.
40 changes: 37 additions & 3 deletions pkg/util/iptables/iptables.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
117 changes: 98 additions & 19 deletions pkg/util/iptables/iptables_test.go
View file
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 },
},
Expand All @@ -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...) },
},
}
Expand All @@ -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])
}
}
Expand All @@ -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 },
},
Expand All @@ -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)
Expand All @@ -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])
}
}

Expand All @@ -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 },
},
Expand All @@ -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)
Expand All @@ -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])
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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} },
},
Expand All @@ -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)
Expand All @@ -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.
Expand All @@ -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...) },
},
}

Expand Down Expand Up @@ -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...) },
},
}

Expand All @@ -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")
}
}

0 comments on commit 3151d32

Please sign in to comment.