agent-substrate / agent-substrate/substrate

TestActorArbitraryPortAccess passes when the worker is unreachable, masking #1050

Đang mở
#1,102 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
area/network area/tests kind/bug
Ngôn ngữ chính
Go
Star
1.8k
Fork
316
Merge trung bình
2 ngày 43 phút
Pull request đã merge (30 ngày)
287

Mô tả

Related to #1050.

`TestActorArbitraryPortAccess/unlisted_port_rejected` treats an unreachable worker as a correct port rejection, so it passes during exactly the failure #1050 is about.

The subtest sends a request through a CONNECT tunnel to a port nothing listens on, and asserts only that the response is not 200:

```go
if resp.StatusCode == http.StatusOK {
t.Fatalf("tunneled request to an unlisted port unexpectedly returned HTTP 200; body: %s", body)
}
```

Three different outcomes all satisfy that:

| Response | What actually happened | Should the subtest pass? |
|---|---|---|
| 502 `bad gateway` | atunnel dialled the unlisted port and failed | yes — this is the behaviour under test |
| 503 `upstream connect error ... connection timeout` | Envoy never reached the worker (#1050) | no — nothing was rejected |
| 504 `actor ... request timed out` | the router's own ext_proc timed out | no |

## Actual Behavior

Six recent `pr-workflow` runs logged the 503 as a pass. The word `correctly` is the test's own:

```
tunneled request to an unlisted port correctly returned HTTP 503;
body: upstream connect error or disconnect/reset before headers. reset reason: connection timeout
```

- https://github.com/agent-substrate/substrate/actions/runs/32196537140 (`atenet-dualstack-verify`)
- https://github.com/agent-substrate/substrate/actions/runs/32200086998 (`main`)
- https://github.com/agent-substrate/substrate/actions/runs/32204069703 (`glutton-actor-memory`)
- https://github.com/agent-substrate/substrate/actions/runs/32242913470 (`chore/consolidate-logging`)
- https://github.com/agent-substrate/substrate/actions/runs/32266040800 (`worker_api`)
- https://github.com/agent-substrate/substrate/actions/runs/32277302523 (`kind-ipv6-coredns`)

The log line's number differs by branch (130, 132, 124), so it is not a stable identifier.

In run 32200086998 the other three tests named in #1050 failed while this one passed, on the same worker, in the same run.

## Why it matters

`TestActorArbitraryPortAccess` is one of the four tests #1050 names. While the assertion stays this loose, that test cannot tell anyone whether #1050 has been fixed — it is green either way.

## Steps to Reproduce the Problem

The failure can be injected rather than waited for. Dropping only SYN from the router to the worker pods reproduces #1050's response exactly, while leaving the control plane and established connections alone:

```
nft add table inet atefault
nft add chain inet atefault blockchain '{ type filter hook forward priority -300; }'
nft add rule inet atefault blockchain ip saddr ip daddr { } \
tcp dport 443 tcp flags and (syn|ack) == syn drop
conntrack -D -s -p tcp --dport 443
```

Restart `atenet-router` first so Envoy has no pooled connection, then run the test:

```
--- PASS: TestActorArbitraryPortAccess/unlisted_port_rejected (5.01s)
tunneled request to an unlisted port correctly returned HTTP 503;
body: upstream connect error or disconnect/reset before headers. reset reason: connection timeout
```

5.01s is `OriginalDstCluster`'s 5s `connect_timeout`, and the body matches the CI logs verbatim.

Two other injections do *not* reproduce it, which is worth recording:

- dropping router → `*`:443 also cuts the router off from ate-api-server, and the ext_proc handler times out first with 504
- dropping every packet rather than just SYN stalls an already-pooled connection, so Envoy reports no failure and the test's own 10s deadline fires

## Specifications

- Version: `origin/main` @ 1d7ce081
- Platform: kind on Linux 6.17.0-1022-azure x86_64

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.