Skip to content

Bugfixes: iptables ipset #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 25, 2019
Merged

Bugfixes: iptables ipset #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
935d947
fix bug in regexp for iptables-save
Mar 25, 2019
ca1a54c
fix ipset listing set entries with different version
Mar 25, 2019
f08567a
fix iptables debug messages and tests
Mar 25, 2019
81096ba
fix unit tests
Mar 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions pkg/nettools/ipset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import (
log "github.com/sirupsen/logrus"
)

const (
// IPSetListWithAwk is a string to execute an ipset list command and filter out results with awk
IPSetListWithAwk = "ipset list %s | awk " + `'$0 ~ "^Members:$" {found=1; ln=NR}; NR>ln && found == 1 {print $1}'`
)

/*
IPSetHelper provides methods to manage ipset sets.

Expand Down Expand Up @@ -110,7 +115,7 @@ func (h *execIPSetHelper) EnsureSetHasOnly(name string, ips []net.IP) error {
ip := iip.(net.IP)
log.Debugf("Adding IP %s to ipset %s", ip.String(), name)
if err := h.addIPToSet(name, ip); err != nil {
log.Debugf("Error adding entry %v to ipset %s", ip, name)
log.Errorf("Error adding entry %v to ipset %s", ip, name)
return err
}
}
Expand All @@ -127,10 +132,10 @@ func (h *execIPSetHelper) EnsureSetHasOnly(name string, ips []net.IP) error {
}

func (h *execIPSetHelper) GetIPs(name string) ([]net.IP, error) {
// # ipset list myset | tail -n +9 | cut -f1 -d" "
// # ipset list myset | awk '$0 ~ "^Members:$" {found=1; ln=NR}; NR>ln && found == 1 {print $1}'
// 127.0.0.1
// 127.0.0.2
cmd := fmt.Sprintf("ipset list %s | tail -n +9 | cut -f1 -d' '", name)
cmd := fmt.Sprintf(IPSetListWithAwk, name)
res := h.exec.RunCommand("sh", "-c", cmd)
if res.Err != nil || res.ExitCode != 0 {
log.Debugf("Problem listing ipset %s - probably it's OK and it just doesn't exist: "+
Expand All @@ -151,7 +156,7 @@ func (h *execIPSetHelper) GetIPs(name string) ([]net.IP, error) {
func (h *execIPSetHelper) addIPToSet(name string, ip net.IP) error {
res := h.exec.RunCommand("ipset", "add", name, ip.String())
if res.Err != nil || res.ExitCode != 0 {
log.Debugf("Error adding IP %s to ipset %s: %v, stdErr: %s",
log.Errorf("Error adding IP %s to ipset %s: %v, stdErr: %s",
ip.String(), name, res.Err, res.StdErr)
return res.Err
}
Expand Down
16 changes: 9 additions & 7 deletions pkg/nettools/ipset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ limitations under the License. */
package nettools_test

