diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index 24d2ef4ef3..761e55c94a 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -326,14 +326,15 @@ func (runner *runner) SaveInto(table Table, buffer *bytes.Buffer) error { args := []string{"-t", string(table)} klog.V(4).Infof("running %s %v", iptablesSaveCmd, args) cmd := runner.exec.Command(iptablesSaveCmd, args...) - // Since CombinedOutput() doesn't support redirecting it to a buffer, - // we need to workaround it by redirecting stdout and stderr to buffer - // and explicitly calling Run() [CombinedOutput() underneath itself - // creates a new buffer, redirects stdout and stderr to it and also - // calls Run()]. cmd.SetStdout(buffer) - cmd.SetStderr(buffer) - return cmd.Run() + stderrBuffer := bytes.NewBuffer(nil) + cmd.SetStderr(stderrBuffer) + + err := cmd.Run() + if err != nil { + stderrBuffer.WriteTo(buffer) // ignore error, since we need to return the original error + } + return err } // Restore is part of Interface. diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index e169468062..dfc5809538 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -932,6 +932,8 @@ func testSaveInto(t *testing.T, protocol Protocol) { COMMIT # Completed on Thu Jan 19 11:38:09 2017`, iptablesSaveCmd+version) + stderrOutput := "#STDERR OUTPUT" // SaveInto() should should NOT capture stderr into the buffer + fcmd := fakeexec.FakeCmd{ CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ // iptables version check @@ -940,8 +942,8 @@ COMMIT func() ([]byte, error) { return []byte(iptablesRestoreCmd + version), nil }, }, RunScript: []fakeexec.FakeRunAction{ - func() ([]byte, []byte, error) { return []byte(output), nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, []byte, error) { return []byte(output), []byte(stderrOutput), nil }, + func() ([]byte, []byte, error) { return nil, []byte(stderrOutput), &fakeexec.FakeExitError{Status: 1} }, }, } fexec := fakeexec.FakeExec{ @@ -962,8 +964,8 @@ COMMIT t.Fatalf("%s: Expected success, got %v", protoStr, err) } - if string(buffer.Bytes()[:len(output)]) != output { - t.Errorf("%s: Expected output '%s', got '%v'", protoStr, output, buffer.Bytes()) + if string(buffer.Bytes()) != output { + t.Errorf("%s: Expected output '%s', got '%v'", protoStr, output, string(buffer.Bytes())) } if fcmd.CombinedOutputCalls != 2 { @@ -982,6 +984,9 @@ COMMIT if err == nil { t.Errorf("%s: Expected failure", protoStr) } + if string(buffer.Bytes()) != stderrOutput { + t.Errorf("%s: Expected output '%s', got '%v'", protoStr, stderrOutput, string(buffer.Bytes())) + } } func TestSaveIntoIPv4(t *testing.T) { diff --git a/pkg/util/iptables/save_restore.go b/pkg/util/iptables/save_restore.go index 172f07e7fa..7ec11e4f00 100644 --- a/pkg/util/iptables/save_restore.go +++ b/pkg/util/iptables/save_restore.go @@ -61,7 +61,11 @@ func GetChainLines(table Table, save []byte) map[Chain][]byte { } else if line[0] == ':' && len(line) > 1 { // We assume that the contains space - chain lines have 3 fields, // space delimited. If there is no space, this line will panic. - chain := Chain(line[1:bytes.Index(line, spaceBytes)]) + spaceIndex := bytes.Index(line, spaceBytes) + if spaceIndex == -1 { + panic(fmt.Sprintf("Unexpected chain line in iptables-save output: %v", string(line))) + } + chain := Chain(line[1:spaceIndex]) chainsMap[chain] = line } }