Skip to content

Commit c795127

Browse files
committed
ci(lint): adopt strict golangci-lint config + cross-platform CI gate
Closes #153. Adopts the `default: all` golangci-lint v2 config proposed in #153 and reconciles the project against it: zero lint findings on both linux/amd64 (host) and windows/amd64, with a CI lint matrix gate so the strict config doesn't erode silently. ## Strict-lint adoption - `.golangci.yml`: `default: all` with the relaxations from the issue (depguard, exhaustruct, gochecknoinits, wsl, lll, errchkjson, ireturn, gocheckcompilerdirectives) plus three documented additions: noinlineerr (200+ inline if-err sites; conversion would create variable-scope leaks), goconst on _test.go (test fixtures live inside backtick strings; substitution desyncs expected vs got), gomodguard (deprecated in v2.12+, neither allow nor blocklist configured). - `gomoddirectives.replace-allow-list` for github.com/siderolabs/talos and .../pkg/machinery — required while upstream declines siderolabs/talos#12652. ## CI lint gate - New `lint:` job in .github/workflows/pr.yml runs golangci-lint v2.12.2 on Ubuntu AND Windows runners (matching the existing `test:` matrix). Build-tagged Windows files (secureperm_windows.go, template_windows_test.go) are now actually evaluated on every PR instead of silently diverging. - `.gitattributes` pins LF for *.go so Windows runners don't surface spurious gofmt errors from autocrlf-converted CRLF endings. ## Bug fixes surfaced by the lint pass - pkg/commands/preflight.go: `nilerr` — two error sites swallowed errors with `return nil` after `if err != nil`. Now propagate. - pkg/commands/rotate_ca_handler.go: `nosprintfhostport` — URL constructed via fmt.Sprintf("%s:%s", host, port) broke for IPv6 endpoints. Switched to net.JoinHostPort. - pkg/age/age.go + pkg/engine/engine.go: `forcetypeassert` — type assertions without ok-form panicked on unexpected YAML shapes. Switched to ok-form with wrapped error returns. - pkg/yamltools/yamltools.go + pkg/engine/engine.go: `exhaustive` — yaml.Kind switches missed DocumentNode/AliasNode/ScalarNode/SequenceNode. Added explicit cases with documented intent. - pkg/commands/init.go: `writeGitignoreFile` reported "Updated" for files it just created (existence check ran AFTER WriteFile). Capture stat before write; pinned by TestContract_WriteGitignoreFile_CreatedVsUpdatedReporting. - pkg/age/age.go: `refuseIfLeftoverBackups` (RotateKeys Phase 0) failed open on permission/sharing/transient I/O errors — only os.IsNotExist is the "no leftover" answer; everything else now fails closed. - pkg/engine/helm/files.go: `Lines()` panicked on empty file content (s[len(s)-1] without length guard). Added early-return guard. Bug is verbatim from upstream Helm v3.20.2; we ship the code so we own the fix. - pkg/commands/template.go: modeline-omitted endpoints overwrote the PreRunE-seeded defaultLocalEndpoint; modeline path generation (root-level basename fallback) was asymmetric with resolution. Both fixed. - pkg/commands/root.go: dropped duplicate stderr print of modeline parse failures — cobra surfaces the wrapped error at the command boundary already. ## Refactors - pkg/age/age.go: EncryptSecretsFile / DecryptSecretsFile / RotateKeys decomposed into named helpers (encryptYAMLPair, decryptYAMLPair, loadOrGenerateIdentity, incrementalEncryptMap, plus rotation-phase helpers). All 19 contract tests still pass. - pkg/commands/apply.go + template.go: split monolithic per-file closures into named helpers (applyOneFileTemplateMode, applyOneFileDirectPatchMode, templateOneFile, buildTemplateRunner, etc.). - pkg/engine/engine.go: extractResourceData rewritten — was an unguarded yamlValue.(string) assertion that would panic; now returns a typed error. ## Error-handling migration ~220 sites migrated from fmt.Errorf("...: %w", err) to cockroachdb/errors.Wrap / Wrapf (project standard). Adds operator-facing hints via errors.WithHint at boundaries. ## Const hoisting ~150 repeated literals (k8s/COSI keys, Helm template names, file modes, preset/chart names, test fixtures) moved to package-level constants with documented intent — single source of truth, single rename per future change. ## Verification - go build ./... — clean (host + GOOS=windows) - go test ./... -count=1 — green (8/8 packages) - go test ./... -race — green - golangci-lint run — 0 issues (host + GOOS=windows) - Manual smoke: talm init --preset cozystack --name test-cluster creates expected files; ancestor-project refusal works; --root . opt-in works. Bulk of the diff is mechanical: const extraction, error-wrap migration, blank-line discipline, and //nolint: annotations with rationale where the rule cannot be satisfied without losing fidelity (project's "200% rule" on disabling lints). Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent b57638a commit c795127

