Skip to content

Commit

Permalink
Merge pull request #108 from Ramkumar-K/master
Browse files Browse the repository at this point in the history 
Fix compilation errors and test and add random-fully to the masquerade rule
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 17, 2023
2 parents ce98851 + 8f9743a commit 6f202f4
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 16 deletions.
8 changes: 5 additions & 3 deletions cmd/ip-masq-agent/ip-masq-agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ import (

utilyaml "k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/component-base/logs"
"k8s.io/component-base/version/verflag"
"k8s.io/ip-masq-agent/cmd/ip-masq-agent/testing/fakefs"
"k8s.io/ip-masq-agent/pkg/version"
utiliptables "k8s.io/kubernetes/pkg/util/iptables"
"k8s.io/component-base/version/verflag"
utilexec "k8s.io/utils/exec"

"github.com/golang/glog"
Expand Down Expand Up @@ -351,8 +351,10 @@ func (m *MasqDaemon) syncMasqRulesIPv6() error {
// Feel free to dig around in iptables and see if you can figure out exactly why; I haven't had time to fully trace how it parses and handle subcommands.
// If you want to investigate, get the source via `git clone git://git.netfilter.org/iptables.git`, `git checkout v1.4.21` (the version I've seen this issue on,
// though it may also happen on others), and start with `git grep XT_EXTENSION_MAXNAMELEN`.
const postRoutingMasqChainCommentFormat = "\"ip-masq-agent: ensure nat POSTROUTING directs all non-LOCAL destination traffic to our custom %s chain\""

func postroutingJumpComment() string {
return fmt.Sprintf("ip-masq-agent: ensure nat POSTROUTING directs all non-LOCAL destination traffic to our custom %s chain", masqChain)
return fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain)
}

func (m *MasqDaemon) ensurePostroutingJump() error {
Expand Down Expand Up @@ -382,7 +384,7 @@ func writeNonMasqRule(lines *bytes.Buffer, cidr string) {
const masqRuleComment = `-m comment --comment "ip-masq-agent: outbound traffic is subject to MASQUERADE (must be last in chain)"`

func writeMasqRule(lines *bytes.Buffer) {
writeRule(lines, utiliptables.Append, masqChain, masqRuleComment, "-j", "MASQUERADE")
writeRule(lines, utiliptables.Append, masqChain, masqRuleComment, "-j", "MASQUERADE", "--random-fully")
}

// Similar syntax to utiliptables.Interface.EnsureRule, except you don't pass a table
Expand Down
77 changes: 64 additions & 13 deletions cmd/ip-masq-agent/ip-masq-agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,40 @@ import (
// turn off glog logging during tests to avoid clutter in output
func TestMain(m *testing.M) {
flag.Set("logtostderr", "false")
flag.Set("masq-chain", "IP-MASQ-AGENT")
ec := m.Run()
os.Exit(ec)
}

// returns a MasqDaemon with empty config values and a fake iptables interface
func NewFakeMasqDaemon() *MasqDaemon {
masqChain = "IP-MASQ-AGENT"
iptables := iptest.NewFake()
iptables.Dump = &iptest.IPTablesDump{
Tables: []iptest.Table{
{
Name: utiliptables.TableNAT,
Chains: []iptest.Chain{
{Name: utiliptables.ChainPostrouting},
},
},
},
}
ip6tables := iptest.NewIPv6Fake()
ip6tables.Dump = &iptest.IPTablesDump{
Tables: []iptest.Table{
{
Name: utiliptables.TableNAT,
Chains: []iptest.Chain{
{Name: utiliptables.ChainPostrouting},
},
},
},
}
return &MasqDaemon{
config: &MasqConfig{},
iptables: iptest.NewFake(),
ip6tables: iptest.NewFake(),
iptables: iptables,
ip6tables: ip6tables,
}
}

Expand Down Expand Up @@ -242,6 +266,7 @@ func TestSyncConfig(t *testing.T) {

// tests MasqDaemon.syncMasqRules
func TestSyncMasqRules(t *testing.T) {
masqChain = utiliptables.Chain("IP-MASQ-AGENT")
var syncMasqRulesTests = []struct {
desc string // human readable description of the test
cfg *MasqConfig // Masq configuration to use
Expand All @@ -252,30 +277,39 @@ func TestSyncMasqRules(t *testing.T) {
desc: "empty config",
cfg: &MasqConfig{},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
{
desc: "default config masquerading reserved ranges",
cfg: NewMasqConfigNoReservedRanges(),
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 10.0.0.0/8 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 172.16.0.0/12 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 192.168.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
{
desc: "default config not masquerading reserved ranges",
cfg: NewMasqConfigWithReservedRanges(),
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 10.0.0.0/8 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 172.16.0.0/12 -j RETURN
Expand All @@ -288,7 +322,7 @@ COMMIT
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 198.51.100.0/24 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 203.0.113.0/24 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 240.0.0.0/4 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -301,10 +335,13 @@ COMMIT
},
},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 169.254.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d 10.244.0.0/16 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -319,15 +356,18 @@ COMMIT
if !ok {
t.Errorf("MasqDaemon wasn't using the expected iptables mock")
}
if string(fipt.Lines) != tt.want {
t.Errorf("syncMasqRules wrote %q, want %q", string(fipt.Lines), tt.want)
buf := bytes.NewBuffer(nil)
fipt.SaveInto("nat", buf)
if string(buf.Bytes()) != string(tt.want) {
t.Errorf("syncMasqRules wrote %q, want %q", string(buf.Bytes()), tt.want)
}
})
}
}

// tests MasqDaemon.syncMasqRulesIPv6
func TestSyncMasqRulesIPv6(t *testing.T) {
masqChain = utiliptables.Chain("IP-MASQ-AGENT")
var syncMasqRulesIPv6Tests = []struct {
desc string // human readable description of the test
cfg *MasqConfig // Masq configuration to use
Expand All @@ -338,9 +378,12 @@ func TestSyncMasqRulesIPv6(t *testing.T) {
desc: "empty config",
cfg: &MasqConfig{},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d fe80::/10 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -353,19 +396,25 @@ COMMIT
},
},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d fe80::/10 -j RETURN
-A ` + string(masqChain) + ` ` + nonMasqRuleComment + ` -d fc00::/7 -j RETURN
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
{
desc: "config has masqLinkLocalIPv6: true",
cfg: &MasqConfig{MasqLinkLocalIPv6: true},
want: `*nat
:` + string(utiliptables.ChainPostrouting) + ` - [0:0]
:` + string(masqChain) + ` - [0:0]
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE
-A ` + string(utiliptables.ChainPostrouting) + ` -m comment --comment ` +
fmt.Sprintf(postRoutingMasqChainCommentFormat, masqChain) + ` -m addrtype ! --dst-type LOCAL -j ` + string(masqChain) + `
-A ` + string(masqChain) + ` ` + masqRuleComment + ` -j MASQUERADE --random-fully
COMMIT
`,
},
Expand All @@ -381,8 +430,10 @@ COMMIT
if !ok {
t.Errorf("MasqDaemon wasn't using the expected iptables mock")
}
if string(fipt6.Lines) != tt.want {
t.Errorf("syncMasqRulesIPv6 wrote %q, want %q", string(fipt6.Lines), tt.want)
buf := bytes.NewBuffer(nil)
fipt6.SaveInto("nat", buf)
if string(buf.Bytes()) != tt.want {
t.Errorf("syncMasqRulesIPv6 wrote %q, want %q", string(buf.Bytes()), tt.want)
}
})
}
Expand Down

0 comments on commit 6f202f4

Please sign in to comment.