perf: reduce init-time memory via lazy loading and build tags#20047
perf: reduce init-time memory via lazy loading and build tags#20047
Conversation
Reduce init-time allocations by deferring computation to first use: - pkg/booleanpolicy: lazy regexp compilation via sync.OnceValue, lazy evaluator.Factory via lazyFactory struct - pkg/branding: lazy branded logo loading - pkg/cloudproviders/aws: lazy root CA pool construction - pkg/probeupload: lazy probe name regex - pkg/printers/table: moved to sub-package to break transitive import chain (roxctl → pkg/printers → tablewriter) - pkg/postgres/id: inline CVE ID utilities (avoids heavyweight pkg/cve import chain) - sensor/kubernetes/fake: //go:build fakeworkloads tag excludes fake workload generator from production binary - sensor/common/centralproxy: move test code to _test.go Depends on: #20024 (for pkg/sync.OnceValue wrapper) AI-assisted. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request introduces lazy initialization patterns using Changes
Sequence Diagram(s)No sequence diagrams generated. The changes consist primarily of lazy initialization refactoring, build constraint additions, and package reorganization without significant new feature flow or multi-component interactions that would benefit from visualization. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In sensor/kubernetes/fake/stub.go, returning nil from NewWorkloadManager/ConfigDefaults makes accidental use in non-fakeworkloads builds fail with nil dereferences; consider having these stubs panic (or otherwise clearly fail fast) so misconfiguration is detected immediately and is easier to diagnose.
- In sensor/common/centralproxy/testutils_test.go, removing the
//nolint:wrapcheckwithout wrapping the error means the linter will likely flag this again; either restore the nolint or wrap the underlying RoundTrip error with additional context.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In sensor/kubernetes/fake/stub.go, returning nil from NewWorkloadManager/ConfigDefaults makes accidental use in non-fakeworkloads builds fail with nil dereferences; consider having these stubs panic (or otherwise clearly fail fast) so misconfiguration is detected immediately and is easier to diagnose.
- In sensor/common/centralproxy/testutils_test.go, removing the `//nolint:wrapcheck` without wrapping the error means the linter will likely flag this again; either restore the nolint or wrap the underlying RoundTrip error with additional context.
## Individual Comments
### Comment 1
<location path="pkg/postgres/id/id.go" line_range="10" />
<code_context>
+import "strings"
+
+// Separator is the separator used in IDs constructed from multiple primary keys.
+const Separator = "#"
+
+// FromPks creates a composite ID from multiple primary key values.
</code_context>
<issue_to_address>
**issue:** Clarify or guard against primary key values that may themselves contain the separator
Since composite IDs rely on `Separator = "#"` with `strings.Join`/`strings.Split`, any PK that can contain `"#"` will break round-tripping and produce ambiguous splits. If that’s possible now or in the future, consider either enforcing that `"#"` is disallowed in PKs used here, or switching to an unambiguous encoding (e.g., length-prefixing or base64-encoding each PK). A brief comment or validation around this invariant would help avoid subtle bugs later.
</issue_to_address>
### Comment 2
<location path="pkg/booleanpolicy/matcher.go" line_range="14" />
<code_context>
"github.com/stackrox/rox/pkg/booleanpolicy/query"
)
+// lazyFactory defers evaluator.Factory creation until first use.
+type lazyFactory struct {
+ once sync.Once
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the custom lazyFactory wrapper with sync.OnceValue-based factory variables to keep lazy initialization while removing extra indirection and dead code.
You can simplify the lazy initialization without the extra `lazyFactory` abstraction and unused `GenerateEvaluator` method by using `sync.OnceValue` directly at the variable level (mirroring the regex pattern).
Instead of:
```go
type lazyFactory struct {
once sync.Once
meta *pathutil.AugmentedObjMeta
val evaluator.Factory
}
func newLazyFactory(meta *pathutil.AugmentedObjMeta) lazyFactory {
return lazyFactory{meta: meta}
}
func (f *lazyFactory) get() *evaluator.Factory {
f.once.Do(func() { f.val = evaluator.MustCreateNewFactory(f.meta) })
return &f.val
}
func (f *lazyFactory) GenerateEvaluator(q *query.Query) (evaluator.Evaluator, error) {
return f.get().GenerateEvaluator(q)
}
var (
deploymentEvalFactory = newLazyFactory(augmentedobjs.DeploymentMeta)
deploymentFileAccessFactory = newLazyFactory(augmentedobjs.DeploymentFileAccessMeta)
// ...
)
```
you can use:
```go
var (
deploymentEvalFactory = sync.OnceValue(func() *evaluator.Factory {
return evaluator.MustCreateNewFactory(augmentedobjs.DeploymentMeta)
})
deploymentFileAccessFactory = sync.OnceValue(func() *evaluator.Factory {
return evaluator.MustCreateNewFactory(augmentedobjs.DeploymentFileAccessMeta)
})
// ...
)
```
and update call sites from:
```go
sectionsAndEvals, err := getSectionsAndEvals(deploymentEvalFactory.get(), p, storage.LifecycleStage_DEPLOY, options...)
```
to:
```go
sectionsAndEvals, err := getSectionsAndEvals(deploymentEvalFactory(), p, storage.LifecycleStage_DEPLOY, options...)
```
This keeps the lazy behavior and global factories exactly as before, but removes the custom type, the `get()` wrapper, and the now-unused `GenerateEvaluator` method, reducing indirection and making the usage clearer at call sites.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import "strings" | ||
|
|
||
| // Separator is the separator used in IDs constructed from multiple primary keys. | ||
| const Separator = "#" |
There was a problem hiding this comment.
issue: Clarify or guard against primary key values that may themselves contain the separator
Since composite IDs rely on Separator = "#" with strings.Join/strings.Split, any PK that can contain "#" will break round-tripping and produce ambiguous splits. If that’s possible now or in the future, consider either enforcing that "#" is disallowed in PKs used here, or switching to an unambiguous encoding (e.g., length-prefixing or base64-encoding each PK). A brief comment or validation around this invariant would help avoid subtle bugs later.
| "github.com/stackrox/rox/pkg/booleanpolicy/query" | ||
| ) | ||
|
|
||
| // lazyFactory defers evaluator.Factory creation until first use. |
There was a problem hiding this comment.
issue (complexity): Consider replacing the custom lazyFactory wrapper with sync.OnceValue-based factory variables to keep lazy initialization while removing extra indirection and dead code.
You can simplify the lazy initialization without the extra lazyFactory abstraction and unused GenerateEvaluator method by using sync.OnceValue directly at the variable level (mirroring the regex pattern).
Instead of:
type lazyFactory struct {
once sync.Once
meta *pathutil.AugmentedObjMeta
val evaluator.Factory
}
func newLazyFactory(meta *pathutil.AugmentedObjMeta) lazyFactory {
return lazyFactory{meta: meta}
}
func (f *lazyFactory) get() *evaluator.Factory {
f.once.Do(func() { f.val = evaluator.MustCreateNewFactory(f.meta) })
return &f.val
}
func (f *lazyFactory) GenerateEvaluator(q *query.Query) (evaluator.Evaluator, error) {
return f.get().GenerateEvaluator(q)
}
var (
deploymentEvalFactory = newLazyFactory(augmentedobjs.DeploymentMeta)
deploymentFileAccessFactory = newLazyFactory(augmentedobjs.DeploymentFileAccessMeta)
// ...
)you can use:
var (
deploymentEvalFactory = sync.OnceValue(func() *evaluator.Factory {
return evaluator.MustCreateNewFactory(augmentedobjs.DeploymentMeta)
})
deploymentFileAccessFactory = sync.OnceValue(func() *evaluator.Factory {
return evaluator.MustCreateNewFactory(augmentedobjs.DeploymentFileAccessMeta)
})
// ...
)and update call sites from:
sectionsAndEvals, err := getSectionsAndEvals(deploymentEvalFactory.get(), p, storage.LifecycleStage_DEPLOY, options...)to:
sectionsAndEvals, err := getSectionsAndEvals(deploymentEvalFactory(), p, storage.LifecycleStage_DEPLOY, options...)This keeps the lazy behavior and global factories exactly as before, but removes the custom type, the get() wrapper, and the now-unused GenerateEvaluator method, reducing indirection and making the usage clearer at call sites.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
pkg/booleanpolicy/value_regex.go (1)
15-17: Keep the signature integration ID prefix in one place.
central/signatureintegration/datastore/validate.gostill validates IDs againstsignatures.SignatureIntegrationIDPrefix, so this inline copy can drift and make policy validation disagree with the rest of the system about which IDs are valid. Moving the prefix to a lightweight shared package would keep the init-time win without introducing a second source of truth.As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/booleanpolicy/value_regex.go` around lines 15 - 17, There is a duplicated constant: remove the inline const signatureIntegrationIDPrefix from pkg/booleanpolicy/value_regex.go and centralize the value into a new lightweight shared package (e.g., a small pkg with exported name SignatureIntegrationIDPrefix) so both callers use the same symbol; update references in pkg/booleanpolicy/value_regex.go and central/signatureintegration/datastore/validate.go to import that lightweight package and use the exported SignatureIntegrationIDPrefix (ensure the new package has no heavy init that would reintroduce the cosign/rekor import).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/booleanpolicy/matcher.go`:
- Around line 15-32: Replace the sync.Once + stored evaluator.Factory pattern
with sync.OnceValue to avoid marking done on panic: change lazyFactory to hold
val sync.OnceValue[evaluator.Factory] (remove once and the plain val field),
update get() to call f.val.Do(func() evaluator.Factory { return
evaluator.MustCreateNewFactory(f.meta) }) and return a pointer to the resulting
evaluator.Factory, and ensure GenerateEvaluator uses that get() result; keep
newLazyFactory and lazyFactory.GenerateEvaluator logic but adapt to the
OnceValue API so initialization is retried on panic rather than leaving a
zero-valued Factory.
In `@pkg/postgres/id/id.go`:
- Around line 13-20: FromPks and ToParts currently join/split raw PK strings
with Separator and will corrupt values that contain the separator (e.g., "a#b").
Fix by making FromPks encode each PK before joining and have ToParts decode each
part after splitting; for example, apply a reversible safe encoding such as
URL-safe base64 (base64url without padding) or percent-encoding to each element
in FromPks and decode in ToParts, ensuring round-trip integrity for any input;
alternatively (if you prefer validation) update FromPks to reject/return error
when any PK contains Separator. Update the implementations of FromPks, ToParts
and any callers that assume raw values, keeping Separator usage only as the
join/split delimiter for encoded parts.
In `@sensor/common/centralproxy/testutils_test.go`:
- Line 62: The test removes the wrapcheck suppression while the production
transport still uses it; restore consistency by adding the same
//nolint:wrapcheck comment to the return of m.base.RoundTrip(reqCopy) in
sensor/common/centralproxy/testutils_test.go (the RoundTrip wrapper that calls
m.base.RoundTrip) so the test matches the production transport behavior.
In `@sensor/kubernetes/fake/flows_test.go`:
- Line 1: The test file flows_test.go is gated by the build tag "//go:build
fakeworkloads" so CI never runs it; either add GOTAGS=fakeworkloads to the
unit-tests matrix in .github/workflows/unit-tests.yaml so these tests run in CI,
or remove the "//go:build fakeworkloads" tag (and any matching "+build" lines)
from sensor/kubernetes/fake/flows_test.go to make the tests run under the
existing GOTAGS; update whichever option you choose and ensure the rest of the
fake workload test files use the same approach.
In `@sensor/kubernetes/fake/virtualmachines_test.go`:
- Line 1: The test file virtualmachines_test.go is build-tagged with //go:build
fakeworkloads and is never executed in CI; update CI to run these tests by
either adding a new CI step that executes go test -tags fakeworkloads
./sensor/kubernetes/fake/... or modify the existing go-unit-tests target to
include GOTAGS=fakeworkloads so the sensor/kubernetes/fake package is tested,
alternatively add a CI entry to run the go test for that package path
specifically or document why the fakeworkloads-tagged tests are intentionally
excluded.
---
Nitpick comments:
In `@pkg/booleanpolicy/value_regex.go`:
- Around line 15-17: There is a duplicated constant: remove the inline const
signatureIntegrationIDPrefix from pkg/booleanpolicy/value_regex.go and
centralize the value into a new lightweight shared package (e.g., a small pkg
with exported name SignatureIntegrationIDPrefix) so both callers use the same
symbol; update references in pkg/booleanpolicy/value_regex.go and
central/signatureintegration/datastore/validate.go to import that lightweight
package and use the exported SignatureIntegrationIDPrefix (ensure the new
package has no heavy init that would reintroduce the cosign/rekor import).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bf54ead-ab23-4fb9-8819-30b08739f814
📒 Files selected for processing (35)
pkg/booleanpolicy/field_metadata.gopkg/booleanpolicy/matcher.gopkg/booleanpolicy/validate_test.gopkg/booleanpolicy/value_regex.gopkg/branding/brandedlogo.gopkg/cloudproviders/aws/certs.gopkg/cloudproviders/aws/metadata.gopkg/postgres/id/id.gopkg/printers/table/table.gopkg/printers/table/table_test.gopkg/probeupload/names.gopkg/sync/common_aliases.goroxctl/common/printer/tabularprinter_factory.gosensor/common/centralproxy/testutils_test.gosensor/kubernetes/fake/common.gosensor/kubernetes/fake/common_test.gosensor/kubernetes/fake/deployment.gosensor/kubernetes/fake/fake.gosensor/kubernetes/fake/flows.gosensor/kubernetes/fake/flows_test.gosensor/kubernetes/fake/labels.gosensor/kubernetes/fake/namespace.gosensor/kubernetes/fake/networkpolicy.gosensor/kubernetes/fake/nodes.gosensor/kubernetes/fake/openshift.gosensor/kubernetes/fake/pebble.gosensor/kubernetes/fake/pools.gosensor/kubernetes/fake/process.gosensor/kubernetes/fake/rbac.gosensor/kubernetes/fake/secret.gosensor/kubernetes/fake/service.gosensor/kubernetes/fake/stub.gosensor/kubernetes/fake/virtualmachines.gosensor/kubernetes/fake/virtualmachines_test.gosensor/kubernetes/fake/workload.go
| type lazyFactory struct { | ||
| once sync.Once | ||
| meta *pathutil.AugmentedObjMeta | ||
| val evaluator.Factory | ||
| } | ||
|
|
||
| func newLazyFactory(meta *pathutil.AugmentedObjMeta) lazyFactory { | ||
| return lazyFactory{meta: meta} | ||
| } | ||
|
|
||
| func (f *lazyFactory) get() *evaluator.Factory { | ||
| f.once.Do(func() { f.val = evaluator.MustCreateNewFactory(f.meta) }) | ||
| return &f.val | ||
| } | ||
|
|
||
| func (f *lazyFactory) GenerateEvaluator(q *query.Query) (evaluator.Evaluator, error) { | ||
| return f.get().GenerateEvaluator(q) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting evaluator.Factory construction and use..."
rg -n -C3 'type Factory struct|func .*MustCreateNewFactory|func .*CreateNewFactory|func .*GenerateEvaluator' pkg/booleanpolicy/evaluator
echo
echo "Looking for assumptions that zero-value Factory is valid or invalid..."
rg -n -C3 'panic|nil|initialized|zero value|GenerateEvaluator' pkg/booleanpolicy/evaluatorRepository: stackrox/stackrox
Length of output: 50374
🏁 Script executed:
#!/bin/bash
# Check Go version in the repository
go version 2>/dev/null || echo "Go version check not available"
# Look for any error handling around lazyFactory usage
rg -n "lazyFactory" pkg/booleanpolicy/
# Check if there's error handling for Factory creation or use
rg -n -A5 "MustCreateNewFactory" pkg/booleanpolicy/ | head -50
# Check if Factory fields are accessed in a way that requires initialization
rg -n "\.fieldToMetaPaths|\.rootType" pkg/booleanpolicy/evaluator/evaluator.goRepository: stackrox/stackrox
Length of output: 2295
🏁 Script executed:
#!/bin/bash
# Let's verify Go 1.26's sync.Once behavior with panic
# Check the Go source/documentation or create a small test
cat > /tmp/test_once_panic.go << 'EOF'
package main
import (
"fmt"
"sync"
)
var once sync.Once
var counter = 0
func main() {
// First call - should panic
fmt.Println("Call 1:")
func() {
defer func() {
if r := recover(); r != nil {
fmt.Println(" Caught panic:", r)
}
}()
once.Do(func() {
counter++
panic("test panic")
})
}()
// Second call - should retry if Once wasn't marked done
fmt.Println("Call 2:")
func() {
defer func() {
if r := recover(); r != nil {
fmt.Println(" Caught panic:", r)
}
}()
once.Do(func() {
counter++
fmt.Println(" Executing once.Do again, counter:", counter)
})
}()
fmt.Println("Final counter:", counter)
fmt.Println("If counter is 2, Once was NOT marked done on panic")
fmt.Println("If counter is 1, Once WAS marked done on panic")
}
EOF
go run /tmp/test_once_panic.go 2>&1Repository: stackrox/stackrox
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Check what errors NewFactory can return
rg -n -A10 "^func NewFactory" pkg/booleanpolicy/evaluator/evaluator.go
# Check what happens when fieldToMetaPaths is nil
rg -n -B2 -A2 "fieldToMetaPaths\.Get" pkg/booleanpolicy/evaluator/Repository: stackrox/stackrox
Length of output: 990
🏁 Script executed:
#!/bin/bash
# Check what utils.CrashOnError does
rg -n -A5 "func CrashOnError"
# Verify that accessing nil.Get panics or errors
cat > /tmp/test_nil_map.go << 'EOF'
package main
import (
"fmt"
)
type MetaPathMap struct {
underlying map[string]interface{}
}
func (m *MetaPathMap) Get(key string) (interface{}, bool) {
if m == nil {
panic("cannot call Get on nil MetaPathMap")
}
val, found := m.underlying[key]
return val, found
}
func main() {
var nilMap *MetaPathMap
defer func() {
if r := recover(); r != nil {
fmt.Println("Panic caught:", r)
}
}()
nilMap.Get("test")
}
EOF
go run /tmp/test_nil_map.go 2>&1Repository: stackrox/stackrox
Length of output: 367
Use sync.OnceValue to preserve fail-fast initialization semantics.
sync.Once has a subtle issue in Go 1.26: when the initializer panics, the Once is still marked done, leaving f.val zero-valued. Subsequent calls then return this uninitialized Factory, which panics later when accessing nil fieldToMetaPaths, masking the original initialization failure. sync.OnceValue[evaluator.Factory] avoids this by not marking done on panic—retrying on each call until successful. This preserves fail-fast behavior and prevents silent initialization failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/booleanpolicy/matcher.go` around lines 15 - 32, Replace the sync.Once +
stored evaluator.Factory pattern with sync.OnceValue to avoid marking done on
panic: change lazyFactory to hold val sync.OnceValue[evaluator.Factory] (remove
once and the plain val field), update get() to call f.val.Do(func()
evaluator.Factory { return evaluator.MustCreateNewFactory(f.meta) }) and return
a pointer to the resulting evaluator.Factory, and ensure GenerateEvaluator uses
that get() result; keep newLazyFactory and lazyFactory.GenerateEvaluator logic
but adapt to the OnceValue API so initialization is retried on panic rather than
leaving a zero-valued Factory.
| func FromPks(pks []string) string { | ||
| return strings.Join(pks, Separator) | ||
| } | ||
|
|
||
| // ToParts splits a composite ID into its primary key parts. | ||
| func ToParts(id string) []string { | ||
| return strings.Split(id, Separator) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate all call sites of FromPks/ToParts:"
rg -n --type=go '\b(FromPks|ToParts)\s*\('
echo
echo "2) For each call site, inspect nearby PK sources/validators for separator constraints:"
rg -n --type=go -C3 'FromPks\(|ToParts\(|PrimaryKey|primary key|pk|ID|Id'
echo
echo "3) Find any existing invariant checks that disallow '#':"
rg -n --type=go 'Contains\(.+Separator|Contains\(.+"#"'Repository: stackrox/stackrox
Length of output: 50376
🏁 Script executed:
cat -n pkg/postgres/id/id.goRepository: stackrox/stackrox
Length of output: 890
🏁 Script executed:
# Search for imports of the id package to confirm no usage
rg -n "pkg/postgres/id|from github.com/stackrox/rox/pkg/postgres/id" --type=goRepository: stackrox/stackrox
Length of output: 43
🏁 Script executed:
# Search for any imports of the id package more broadly
rg -n 'import.*".*postgres/id"' --type=goRepository: stackrox/stackrox
Length of output: 43
Composite ID encoding lacks escape handling for PK values containing #.
The FromPks and ToParts functions will silently corrupt IDs if any primary key contains the separator. For example: FromPks(["a#b", "c"]) produces "a#b#c", which ToParts splits back to ["a", "b", "c"] instead of ["a#b", "c"]. This breaks round-trip integrity and record addressing.
Implement a safe encoding strategy (e.g., URL encoding, base64, length-prefixed format) before joining, or validate/reject PKs containing the separator at the creation boundary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/postgres/id/id.go` around lines 13 - 20, FromPks and ToParts currently
join/split raw PK strings with Separator and will corrupt values that contain
the separator (e.g., "a#b"). Fix by making FromPks encode each PK before joining
and have ToParts decode each part after splitting; for example, apply a
reversible safe encoding such as URL-safe base64 (base64url without padding) or
percent-encoding to each element in FromPks and decode in ToParts, ensuring
round-trip integrity for any input; alternatively (if you prefer validation)
update FromPks to reject/return error when any PK contains Separator. Update the
implementations of FromPks, ToParts and any callers that assume raw values,
keeping Separator usage only as the join/split delimiter for encoded parts.
| reqCopy := req.Clone(req.Context()) | ||
| reqCopy.Header.Set("Authorization", fmt.Sprintf("Bearer %s", m.token)) | ||
| return m.base.RoundTrip(reqCopy) //nolint:wrapcheck | ||
| return m.base.RoundTrip(reqCopy) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if wrapcheck linter is configured and would flag this line
# Check if golangci-lint config exists and includes wrapcheck
if [ -f .golangci.yml ] || [ -f .golangci.yaml ]; then
echo "=== Checking for wrapcheck linter configuration ==="
rg -i "wrapcheck" .golangci.yml .golangci.yaml 2>/dev/null || echo "wrapcheck not found in config"
fi
# Search for other wrapcheck suppressions in the codebase to understand the pattern
echo ""
echo "=== Other wrapcheck suppressions in codebase ==="
rg "nolint:wrapcheck" --type go -C 2Repository: stackrox/stackrox
Length of output: 5174
Align suppression comment usage with production code.
The //nolint:wrapcheck suppression was removed from this line, but the identical pattern in the production version (sensor/common/centralproxy/transport.go line ~113) still retains it. Since wrapcheck is configured and enabled in .golangci.yml, either the test code should keep the suppression to match production, or the production code should be updated to align if the suppression is no longer needed. Ensure consistency across the transport and test utilities.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sensor/common/centralproxy/testutils_test.go` at line 62, The test removes
the wrapcheck suppression while the production transport still uses it; restore
consistency by adding the same //nolint:wrapcheck comment to the return of
m.base.RoundTrip(reqCopy) in sensor/common/centralproxy/testutils_test.go (the
RoundTrip wrapper that calls m.base.RoundTrip) so the test matches the
production transport behavior.
| @@ -1,3 +1,5 @@ | |||
| //go:build fakeworkloads | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether repository automation references fakeworkloads-tagged test execution.
# Expected result: at least one workflow/script/Make target includes go test with fakeworkloads.
rg -n -C2 -P 'fakeworkloads|go\s+test[^\n]*-tags(?:=|\s+)[^\n]*fakeworkloads'Repository: stackrox/stackrox
Length of output: 4340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find CI configuration files
echo "=== Looking for CI configuration files ==="
find . -type f \( -name "Makefile" -o -name "*.yml" -o -name "*.yaml" -o -name ".circleci" \) | head -20
# Search GitHub workflows specifically
echo "=== GitHub workflow files ==="
find .github/workflows -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -20
# Look for Makefile targets related to testing
echo "=== Makefile test targets ==="
fd -t f "Makefile" | xargs grep -l "test" 2>/dev/null | head -5Repository: stackrox/stackrox
Length of output: 983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search GitHub workflows for fakeworkloads tag in test execution
echo "=== Searching GitHub workflows for fakeworkloads ==="
grep -r "fakeworkloads" .github/workflows/ 2>/dev/null || echo "No matches found in workflows"
# Search for go test commands in workflows
echo -e "\n=== Searching for 'go test' patterns in workflows ==="
grep -r "go.*test" .github/workflows/ -A2 -B2 2>/dev/null | head -100
# Check build.yaml specifically
echo -e "\n=== Checking build.yaml for test targets ==="
cat .github/workflows/build.yaml | grep -A10 "test" | head -50Repository: stackrox/stackrox
Length of output: 7422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full unit-tests.yaml to see matrix definition
echo "=== Full unit-tests.yaml (first 200 lines) ==="
head -200 .github/workflows/unit-tests.yaml
# Look for Makefile targets related to testing
echo -e "\n=== Makefile go-unit-tests target ==="
grep -A20 "^go-unit-tests" Makefile || echo "Not found in Makefile root"
# Search for Makefile in repository
echo -e "\n=== Finding Makefile files ==="
find . -name "Makefile" -type f | head -5Repository: stackrox/stackrox
Length of output: 8000
Add fakeworkloads to CI test matrix or remove the build tag.
The //go:build fakeworkloads tag gates flows_test.go and related tests, but the CI matrix in .github/workflows/unit-tests.yaml only runs with GOTAGS="" and GOTAGS=release. Add GOTAGS=fakeworkloads to the matrix to exercise these tests in CI, or remove the build tag if they're meant for local development only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sensor/kubernetes/fake/flows_test.go` at line 1, The test file flows_test.go
is gated by the build tag "//go:build fakeworkloads" so CI never runs it; either
add GOTAGS=fakeworkloads to the unit-tests matrix in
.github/workflows/unit-tests.yaml so these tests run in CI, or remove the
"//go:build fakeworkloads" tag (and any matching "+build" lines) from
sensor/kubernetes/fake/flows_test.go to make the tests run under the existing
GOTAGS; update whichever option you choose and ensure the rest of the fake
workload test files use the same approach.
| @@ -1,3 +1,5 @@ | |||
| //go:build fakeworkloads | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for fakeworkloads-tagged Go invocations in CI/build config.
rg -n --hidden \
--glob '.github/workflows/**' \
--glob 'Makefile*' \
--glob '**/*.mk' \
--glob '**/Jenkinsfile*' \
--glob '**/*.yaml' \
--glob '**/*.yml' \
'go (test|build).*(-tags[ =][^\\n]*fakeworkloads)|fakeworkloads'Repository: stackrox/stackrox
Length of output: 43
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. Search for any Go test invocations in CI/build files (broader)
echo "=== Go test commands in CI/build config ==="
rg -n --hidden \
--glob '.github/**' \
--glob 'Makefile*' \
--glob '**/*.mk' \
'go test' \
| head -20
# 2. Search for any mention of fakeworkloads anywhere in the repo
echo -e "\n=== All mentions of fakeworkloads ==="
rg -n --type go 'fakeworkloads' | head -30
# 3. Check if virtualmachines_test.go file exists and is newly tagged
echo -e "\n=== virtualmachines_test.go file check ==="
find . -name 'virtualmachines_test.go' -type f 2>/dev/null | head -5
# 4. Look for any CI build/test steps that run without tag restrictions
echo -e "\n=== CI files present ==="
find .github/workflows -type f -name '*.yaml' -o -name '*.yml' 2>/dev/null | head -10Repository: stackrox/stackrox
Length of output: 3714
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. Search for sensor package test invocations in CI
echo "=== Sensor package tests in CI ==="
rg -n --hidden \
--glob '.github/**' \
'sensor' | grep -i 'test\|build' | head -20
# 2. Check git history for this specific file to see if tag is new or pre-existing
echo -e "\n=== Git log for virtualmachines_test.go (limited) ==="
git log --oneline sensor/kubernetes/fake/virtualmachines_test.go 2>/dev/null | head -10 || echo "Git history unavailable or file newly added"
# 3. Check if stub pattern was already present (examine stub.go more closely)
echo -e "\n=== Stub implementation pattern ==="
cat -n sensor/kubernetes/fake/stub.go | head -30
# 4. Look for build constraints in other test files in same package
echo -e "\n=== Build constraints in sensor/kubernetes/fake/ ==="
rg -n '^//' sensor/kubernetes/fake/ | grep -E 'build|go:' | head -20Repository: stackrox/stackrox
Length of output: 5552
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the sensor-integration-test target in Makefile
echo "=== sensor-integration-test Makefile target ==="
rg -A 10 'sensor-integration-test:' Makefile
# Also check if there are any other references to fakeworkloads in build/test scripts
echo -e "\n=== fakeworkloads in scripts or configs (non-Go files) ==="
rg --type sh --type yaml 'fakeworkloads' 2>/dev/null | head -20Repository: stackrox/stackrox
Length of output: 919
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check GOTAGS default value
echo "=== GOTAGS definition in Makefile ==="
rg -B 2 -A 2 'GOTAGS\s*=' Makefile | head -30
# Check if any CI command runs sensor/kubernetes/fake tests
echo -e "\n=== CI test paths ==="
rg 'go test.*sensor' .github/workflows/*.yaml | head -20
# Check the actual go-test.sh script to see how it handles tags
echo -e "\n=== go-test.sh script ==="
if [ -f scripts/go-test.sh ]; then
head -50 scripts/go-test.sh
fiRepository: stackrox/stackrox
Length of output: 2453
Ensure fakeworkloads-tagged tests run in CI.
This test file is excluded from default go test ./... runs due to the //go:build fakeworkloads constraint. The go-unit-tests target discovers this test file but skips it because GOTAGS does not include fakeworkloads, and the sensor-integration-test runs a different test path (./sensor/tests/...). Without explicit CI coverage, this test codepath remains unexecuted.
Add a CI step or extend an existing one to run tests with -tags fakeworkloads for the sensor/kubernetes/fake/... package, or document why this test is intentionally excluded from coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sensor/kubernetes/fake/virtualmachines_test.go` at line 1, The test file
virtualmachines_test.go is build-tagged with //go:build fakeworkloads and is
never executed in CI; update CI to run these tests by either adding a new CI
step that executes go test -tags fakeworkloads ./sensor/kubernetes/fake/... or
modify the existing go-unit-tests target to include GOTAGS=fakeworkloads so the
sensor/kubernetes/fake package is tested, alternatively add a CI entry to run
the go test for that package path specifically or document why the
fakeworkloads-tagged tests are intentionally excluded.
🚀 Build Images ReadyImages are ready for commit dc8d15b. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-660-gdc8d15b4c0 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #20047 +/- ##
==========================================
+ Coverage 49.61% 49.71% +0.09%
==========================================
Files 2765 2749 -16
Lines 208643 206632 -2011
==========================================
- Hits 103520 102727 -793
+ Misses 97467 96258 -1209
+ Partials 7656 7647 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Reduce init-time allocations by deferring computation to first use and excluding test-only code from production binaries.
Changes
sync.OnceValue, lazyevaluator.Factorypkg/cveimport)//go:build fakeworkloadsexcludes fake workload generator from production binary_test.goDependencies
Depends on #20024 (schema lazy loading — provides
pkg/sync.OnceValuewrapper).User-facing documentation
Testing and quality
Automated testing
How I validated my change
go build ./...passesAI-assisted.