62 files changed

Lines changed: 4109 additions & 2028 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.gitattributes

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
* text=auto eol=lf
2+
3+
# Go source must keep LF endings; gofmt rejects CRLF.
4+
*.go text eol=lf
5+
6+
# Go module / sum / config files.
7+
go.mod text eol=lf
8+
go.sum text eol=lf
9+
*.yaml text eol=lf
10+
*.yml text eol=lf
11+
*.sh text eol=lf
12+
13+
# Binaries
14+
*.png binary
15+
*.jpg binary
16+
*.jpeg binary
17+
*.gif binary

.github/workflows/pr.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,30 @@ jobs:
2222
- name: Run tests
2323
run: go test ./...
2424

25+
lint:
26+
# Run golangci-lint on the same OS matrix as test:. The Windows
27+
# runner is essential — secureperm_windows.go is build-tagged
28+
# (//go:build windows) and never gets evaluated on a Linux/macOS
29+
# host. Without a Windows lint pass, build-tagged files diverge
30+
# from the rest of the tree silently.
31+
strategy:
32+
fail-fast: false
33+
matrix:
34+
os: [ubuntu-latest, windows-latest]
35+
runs-on: ${{ matrix.os }}
36+
steps:
37+
- name: Checkout
38+
uses: actions/checkout@v6
39+
- name: Set up Go
40+
uses: actions/setup-go@v6
41+
with:
42+
go-version: stable
43+
- name: Run golangci-lint
44+
uses: golangci/golangci-lint-action@v7
45+
with:
46+
version: v2.12.2
47+
args: --timeout=5m
48+
2549
dco:
2650
runs-on: ubuntu-latest
2751
steps:

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
talm
22
dist/
3+
.claude/

