From bad2ce562edec7ecda63830f4bcf1799cb02eda6 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Wed, 17 Jan 2024 12:59:45 +0400 Subject: [PATCH] fix: stop asserting fuzz bytes written in test Fixes a flake seen here: https://github.com/coder/coder/actions/runs/7541558190/job/20528545916 ``` === FAIL: enterprise/provisionerd TestRemoteConnector_Fuzz (0.06s) t.go:84: 2024-01-16 12:32:27.024 [info] connector: failed provisioner authentication remote_addr=[::1]:45138 ... error= failed to receive jobID: github.com/coder/coder/v2/enterprise/provisionerd.(*remoteConnector).authenticate /home/runner/actions-runner/_work/coder/coder/enterprise/provisionerd/remoteprovisioners.go:438 - bufio.Scanner: token too long t.go:84: 2024-01-16 12:32:27.024 [debu] connector: closed connection remote_addr=[::1]:45138 error= remoteprovisioners_test.go:209: Error Trace: /home/runner/actions-runner/_work/coder/coder/enterprise/provisionerd/remoteprovisioners_test.go:209 Error: "2992256" is not less than "2097152" Test: TestRemoteConnector_Fuzz Messages: should not allow more than 1 MiB ``` This was an attempt to test that malicious actors can't abuse our authentication protocol to make us allocate a bunch of memory. However, the test asserted on the number of bytes sent by the fuzzer, not the number of bytes read (& allocated) by the service. The former is affected by network queue sizes and is thus flaky without actively managing the socket queues, which I don't think we want to do. In actual practise, the thing that matters is how much memory the bufio Scanner allocates. By inspection, the scanner will allocate up to 64k, and testing this is true devolves into testing the go standard library, which I don't think is worth doing. So... let's just drop the assertion because a) its flaky, b) it doesn't test what we actually want to test, c) the behavior we actually care about is part of the standard library. --- enterprise/provisionerd/remoteprovisioners_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/enterprise/provisionerd/remoteprovisioners_test.go b/enterprise/provisionerd/remoteprovisioners_test.go index 1e1ca3d788..38c8bc1605 100644 --- a/enterprise/provisionerd/remoteprovisioners_test.go +++ b/enterprise/provisionerd/remoteprovisioners_test.go @@ -206,7 +206,6 @@ func TestRemoteConnector_Fuzz(t *testing.T) { case <-exec.done: // Connector hung up on the fuzzer } - require.Less(t, exec.bytesFuzzed, 2<<20, "should not allow more than 1 MiB") connectCtxCancel() var resp agpl.ConnectResponse select {