From c1a40d98a8ef5acf48c0cb944ad492625673cf1a Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:50:23 +0000 Subject: [PATCH 01/10] build: add make vulncheck target To be able to detect known vulnerabilities reachable from the repository's Go code, add a vulncheck target that runs govulncheck over all Go packages. The tool is pinned (currently v1.9.0) and downloaded to ./bin/ by the existing go-install-tool machinery, so the version stays out of go.mod. The scan fetches the Go vulnerability database on demand and therefore needs network access. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 2e34693172337d45db30dcb798532872ab5a47b7) --- Makefile | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Makefile b/Makefile index e222078c..e28a9960 100644 --- a/Makefile +++ b/Makefile @@ -98,6 +98,10 @@ go-lines: get-golines go-lines-fix: get-golines "$(GOLINES_BIN)" -w . +.PHONY: vulncheck +vulncheck: govulncheck ## Check Go code for known vulnerabilities (requires network access to the Go vulnerability database). + "$(GOVULNCHECK)" ./... + .PHONY: e2e e2e: ## Run e2e tests (requires: make deploy-bink). V=1 for verbose. RUN= to filter. # NB: we `cd` here instead of passing a package path to `go test` so that `-v` @@ -288,11 +292,13 @@ CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest GOLANGCI_LINT = $(LOCALBIN)/golangci-lint GOLINES_BIN = $(LOCALBIN)/golines +GOVULNCHECK = $(LOCALBIN)/govulncheck YQ ?= $(LOCALBIN)/yq KUSTOMIZE_VERSION ?= v5.8.1 CONTROLLER_TOOLS_VERSION ?= v0.20.1 GOLANGCI_LINT_VERSION ?= v2.11.4 +GOVULNCHECK_VERSION ?= v1.9.0 YQ_VERSION ?= v4.53.2 # ENVTEST_VERSION is the version of controller-runtime release branch to fetch the envtest setup script (i.e. release-0.20) @@ -332,6 +338,11 @@ golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. $(GOLANGCI_LINT): $(LOCALBIN) $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION)) +.PHONY: govulncheck +govulncheck: $(GOVULNCHECK) ## Download govulncheck locally if necessary. +$(GOVULNCHECK): $(LOCALBIN) + $(call go-install-tool,$(GOVULNCHECK),golang.org/x/vuln/cmd/govulncheck,$(GOVULNCHECK_VERSION)) + .PHONY: get-golines get-golines: $(GOLINES_BIN) ## Download golines locally if necessary. $(GOLINES_BIN): $(LOCALBIN) From 3c44169c09f9b7e7ef60ae0112542dfab5b3510e Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:51:01 +0000 Subject: [PATCH 02/10] chore(deps): update golang.org/x/net to v0.60.0 govulncheck reports five vulnerabilities reachable from this repository's code, all in golang.org/x/net v0.59.0 and fixed in v0.60.0: GO-2026-6603, GO-2026-6610, GO-2026-6611, GO-2026-6612 and GO-2026-6617 (HTTP/2 flow control, HPACK and header handling issues). Bump to the minimum version that fixes them so the vulnerability check can be enforced in CI without any existing finding to waive. Two further findings in x/net/html (GO-2026-6860 and GO-2026-6866) are not reachable from this code and stay informational; they do not fail the check. Assisted-by: AI (cherry picked from commit 0d424cb2eb43cefac7eeaf6115c70411320a3c60) Signed-off-by: Alice Frosi --- go.mod | 14 +++++++------- go.sum | 28 ++++++++++++++-------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index bc27b942..6a871045 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,6 @@ require ( k8s.io/client-go v0.37.1 k8s.io/kubectl v0.37.1 sigs.k8s.io/controller-runtime v0.25.2 - sigs.k8s.io/yaml v1.6.0 ) require ( @@ -95,12 +94,12 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect - golang.org/x/net v0.57.0 // indirect - golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sync v0.22.0 // indirect - golang.org/x/sys v0.47.0 // indirect - golang.org/x/term v0.45.0 // indirect - golang.org/x/text v0.40.0 // indirect + golang.org/x/net v0.60.0 // indirect + golang.org/x/oauth2 v0.37.0 // indirect + golang.org/x/sync v0.23.0 // indirect + golang.org/x/sys v0.48.0 // indirect + golang.org/x/term v0.46.0 // indirect + golang.org/x/text v0.42.0 // indirect golang.org/x/time v0.15.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/protobuf v1.36.12-0.20260120151049-f2248ac996af // indirect @@ -117,4 +116,5 @@ require ( sigs.k8s.io/kustomize/kyaml v0.21.1 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.4.2 // indirect + sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/go.sum b/go.sum index eee757bd..f00c7d40 100644 --- a/go.sum +++ b/go.sum @@ -207,21 +207,21 @@ go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= -golang.org/x/mod v0.39.0 h1:UF5zwQdCRRUpHfyPwr7d4UrGiVeldIsogtzWVnczL74= -golang.org/x/mod v0.39.0/go.mod h1:bvIbwjQ0HUFFf5AKukeeYQG4ZBUG9yxQbR9aEweIwYY= -golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= -golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= -golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= -golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= -golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= -golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c= +golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o= +golang.org/x/net v0.60.0 h1:79p50tfZlm0J9YfoDsSi639qSXNGVwEzOPLCxM2FsYU= +golang.org/x/net v0.60.0/go.mod h1:2DA/G1UfVbCpQPeWTmMPGY7Cs2PkBkwu743bVX5PIVg= +golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= +golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= +golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk= +golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= -golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= -golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= -golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= -golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= +golang.org/x/sys v0.48.0/go.mod h1:hNLxWAXmnKAxqDtdwIYC4bM9oQPEecfsnNMuSxOs3og= +golang.org/x/term v0.46.0 h1:3+OXuTbaKDgwk8jTi3aSLHRlmWqHEUDUtxnbFigO4YE= +golang.org/x/term v0.46.0/go.mod h1:+K02xbkittuwc0Am4abfA3Fc+XRGXkvBXNO88NCXPoc= +golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= +golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= From a6c01bcc5fd89b473a1227195db7389805f2210a Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:51:06 +0000 Subject: [PATCH 03/10] ci: run vulnerability check in unit job Run make vulncheck as a dedicated step in the unit job so pull requests fail when a known vulnerability is reachable from the repository's Go code. It is a separate step rather than part of make lint so the two checks can be run and diagnosed independently, and it reuses the Makefile target instead of duplicating the invocation. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 2985ad1eeca33cf7285d60137cbd00cda6f2a6a6) --- .github/workflows/ci.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d68dd1fc..18ad6190 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -48,6 +48,9 @@ jobs: - name: Lint run: make lint + - name: Vulnerability check + run: make vulncheck + build-bink: uses: $/.github/workflows/build-bink.yaml with: From 514cd1f4f68df1545ce15d55078f4c004d09c112 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 13:51:11 +0000 Subject: [PATCH 04/10] docs: document make vulncheck Describe the new target in CONTRIBUTING.md: what it scans, that it downloads both the govulncheck binary and the Go vulnerability database and so needs network access, and how to interpret its output. Only findings in functions the code actually calls fail the check; findings in imported packages or required modules that are not called are informational. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit cbb242ecbb778b870eb52733c13933603deea776) --- CONTRIBUTING.md | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6e4ee6a8..452c526f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,9 +34,9 @@ For end-to-end testing you will also need: backed by bootc nodes - [skopeo](https://github.com/containers/skopeo) — container image inspection -Tool dependencies like `controller-gen`, `kustomize`, `golangci-lint`, and -`setup-envtest` are downloaded automatically to `./bin/` by the Makefile when -needed. +Tool dependencies like `controller-gen`, `kustomize`, `golangci-lint`, +`govulncheck`, and `setup-envtest` are downloaded automatically to `./bin/` by +the Makefile when needed. ### Development Environment @@ -106,6 +106,26 @@ make unit V=1 # Verbose output make unit RUN=Foo # Run tests matching "Foo" ``` +### Checking for Vulnerabilities + +```shell +make vulncheck +``` + +This runs [govulncheck](https://go.dev/blog/vuln) against all Go packages in +the repository, looking for known vulnerabilities from the +[Go vulnerability database](https://vuln.go.dev/). Both the `govulncheck` +binary and the database are downloaded on demand, so the target requires +network access. + +Only reachable findings — vulnerabilities in functions that the code actually +calls — fail the check. Fix them by upgrading the affected dependency or +changing the affected code. The scan may also report informational findings +in packages you import or modules you require whose vulnerable functions are +not called; these do not fail the check. + +CI runs this check in the `unit` job. + ### Running End-to-End Tests E2E tests live in `test/e2e/` and run against a real @@ -247,6 +267,7 @@ Follow a commit style similar to the Linux kernel: make vet make lint make unit + make vulncheck ``` 4. Push your branch and open a pull request against `main`. From 56ca8755bba75d4e3ef68f7b4e016fa0eac7d953 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:37 +0000 Subject: [PATCH 05/10] internal: add package doc comments Every internal package lacked a package comment, which staticcheck's ST1000 check flags. Add a short one to each so the full staticcheck suite can be enabled. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 1caecb62d8d1948fa212d49e746a4686e018f9db) --- internal/bootc/executor.go | 1 + internal/controller/bootcnodepool_controller.go | 2 ++ internal/daemon/reconciler.go | 2 ++ internal/image/digest.go | 2 ++ internal/registry/resolver.go | 1 + internal/version/version.go | 1 + 6 files changed, 9 insertions(+) diff --git a/internal/bootc/executor.go b/internal/bootc/executor.go index 6d7a0a7c..4554966a 100644 --- a/internal/bootc/executor.go +++ b/internal/bootc/executor.go @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +// Package bootc runs bootc commands on the host and parses their output. package bootc import ( diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 7d334476..6f9dee32 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 +// Package controller implements the BootcNodePool controller, which +// resolves a pool's target image and rolls it out across member nodes. package controller import ( diff --git a/internal/daemon/reconciler.go b/internal/daemon/reconciler.go index 6bf422b3..afb3e11a 100644 --- a/internal/daemon/reconciler.go +++ b/internal/daemon/reconciler.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 +// Package daemon implements the per-node daemon that watches bootc +// status and reconciles the node's BootcNode. package daemon import ( diff --git a/internal/image/digest.go b/internal/image/digest.go index 82a9c545..f46b5fd4 100644 --- a/internal/image/digest.go +++ b/internal/image/digest.go @@ -1,5 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 +// Package image provides helpers for matching container image +// references against digests. package image import ( diff --git a/internal/registry/resolver.go b/internal/registry/resolver.go index b9ee71e6..0c8631ee 100644 --- a/internal/registry/resolver.go +++ b/internal/registry/resolver.go @@ -1,3 +1,4 @@ +// Package registry resolves container image tags to digests. package registry import ( diff --git a/internal/version/version.go b/internal/version/version.go index 07251d1a..de51aa6f 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -1,5 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +// Package version holds build metadata set via linker flags. package version var ( From 1e9fe099bab1c67bbdae95e9d40fe5f5385a59e0 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:37 +0000 Subject: [PATCH 06/10] controller: saturate status counts instead of wrapping The pool status counts are int32 in the API but computed from int slice lengths, and the bare conversion would silently wrap on overflow (gosec G115). Node counts can't realistically get there, but a saturating conversion makes the intent explicit and keeps the linter quiet without a suppression. The counts can never be negative, but they stay int32 rather than uint32: the Kubernetes API conventions advise against unsigned integers because client support across languages is uneven, and recommend validating that the value is non-negative instead. Mark the counts with a minimum of 0 in the CRD schema, and clamp the conversion to [0, math.MaxInt32] so the controller never writes a value the schema would reject. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 971a76abc85b376d8072652ab853221ef012a15c) --- api/v1alpha1/bootcnodepool_types.go | 4 ++++ .../bases/node.bootc.dev_bootcnodepools.yaml | 4 ++++ internal/controller/status.go | 21 +++++++++++++++---- internal/controller/status_test.go | 13 ++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/bootcnodepool_types.go b/api/v1alpha1/bootcnodepool_types.go index 2277cbbc..559c691a 100644 --- a/api/v1alpha1/bootcnodepool_types.go +++ b/api/v1alpha1/bootcnodepool_types.go @@ -200,19 +200,23 @@ type BootcNodePoolStatus struct { // nodeCount is the total number of nodes in this pool. // +optional + // +kubebuilder:validation:Minimum=0 NodeCount int32 `json:"nodeCount,omitempty"` // updatedCount is the number of nodes running the target digest. // +optional + // +kubebuilder:validation:Minimum=0 UpdatedCount int32 `json:"updatedCount,omitempty"` // updatingCount is the number of nodes actively being updated // (staging, staged, or rebooting). // +optional + // +kubebuilder:validation:Minimum=0 UpdatingCount int32 `json:"updatingCount,omitempty"` // degradedCount is the number of nodes in a degraded state. // +optional + // +kubebuilder:validation:Minimum=0 DegradedCount int32 `json:"degradedCount,omitempty"` // conditions represent the current state of the pool. diff --git a/config/crd/bases/node.bootc.dev_bootcnodepools.yaml b/config/crd/bases/node.bootc.dev_bootcnodepools.yaml index db056742..2332791a 100644 --- a/config/crd/bases/node.bootc.dev_bootcnodepools.yaml +++ b/config/crd/bases/node.bootc.dev_bootcnodepools.yaml @@ -263,6 +263,7 @@ spec: degradedCount: description: degradedCount is the number of nodes in a degraded state. format: int32 + minimum: 0 type: integer deployedDigest: description: |- @@ -278,6 +279,7 @@ spec: nodeCount: description: nodeCount is the total number of nodes in this pool. format: int32 + minimum: 0 type: integer observedGeneration: description: |- @@ -300,12 +302,14 @@ spec: description: updatedCount is the number of nodes running the target digest. format: int32 + minimum: 0 type: integer updatingCount: description: |- updatingCount is the number of nodes actively being updated (staging, staged, or rebooting). format: int32 + minimum: 0 type: integer type: object required: diff --git a/internal/controller/status.go b/internal/controller/status.go index 83fc2fee..266d0ac4 100644 --- a/internal/controller/status.go +++ b/internal/controller/status.go @@ -4,6 +4,7 @@ package controller import ( "fmt" + "math" "strings" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -19,12 +20,12 @@ func syncPoolStatus(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { return } pool.Status.ObservedGeneration = pool.Generation - pool.Status.NodeCount = int32(rs.nodeCount()) - pool.Status.UpdatedCount = int32(len(rs.upToDate)) - pool.Status.UpdatingCount = int32( + pool.Status.NodeCount = clampCount(rs.nodeCount()) + pool.Status.UpdatedCount = clampCount(len(rs.upToDate)) + pool.Status.UpdatingCount = clampCount( len(rs.pending) + len(rs.staging) + len(rs.staged) + len(rs.rebooting), ) - pool.Status.DegradedCount = int32(len(rs.degraded)) + pool.Status.DegradedCount = clampCount(len(rs.degraded)) if pool.Status.NodeCount == pool.Status.UpdatedCount { pool.Status.DeployedDigest = pool.Status.TargetDigest @@ -34,6 +35,18 @@ func syncPoolStatus(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { syncUpToDateCondition(pool, rs) } +// clampCount converts a count to int32 for the status API, saturating to +// [0, math.MaxInt32] to match the schema instead of wrapping. +func clampCount(n int) int32 { + if n < 0 { + return 0 + } + if n > math.MaxInt32 { + return math.MaxInt32 + } + return int32(n) +} + func syncUpToDateCondition(pool *bootcv1alpha1.BootcNodePool, rs *rolloutState) { if pool == nil || rs == nil { return diff --git a/internal/controller/status_test.go b/internal/controller/status_test.go index 7568b51b..20dcc0df 100644 --- a/internal/controller/status_test.go +++ b/internal/controller/status_test.go @@ -3,6 +3,7 @@ package controller import ( + "math" "testing" . "github.com/onsi/gomega" @@ -170,3 +171,15 @@ func TestSyncPoolStatusDeployedDigestPreserved(t *testing.T) { g.Expect(pool.Status.DeployedDigest).To(Equal(testDigestA)) g.Expect(pool.Status.UpdateAvailable).To(BeTrue()) } + +// Verifies that clampCount passes in-range counts through and saturates +// instead of wrapping when a count does not fit in int32. +func TestClampCount(t *testing.T) { + g := NewWithT(t) + + g.Expect(clampCount(0)).To(Equal(int32(0))) + g.Expect(clampCount(42)).To(Equal(int32(42))) + g.Expect(clampCount(math.MaxInt32)).To(Equal(int32(math.MaxInt32))) + g.Expect(clampCount(math.MaxInt32 + 1)).To(Equal(int32(math.MaxInt32))) + g.Expect(clampCount(-1)).To(Equal(int32(0))) +} From 87ebe4c8c07f230324f8ded01596db67a2090b42 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:37:14 +0000 Subject: [PATCH 07/10] controller: release drain context once the drain is collected collectDrainResults dropped finished drains from r.drains without calling their cancel func, which is only invoked when a node leaves the pool. Each drain context derives from the reconcile context, which has no deadline, so every completed or failed drain stayed registered with the long-lived parent until the process exited. Cancel the context when the result is collected, after checking whether it was cancelled so the existing cancellation handling still works. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit c4107895bd3cbe6579160bac5590e947f6d105e2) --- internal/controller/rollout.go | 7 +++++-- internal/controller/rollout_test.go | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index f1c3d069..461d8f18 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -371,11 +371,14 @@ func (r *BootcNodePoolReconciler) collectDrainResults( continue } - // Drain finished; remove from the map. + // Drain finished; remove from the map and release its context, + // which would otherwise stay registered with the parent. delete(r.drains, nodeName) + cancelled := ds.ctx.Err() != nil + ds.cancel() if drainErr != nil { - if ds.ctx.Err() != nil { + if cancelled { // Context was cancelled (node left pool or controller // shutdown). The canceller is responsible for cleanup. log.V(1).Info("Drain cancelled", "node", nodeName) diff --git a/internal/controller/rollout_test.go b/internal/controller/rollout_test.go index 18ffd244..b35b039f 100644 --- a/internal/controller/rollout_test.go +++ b/internal/controller/rollout_test.go @@ -3,6 +3,8 @@ package controller import ( + "context" + "errors" "testing" "time" @@ -522,3 +524,23 @@ func TestFindUnhealthySlots(t *testing.T) { g.Expect(findUnhealthySlots(rs, targetDigest)).To(BeEmpty()) }) } + +// Verifies that collecting a finished drain cancels its context, so it +// does not stay registered with the long-lived parent context. +func TestCollectDrainResultsReleasesContext(t *testing.T) { + g := NewWithT(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + drainCtx, drainCancel := context.WithCancel(ctx) + defer drainCancel() + ds := &drainStatus{result: make(chan error, 1), ctx: drainCtx, cancel: drainCancel} + ds.result <- errors.New("drain failed") + + r := &BootcNodePoolReconciler{drains: map[string]*drainStatus{"n1": ds}} + pool := testutil.NewPool("drain-release", testImageDigestRefA, testutil.WithWorkerSelector()) + + g.Expect(r.collectDrainResults(ctx, pool, nil)).To(Succeed()) + g.Expect(r.drains).To(BeEmpty()) + g.Expect(drainCtx.Err()).To(MatchError(context.Canceled)) +} From 6dc61a44efc06c3164d8569a715519618df48ac4 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:37 +0000 Subject: [PATCH 08/10] internal: address gosec findings Prepare for enabling gosec by resolving its findings outside the e2e suite: - Tighten directory permissions in watcher tests to 0750 (G301). - Annotate the remaining reports as false positives, each with the reason: - G204: nsenter callers pass fixed program names, and the only external input is a validated, digested image reference; nothing goes through a shell. - G118 (drain): the cancel func is stored in r.drains and called when the drain is collected. - G118 (staging): staging intentionally outlives the reconcile, and its parentless context cannot leak. - G301: /run/ostree on the host keeps the conventional 0755 mode. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 0bedf3d56044b5e4030a50e618c39b3c268f856c) --- internal/bootc/executor.go | 5 ++++- internal/controller/rollout.go | 3 ++- internal/daemon/reconciler.go | 16 ++++++++++++++-- internal/daemon/watcher_test.go | 10 +++++----- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/internal/bootc/executor.go b/internal/bootc/executor.go index 4554966a..eaaa9bf7 100644 --- a/internal/bootc/executor.go +++ b/internal/bootc/executor.go @@ -61,7 +61,10 @@ func (e *HostExecutor) nsenterCmd(ctx context.Context, args ...string) *exec.Cmd "--setuid", "0", "--setgid", "0", "--env", "--", } - return exec.CommandContext(ctx, "nsenter", append(base, args...)...) + // Callers in this package pass fixed program names; the only external + // input is a validated, digested image reference. Arguments are not + // passed through a shell. + return exec.CommandContext(ctx, "nsenter", append(base, args...)...) //nolint:gosec // G204 } func (e *HostExecutor) Status(ctx context.Context) ([]byte, error) { diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index 461d8f18..212cb1cc 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -307,7 +307,8 @@ func (r *BootcNodePoolReconciler) ensureDrain( log.Info("Starting drain", "node", bn.Name) - drainCtx, cancel := context.WithCancel(ctx) + // cancel is stored in r.drains and called when the drain is collected. + drainCtx, cancel := context.WithCancel(ctx) //nolint:gosec // G118 status := &drainStatus{ result: make(chan error, 1), ctx: drainCtx, diff --git a/internal/daemon/reconciler.go b/internal/daemon/reconciler.go index afb3e11a..7a873336 100644 --- a/internal/daemon/reconciler.go +++ b/internal/daemon/reconciler.go @@ -236,7 +236,17 @@ func (r *BootcNodeReconciler) reconcileBootcNode( switchCtx, cancel := context.WithCancel(context.Background()) r.inflight.acquire(log, desiredImage, cancel) log.Info("Starting staging", "image", desiredImage) - go r.inflight.run(switchCtx, r.NodeName, desiredImage, r.Executor, r.stageDone) + // Staging must outlive this reconcile, so it is not derived from + // the request context. The context has no parent, so it cannot + // leak; acquire cancels it when a newer stage replaces it. + //nolint:gosec // G118 + go r.inflight.run( + switchCtx, + r.NodeName, + desiredImage, + r.Executor, + r.stageDone, + ) case actionAwaitStage: reason = bootcv1alpha1.NodeReasonStaging @@ -440,7 +450,9 @@ func (r *BootcNodeReconciler) writeAuthFile(data []byte) error { } dir := filepath.Join(hostRoot, "run", "ostree") - if err := os.MkdirAll(dir, 0755); err != nil { + // Standard mode for a host system directory under /run; it only + // applies if the host has not created the directory already. + if err := os.MkdirAll(dir, 0755); err != nil { //nolint:gosec // G301 return fmt.Errorf("creating auth dir: %w", err) } diff --git a/internal/daemon/watcher_test.go b/internal/daemon/watcher_test.go index ccf59a6c..9fe13f57 100644 --- a/internal/daemon/watcher_test.go +++ b/internal/daemon/watcher_test.go @@ -78,12 +78,12 @@ func TestWatcherEvents(t *testing.T) { composefsPath := filepath.Join(dir, "deploy") if tt.mkOstree { - if err := os.Mkdir(ostreePath, 0o755); err != nil { + if err := os.Mkdir(ostreePath, 0o750); err != nil { t.Fatal(err) } } if tt.mkComposefs { - if err := os.Mkdir(composefsPath, 0o755); err != nil { + if err := os.Mkdir(composefsPath, 0o750); err != nil { t.Fatal(err) } } @@ -100,7 +100,7 @@ func TestWatcherEvents(t *testing.T) { t.Fatal(err) } case triggerCreate: - if err := os.Mkdir(filepath.Join(composefsPath, "new-deploy"), 0o755); err != nil { + if err := os.Mkdir(filepath.Join(composefsPath, "new-deploy"), 0o750); err != nil { t.Fatal(err) } } @@ -204,7 +204,7 @@ func TestWatcherGetStatusColdCache(t *testing.T) { func TestWatcherShutdown(t *testing.T) { dir := t.TempDir() watchDir := filepath.Join(dir, "bootc") - if err := os.Mkdir(watchDir, 0o755); err != nil { + if err := os.Mkdir(watchDir, 0o750); err != nil { t.Fatal(err) } @@ -226,7 +226,7 @@ func TestWatcherShutdown(t *testing.T) { func TestWatcherNoPollWhenFsnotifyHealthy(t *testing.T) { dir := t.TempDir() ostreePath := filepath.Join(dir, "bootc") - if err := os.Mkdir(ostreePath, 0o755); err != nil { + if err := os.Mkdir(ostreePath, 0o750); err != nil { t.Fatal(err) } From 4f90bc53c36e53b03b6ff89c0b93bdcec04aa458 Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 14:32:49 +0000 Subject: [PATCH 09/10] lint: enable gosec and full staticcheck suite Without a config file golangci-lint runs only its standard linters, and staticcheck runs without the stylistic checks golangci-lint disables by default (package comments, naming, receiver and doc comment consistency). Add .golangci.yml to keep the standard set, enable all staticcheck checks, and add gosec for security issues. gosec is excluded from test/e2e, which intentionally drives kubectl, helm and bink with computed arguments. Because make lint already runs in CI, this needs no workflow change. Assisted-by: AI Signed-off-by: Alice Frosi (cherry picked from commit 2aa6aec456cf90dc49554049cfca385f4f22a498) --- .golangci.yml | 19 +++++++++++++++++++ CONTRIBUTING.md | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..0286eabe --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,19 @@ +version: "2" + +linters: + default: standard + enable: + - gosec + settings: + staticcheck: + # golangci-lint disables some stylistic checks by default; enable + # the full staticcheck suite. + checks: + - all + exclusions: + rules: + # The e2e suite drives external tools (kubectl, helm, bink) with + # computed arguments by design. + - path: ^test/e2e/ + linters: + - gosec diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 452c526f..779e1365 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -216,7 +216,10 @@ matching the structure described in [Finding Test Logs](#finding-test-logs). - Run `make fmt` and `make vet` before submitting. - Run `make lint` to check with [golangci-lint](https://golangci-lint.run/). Use `make lint-fix` to apply - automatic fixes. + automatic fixes. The configuration in `.golangci.yml` enables the full + staticcheck suite and gosec (except for the e2e tests). If a gosec finding + is a false positive, suppress it with `//nolint:gosec // ` and a + comment explaining why. - All Go files must include the SPDX license header: ```go From a430a72db31b2d4b8541d5c3f0b2dc3ca519b59c Mon Sep 17 00:00:00 2001 From: Alice Frosi Date: Wed, 28 Oct 2026 15:20:35 +0000 Subject: [PATCH 10/10] build: require Go 1.26.10 CI installs the Go version named by the go directive in go.mod, so the vulnerability check ran against Go 1.26.0 and reported ten standard library vulnerabilities reachable from this repository's code, in net/http, net/textproto, crypto/tls and crypto/x509 (GO-2026-6603 through GO-2026-6809). They are fixed in Go 1.26.9 and 1.26.10. Raise the minimum to 1.26.10, the first release that fixes all of them, so CI builds and scans with a patched toolchain. Use the same patched Go version for the operator container builder. Assisted-by: AI (cherry picked from commit 67487724d966b1b02211447b401cfc8e2a2a1bec) Signed-off-by: Alice Frosi --- Containerfile | 2 +- go.mod | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Containerfile b/Containerfile index afcda61b..158b32c3 100644 --- a/Containerfile +++ b/Containerfile @@ -1,4 +1,4 @@ -ARG GO_BUILDER_IMAGE=quay.io/hummingbird/go:1.26.8-builder +ARG GO_BUILDER_IMAGE=quay.io/hummingbird/go:1.26.10-builder ARG CORE_RUNTIME_IMAGE=quay.io/hummingbird/core-runtime:2.43 diff --git a/go.mod b/go.mod index 6a871045..91348bdd 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/bootc-dev/bootc-operator -go 1.26.0 +go 1.26.10 require ( github.com/aws/aws-sdk-go-v2 v1.47.1