cmcfarlen opened a new issue, #13501:
URL: https://github.com/apache/trafficserver/issues/13501
## Description
`tests/gold_tests/tls/tls_check_cert_select_plugin.test.py` fails
intermittently in CI, depending purely on which ephemeral port AuTest happens
to assign to the `ts` process.
The test asserts on the bare substring `"404"`:
```python
tr.Processes.Default.Streams.All += Testers.ExcludesExpression("404",
"Should make an exchange")
```
That substring is not anchored to an HTTP status line, so it also matches
the *port number* in ordinary curl output. When AuTest assigns `ts` a port
containing `404` — for example `62404` — the excluded expression matches lines
like:
```
* Added bar.com:62404:127.0.0.1 to DNS cache
* Trying 127.0.0.1:62404...
```
and the run fails:
```
file .../6-tr-Default/stream.all.txt : Should make an exchange - Failed
Reason: Contents ... contains expression: "404"
Details:
* Added bar.com:62404:127.0.0.1 to DNS cache : 1
... 0* Trying 127.0.0.1:62404... : 6
```
Test run 6 ("Test new version of bar cert with bad CA") is the one that
fails, because it is the only run using `ExcludesExpression` for `"404"`. The
`ContainsExpression("404")` assertions in the other runs are spuriously
*satisfied* by the same collision, which is arguably worse — they can pass
without a real HTTP exchange having occurred.
## Second, related bug in the same file
Four assertions write to the wrong test run variable. Lines 103, 116, and
130 use `tr` where the surrounding run is `tr2`:
```python
tr2 = Test.AddTestRun("foo.com cert")
...
tr2.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not
Connect", ...)
tr2.Processes.Default.Streams.All +=
Testers.ContainsExpression("CN=foo.com", ...)
tr2.Processes.Default.Streams.All +=
Testers.ExcludesExpression("CN=bar.com", ...)
tr.Processes.Default.Streams.All += Testers.ContainsExpression("404",
"Should make an exchange") # <-- tr, not tr2
```
The effect is that the "Should make an exchange" check for runs 1, 2 and 3
is appended to run 0 instead. Run 0's report shows "Should make an exchange"
four times, while runs 1-3 never assert it at all. This also partially masks
the port collision, since those misdirected checks are `Contains` rather than
`Excludes`.
## Suggested fix
1. Anchor the assertion so it cannot match a port number — e.g. match
`HTTP/1.1 404` or the curl status line `< 404`, rather than the bare digits.
2. Change the three misdirected `tr.` references to `tr2.` so runs 1-3
assert what they were meant to.
## Affected versions
Both bugs were introduced together in 7dbb6cb188 ("Add hook for loading
certificate and key data from plugin", #6609, 2021-06-25), verified with `git
log -S` on the exact strings. The file has not been meaningfully changed since
in this respect, so this affects every current line: `9.2.x`, `10.0.x`,
`10.1.x`, `10.2.x` and `master`. The earliest release tag containing it is
`9.2.0`.
## Notes
Not a regression from any recent change — it surfaced while reviewing an
unrelated 10.2.x backport batch, where the shard drew port `62404`. Re-running
the shard makes it pass, which is consistent with the port-draw explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
