Merge pull request #1 from kradalby/ip-pool-test

Fix empty ip issue and remove network/broadcast addresses

api.go

@@ -445,6 +445,7 @@ func (h *Headscale) handleAuthKey(c *gin.Context, db *gorm.DB, idKey wgkey.Key,
 		log.Println(err)
 		return
 	}
+	log.Printf("Assigning %s to %s", ip, m.Name)
 
 	m.AuthKeyID = uint(pak.ID)
 	m.IPAddress = ip.String()

utils.go

@@ -79,21 +79,6 @@ func (h *Headscale) getAvailableIP() (*netaddr.IP, error) {
 		return nil, err
 	}
 
-	// for _, ip := range usedIps {
-	// 	nextIP := ip.Next()
-
-	// 	if !containsIPs(usedIps, nextIP) && ipPrefix.Contains(nextIP) {
-	// 		return &nextIP, nil
-	// 	}
-	// }
-
-	// // If there are no IPs in use, we are starting fresh and
-	// // can issue IPs from the beginning of the prefix.
-	// ip := ipPrefix.IP()
-	// return &ip, nil
-
-	// return nil, fmt.Errorf("failed to find any available IP in %s", ipPrefix)
-
 	// Get the first IP in our prefix
 	ip := ipPrefix.IP()
 
@@ -102,8 +87,19 @@ func (h *Headscale) getAvailableIP() (*netaddr.IP, error) {
 			return nil, fmt.Errorf("could not find any suitable IP in %s", ipPrefix)
 		}
 
+		// Some OS (including Linux) does not like when IPs ends with 0 or 255, which
+		// is typically called network or broadcast. Lets avoid them and continue
+		// to look when we get one of those traditionally reserved IPs.
+		ipRaw := ip.As4()
+		if ipRaw[3] == 0 || ipRaw[3] == 255 {
+			ip = ip.Next()
+			continue
+		}
+
 		if ip.IsZero() &&
 			ip.IsLoopback() {
+
+			ip = ip.Next()
 			continue
 		}
 
@@ -121,6 +117,7 @@ func (h *Headscale) getUsedIPs() ([]netaddr.IP, error) {
 
 	ips := make([]netaddr.IP, len(addresses))
 	for index, addr := range addresses {
+		if addr != "" {
 			ip, err := netaddr.ParseIP(addr)
 			if err != nil {
 				return nil, fmt.Errorf("failed to parse ip from database, %w", err)
@@ -128,6 +125,7 @@ func (h *Headscale) getUsedIPs() ([]netaddr.IP, error) {
 
 			ips[index] = ip
 		}
+	}
 
 	return ips, nil
 }

utils_test.go

@@ -10,7 +10,7 @@ func (s *Suite) TestGetAvailableIp(c *check.C) {
 
 	c.Assert(err, check.IsNil)
 
-	expected := netaddr.MustParseIP("10.27.0.0")
+	expected := netaddr.MustParseIP("10.27.0.1")
 
 	c.Assert(ip.String(), check.Equals, expected.String())
 }
@@ -46,7 +46,7 @@ func (s *Suite) TestGetUsedIps(c *check.C) {
 
 	c.Assert(err, check.IsNil)
 
-	expected := netaddr.MustParseIP("10.27.0.0")
+	expected := netaddr.MustParseIP("10.27.0.1")
 
 	c.Assert(ips[0], check.Equals, expected)
 
@@ -91,20 +91,20 @@ func (s *Suite) TestGetMultiIp(c *check.C) {
 
 	c.Assert(len(ips), check.Equals, 350)
 
-	c.Assert(ips[0], check.Equals, netaddr.MustParseIP("10.27.0.0"))
+	c.Assert(ips[0], check.Equals, netaddr.MustParseIP("10.27.0.1"))
-	c.Assert(ips[9], check.Equals, netaddr.MustParseIP("10.27.0.9"))
+	c.Assert(ips[9], check.Equals, netaddr.MustParseIP("10.27.0.10"))
-	c.Assert(ips[300], check.Equals, netaddr.MustParseIP("10.27.1.44"))
+	c.Assert(ips[300], check.Equals, netaddr.MustParseIP("10.27.1.47"))
 
 	// Check that we can read back the IPs
 	m1, err := h.GetMachineByID(1)
 	c.Assert(err, check.IsNil)
-	c.Assert(m1.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.0").String())
+	c.Assert(m1.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.1").String())
 
 	m50, err := h.GetMachineByID(50)
 	c.Assert(err, check.IsNil)
-	c.Assert(m50.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.49").String())
+	c.Assert(m50.IPAddress, check.Equals, netaddr.MustParseIP("10.27.0.50").String())
 
-	expectedNextIP := netaddr.MustParseIP("10.27.1.94")
+	expectedNextIP := netaddr.MustParseIP("10.27.1.97")
 	nextIP, err := h.getAvailableIP()
 	c.Assert(err, check.IsNil)
 
@@ -117,3 +117,39 @@ func (s *Suite) TestGetMultiIp(c *check.C) {
 
 	c.Assert(nextIP2.String(), check.Equals, expectedNextIP.String())
 }
+
+func (s *Suite) TestGetAvailableIpMachineWithoutIP(c *check.C) {
+	ip, err := h.getAvailableIP()
+	c.Assert(err, check.IsNil)
+
+	expected := netaddr.MustParseIP("10.27.0.1")
+
+	c.Assert(ip.String(), check.Equals, expected.String())
+
+	n, err := h.CreateNamespace("test_ip")
+	c.Assert(err, check.IsNil)
+
+	pak, err := h.CreatePreAuthKey(n.Name, false, false, nil)
+	c.Assert(err, check.IsNil)
+
+	_, err = h.GetMachine("test", "testmachine")
+	c.Assert(err, check.NotNil)
+
+	m := Machine{
+		ID:             0,
+		MachineKey:     "foo",
+		NodeKey:        "bar",
+		DiscoKey:       "faa",
+		Name:           "testmachine",
+		NamespaceID:    n.ID,
+		Registered:     true,
+		RegisterMethod: "authKey",
+		AuthKeyID:      uint(pak.ID),
+	}
+	h.db.Save(&m)
+
+	ip2, err := h.getAvailableIP()
+	c.Assert(err, check.IsNil)
+
+	c.Assert(ip2.String(), check.Equals, expected.String())
+}