cloudflare / cloudflare/pingora

connectors tests connect to the real 1.1.1.1, so the suite cannot run without internet access

Open
#999 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
27.4k
Forks
1.7k
Avg merge
6h 22m
Merged PRs (30d)
3

Description

## Describe the bug

A number of tests in `pingora-core` connect to the real `1.1.1.1` on port 80 and 443, with
`one.one.one.one` as the SNI and `Host`, and assert on what comes back:

```rust
// pingora-core/src/connectors/http/v1.rs
let peer = HttpPeer::new(("1.1.1.1", 80), false, "".into());
// make a new connection to 1.1.1.1
let (http, reused) = connector.get_http_session(&peer).await.unwrap();
...
get_http(&mut http, 301).await; // asserts the upstream returns 301
```

So `cargo test` on this crate needs working egress to a third party, and it asserts on that
third party's current behaviour. Two things follow from that:

- The suite cannot run in an isolated network. That includes air-gapped build machines,
CI with egress restrictions, and just working on a train.
- Every test run sends real traffic to a service the project does not control, and the
assertions (`301` from `http://1.1.1.1`) are only true as long as Cloudflare keeps that
redirect. If it ever changes, the tests break for everyone at once, and the failure will
look like a pingora regression.

I do not think the tests themselves are wrong — they cover real behaviour (h1/h2
negotiation, ALPN, keepalive reuse, fd matching). It is the choice of peer that makes them
non-hermetic.

## Pingora info

**Pingora version**: 0.8.1; the same code is on `main` @ `09696b5`
**Rust version**: `cargo 1.98.0 (797e8a9bc 2026-08-05)`
**Operating system version**: Debian 13 (trixie), x86_64, in Docker

## Steps to reproduce

Block only `1.1.1.1` and leave the rest of the network stack alone, so the one variable
being changed is reachability of that address. I also drop `192.0.2.0/24` in both runs,
because `test_conn_timeout` needs that address to black-hole rather than reject, and I did
not want it moving between runs.

```
iptables -A OUTPUT -d 192.0.2.0/24 -j DROP # both runs
iptables -A OUTPUT -d 1.1.1.1 -j REJECT --reject-with icmp-net-unreachable # blocked run only

cargo test -p pingora-core --lib --no-fail-fast
```

With default features (`pingora-core`'s `default = []`):

```
network available 339 passed; 2 failed
1.1.1.1 blocked 334 passed; 7 failed
```

The two failures in the baseline are `connectors::l4::tests::test_bind_to_port_range_on_connect`
(fails on this host regardless — it depends on how fast the host recycles source ports) and
`protocols::http::v2::server::test::test_req_header_no_eos_empty_data_with_eos` (fails on
0.8.1 as released too). Neither is related to this. The difference between the two runs is
exactly five tests:

```
connectors::http::v1::tests::test_connect
connectors::http::v1::tests::test_reuse_rejects_fd_mismatch
connectors::http::v2::tests::test_connect_h1_plaintext
connectors::l4::tests::test_custom_connect
connectors::l4::tests::test_tweak_hook
```

Those five are in the **default** configuration — no TLS feature, nothing opted into. Plain
`cargo test -p pingora-core` needs the internet.

With `--features rustls` the same comparison gives 20 failures, and after subtracting the
same two unrelated ones, **18 tests** depend on reaching `1.1.1.1`. They are spread over
`connectors/http/mod.rs`, `connectors/http/v1.rs`, `connectors/http/v2.rs`,
`connectors/l4.rs` and `connectors/mod.rs`.

A sample failure, so it is clear what a user sees:

```
thread 'connectors::http::v1::tests::test_connect' panicked at
pingora-core/src/connectors/http/v1.rs:90:70:
called `Result::unwrap()` on an `Err` value: Error { etype: ConnectNoRoute, ...
cause: Some(Os { code: 101, kind: NetworkUnreachable, message: "Network is unreachable" }),
context: Some(Owned("Fail to connect to 1.1.1.1:80")) }
```

That one is readable. The TLS ones are less obvious, because a blocked connect surfaces as
a handshake or timeout error further along.

## Expected results

`cargo test` passes without egress to the public internet, and does not depend on a third
party's responses staying the same.

## Observed results

Five tests fail in the default configuration, eighteen with `--features rustls`, purely
because `1.1.1.1` is unreachable.

## Additional context

The pattern for fixing this already exists in the same file: `connectors/mod.rs` has
`test_utils::spawn_mock_uds_server`, and the UDS tests use it instead of a real peer. A
local TCP listener speaking h1, plus a TLS one with a generated cert for the ALPN and
handshake cases, would cover the same ground.

If it is useful I am happy to work on it, but this touches quite a few tests and I would
rather not guess at the shape you want — in particular whether you would prefer a shared
fixture in `test_utils` or a small server per test module, and whether generating a
throwaway CA at test time is acceptable or you would want a checked-in cert.

For what it is worth, I have been reading this area recently and filed #967 and #998 for two
unrelated problems in the same connector tests. This one is separate from both.

Contributor guide

Open the contributing guide

Research direction

Start with the connector tests in pingora-core/src/connectors/http/mod.rs, http/v1.rs, http/v2.rs, l4.rs, and mod.rs, then read test_utils::spawn_mock_uds_server in connectors/mod.rs. Run cargo test -p pingora-core with and without rustls while blocking 1.1.1.1; done means the affected tests pass without public-network egress or assertions against third-party responses.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.