add kindling tester

[WendelHime]

No description provided.

[Copilot]

Pull request overview

This PR adds a new kindling-tester tool for testing individual kindling transports (proxyless, fronted, amp, dnstt). The implementation refactors the kindling client initialization to support selectively enabling/disabling transports via a configurable map, and introduces a standalone CLI tool that can be packaged as Docker images.

Changes:

• Refactored kindling transport initialization to be configurable via an EnabledTransports map
• Added kindling-tester CLI tool for testing individual transports
• Added Dockerfile for building transport-specific test images

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
kindling/client.go	Refactored NewKindling() to conditionally initialize transports based on EnabledTransports map; added dnstt transport support
cmd/kindling-tester/main.go	New CLI tool that tests a single kindling transport by setting environment variables and making HTTP requests
cmd/kindling-tester/README.md	Documentation for the kindling-tester tool with CLI and Docker usage examples
docker/Dockerfile.kindling-tester	Multi-stage Dockerfile for building kindling-tester binary
.gitignore	Added patterns to ignore local environment files for kindling-tester

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The code calls dnsttOptions.Close() immediately after creating it (line 134), then adds the same Close method to closeTransports (line 136), and then passes the already-closed dnsttOptions to kindling.WithDNSTunnel (line 137). This means the dnstt transport will be unusable since it's closed before being used. The immediate Close call on line 134 should be removed, as the closeTransports slice already manages cleanup via the Close() function.

Suggested change

	dnsttOptions.Close()
	}
	closeTransports = append(closeTransports, dnsttOptions.Close)
	kindlingOptions = append(kindlingOptions, kindling.WithDNSTunnel(dnsttOptions))
	closeTransports = append(closeTransports, dnsttOptions.Close)
	kindlingOptions = append(kindlingOptions, kindling.WithDNSTunnel(dnsttOptions))
	}

[Copilot]

The dnsttOptions.Close function pointer is being added to closeTransports even when dnsttOptions might be nil (which happens when DNSTTOptions returns an error). This will cause a nil pointer dereference when Close() is called. The append to closeTransports should only happen when dnsttOptions is not nil, similar to how the fronted transport is handled with a nil check in the closure on lines 109-110.

Suggested change

	}
	closeTransports = append(closeTransports, dnsttOptions.Close)
	kindlingOptions = append(kindlingOptions, kindling.WithDNSTunnel(dnsttOptions))
	closeTransports = append(closeTransports, dnsttOptions.Close)
	kindlingOptions = append(kindlingOptions, kindling.WithDNSTunnel(dnsttOptions))
	}

[Copilot]

The EnabledTransports map is accessed concurrently without synchronization. It's read in NewKindling() and written in cmd/kindling-tester/main.go (lines 101-102) without any mutex protection. This creates a data race condition. Consider adding a sync.RWMutex to protect access to this map, similar to how other shared state is protected in the codebase (e.g., kindlingMutex for the kindling client).

[Copilot]

Variable shadowing issue: the loop variable 'transport' on line 101 shadows the 'transport' variable declared on line 76. This causes line 105 to always set the last transport from the EnabledTransports map to true, rather than the intended transport from the TRANSPORT environment variable. The loop variable should be renamed (e.g., to 'key' or 'transportName') to avoid this shadowing.

Suggested change

	for transport := range kindling.EnabledTransports {
	kindling.EnabledTransports[transport] = false
	for name := range kindling.EnabledTransports {
	kindling.EnabledTransports[name] = false

[Copilot]

When NewAMPClient returns an error, the code logs the error but still proceeds to append kindling.WithAMPCache(ampClient) to kindlingOptions on line 124. This passes a nil ampClient to the kindling configuration, which could cause nil pointer dereferences. The code should only append the AMP option when ampClient is successfully created, similar to how the fronted transport checks for nil before adding to closeTransports.

Suggested change

	}
	// Kindling will skip amp transports if the request has a payload larger than 6kb
	kindlingOptions = append(kindlingOptions, kindling.WithAMPCache(ampClient))
	} else if ampClient != nil {
	// Kindling will skip amp transports if the request has a payload larger than 6kb
	kindlingOptions = append(kindlingOptions, kindling.WithAMPCache(ampClient))
	}

[Copilot]

In the stage environment path (lines 84-92), the code returns early without setting stopUpdater or closeTransports. This means that if Close() is called after NewKindling() in a stage environment, stopUpdater will be nil and won't be called, though this is handled safely by the nil check on line 49. However, closeTransports won't be initialized, which could cause issues if other code expects it to exist. Consider initializing closeTransports to an empty slice even in the stage path for consistency.

[Copilot]

The documentation states that USER_ID is required, but the code in main.go (lines 82-85) does not enforce this. The USER_ID environment variable is not included in the required check on line 82, and the code handles an empty userID by using a default value of 0 (line 87). If USER_ID is truly required, it should be added to the validation check on line 82. If it's optional, the documentation should be updated to reflect this.

[Copilot]

The transport value from the TRANSPORT environment variable is used directly to index into kindling.EnabledTransports on line 105 without validating that it's a valid transport name. If an invalid transport name is provided, this will silently add a new key to the map without actually enabling any transport. The code should validate that the transport is one of the expected values ("dnstt", "amp", "proxyless", "fronted") before attempting to enable it.

[Copilot]

The documentation states that TOKEN is required, but the code in main.go does not enforce this. The TOKEN environment variable is not included in the required check on line 82 (cmd/kindling-tester/main.go), and the code will accept an empty token value. If TOKEN is truly required for the tester to function properly, it should be added to the validation check. If it's optional, the documentation should be updated to reflect this.

[Copilot]

The error returned from os.MkdirAll is ignored on line 17. If directory creation fails (e.g., due to permissions issues), the code will continue and later fail when trying to write files to the directory. The error should be checked and handled appropriately, either by returning the error or logging it and exiting.

Suggested change

	os.MkdirAll(dataDir, 0o755)
	if err := os.MkdirAll(dataDir, 0o755); err != nil {
	slog.Error("failed to create data directory", slog.Any("error", err), slog.String("path", dataDir))
	return err
	}