.golangci.yml

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
version: "2"
2+
3+
linters:
4+
default: all
5+
disable:
6+
- depguard
7+
- exhaustruct
8+
- gochecknoinits
9+
- wsl
10+
- lll
11+
- errchkjson
12+
- ireturn
13+
- gocheckcompilerdirectives
14+
# noinlineerr: this codebase wholesale uses 'if err := X(); err != nil' inline form
15+
# — 219+ occurrences across packages. Converting to plain-assignment style would
16+
# produce a noisy diff with zero correctness benefit AND create variable-scope leaks
17+
# in many places where err is locally scoped to the check.
18+
- noinlineerr
19+
# gomodguard: deprecated in v2.12+ in favour of gomodguard_v2; we don't use
20+
# either (no allow/blocklists configured), so disable to silence the warning.
21+
- gomodguard
22+
settings:
23+
dupl:
24+
threshold: 100
25+
goconst:
26+
min-len: 2
27+
min-occurrences: 2
28+
gocritic:
29+
disabled-checks:
30+
- dupImport
31+
- unnamedResult
32+
enabled-tags:
33+
- diagnostic
34+
- experimental
35+
- opinionated
36+
- performance
37+
- style
38+
funlen:
39+
lines: 60
40+
statements: 60
41+
gomoddirectives:
42+
# The cozystack fork of Talos carries a downstream-only patch
43+
# (siderolabs/talos#12652, --skip-verify) that upstream declined.
44+
# Until that flag lands upstream, the replace directive is the
45+
# only way to consume the fork — it is not generic dependency
46+
# rewriting and must stay.
47+
replace-allow-list:
48+
- github.com/siderolabs/talos
49+
- github.com/siderolabs/talos/pkg/machinery
50+
gocyclo:
51+
min-complexity: 15
52+
cyclop:
53+
max-complexity: 15
54+
mnd:
55+
ignored-numbers:
56+
- "10"
57+
- "100"
58+
- "1000"
59+
- "2"
60+
- "60"
61+
- "60.0"
62+
- "64"
63+
- "500"
64+
nolintlint:
65+
require-explanation: true
66+
require-specific: true
67+
allow-unused: false
68+
varnamelen:
69+
max-distance: 5
70+
min-name-length: 3
71+
check-receiver: false
72+
check-return: false
73+
ignore-type-assert-ok: false
74+
ignore-map-index-ok: false
75+
ignore-chan-recv-ok: false
76+
ignore-decls:
77+
- wg sync.WaitGroup
78+
- wg *sync.WaitGroup
79+
- mu sync.Mutex
80+
- ok bool
81+
ignore-names:
82+
- i
83+
- w
84+
- r
85+
- b
86+
- c
87+
- m
88+
- n
89+
- tt
90+
- rw
91+
exclusions:
92+
generated: lax
93+
presets:
94+
- comments
95+
- common-false-positives
96+
- legacy
97+
- std-error-handling
98+
paths:
99+
- third_party$
100+
- builtin$
101+
- generated\.go$
102+
- pkg/generated/
103+
- \.claude/
104+
rules:
105+
- linters:
106+
- funlen
107+
- dupl
108+
- gocognit
109+
- gocyclo
110+
- cyclop
111+
- errcheck
112+
- testableexamples
113+
- testpackage
114+
- forcetypeassert
115+
- gocritic
116+
- nlreturn
117+
- wsl_v5
118+
- varnamelen
119+
- unparam
120+
- modernize
121+
- gosec
122+
- testifylint
123+
- perfsprint
124+
- paralleltest
125+
- maintidx
126+
# goconst on _test.go: tests intentionally repeat literals
127+
# (IPs, CIDRs, MAC addresses, YAML keys) inside backtick raw
128+
# strings that are EXPECTED template outputs, alongside Go
129+
# string literals used as assertion values. Substituting the
130+
# Go literal into a const desynchronises it from the
131+
# backtick fixture, breaking the test silently. Empirically
132+
# observed during the strict-lint adoption pass: every
133+
# blanket goconst substitution in pkg/engine/contract_*.go
134+
# broke at least one TestRender* case. Sub-agent attempts
135+
# failed for the same reason. Disable goconst on test files.
136+
- goconst
137+
path: _test\.go
138+
139+
formatters:
140+
enable:
141+
- gofmt
142+
- gofumpt
143+
- goimports
144+
exclusions:
145+
generated: lax
146+
paths:
147+
- third_party$
148+
- builtin$
149+
- generated\.go$
150+
- pkg/generated/
151+
- \.claude/

charts/charts.go

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"strings"
99
)
1010

11+
const presetGenericName = "generic"
12+
1113
//go:embed all:cozystack all:generic all:talm
1214
var embeddedCharts embed.FS
1315

