7
High
// The Bug
Helm #32047 (PR #32337): testing import in capabilities.go non-test code detected test builds (no debug info). Fix replaced DefaultCapabilities var+panic…
// Root Cause
The edge case in `import (` at `pkg/chart/common/capabilities.go` causes incorrect behavior when
a specific input condition is met. In Go, this pattern is easy to miss because
standard test suites rarely cover every boundary condition.
The fix is a moderate change — it addresses exactly the failing condition without
refactoring surrounding code. This minimizes the risk of introducing new bugs.
**Impact**: The bug affects users who hit the specific edge case. For helm,
this means 30 lines fixes a scenario that could
cause incorrect output, crashes, or silent data corruption depending on the code path.
a specific input condition is met. In Go, this pattern is easy to miss because
standard test suites rarely cover every boundary condition.
The fix is a moderate change — it addresses exactly the failing condition without
refactoring surrounding code. This minimizes the risk of introducing new bugs.
**Impact**: The bug affects users who hit the specific edge case. For helm,
this means 30 lines fixes a scenario that could
cause incorrect output, crashes, or silent data corruption depending on the code path.
// The Fix
Diff showing the exact changes made to fix the bug.
@@ -20,7 +20,6 @@ import (
"slices"
"strconv"
"strings"
- "testing"
"github.com/Masterminds/semver/v3"
"k8s.io/client-go/kubernetes/scheme"
@@ -41,16 +40,18 @@ var (
// DefaultVersionSet is the default version set, which includes only Core V1 ("v1").
DefaultVersionSet = allKnownVersions()
- DefaultCapabilities = func() *Capabilities {
- caps, err := makeDefaultCapabilities()
- if err != nil {
- panic(fmt.Sprintf("failed to create default capabilities: %v", err))
- }
- return caps
-
- }()
+ // DefaultCapabilities is initialized during init().
+ DefaultCapabilities *Capabilities
)
+func init() {
+ var err error
+ DefaultCapabilities, err = makeDefaultCapabilities()
+ if err != nil {
+ panic(fmt.Sprintf("failed to create default capabilities: %v", err))
+ }
+}
+
// Capabilities describes the capabilities of the Kubernetes cluster.
type Capabilities struct {
// KubeVersion is the Kubernetes version.
@@ -143,16 +144,11 @@ func allKnownVersions() VersionSet {
}
func makeDefaultCapabilities() (*Capabilities, error) {
- // Test builds don't include debug info / module info
- // (And even if they did, we probably want stable capabilities for tests anyway)
- // Return a default value for test builds
- if testing.Testing() {
- return newCapabilities(kubeVersionMajorTesting, kubeVersionMinorTesting)
- }
-
vstr, err := helmversion.K8sIOClientGoModVersion()
if err != nil {
- return nil, fmt.Errorf("failed to retrieve k8s.io/client-go version: %w", err)
+ // Test builds and environments without k8s.io/client-go build info
+ // fall back to a stable default version.
+ return newCapabilities(kubeVersionMajorTesting, kubeVersionMinorTesting)
}
v, err := semver.NewVersion(vstr)// Pattern & Takeaways
**Pattern**: Edge case in `import (` — the Go code path was not tested with the specific input that triggers the failure. The moderate fix demonstrates that the most reliable approach is to change the minimum necessary code.
### How to Apply This to Your Go Codebase
When reviewing Go code for testing-import leaks, follow this checklist:
1. **Scan non-test files for `"testing"` imports** — `grep '"testing"' --include='*.go' | grep -v '_test.go'` catches the pattern instantly.
2. **Replace `testing.Testing()` guards with error-based fallbacks** — Instead of checking `testing.Testing()`, restructure the code to attempt the real operation and fall back to a default value on error (as shown in the diff above).
3. **Run `go vet` with all analyzers** — `go vet ./...` catches the `testing` import in non-test code on recent Go versions.
4. **Add a CI check** — A simple `! grep -r '"testing"' $(find . -name '*.go' ! -name '*_test.go')` in your CI pipeline prevents regression.
**Key insight**: The most predictable bugs are edge cases at input boundaries. Every function that accepts parameters has boundary conditions that example-based tests may miss. Code review should focus on: (1) What happens with empty/null input? (2) What happens at iteration boundaries? (3) What happens with unexpected types?
**Transfer potential**: **Varies** — edge case fixes are repo-specific in detail but universal in pattern. The minimal-change principle and boundary-condition thinking transfer to any codebase. Reading this post helps recognize similar patterns in your own projects.
### How to Apply This to Your Go Codebase
When reviewing Go code for testing-import leaks, follow this checklist:
1. **Scan non-test files for `"testing"` imports** — `grep '"testing"' --include='*.go' | grep -v '_test.go'` catches the pattern instantly.
2. **Replace `testing.Testing()` guards with error-based fallbacks** — Instead of checking `testing.Testing()`, restructure the code to attempt the real operation and fall back to a default value on error (as shown in the diff above).
3. **Run `go vet` with all analyzers** — `go vet ./...` catches the `testing` import in non-test code on recent Go versions.
4. **Add a CI check** — A simple `! grep -r '"testing"' $(find . -name '*.go' ! -name '*_test.go')` in your CI pipeline prevents regression.
**Key insight**: The most predictable bugs are edge cases at input boundaries. Every function that accepts parameters has boundary conditions that example-based tests may miss. Code review should focus on: (1) What happens with empty/null input? (2) What happens at iteration boundaries? (3) What happens with unexpected types?
**Transfer potential**: **Varies** — edge case fixes are repo-specific in detail but universal in pattern. The minimal-change principle and boundary-condition thinking transfer to any codebase. Reading this post helps recognize similar patterns in your own projects.