feature/conn25/addrAssignments.go

@@ -58,22 +58,16 @@ func (a *addrAssignments) insertWithExpiry(as *addrs, d time.Duration) error {
 	if !as.expiresAt.IsZero() && !as.expiresAt.Before(now) {
 		return errors.New("expiresAt already set")
 	}
-	// we don't expect for addresses to be reused before expiry
-	if existing, ok := a.byMagicIP[as.magic]; ok {
-		if !existing.expiresAt.Before(now) {
-			return errors.New("byMagicIP key exists")
-		}
+	// addresses must be removed (eg by popExpired) before they can be reused
+	if _, ok := a.byMagicIP[as.magic]; ok {
+		return errors.New("byMagicIP key exists")
 	}
 	ddst := domainDst{domain: as.domain, dst: as.dst}
-	if existing, ok := a.byDomainDst[ddst]; ok {
-		if !existing.expiresAt.Before(now) {
-			return errors.New("byDomainDst key exists")
-		}
+	if _, ok := a.byDomainDst[ddst]; ok {
+		return errors.New("byDomainDst key exists")
 	}
-	if existing, ok := a.byTransitIP[as.transit]; ok {
-		if !existing.expiresAt.Before(now) {
-			return errors.New("byTransitIP key exists")
-		}
+	if _, ok := a.byTransitIP[as.transit]; ok {
+		return errors.New("byTransitIP key exists")
 	}
 	as.expiresAt = now.Add(d)
 	mak.Set(&a.byMagicIP, as.magic, as)

feature/conn25/addrAssignments_test.go

@@ -51,20 +51,10 @@ func TestAssignmentsExpire(t *testing.T) {
 	if foundAsAfter.isValid() {
 		t.Fatal("expected zero val")
 	}
-	// Now we can reuse the addresses
+	// We should only be able to write old addresses again if they've been removed from the maps (eg with popExpired).
 	err = assignments.insert(as)
-	if err != nil {
-		t.Fatal(err)
-	}
-	foundAs, ok = assignments.lookupByMagicIP(as.magic)
-	if !ok {
-		t.Fatal("expected to find")
-	}
-	if foundAs.dst != as.dst {
-		t.Fatalf("want %v; got %v", as.dst, foundAs.dst)
-	}
-	if !foundAs.expiresAt.After(clock.Now()) {
-		t.Fatalf("expected foundAs to expire after now")
+	if err == nil {
+		t.Fatal("expected an error but got nil")
 	}
 }