Pipes have a finite OS buffer (\~64KB Linux, \~4KB macOS, \~4KB Windows). Since nobody was reading stdout/stderr during the process lifetime, cloudflared would block once the buffer filled up. The post-terminate()/read() could only get whatever fit in the buffer, causing truncated logs.
There was also a race between terminate() and read(): the process might not have flushed its final output yet.
We're also deleting `test_default_only`. Since we changed `edge-ip-version` to auto, this test became redundant.
Fixing some bugs with DNS targets. Most importantly, these changes also fix some wrong assumptionsmade when trying to add support for the `--edge` flag:
1. Removes `StaticEdgeDNSResolver` in favor `resolveStaticEdge`. Since --edge does not imply resolving DNS, this fixes that assumption.
2. Adds EdgeAddrs, which allows us to skip DNS probes when set. This fixes the targets in the DNS rows.
3. Added a new `ResolvedTarget` struct, which joins addresses with the respective DNS results. This avoids the brittle logic we had before, where we assumed there were always two groups (one for each region) when running probes. So this not only makes the code more extensible in case we want to add more regions in the future but also adds support for multiple targets supplied via `--edge`.
4. Changes the existing nomenclature, going from calling things `region` to `target`. The term `region` works when resolving production regions (region1 and region2), but becomes misleading when we add the logic for `--edge`.
The end result of these changes is that we now see the correct addresses when you supply targets via `--edge`, while also making the code a bit clearer.
Instead of having the --precheck flag in cloudflared, we allow controlling prechecks via a DNS flag, so we can short-circuit this behavior in case anything goes wrong. Although we don't expect pre-checks to add that much traffic, we should still guarantee that we can stop pre-checks in case something goes wrong.
Final run method, which runs cloudlflared pre-checks for both the normal startup procedure, as well as cloudflared diag.
For cloudflared diag, this produces a new json output to which is added to the final zip file.
Also added in a new flag to prevent this from running all the time, at least for now until we are 100% sure this works as intended. We will later remove this flag, only leaving in `--no-prechecks`, so this runs by default for everyone using cloudflared.
Tested pre-checks locally with origintunneld. The results show all pre-checks succeeding. In this case, it ran with only 1 region, since locally we run it with `--edge origintunneld1:7844`.
{width=900 height=217}
This PR allows us to use edge overrides with pre-checks. I forgot to account for the --edge flag when developing pre-check probes in TUN-10388. This should allow us to wire this flag in.
DNSResolver will still be injected via RunDialers, and we will take care of initialization in cmd.go. This allows us to keep pre-checks testable and inject mock DNSResolvers when needed.
I thought about overriding `edgediscovery.ResolveEdge` and `edgeDiscovery.StaticEdge` instead of `allregion.EdgeDiscovery` and `allregionResolveAddrs`, respectively. But that would imply changing our existing probe logic to support `Regions` instead of `[][]EdgeAddr`, which would mean more work. Additionally (and perhaps most importantly), using `Regions` would also require us to create new functions to extract a list of edge addresses for our probe tests. I don't think this would go well with the current implementation of `Regions`, as I believe it's intent is to encapsulate the logic around managing addresses per regions. Adding these functions would mean breaking this encapsulation.
Fixing missing TLS settings. While developing the pre-check probes, I forgot to add the certificate settings, which are essential for establishing a connection to origintunneld. I discovered this while testing cloudflared locally.
Fixing warnings in cloudflared log collector.
This attempts to fix errors like the ones shown below:
```
diagnostic/diagnostic.go:132:23: Error return value of `logHandle.Close` is not checked (errcheck)
defer logHandle.Close()
diagnostic/diagnostic.go:134:26: G303: File creation in shared tmp directory without using ioutil.Tempfile (gosec)
outputLogHandle, err := os.Create(filepath.Join(os.TempDir(), logFilename))
```
This introduces the main precheck logic. This will follow concurrency model, timeout handling, and retry logic established in the SPEC. We will follow the decision flow in the [connectivity pre-checks SPEC](https://wiki.cfdata.org/spaces/TUN/pages/1374967685/Connectivity+Pre-checks+for+cloudflared). You can find an attached image of what the decision flow should look like.
{width=900 height=235}
This PR implements all the dialers and resolvers needed to make pre-checks happen. So this task focuses on the following:
1. Implement the DNS probe: call DNSResolver.Resolve(region)
2. Implement the QUIC probe: call QUICDialer.DialQuic (handshake only, no stream opened) and record the result.
3. Implement the HTTP/2 probe: call TCPDialer.DialEdge (TCP + TLS handshake only, no frames sent) and record the result.
4. Implement the Management API probe: call ManagementDialer.DialContext to api.cloudflare.com:443 and record the result.
5. Export edgeDiscovery as EdgeDiscovery in edgediscovery/allregions/discovery.go so the pre-check can reuse the production DNS path.
This sets up the main components to implement the checker.
Switching `SuggestedProtocol` to a pointer, so we can pass nil whenever both QUIC and HTTP2 fail. We should not be suggesting anything when all our protocols have failed.
Adding pre-push hooks to cloudflared. While developing in cloudflared, I found myself constantly bumping into issues in CI, as I was forgetting to run linters and tests at times. We should run these before pushing any code to our repo.
The goals of this PR are twofold:
## **1. Introduce a new renderer to output to `stdout`**
Implement the table renderer that will be used to report the results to stdout. The renderer should output something similar to this:
```
─── CONNECTIVITY PRE-CHECKS ──────────────────────────────────────────────────
COMPONENT TARGET STATUS DETAILS
DNS Resolution region1.v2.argotunnel.com PASS Resolved successfully
DNS Resolution region2.v2.argotunnel.com PASS Resolved successfully
UDP Connectivity Port 7844 (QUIC) PASS Handshake successful
TCP Connectivity Port 7844 (HTTP/2) PASS TLS handshake successful
Cloudflare API api.cloudflare.com:443 PASS Reachable
SUMMARY: Environment is healthy. cloudflared will use 'quic' as primary protocol.
──────────────────────────────────────────────────────────────────────────────
```
## **2. Add a log-level renderer**
Add support for structured logging to print the table results as logs. Below is an example of how logs should look like:
```
{"level":"info","run_id":"52828729-dfwd-45b3-w12f-727cbdb4cbd4", "component":"DNS Resolution","target":"region1.v2.argotunnel.com","status":"pass","details":"Resolved successfully","time":"2024-01-15T10:30:00Z","message":"precheck"}
{"level":"info","run_id":"52828729-dfwd-45b3-w12f-727cbdb4cbd4", "component":"DNS Resolution","target":"region2.v2.argotunnel.com","status":"pass","details":"Resolved successfully","time":"2024-01-15T10:30:00Z","message":"precheck"}
{"level":"info","run_id":"52828729-dfwd-45b3-w12f-727cbdb4cbd4", "component":"QUIC Connectivity","target":"Port 7844 (QUIC)","status":"pass","details":"Handshake successful","time":"2024-01-15T10:30:01Z","message":"precheck"}
{"level":"info","run_id":"52828729-dfwd-45b3-w12f-727cbdb4cbd4", "component":"HTTP/2 Connectivity","target":"Port 7844 (HTTP/2)","status":"pass","details":"TLS handshake successful","time":"2024-01-15T10:30:01Z","message":"precheck"}
{"level":"info","run_id":"52828729-dfwd-45b3-w12f-727cbdb4cbd4", "component":"Management API","target":"api.cloudflare.com:443","status":"pass","details":"Reachable","time":"2024-01-15T10:30:01Z","message":"precheck"}
{"level":"info","run_id":"52828729-dfwd-45b3-w12f-727cbdb4cbd4", "hard_fail":false,"suggested_protocol":"quic","time":"2024-01-15T10:30:01Z","message":"precheck complete"}
```
This adds the foundation for the new cloudflared pre-checks by creating a new prechecks package. This adds the following:
* types.go: Status, CheckResult, Report, Config (add IPVersion allregions.ConfigIPVersion field to Config)
* interfaces.go: DNSResolver, TCPDialer, QUICDialer, ManagementDialer
To allow pre-checks to test both IPv6 and IPv4, we must change the default value of edge-ip-version's from 4 to auto. This will allows the tunnel (and pre-check) to probe both IPv4 and IPv6 addresses by default, respecting the system's DNS preference. Instead of always preferring IPv4, cloudflared will now use whichever address family the system resolver returns first.
Trying to fix the following errors that showed up in CI, which became an issue when doing the pre-check work in https://gitlab.cfdata.org/cloudflare/tun/cloudflared/-/merge_requests/1814:
```
cmd/cloudflared/tunnel/cmd.go:454:29: Error return value of `metricsListener.Close` is not checked (errcheck)
defer metricsListener.Close()
^
cmd/cloudflared/tunnel/cmd.go:573:18: Error return value of `file.Close` is not checked (errcheck)
defer file.Close()
^
cmd/cloudflared/tunnel/cmd.go:574:13: Error return value of `fmt.Fprintf` is not checked (errcheck)
fmt.Fprintf(file, "%d", os.Getpid())
^
cmd/cloudflared/tunnel/cmd.go:47:2: G101: Potential hardcoded credentials: Password in URL (gosec)
sentryDSN = "https://56a9c9fa5c364ab28f34b14f35ea0f1b:3e8827f6f9f740738eb11138f7bebb68@sentry.io/189878"
^
cmd/cloudflared/tunnel/cmd.go:348:23: G703: Path traversal via taint analysis (gosec)
if err := os.Rename(tmpTraceFile.Name(), traceOutputFilepath); err != nil {
^
cmd/cloudflared/tunnel/cmd.go:354:21: G703: Path traversal via taint analysis (gosec)
err := os.Remove(tmpTraceFile.Name())
^
cmd/cloudflared/tunnel/cmd.go:568:15: G304: Potential file inclusion via variable (gosec)
file, err := os.Create(expandedPath)
^
cmd/cloudflared/tunnel/cmd.go:260:10: ST1005: error strings should not be capitalized (staticcheck)
return fmt.Errorf("Use `cloudflared tunnel run` to start tunnel %s", ref)
^
cmd/cloudflared/tunnel/cmd.go:1146:5: SA4011: ineffective break statement. Did you mean to break out of the outer loop? (staticcheck)
break
^
9 issues:
* errcheck: 3
* gosec: 4
* staticcheck: 2
```
