소스 검색

test(iplimit): align ban-policy tests with last-IP-wins (#4699)

PR #4699 restored the "keep newest live IP, ban the oldest" policy in
check_client_ip_job.go but left the integration test asserting the old
"protect original, ban newcomer" behavior, so it failed. Update the test
to expect the oldest live IP banned and the newest kept, and fix the now
misleading name/comment on the partitionLiveIps concurrency unit test.
MHSanaei 11 시간 전
부모
커밋
72121784fe
2개의 변경된 파일15개의 추가작업 그리고 13개의 파일을 삭제
  1. 11 10
      web/job/check_client_ip_job_integration_test.go
  2. 4 3
      web/job/check_client_ip_job_test.go

+ 11 - 10
web/job/check_client_ip_job_integration_test.go

@@ -179,7 +179,8 @@ func TestUpdateInboundClientIps_LiveIpNotBannedByStillFreshHistoricals(t *testin
 }
 
 // opposite invariant: when several ips are actually live and exceed
-// the limit, the newcomer still gets banned.
+// the limit, the oldest connection is dropped and the most recent one
+// keeps the slot (last-IP-wins policy from #3735, restored in #4699).
 func TestUpdateInboundClientIps_ExcessLiveIpIsStillBanned(t *testing.T) {
 	setupIntegrationDB(t)
 
@@ -193,8 +194,8 @@ func TestUpdateInboundClientIps_ExcessLiveIpIsStillBanned(t *testing.T) {
 
 	j := NewCheckClientIpJob()
 	// both live, limit=1. use distinct timestamps so sort-by-timestamp
-	// is deterministic: 10.1.0.1 is the original (older), 192.0.2.9
-	// joined later and must get banned.
+	// is deterministic: 10.1.0.1 is the original (older) and must get
+	// banned; 192.0.2.9 joined later and keeps the slot (last IP wins).
 	live := []IPWithTimestamp{
 		{IP: "10.1.0.1", Timestamp: now - 5},
 		{IP: "192.0.2.9", Timestamp: now},
@@ -205,16 +206,16 @@ func TestUpdateInboundClientIps_ExcessLiveIpIsStillBanned(t *testing.T) {
 	if !shouldCleanLog {
 		t.Fatalf("shouldCleanLog must be true when the live set exceeds the limit")
 	}
-	if len(j.disAllowedIps) != 1 || j.disAllowedIps[0] != "192.0.2.9" {
-		t.Fatalf("expected 192.0.2.9 to be banned; disAllowedIps = %v", j.disAllowedIps)
+	if len(j.disAllowedIps) != 1 || j.disAllowedIps[0] != "10.1.0.1" {
+		t.Fatalf("expected 10.1.0.1 to be banned; disAllowedIps = %v", j.disAllowedIps)
 	}
 
 	persisted := ipSet(readClientIps(t, email))
-	if _, ok := persisted["10.1.0.1"]; !ok {
-		t.Errorf("original IP 10.1.0.1 must still be persisted; got %v", persisted)
+	if _, ok := persisted["192.0.2.9"]; !ok {
+		t.Errorf("newest IP 192.0.2.9 must still be persisted; got %v", persisted)
 	}
-	if _, ok := persisted["192.0.2.9"]; ok {
-		t.Errorf("banned IP 192.0.2.9 must NOT be persisted; got %v", persisted)
+	if _, ok := persisted["10.1.0.1"]; ok {
+		t.Errorf("banned IP 10.1.0.1 must NOT be persisted; got %v", persisted)
 	}
 
 	// 3xipl.log must contain the ban line in the exact fail2ban format.
@@ -222,7 +223,7 @@ func TestUpdateInboundClientIps_ExcessLiveIpIsStillBanned(t *testing.T) {
 	if err != nil {
 		t.Fatalf("read 3xipl.log: %v", err)
 	}
-	wantSubstr := "[LIMIT_IP] Email = pr4091-abuse || Disconnecting OLD IP = 192.0.2.9"
+	wantSubstr := "[LIMIT_IP] Email = pr4091-abuse || Disconnecting OLD IP = 10.1.0.1"
 	if !contains(string(body), wantSubstr) {
 		t.Fatalf("3xipl.log missing expected ban line %q\nfull log:\n%s", wantSubstr, body)
 	}

+ 4 - 3
web/job/check_client_ip_job_test.go

@@ -107,9 +107,10 @@ func TestPartitionLiveIps_SingleLiveNotStarvedByStillFreshHistoricals(t *testing
 	}
 }
 
-func TestPartitionLiveIps_ConcurrentLiveIpsStillBanNewcomers(t *testing.T) {
-	// keep the "protect original, ban newcomer" policy when several ips
-	// are really live. with limit=1, A must stay and B must be banned.
+func TestPartitionLiveIps_ConcurrentLiveIpsSortedAscending(t *testing.T) {
+	// when several ips are really live, partition returns them all in the
+	// live set sorted ascending by timestamp. updateInboundClientIps then
+	// keeps the newest and bans the oldest (last-IP-wins, #4699).
 	ipMap := map[string]int64{
 		"A": 5000,
 		"B": 5500,