@@ -16,83 +18,83 @@ var embeddedCharts embed.FS
1618
func PresetFiles() (map[string]string, error) {
1719
filesMap := make(map[string]string)
1820
regex := regexp.MustCompile(`(name|version): \S+`)
19-
21+
//nolint:wrapcheck // wrapper around embedded FS WalkDir; inner func returns wrapped errors with file context.
2022
err := fs.WalkDir(embeddedCharts, ".", func(filePath string, d fs.DirEntry, err error) error {
2123
if err != nil {
22-
return err
24+
return err //nolint:wrapcheck // wrapper around embedded FS WalkDir.
2325
}
24-
26+
2527
if d.IsDir() {
2628
return nil
2729
}
28-
30+
2931
// Skip talm subdirectories in preset charts (cozystack/charts/talm, generic/charts/talm)
3032
// but include files from the main talm chart (talm/templates/_helpers.tpl, etc.)
31-
if strings.HasPrefix(filePath, "cozystack/charts/talm/") ||
32-
strings.HasPrefix(filePath, "generic/charts/talm/") {
33+
if strings.HasPrefix(filePath, "cozystack/charts/talm/") ||
34+
strings.HasPrefix(filePath, "generic/charts/talm/") {
3335
return nil
3436
}
35-
37+
3638
// Read file content
3739
data, err := embeddedCharts.ReadFile(filePath)
3840
if err != nil {
3941
return err
4042
}
41-
43+
4244
content := string(data)
43-
45+
4446
// For Chart.yaml files, replace name and version with %s
4547
if path.Base(filePath) == "Chart.yaml" {
4648
content = regex.ReplaceAllString(content, "$1: %s")
4749
}
48-
50+
4951
// Use the file path as-is (relative to charts directory)
5052
filesMap[filePath] = content
51-
53+
5254
return nil
5355
})
54-
5556
if err != nil {
56-
return nil, err
57+
return nil, err //nolint:wrapcheck // bubble WalkDir error.
5758
}
58-
59+
5960
return filesMap, nil
6061
}
6162

6263
// AvailablePresets returns a list of available preset chart names.
63-
// The "generic" preset is always first if it exists.
64+
// The presetGenericName preset is always first if it exists.
6465
func AvailablePresets() ([]string, error) {
65-
var presets []string
66-
var hasGeneric bool
67-
66+
var (
67+
presets []string
68+
hasGeneric bool
69+
)
70+
6871
entries, err := embeddedCharts.ReadDir(".")
6972
if err != nil {
70-
return nil, err
73+
return nil, err //nolint:wrapcheck // wrapper around embedded FS ReadDir.
7174
}
72-
75+
7376
for _, entry := range entries {
7477
if !entry.IsDir() {
7578
continue
7679
}
77-
80+
7881
name := entry.Name()
7982
// Skip talm as it's a library chart, not a preset
8083
if name == "talm" {
8184
continue
8285
}
83-
84-
if name == "generic" {
86+
87+
if name == presetGenericName {
8588
hasGeneric = true
8689
} else {
8790
presets = append(presets, name)
8891
}
8992
}
90-
93+
9194
// Put generic first if it exists
9295
if hasGeneric {
93-
presets = append([]string{"generic"}, presets...)
96+
presets = append([]string{presetGenericName}, presets...)
9497
}
95-
98+
9699
return presets, nil
97100
}
98-

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/cozystack/talm
22

3-
go 1.26.2
3+
go 1.26.3
44

55
// Kubernetes dependencies sharing the same version.
66
require (
@@ -96,7 +96,6 @@ require (
9696
github.com/Masterminds/sprig/v3 v3.3.0
9797
github.com/cockroachdb/errors v1.13.0
9898
github.com/gobwas/glob v0.2.3
99-
github.com/pkg/errors v0.9.1
10099
github.com/siderolabs/talos v1.12.6
101100
helm.sh/helm/v3 v3.20.2
102101
)
@@ -227,6 +226,7 @@ require (
227226
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
228227
github.com/petermattis/goid v0.0.0-20260330135022-df67b199bc81 // indirect
229228
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
229+
github.com/pkg/errors v0.9.1 // indirect
230230
github.com/pkg/xattr v0.4.12 // indirect
231231
github.com/planetscale/vtprotobuf v0.6.1-0.20250313105119-ba97887b0a25 // indirect
232232
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect

0 commit comments

Comments
 (0)