Skip to content
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.

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

ipmasq: Add default nonMasq CIDRs if config is empty #11409

Merged
merged 2 commits into from May 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -295,7 +295,6 @@ data:
{{- end }}
{{- if and .Values.global.ipMasqAgent .Values.global.ipMasqAgent.enabled }}
enable-ip-masq-agent: "true"
ip-masq-agent-sync-period: {{ .Values.global.ipMasqAgent.syncPeriod | quote }}
{{- end }}

{{- if .Values.global.encryption.enabled }}
Expand Down
1 change: 0 additions & 1 deletion install/kubernetes/cilium/values.yaml
Expand Up @@ -129,7 +129,6 @@ global:
# ipMasqAgent enables and controls BPF ip-masq-agent
ipMasqAgent:
enabled: false
syncPeriod: 60s

# autoDirectNodeRoutes enables installation of PodCIDR routes between worker
# nodes if worker nodes share a common L2 network segment.
Expand Down
91 changes: 74 additions & 17 deletions pkg/ipmasq/ipmasq.go
Expand Up @@ -33,6 +33,24 @@ import (

var (
log = logging.DefaultLogger.WithField(logfields.LogSubsys, "ipmasq")

// The following reserved by RFCs IP addr ranges are used by
// https://github.com/kubernetes-sigs/ip-masq-agent
defaultNonMasqCIDRs = map[string]net.IPNet{
"10.0.0.0/8": mustParseCIDR("10.0.0.0/8"),
"172.16.0.0/12": mustParseCIDR("172.16.0.0/12"),
"192.168.0.0/16": mustParseCIDR("192.168.0.0/16"),
"100.64.0.0/10": mustParseCIDR("100.64.0.0/10"),
"192.0.0.0/24": mustParseCIDR("192.0.0.0/24"),
"192.0.2.0/24": mustParseCIDR("192.0.2.0/24"),
"192.88.99.0/24": mustParseCIDR("192.88.99.0/24"),
"198.18.0.0/15": mustParseCIDR("198.18.0.0/15"),
"198.51.100.0/24": mustParseCIDR("198.51.100.0/24"),
"203.0.113.0/24": mustParseCIDR("203.0.113.0/24"),
"240.0.0.0/4": mustParseCIDR("240.0.0.0/4"),
brb marked this conversation as resolved.
Show resolved Hide resolved
}
linkLocalCIDRStr = "169.254.0.0/16"
linkLocalCIDR = mustParseCIDR(linkLocalCIDRStr)
)

// ipnet is a wrapper type for net.IPNet to enable de-serialization of CIDRs
Expand All @@ -45,21 +63,30 @@ func (c *Ipnet) UnmarshalJSON(json []byte) error {
return fmt.Errorf("Invalid CIDR: %s", str)
}

ip, n, err := net.ParseCIDR(strings.Trim(str, `"`))
n, err := parseCIDRv4(strings.Trim(str, `"`))
if err != nil {
return fmt.Errorf("Invalid CIDR %s: %s", str, err)
}
if ip.To4() == nil {
return fmt.Errorf("Invalid CIDR %s: only IPv4 is supported", str)
return err
}

*c = Ipnet(*n)
return nil
}

func parseCIDRv4(c string) (*net.IPNet, error) {
ip, n, err := net.ParseCIDR(c)
if err != nil {
return nil, fmt.Errorf("Invalid CIDR %s: %s", c, err)
}
if ip.To4() == nil {
return nil, fmt.Errorf("Invalid CIDR %s: only IPv4 is supported", c)
}
return n, nil
}

// config represents the ip-masq-agent configuration file encoded as YAML
type config struct {
NonMasqCIDRs []Ipnet `json:"nonMasqueradeCIDRs"`
NonMasqCIDRs []Ipnet `json:"nonMasqueradeCIDRs"`
MasqLinkLocal bool `json:"masqLinkLocal"`
}