import (
"fmt"
"net"
"os/exec"
"testing"

"github.com/DevFactory/go-tools/pkg/linux/command"
cmdmock "github.com/DevFactory/go-tools/pkg/linux/command/mock"
"github.com/DevFactory/go-tools/pkg/nettools"
nt "github.com/DevFactory/go-tools/pkg/nettools"
netth "github.com/DevFactory/go-tools/pkg/nettools/testhelpers"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -113,7 +115,7 @@ func Test_execIPSetHelper_GetIPs(t *testing.T) {
expected: []net.IP{},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: netth.ExecResultOKNoOutput(),
},
},
Expand All @@ -127,7 +129,7 @@ func Test_execIPSetHelper_GetIPs(t *testing.T) {
expected: []net.IP{},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: execResultIpsetNotFound(),
},
},
Expand All @@ -139,7 +141,7 @@ func Test_execIPSetHelper_GetIPs(t *testing.T) {
expected: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("127.0.0.2")},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: execResultIpsetIPs(),
},
},
Expand Down Expand Up @@ -172,7 +174,7 @@ func Test_execIPSetHelper_EnsureSetHasOnly(t *testing.T) {
addresses: []net.IP{},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: netth.ExecResultOKNoOutput(),
},
},
Expand All @@ -184,7 +186,7 @@ func Test_execIPSetHelper_EnsureSetHasOnly(t *testing.T) {
addresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("127.0.0.2")},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: netth.ExecResultOKNoOutput(),
},
{
Expand All @@ -204,7 +206,7 @@ func Test_execIPSetHelper_EnsureSetHasOnly(t *testing.T) {
addresses: []net.IP{},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: execResultIpsetIPs(),
},
{
Expand All @@ -224,7 +226,7 @@ func Test_execIPSetHelper_EnsureSetHasOnly(t *testing.T) {
addresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("127.0.0.3")},
mockInfo: []*cmdmock.ExecInfo{
{
Expected: "sh -c ipset list 12341234abc | tail -n +9 | cut -f1 -d' '",
Expected: fmt.Sprintf("sh -c %s", fmt.Sprintf(nettools.IPSetListWithAwk, "12341234abc")),
Returned: execResultIpsetIPs(),
},
{
Expand Down
14 changes: 7 additions & 7 deletions pkg/nettools/iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const (
iptablesRetriesDelayMSec = 100
// this uses awk to list the content of a single chain in a table using iptables-save command
awkIptablesSaveMagicFilter = "iptables-save | awk -v table=%s -v chain=%s " +
`'$0 ~ "^*"table"$" {in_table=1};$1 ~ "^COMMIT$" {in_table=0};in_table == 1 && $2 ~ "^"chain"$" {print $0}'`
`'$0 ~ "^\*"table"$" {in_table=1};$1 ~ "^COMMIT$" {in_table=0};in_table == 1 && $2 ~ "^"chain"$" {print $0}'`
)

// IPTablesRuleArgs provides arguments for an iptables rule
Expand Down Expand Up @@ -144,8 +144,8 @@ func (h *execIPTablesHelper) EnsureExistsOnlyAppend(args IPTablesRuleArgs) error
selector, action := rule.GetSelectorAndAction()
err = h.runChangingRule(rule.Table, rule.ChainName, "-D", selector, rule.Comment, action, nil)
if err != nil {
log.Debug("Error deleting rule by comment in table %s chain %s; exact info above; error: %v",
args.Table, args.ChainName, "-D", selector, action)
log.Debugf("Error deleting rule by comment in table %s chain %s; exact info above; error: %v",
args.Table, args.ChainName, action)
return err
}
}
Expand Down Expand Up @@ -176,7 +176,7 @@ func (h *execIPTablesHelper) DeleteByComment(table, chain, comment string) error
selector, action := rule.GetSelectorAndAction()
err = h.runChangingRule(rule.Table, rule.ChainName, "-D", selector, rule.Comment, action, nil)
if err != nil {
log.Debug("Error deleting rule by comment in table %s chain %s; exact info above; error: %v",
log.Debugf("Error deleting rule by comment in table %s chain %s; exact info above; error: %v",
table, chain, err)
return err
}
Expand Down Expand Up @@ -302,8 +302,8 @@ func (h *execIPTablesHelper) listRules(tableName, chainName, regexpFilter string
res := h.exec.RunCommandWithRetriesAndDelay(iptablesRetries, iptablesRetriesDelayMSec, []int{0},
"sh", "-c", shCommand)
if res.Err != nil || res.StdErr != "" {
log.Debug("Error running iptables-save with awk filter for table %s and chain %s: %v",
tableName, chainName, res.Err)
log.Errorf("Error running iptables-save with awk filter for table %s and chain %s: %v - %v",
tableName, chainName, res.Err, res.StdErr)
if res.Err != nil {
return nil, res.Err
}
Expand Down Expand Up @@ -385,7 +385,7 @@ func (h *execIPTablesHelper) loadRulesWithComment(tableName, chainName, comment
for i, entry := range entries {
rule, err := h.parseIPTablesSaveEntry(tableName, chainName, entry)
if err != nil {
log.Debug("Can't parse rules loaded from table %s and chain %s", tableName, chainName)
log.Debugf("Can't parse rules loaded from table %s and chain %s", tableName, chainName)
}
result[i] = rule
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/nettools/iptables_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

const (
awkIPTablesForNatTest = "sh -c iptables-save | awk -v table=nat -v chain=test " +
`'$0 ~ "^*"table"$" {in_table=1};$1 ~ "^COMMIT$" {in_table=0};in_table == 1 && $2 ~ "^"chain"$" {print $0}'`
`'$0 ~ "^\*"table"$" {in_table=1};$1 ~ "^COMMIT$" {in_table=0};in_table == 1 && $2 ~ "^"chain"$" {print $0}'`
)

func Test_execIPTablesHelper_EnsureChainExists(t *testing.T) {
Expand Down