// IPMasqMap is an interface describing methods for manipulating an ipmasq map
Expand All @@ -72,6 +99,7 @@ type IPMasqMap interface {
// IPMasqAgent represents a state of the ip-masq-agent
type IPMasqAgent struct {
configPath string
masqLinkLocal bool
nonMasqCIDRsFromConfig map[string]net.IPNet
nonMasqCIDRsInMap map[string]net.IPNet
ipMasqMap IPMasqMap
Expand Down Expand Up @@ -99,10 +127,11 @@ func newIPMasqAgent(configPath string, ipMasqMap IPMasqMap) (*IPMasqAgent, error
}

a := &IPMasqAgent{
configPath: configPath,
nonMasqCIDRsInMap: map[string]net.IPNet{},
ipMasqMap: ipMasqMap,
watcher: watcher,
configPath: configPath,
nonMasqCIDRsFromConfig: map[string]net.IPNet{},
nonMasqCIDRsInMap: map[string]net.IPNet{},
ipMasqMap: ipMasqMap,
watcher: watcher,
}

return a, nil
Expand Down Expand Up @@ -155,10 +184,22 @@ func (a *IPMasqAgent) Stop() {

// Update updates the ipmasq BPF map entries with ones from the config file.
func (a *IPMasqAgent) Update() error {
if err := a.readConfig(); err != nil {
isEmpty, err := a.readConfig()
if err != nil {
return err
}

// Set default nonMasq CIDRS if user hasn't specified any
if isEmpty {
for cidrStr, cidr := range defaultNonMasqCIDRs {
a.nonMasqCIDRsFromConfig[cidrStr] = cidr
}
}

if !a.masqLinkLocal {
a.nonMasqCIDRsFromConfig[linkLocalCIDRStr] = linkLocalCIDR
}

for cidrStr, cidr := range a.nonMasqCIDRsFromConfig {
if _, ok := a.nonMasqCIDRsInMap[cidrStr]; !ok {
log.WithField(logfields.CIDR, cidrStr).Info("Adding CIDR")
Expand All @@ -180,26 +221,33 @@ func (a *IPMasqAgent) Update() error {

// readConfig reads the config file and populates IPMasqAgent.nonMasqCIDRsFromConfig
// with the CIDRs from the file.
func (a *IPMasqAgent) readConfig() error {
func (a *IPMasqAgent) readConfig() (bool, error) {
var cfg config

raw, err := ioutil.ReadFile(a.configPath)
if err != nil {
if os.IsNotExist(err) {
log.WithField(logfields.Path, a.configPath).Info("Config file not found")
a.nonMasqCIDRsFromConfig = map[string]net.IPNet{}
return nil
a.masqLinkLocal = false
return true, nil
}
return fmt.Errorf("Failed to read %s: %s", a.configPath, err)
return false, fmt.Errorf("Failed to read %s: %s", a.configPath, err)
}

if len(raw) == 0 {
a.nonMasqCIDRsFromConfig = map[string]net.IPNet{}
a.masqLinkLocal = false
return true, nil
}

jsonStr, err := yaml.ToJSON(raw)
if err != nil {
return fmt.Errorf("Failed to convert to json: %s", err)
return false, fmt.Errorf("Failed to convert to json: %s", err)
}

if err := json.Unmarshal(jsonStr, &cfg); err != nil {
return fmt.Errorf("Failed to de-serialize json: %s", err)
return false, fmt.Errorf("Failed to de-serialize json: %s", err)
}

nonMasqCIDRs := map[string]net.IPNet{}
Expand All @@ -208,8 +256,9 @@ func (a *IPMasqAgent) readConfig() error {
nonMasqCIDRs[n.String()] = n
}
a.nonMasqCIDRsFromConfig = nonMasqCIDRs
a.masqLinkLocal = cfg.MasqLinkLocal

return nil
return false, nil
}

// restore dumps the ipmasq BPF map and populates IPMasqAgent.nonMasqCIDRsInMap
Expand All @@ -228,3 +277,11 @@ func (a *IPMasqAgent) restore() error {

return nil
}

func mustParseCIDR(c string) net.IPNet {
n, err := parseCIDRv4(c)
if err != nil {
panic(err)
}
return *n
}
80 changes: 47 additions & 33 deletions pkg/ipmasq/ipmasq_test.go
Expand Up @@ -89,9 +89,9 @@ func (m *ipMasqMapMock) dumpToSet() map[string]struct{} {
}

type IPMasqTestSuite struct {
ipMasqMap *ipMasqMapMock
ipMasqAgent *IPMasqAgent
configFile *os.File
ipMasqMap *ipMasqMapMock
ipMasqAgent *IPMasqAgent
configFilePath string
}

var _ = check.Suite(&IPMasqTestSuite{})
Expand All @@ -101,50 +101,61 @@ func (i *IPMasqTestSuite) SetUpTest(c *check.C) {

configFile, err := ioutil.TempFile("", "ipmasq-test")
c.Assert(err, check.IsNil)
i.configFile = configFile
i.configFilePath = configFile.Name()

agent, err := newIPMasqAgent(configFile.Name(), i.ipMasqMap)
agent, err := newIPMasqAgent(i.configFilePath, i.ipMasqMap)
c.Assert(err, check.IsNil)
i.ipMasqAgent = agent
i.ipMasqAgent.Start()
}

func (i *IPMasqTestSuite) TearDownTest(c *check.C) {
i.ipMasqAgent.Stop()
os.Remove(i.configFile.Name())
os.Remove(i.configFilePath)
}

func (i *IPMasqTestSuite) TestUpdate(c *check.C) {
_, err := i.configFile.WriteString("nonMasqueradeCIDRs:\n- 1.1.1.1/32\n- 2.2.2.2/16")
func (i *IPMasqTestSuite) writeConfig(cfg string, c *check.C) {
err := ioutil.WriteFile(i.configFilePath, []byte(cfg), 0644)
c.Assert(err, check.IsNil)
}

func (i *IPMasqTestSuite) TestUpdate(c *check.C) {
i.writeConfig("nonMasqueradeCIDRs:\n- 1.1.1.1/32\n- 2.2.2.2/16", c)
time.Sleep(300 * time.Millisecond)

ipnets := i.ipMasqMap.dumpToSet()
c.Assert(len(ipnets), check.Equals, 2)
c.Assert(len(ipnets), check.Equals, 3)
_, ok := ipnets["1.1.1.1/32"]
c.Assert(ok, check.Equals, true)
_, ok = ipnets["2.2.0.0/16"]
c.Assert(ok, check.Equals, true)
_, ok = ipnets[linkLocalCIDRStr]
c.Assert(ok, check.Equals, true)

// Write new config
_, err = i.configFile.Seek(0, 0)
c.Assert(err, check.IsNil)
_, err = i.configFile.WriteString("nonMasqueradeCIDRs:\n- 8.8.0.0/16\n- 2.2.2.2/16")
c.Assert(err, check.IsNil)
i.writeConfig("nonMasqueradeCIDRs:\n- 8.8.0.0/16\n- 2.2.2.2/16", c)
time.Sleep(300 * time.Millisecond)

ipnets = i.ipMasqMap.dumpToSet()
c.Assert(len(ipnets), check.Equals, 2)
c.Assert(len(ipnets), check.Equals, 3)
_, ok = ipnets["8.8.0.0/16"]
c.Assert(ok, check.Equals, true)
_, ok = ipnets["2.2.0.0/16"]
c.Assert(ok, check.Equals, true)
_, ok = ipnets[linkLocalCIDRStr]
c.Assert(ok, check.Equals, true)

// Write config with no CIDRs
i.writeConfig("nonMasqueradeCIDRs:\n", c)
time.Sleep(300 * time.Millisecond)

ipnets = i.ipMasqMap.dumpToSet()
c.Assert(len(ipnets), check.Equals, 1)
_, ok = ipnets[linkLocalCIDRStr]
c.Assert(ok, check.Equals, true)

// Write new config in JSON
_, err = i.configFile.Seek(0, 0)
c.Assert(err, check.IsNil)
_, err = i.configFile.WriteString(`{"nonMasqueradeCIDRs": ["8.8.0.0/16", "1.1.2.3/16"]}`)
c.Assert(err, check.IsNil)
i.writeConfig(`{"nonMasqueradeCIDRs": ["8.8.0.0/16", "1.1.2.3/16"], "masqLinkLocal": true}`, c)
time.Sleep(300 * time.Millisecond)

ipnets = i.ipMasqMap.dumpToSet()
Expand All @@ -154,48 +165,51 @@ func (i *IPMasqTestSuite) TestUpdate(c *check.C) {
_, ok = ipnets["1.1.0.0/16"]
c.Assert(ok, check.Equals, true)

// Delete file, should remove the CIDRs
err = os.Remove(i.configFile.Name())
c.Assert(err, check.IsNil)
err = i.configFile.Close()
// Delete file, should remove the CIDRs and add default nonMasq CIDRs
err := os.Remove(i.configFilePath)
c.Assert(err, check.IsNil)
time.Sleep(300 * time.Millisecond)
ipnets = i.ipMasqMap.dumpToSet()
c.Assert(len(ipnets), check.Equals, 0)
c.Assert(len(ipnets), check.Equals, len(defaultNonMasqCIDRs)+1)
for cidrStr := range defaultNonMasqCIDRs {
_, ok := ipnets[cidrStr]
c.Assert(ok, check.Equals, true)
}
_, ok = ipnets[linkLocalCIDRStr]
c.Assert(ok, check.Equals, true)
}

func (i *IPMasqTestSuite) TestRestore(c *check.C) {
var err error

// Check that stale entry is removed from the map after restore
i.ipMasqAgent.Stop()

_, cidr, _ := net.ParseCIDR("3.3.3.0/24")
i.ipMasqMap.cidrs[cidr.String()] = *cidr
_, cidr, _ = net.ParseCIDR("4.4.0.0/16")
i.ipMasqMap.cidrs[cidr.String()] = *cidr
i.writeConfig("nonMasqueradeCIDRs:\n- 4.4.0.0/16", c)

_, err := i.configFile.WriteString("nonMasqueradeCIDRs:\n- 4.4.0.0/16")
c.Assert(err, check.IsNil)

i.ipMasqAgent, err = newIPMasqAgent(i.configFile.Name(), i.ipMasqMap)
i.ipMasqAgent, err = newIPMasqAgent(i.configFilePath, i.ipMasqMap)
c.Assert(err, check.IsNil)
i.ipMasqAgent.Start()
time.Sleep(300 * time.Millisecond)

ipnets := i.ipMasqMap.dumpToSet()
c.Assert(len(ipnets), check.Equals, 1)
c.Assert(len(ipnets), check.Equals, 2)
_, ok := ipnets["4.4.0.0/16"]
c.Assert(ok, check.Equals, true)
_, ok = ipnets[linkLocalCIDRStr]
c.Assert(ok, check.Equals, true)

// Now stop the goroutine, and also remove the maps. It should bootstrap from
// the config
i.ipMasqAgent.Stop()
i.ipMasqMap = &ipMasqMapMock{cidrs: map[string]net.IPNet{}}
i.ipMasqAgent.ipMasqMap = i.ipMasqMap
_, err = i.configFile.Seek(0, 0)
c.Assert(err, check.IsNil)
_, err = i.configFile.WriteString("nonMasqueradeCIDRs:\n- 3.3.0.0/16")
c.Assert(err, check.IsNil)
i.ipMasqAgent, err = newIPMasqAgent(i.configFile.Name(), i.ipMasqMap)
i.writeConfig("nonMasqueradeCIDRs:\n- 3.3.0.0/16\nmasqLinkLocal: true", c)
i.ipMasqAgent, err = newIPMasqAgent(i.configFilePath, i.ipMasqMap)
c.Assert(err, check.IsNil)
i.ipMasqAgent.Start()

Expand Down