Conversation
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: Duck <70207455+entlein@users.noreply.github.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
testing if nodeagent ci runs against fork
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
…e storage module ; ; Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
Signed-off-by: entlein <einentlein@gmail.com>
The local `replace github.com/kubescape/storage => ../storage` was accidentally committed. This restores go.mod to the clean upstream state. go.sum was already correct (no net change from the two prior fixup commits). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: entlein <einentlein@gmail.com>
Several upstream modules (anchore/go-macholibre, charmbracelet/x/ansi) have stale hashes in go.sum that don't match current downloads. Deleting go.sum entirely lets CI regenerate it fresh with correct hashes via `go mod tidy`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # go.mod # go.sum
Signed-off-by: entlein <einentlein@gmail.com>
Add profile state to cache from fetched profile annotations Signed-off-by: Duck <70207455+entlein@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request introduces a new KubeletTLS tracer feature for monitoring plaintext TLS traffic from kubelet using eBPF probes, alongside a migration from Quay.io to GitHub Container Registry across CI/CD workflows. It includes eBPF gadget implementation, Go tracer code, event system updates, rule adapters, and associated configuration changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Tracer as KubeletTLSTracer
participant GadgetCtx as Gadget Context
participant eBPF as eBPF Probes
participant DataOp as Data Operators
participant EventCB as Event Callback
participant RuleEngine as Rule Engine
Tracer->>GadgetCtx: Start(ctx)
GadgetCtx->>DataOp: Initialize with kubeManager, OciHandler, eventOperator
GadgetCtx->>eBPF: Launch gadget async (kubelet_tls:latest)
eBPF->>eBPF: Monitor crypto/tls.(*Conn).Write/Read
eBPF->>eBPF: Parse Go stack arguments (is_register_abi)
eBPF->>eBPF: Filter ApplicationData records
eBPF->>eBPF: Capture TLS data (up to 16 KiB)
eBPF->>DataOp: Emit DataSource/Data events
DataOp->>EventCB: Forward as DatasourceEvent with KubeletTLSEventType
EventCB->>RuleEngine: Invoke external callback with container/process IDs
RuleEngine->>RuleEngine: Evaluate KubeletTLS rules via adapters
Tracer->>GadgetCtx: Stop()
GadgetCtx->>eBPF: Cancel context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/incluster-comp-pr-merged.yaml (1)
339-357:⚠️ Potential issue | 🟠 Major
create-release-and-retagjob is missingpackages: writepermission forghcr.io.The
docker-buildjob (line 64) correctly declarespackages: write, butcreate-release-and-retagonly declarescontents: write(line 346). Theregctl image copycommand at line 365 retagging toghcr.iowill fail with an authorization error withoutpackages: write.🔧 Proposed fix
permissions: contents: write + packages: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/incluster-comp-pr-merged.yaml around lines 339 - 357, The create-release-and-retag job lacks the packages write permission needed to push/retag images to ghcr.io; update the job's permissions block in the create-release-and-retag job (referenced by name "create-release-and-retag") to include packages: write alongside contents: write so the regctl image copy step (the retag to ghcr.io) can authenticate and succeed..github/workflows/component-tests.yaml (2)
45-45:⚠️ Potential issue | 🟡 MinorStep name still references "Quay.io" but now pushes to ghcr.io.
The step name
Build the Image and Push to Quay.iois stale after the registry migration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/component-tests.yaml at line 45, Update the GitHub Actions step name string "Build the Image and Push to Quay.io" to accurately reflect the current registry by renaming it to mention ghcr.io (e.g., "Build the Image and Push to ghcr.io") so the step description matches the action performed; locate the step that currently uses the exact string "Build the Image and Push to Quay.io" and change only the human-readable name field.
114-122:⚠️ Potential issue | 🟠 MajorComponent tests ignore the built image and always pull
latestfromk8sstormcenter.The
component-testsjob hardcodesIMAGE_TAG="latest"andIMAGE_REPO='ghcr.io/k8sstormcenter/node-agent'instead of consuming the outputs from thebuild-and-push-imagejob. This means even when a new image is built, tests always run against a stalelatesttag. If this is intentional for fork testing, consider adding a comment explaining why and how to test with a freshly built image.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/component-tests.yaml around lines 114 - 122, The workflow step "Install Node Agent Chart" currently hardcodes IMAGE_TAG="latest" and IMAGE_REPO='ghcr.io/k8sstormcenter/node-agent', causing component-tests to ignore the image produced by the build-and-push-image job; update the step to consume the outputs of the build-and-push-image job (replace the hardcoded IMAGE_TAG and IMAGE_REPO with the job outputs/outputs.* or job-level outputs secrets passed via needs.build-and-push-image.outputs.<name>) so Helm uses the freshly built image, or if intentional keep the hardcoded values but add a clear comment explaining fork/testing behavior; specifically change the variables referenced in this step (IMAGE_TAG, IMAGE_REPO) to use the workflow output names produced by build-and-push-image.
🟡 Minor comments (10)
tests/scripts/storage-tag.sh-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorInvalid shebang — missing
!.
#/bin/bashis treated as a comment, not a shebang directive; some shells and executors will not invoke Bash for this script.🔧 Proposed fix
-#/bin/bash +#!/bin/bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/scripts/storage-tag.sh` at line 1, The shebang in tests/scripts/storage-tag.sh is incorrect ("#/bin/bash"); replace it with a valid shebang ("#!/bin/bash") on the first line so the script is executed with Bash, and re-save the file (also ensure the script remains executable if required by CI). Target the top of tests/scripts/storage-tag.sh where the current shebang appears.pkg/ebpf/gadgets/kubelet_tls/gadget.yaml-3-5 (1)
3-5:⚠️ Potential issue | 🟡 MinorGadget metadata URLs point to the fork (
k8sstormcenter) instead of the canonicalkubescapeorg.All three URL fields reference the author's personal fork and will redirect users and tooling to the wrong repository.
🔧 Proposed fix
-homepageURL: https://github.com/k8sstormcenter/node-agent -documentationURL: https://github.com/k8sstormcenter/node-agent/tree/main/pkg/ebpf/gadgets/kubelet_tls -sourceURL: https://github.com/k8sstormcenter/node-agent/tree/main/pkg/ebpf/gadgets/kubelet_tls +homepageURL: https://github.com/kubescape/node-agent +documentationURL: https://github.com/kubescape/node-agent/tree/main/pkg/ebpf/gadgets/kubelet_tls +sourceURL: https://github.com/kubescape/node-agent/tree/main/pkg/ebpf/gadgets/kubelet_tls🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/kubelet_tls/gadget.yaml` around lines 3 - 5, The gadget metadata fields homepageURL, documentationURL, and sourceURL currently point to the forked repo (k8sstormcenter); update these three YAML values to the canonical kubescape organization repository URLs (replace "https://github.com/k8sstormcenter/node-agent" and the two tree links with the equivalent "https://github.com/kubescape/node-agent" and the corresponding documentation/source paths) so tooling and users are directed to the upstream kubescape/node-agent locations.pkg/ebpf/gadgets/kubelet_tls/program.bpf.c-19-19 (1)
19-19:⚠️ Potential issue | 🟡 MinorWrap macro value in parentheses to avoid operator-precedence surprises.
#define MAX_DATA_SIZE_OPENSSL 1024 * 16can misexpand in certain expressions. Standard practice is to parenthesize.Proposed fix
-#define MAX_DATA_SIZE_OPENSSL 1024 * 16 +#define MAX_DATA_SIZE_OPENSSL (1024 * 16)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/kubelet_tls/program.bpf.c` at line 19, The macro MAX_DATA_SIZE_OPENSSL should be wrapped in parentheses to prevent operator-precedence surprises during expansion; update the definition of MAX_DATA_SIZE_OPENSSL so its value is parenthesized (e.g. change the current definition of MAX_DATA_SIZE_OPENSSL to use parentheses around the arithmetic expression) so any usage like multiplications or additions involving MAX_DATA_SIZE_OPENSSL expands safely.pkg/ebpf/gadgets/kubelet_tls/program.bpf.c-122-124 (1)
122-124:⚠️ Potential issue | 🟡 MinorPointer null-check uses
<=comparison.
if (str <= 0)compares a pointer against0using<=, which is non-idiomatic and technically compares as a signed value in some eBPF verifier contexts. Use a standard null check.Proposed fix
- if (str <= 0) { + if (!str) { return 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/kubelet_tls/program.bpf.c` around lines 122 - 124, The null-pointer check for the pointer variable `str` currently uses a non-idiomatic `if (str <= 0)` comparison; update this to a proper null check (e.g., `if (!str)` or `if (str == NULL)`) in `program.bpf.c` where `str` is validated so the eBPF verifier treats it correctly and the intent is clear—replace the `if (str <= 0) { return 0; }` with a standard null-pointer check referencing the same `str` symbol.pkg/ebpf/gadgets/kubelet_tls/Makefile-4-8 (1)
4-8:⚠️ Potential issue | 🟡 MinorInconsistent
TMPDIRusage betweenbuildandbuild-with-metadata.
buildpassesTMPDIR=$$TMPDIRto theig image buildcommand butbuild-with-metadatadoes not. If TMPDIR is needed for build correctness (e.g., space or permission constraints), this omission could cause failures in the metadata build path.Proposed fix
build-with-metadata: - sudo ig image build -t $(IMAGE):$(TAG) . --update-metadata + sudo TMPDIR=$$TMPDIR ig image build -t $(IMAGE):$(TAG) . --update-metadata🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/kubelet_tls/Makefile` around lines 4 - 8, The Makefile has inconsistent TMPDIR usage: the build target sets TMPDIR=$$TMPDIR for the sudo ig image build command but build-with-metadata omits it; update the build-with-metadata target (and ensure syntax matches the build target) to export TMPDIR=$$TMPDIR before calling sudo ig image build -t $(IMAGE):$(TAG) . --update-metadata so both targets use the same TMPDIR environment for correct builds..github/workflows/component-tests.yaml-18-20 (1)
18-20:⚠️ Potential issue | 🟡 MinorBoolean input comparison may not work as expected.
github.event.inputs.build_imageis a string ("true"/"false"), not a native boolean. Comparing with== true(no quotes) may silently fail. Useinputs.build_imagecontext instead, which handles type coercion correctly forworkflow_dispatch.Proposed fix
- if: github.event_name == 'workflow_dispatch' && github.event.inputs.build_image == true + if: github.event_name == 'workflow_dispatch' && inputs.build_image == true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/component-tests.yaml around lines 18 - 20, The job-level condition for build-and-push-image compares github.event.inputs.build_image to a boolean and will fail because inputs are strings; update the job's if expression (the if on the build-and-push-image job) to use the workflow inputs context instead and rely on its coercion (e.g., use the inputs.build_image expression form) so the check reads the declared workflow_dispatch input correctly rather than comparing to a raw boolean..github/workflows/build.yaml-99-104 (1)
99-104:⚠️ Potential issue | 🟡 MinorQuote glob patterns to prevent shell expansion (SC2125).
The
*inGONOSUMDBandGONOSUMCHECKassignments can be expanded by the shell. Also,github.com/matthyx/*is again a hardcoded personal GitHub reference.Proposed fix
- GONOSUMDB=github.com/k8sstormcenter/*,github.com/matthyx/* GONOSUMCHECK=github.com/k8sstormcenter/*,github.com/matthyx/* go mod tidy + GONOSUMDB="github.com/k8sstormcenter/*,github.com/matthyx/*" GONOSUMCHECK="github.com/k8sstormcenter/*,github.com/matthyx/*" go mod tidy🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yaml around lines 99 - 104, The shell glob in the workflow step is unquoted causing expansion for GONOSUMDB and GONOSUMCHECK and hardcodes a personal repo; update the run block to quote the glob patterns and avoid the literal personal reference by using the REPO_OWNER variable: change the environment assignments for GONOSUMDB and GONOSUMCHECK to use quoted strings (e.g., "github.com/${REPO_OWNER}/*") and ensure any other literal "github.com/matthyx/*" is replaced with the appropriate variable so that go mod tidy sees the patterns as literals rather than being expanded by the shell.tests/component_test.go-1630-1631 (1)
1630-1631:⚠️ Potential issue | 🟡 MinorErrors from
ExecIntoPodare silently discarded.Both exec calls discard all return values. At minimum, the first (
cat /etc/nginx/nginx.conf) is expected to succeed and should be checked. The second (cat /etc/hostname) may also succeed (the file exists in most containers), even though it should trigger an alert.Proposed fix
- _, _, _ = wl.ExecIntoPod([]string{"cat", "/etc/nginx/nginx.conf"}, "nginx") // Should be allowed by regex - _, _, _ = wl.ExecIntoPod([]string{"cat", "/etc/hostname"}, "nginx") // Should be blocked (alert) + _, _, err = wl.ExecIntoPod([]string{"cat", "/etc/nginx/nginx.conf"}, "nginx") // Should be allowed by regex + require.NoError(t, err, "Failed to cat nginx.conf") + _, _, err = wl.ExecIntoPod([]string{"cat", "/etc/hostname"}, "nginx") // Should be blocked (alert) + require.NoError(t, err, "Failed to cat hostname")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/component_test.go` around lines 1630 - 1631, The test currently discards all return values from wl.ExecIntoPod; update the two calls to capture and assert on their results: call ExecIntoPod in the test, assign its returned stdout, stderr and error (from wl.ExecIntoPod), then assert the first call (ExecIntoPod([... "cat", "/etc/nginx/nginx.conf"], "nginx")) returned no error and stdout contains expected nginx config content; for the second call (ExecIntoPod([... "cat", "/etc/hostname"], "nginx")) assert the call either returned an error or produced an alert condition (depending on how alerts are surfaced in the test harness) — e.g., check stderr/exit error or the test alerting mechanism — so failures are surfaced instead of being silently ignored.Makefile-35-38 (1)
35-38:⚠️ Potential issue | 🟡 Minor
go.modleft dirty if tests fail.If
go teston line 37 exits non-zero, Make will abort andunlocal(line 38) will never execute, leavinggo.modwith the local storage replace directive. Consider using a trap or restructuring:Proposed fix
.PHONY: test -test: local - go test ./pkg/rulemanager/cel/libraries/applicationprofile/... -v -count=1 - @$(MAKE) unlocal +test: local + go test ./pkg/rulemanager/cel/libraries/applicationprofile/... -v -count=1; \ + status=$$?; \ + $(MAKE) unlocal; \ + exit $$status🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 35 - 38, The test target leaves go.mod dirty if go test fails because Make aborts before running the unlocal target; update the test target so it always invokes the unlocal target regardless of go test exit status (capture the go test exit code, run $(MAKE) unlocal, then exit with the original code). Modify the Makefile's test target (the rule named "test" that depends on "local" and currently runs go test and "@$(MAKE) unlocal") to run go test in a shell that preserves the exit status and guarantees calling the "unlocal" target even on failure.tests/chart/templates/node-agent/default-rules.yaml-591-592 (1)
591-592:⚠️ Potential issue | 🟡 Minor
uniqueIdmay over-deduplicate distinct exec requests.The
uniqueIdisevent.comm + '_' + string(event.tlsEventType), which means all kubelet TLS events from the same process with the same direction (read/write) will share the same unique ID. If multiple distinct exec requests are intercepted (e.g., different commands), they'll be treated as duplicate alerts. Consider incorporatingevent.tlsDataor a hash/substring of it intouniqueIdto distinguish distinct exec payloads.For comparison, other rules use more granular identifiers (e.g., R0001 uses
event.comm + '_' + event.exepath).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/chart/templates/node-agent/default-rules.yaml` around lines 591 - 592, The uniqueId currently uses only event.comm and event.tlsEventType which will collapse distinct kubelet TLS exec payloads into one alert; update the uniqueId for this rule to include event.tlsData (or a stable hash/substring of event.tlsData) in addition to event.comm and event.tlsEventType so different exec requests produce distinct IDs—modify the uniqueId expression referencing uniqueId, event.tlsData, event.tlsEventType, and event.comm to include the extra payload-based discriminator.
🧹 Nitpick comments (7)
pkg/objectcache/applicationprofilecache/applicationprofilecache.go (1)
534-542: LGTM — correctly aligns user-defined profile path with existing state-tracking pattern.The new block mirrors what
updateAllProfilesdoes at lines 180–186 andhandleUserManagedProfiledoes at lines 333–339, soworkloadIDToProfileStateis now consistently populated across all three profile-fetch paths. The nil state written at line 497 for new/pre-running containers is properly replaced here on a successful fetch.One optional hardening:
fullProfileis not nil-guarded before its fields are read at lines 537–538. A storage client returning(nil, nil)would panic here (same latent risk already exists at line 534 and line 247 inupdateAllProfiles).🛡️ Optional nil guard
apc.workloadIDToProfile.Set(workloadID, fullProfile) +if fullProfile != nil { // Set the profile state from the fetched profile's annotations profileState := &objectcache.ProfileState{ Completion: fullProfile.Annotations[helpersv1.CompletionMetadataKey], Status: fullProfile.Annotations[helpersv1.StatusMetadataKey], Name: fullProfile.Name, Error: nil, } apc.workloadIDToProfileState.Set(workloadID, profileState) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/objectcache/applicationprofilecache/applicationprofilecache.go` around lines 534 - 542, The new block assumes fullProfile is non-nil before reading its fields; add a nil-guard to avoid panics from a storage client returning (nil, nil): check if fullProfile == nil and handle it (e.g., log/warn and skip setting apc.workloadIDToProfile.Set and apc.workloadIDToProfileState.Set or set an appropriate error state) before accessing fullProfile.Annotations or fullProfile.Name; apply the same pattern used in updateAllProfiles and handleUserManagedProfile around the apc.workloadIDToProfile / apc.workloadIDToProfileState updates to keep behavior consistent.pkg/rulemanager/cel/libraries/applicationprofile/open_test.go (1)
150-190:TestOpenWithWildcardProfilehas no negative (false) test case.All 7 test cases assert
expectedResult: true. An implementation that unconditionally returnstruewould satisfy all of them. Even a single negative case — e.g., a relative path"relative/path"or""that the profile does not cover — would meaningfully raise the bar. Consider adding at least one such case.✅ Example negative case
+ { + name: "Relative path does not match wildcard profile", + path: "relative/path", + expectedResult: false, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/rulemanager/cel/libraries/applicationprofile/open_test.go` around lines 150 - 190, TestOpenWithWildcardProfile's testCases slice only contains positive expectations so a buggy implementation that always returns true would pass; update the testCases in TestOpenWithWildcardProfile to include at least one negative case (e.g., name "Relative path not matched", path "relative/path" or empty string "", expectedResult: false) so the test asserts a false outcome and fails implementations that unconditionally return true; modify the testCases variable in open_test.go and ensure the test loop still uses expectedResult to validate the matcher.pkg/ebpf/gadgets/kubelet_tls/Makefile (1)
1-13: Missing.PHONYdeclarations.None of the targets (
build,build-with-metadata,run,build-and-run) are declared.PHONY. If files matching these names ever exist in the directory, Make will consider them up-to-date and skip execution.Proposed fix
+.PHONY: build build-with-metadata run build-and-run + IMAGE?=kubelet_tls TAG?=latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/kubelet_tls/Makefile` around lines 1 - 13, Add a .PHONY declaration for the Makefile targets to ensure they always run: declare .PHONY for build, build-with-metadata, run, and build-and-run (e.g., add a line ".PHONY: build build-with-metadata run build-and-run" near the top of the Makefile) so Make doesn’t treat same-named files as up-to-date; keep the existing target definitions (build, build-with-metadata, run, build-and-run) unchanged.pkg/ebpf/gadgets/kubelet_tls/go_argument.h (1)
15-16: Include guard name is a leftover from the eCapture project.
ECAPTURE_GOTLS_Hdoesn't reflect this project's namespace. Consider renaming to something likeKUBELET_TLS_GO_ARGUMENT_Hto avoid potential collisions and improve clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/ebpf/gadgets/kubelet_tls/go_argument.h` around lines 15 - 16, The include guard ECAPTURE_GOTLS_H is a leftover name and should be renamed to a project-specific guard to avoid collisions; update the macro in go_argument.h (replace ECAPTURE_GOTLS_H) with a clearer name such as KUBELET_TLS_GO_ARGUMENT_H and ensure you change both the `#ifndef/`#define and the corresponding `#endif` comment (if present) so the symbol is consistent across the file..github/workflows/build.yaml (1)
106-109: Hardcodedigversion may drift.The
igtool versionv0.45.0is hardcoded. Consider extracting this to a workflow input or env variable for easier updates, similar to howVERSIONis handled in the Makefile.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yaml around lines 106 - 109, The workflow step "Ensure ig is installed" hardcodes the ig version string ("v0.45.0" and ig_0.45.0_amd64.deb); make this configurable by introducing a workflow input or environment variable (e.g., IG_VERSION) and update the curl URL and downloaded filename to use that variable instead (mirroring how VERSION is used in the Makefile) so future updates only require changing the variable.Makefile (1)
27-28: Hardcoded personal GitHub username inGONOSUMDB/GONOSUMCHECK.
github.com/matthyx/*is a personal GitHub account reference baked into the shared Makefile. This should either be parameterized or use the project's organization namespace. Same issue on line 33.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 27 - 28, The Makefile currently hardcodes a personal GitHub namespace via the GONOSUMDB and GONOSUMCHECK values (e.g., "github.com/matthyx/*"); update these to a parameterized variable (for example GITHUB_ORG or PROJECT_NAMESPACE) and use that variable in both GONOSUMDB and GONOSUMCHECK instead of the literal "github.com/matthyx/*", or replace with the project's organization namespace, ensuring you change every occurrence (including the second occurrence that mirrors this line) so the Makefile no longer embeds a personal username.pkg/containerwatcher/v2/tracers/kubelet_tls.go (1)
52-74:RunGadgeterrors are silently swallowed afterStartreturnsnil.
Startlaunches the gadget in a fire-and-forget goroutine (line 64) and immediately returnsnil. IfRunGadgetfails (e.g., invalid eBPF program, missing permissions), the caller has no way to detect the failure — it only gets logged. Consider adding a short startup probe (e.g., a channel that signals successful init) or at minimum documenting that errors are only logged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/containerwatcher/v2/tracers/kubelet_tls.go` around lines 52 - 74, Start currently launches RunGadget in a fire-and-forget goroutine so errors are only logged; modify KubeletTLSTracer.Start to perform a short startup probe: create a result channel (e.g., errCh) and run runtime.RunGadget inside the goroutine sending nil on success or the error on failure, then after launching wait for either an error, a success signal, context cancellation, or a timeout and return the received error (or context/timeout error) instead of always returning nil; ensure you still log failures inside the goroutine and respect the provided ctx when waiting so callers can detect startup failures of RunGadget.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/component-tests.yaml:
- Around line 86-88: The YAML flow sequence listing test names is missing a
comma between items: add a comma after Test_24_ProcessTreeDepthTest so the
sequence reads "Test_23_RuleCooldownTest, Test_24_ProcessTreeDepthTest,
Test_27_RegexFileOpenMatchTest"; update the sequence that contains
Test_23_RuleCooldownTest, Test_24_ProcessTreeDepthTest, and
Test_27_RegexFileOpenMatchTest to ensure all items are comma-separated to allow
the workflow to parse.
In `@go.mod`:
- Line 463: Remove the local filesystem replace directive `replace
github.com/kubescape/storage => ../storage` from go.mod before merging: either
publish the updated github.com/kubescape/storage module (including
CompareExecArgs) and update the version in go.mod to that new release, or keep
the replace only in a developer-local overlay (e.g., untracked local config) so
it is not committed; ensure the committed go.mod references a resolvable module
version instead of a relative path.
In `@pkg/containerwatcher/v2/tracers/kubelet_tls.go`:
- Around line 3-18: The imports for the operators packages are using a forked
path; update the import paths in kubelet_tls.go so they match the official
module: replace "github.com/dorkamotorka/inspektor-gadget/pkg/operators" with
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators" and replace
"github.com/dorkamotorka/inspektor-gadget/pkg/operators/simple" with
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators/simple" so the
compiler will resolve the packages already declared in go.mod; keep all other
imports unchanged and ensure any references to the operators package (e.g.,
symbols in operators or simple) continue to compile.
In `@pkg/ebpf/gadgets/kubelet_tls/go_argument.h`:
- Around line 74-108: The three functions go_get_argument_by_reg,
go_get_argument_by_stack, and go_get_argument in go_argument.h must be declared
static __always_inline to avoid multiple-definition linkage issues and to
satisfy the eBPF verifier; update their declarations/definitions to prepend
static __always_inline (matching gotls_write/gotls_read style) so they are
inlined into callers and not emitted as external symbols.
In `@pkg/ebpf/gadgets/kubelet_tls/program.bpf.c`:
- Around line 50-57: Remove or guard the noisy bpf_printk debug prints in
program.bpf.c so they don't run in production: locate the bpf_printk calls in
the TLS write and read paths (e.g., inside the gotls_write and gotls_read logic
where messages like "first return", "gotls_write record_type:%d, len:%d", and
the other ~10 printk sites appear) and either delete them or wrap each in a
compile-time guard such as `#ifdef` DEBUG ... `#endif` (or a similarly named macro)
so they are only compiled in when debugging is enabled; ensure all occurrences
noted in the review (including the ones around the early returns and record-type
checks) are handled consistently.
- Line 36: The perf buffer size is incorrectly set to GADGET_TRACER_MAP(events,
4096*4096) which would allocate ~16M entries (multiplied by sizeof(struct
event)) — change the map declaration to a reasonable size (e.g.,
GADGET_TRACER_MAP(events, 1024) or GADGET_TRACER_MAP(events, 4096)) to avoid
huge memory allocation; update the entry count used in the GADGET_TRACER_MAP
macro (referencing the events map and struct event) to one of those realistic
values.
- Around line 74-87: Remove the debug byte-printing loop that iterates with
"#pragma unroll" over event->data and instead restore the bpf_probe_read_user
error handling in the write path: after attempting to read into event->data (the
bpf_probe_read_user/bpf_probe_read_user_str call in the gotls_write handling),
check if ret < 0, call gadget_discard_buf(&event), emit a single bpf_printk
including the ret/error and context (e.g., "gotls_write bpf_probe_read_user_str
failed, ret:%d"), and return 0 to avoid submitting corrupted/partial data; apply
the same removal and error-check re-enabling to the read path handling (the
corresponding code around the read path that mirrors gotls_write).
In `@pkg/rulemanager/cel/libraries/applicationprofile/exec.go`:
- Around line 12-13: The code references dynamicpathdetector.CompareExecArgs
which does not exist in the published kubescape/storage v0.0.239 and will break
builds when the local replace is removed; update exec.go to avoid the missing
symbol by implementing the argument comparison locally (create a local helper
like compareExecArgs that reproduces the intended logic) or by switching to the
existing dynamicpathdetector.CompareDynamic if it provides the needed semantics;
ensure the call sites in exec.go that currently invoke CompareExecArgs are
updated to call the new local helper (or CompareDynamic) and include unit
coverage for the comparison logic so future dependency differences won't break
the build.
In `@tests/chart/values.yaml`:
- Line 35: The image repository values point to a forked registry; update the
values for storage.image.repository and nodeAgent.image.repository to reference
the upstream registry (e.g., ghcr.io/kubescape/...) or make them configurable
via a new parameter (e.g., image.repositoryOverride) so tests don’t depend on
the k8sstormcenter fork; ensure both occurrences (storage.image.repository and
nodeAgent.image.repository) are changed consistently and document the new
parameter if added.
In `@tests/component_test.go`:
- Around line 1636-1639: The assertions use the wrong rule and container names
causing failures; update the two assertions that call
testutils.AssertNotContains and testutils.AssertContains to use the actual rule
"Files Access Anomalies in container" instead of "Unexpected file open" and use
the actual container name "nginx" (not "hostname"), keeping the same alerts
variable and wl.Namespace context so the label-based searches in
testutils.AssertContains/AssertNotContains can find the expected alerts.
In `@tests/scripts/storage-tag.sh`:
- Line 2: The curl in tests/scripts/storage-tag.sh is pointing to a personal
fork URL
(https://raw.githubusercontent.com/bob/test/localtestbuild/kubescape/values.yaml);
replace that with the official upstream raw URL for the Kubescape chart values
(use the kubescape/helm-charts repo and the appropriate branch/tag) so the curl
command fetches the canonical values.yaml for resolving the storage image tag;
update the single curl invocation in the script accordingly and verify the
script still extracts the image tag as before.
---
Outside diff comments:
In @.github/workflows/component-tests.yaml:
- Line 45: Update the GitHub Actions step name string "Build the Image and Push
to Quay.io" to accurately reflect the current registry by renaming it to mention
ghcr.io (e.g., "Build the Image and Push to ghcr.io") so the step description
matches the action performed; locate the step that currently uses the exact
string "Build the Image and Push to Quay.io" and change only the human-readable
name field.
- Around line 114-122: The workflow step "Install Node Agent Chart" currently
hardcodes IMAGE_TAG="latest" and IMAGE_REPO='ghcr.io/k8sstormcenter/node-agent',
causing component-tests to ignore the image produced by the build-and-push-image
job; update the step to consume the outputs of the build-and-push-image job
(replace the hardcoded IMAGE_TAG and IMAGE_REPO with the job outputs/outputs.*
or job-level outputs secrets passed via
needs.build-and-push-image.outputs.<name>) so Helm uses the freshly built image,
or if intentional keep the hardcoded values but add a clear comment explaining
fork/testing behavior; specifically change the variables referenced in this step
(IMAGE_TAG, IMAGE_REPO) to use the workflow output names produced by
build-and-push-image.
In @.github/workflows/incluster-comp-pr-merged.yaml:
- Around line 339-357: The create-release-and-retag job lacks the packages write
permission needed to push/retag images to ghcr.io; update the job's permissions
block in the create-release-and-retag job (referenced by name
"create-release-and-retag") to include packages: write alongside contents: write
so the regctl image copy step (the retag to ghcr.io) can authenticate and
succeed.
---
Nitpick comments:
In @.github/workflows/build.yaml:
- Around line 106-109: The workflow step "Ensure ig is installed" hardcodes the
ig version string ("v0.45.0" and ig_0.45.0_amd64.deb); make this configurable by
introducing a workflow input or environment variable (e.g., IG_VERSION) and
update the curl URL and downloaded filename to use that variable instead
(mirroring how VERSION is used in the Makefile) so future updates only require
changing the variable.
In `@Makefile`:
- Around line 27-28: The Makefile currently hardcodes a personal GitHub
namespace via the GONOSUMDB and GONOSUMCHECK values (e.g.,
"github.com/matthyx/*"); update these to a parameterized variable (for example
GITHUB_ORG or PROJECT_NAMESPACE) and use that variable in both GONOSUMDB and
GONOSUMCHECK instead of the literal "github.com/matthyx/*", or replace with the
project's organization namespace, ensuring you change every occurrence
(including the second occurrence that mirrors this line) so the Makefile no
longer embeds a personal username.
In `@pkg/containerwatcher/v2/tracers/kubelet_tls.go`:
- Around line 52-74: Start currently launches RunGadget in a fire-and-forget
goroutine so errors are only logged; modify KubeletTLSTracer.Start to perform a
short startup probe: create a result channel (e.g., errCh) and run
runtime.RunGadget inside the goroutine sending nil on success or the error on
failure, then after launching wait for either an error, a success signal,
context cancellation, or a timeout and return the received error (or
context/timeout error) instead of always returning nil; ensure you still log
failures inside the goroutine and respect the provided ctx when waiting so
callers can detect startup failures of RunGadget.
In `@pkg/ebpf/gadgets/kubelet_tls/go_argument.h`:
- Around line 15-16: The include guard ECAPTURE_GOTLS_H is a leftover name and
should be renamed to a project-specific guard to avoid collisions; update the
macro in go_argument.h (replace ECAPTURE_GOTLS_H) with a clearer name such as
KUBELET_TLS_GO_ARGUMENT_H and ensure you change both the `#ifndef/`#define and the
corresponding `#endif` comment (if present) so the symbol is consistent across the
file.
In `@pkg/ebpf/gadgets/kubelet_tls/Makefile`:
- Around line 1-13: Add a .PHONY declaration for the Makefile targets to ensure
they always run: declare .PHONY for build, build-with-metadata, run, and
build-and-run (e.g., add a line ".PHONY: build build-with-metadata run
build-and-run" near the top of the Makefile) so Make doesn’t treat same-named
files as up-to-date; keep the existing target definitions (build,
build-with-metadata, run, build-and-run) unchanged.
In `@pkg/objectcache/applicationprofilecache/applicationprofilecache.go`:
- Around line 534-542: The new block assumes fullProfile is non-nil before
reading its fields; add a nil-guard to avoid panics from a storage client
returning (nil, nil): check if fullProfile == nil and handle it (e.g., log/warn
and skip setting apc.workloadIDToProfile.Set and
apc.workloadIDToProfileState.Set or set an appropriate error state) before
accessing fullProfile.Annotations or fullProfile.Name; apply the same pattern
used in updateAllProfiles and handleUserManagedProfile around the
apc.workloadIDToProfile / apc.workloadIDToProfileState updates to keep behavior
consistent.
In `@pkg/rulemanager/cel/libraries/applicationprofile/open_test.go`:
- Around line 150-190: TestOpenWithWildcardProfile's testCases slice only
contains positive expectations so a buggy implementation that always returns
true would pass; update the testCases in TestOpenWithWildcardProfile to include
at least one negative case (e.g., name "Relative path not matched", path
"relative/path" or empty string "", expectedResult: false) so the test asserts a
false outcome and fails implementations that unconditionally return true; modify
the testCases variable in open_test.go and ensure the test loop still uses
expectedResult to validate the matcher.
| Test_23_RuleCooldownTest, | ||
| Test_24_ProcessTreeDepthTest | ||
| Test_27_RegexFileOpenMatchTest |
There was a problem hiding this comment.
Missing comma in YAML flow sequence — workflow will fail to parse.
In YAML flow sequences (bracket notation), items must be comma-separated. There's no comma after Test_24_ProcessTreeDepthTest on line 87 before Test_27_RegexFileOpenMatchTest on line 88.
Proposed fix
- Test_24_ProcessTreeDepthTest
- Test_27_RegexFileOpenMatchTest
+ Test_24_ProcessTreeDepthTest,
+ Test_27_RegexFileOpenMatchTest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Test_23_RuleCooldownTest, | |
| Test_24_ProcessTreeDepthTest | |
| Test_27_RegexFileOpenMatchTest | |
| Test_23_RuleCooldownTest, | |
| Test_24_ProcessTreeDepthTest, | |
| Test_27_RegexFileOpenMatchTest |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/component-tests.yaml around lines 86 - 88, The YAML flow
sequence listing test names is missing a comma between items: add a comma after
Test_24_ProcessTreeDepthTest so the sequence reads "Test_23_RuleCooldownTest,
Test_24_ProcessTreeDepthTest, Test_27_RegexFileOpenMatchTest"; update the
sequence that contains Test_23_RuleCooldownTest, Test_24_ProcessTreeDepthTest,
and Test_27_RegexFileOpenMatchTest to ensure all items are comma-separated to
allow the workflow to parse.
|
|
||
| replace github.com/inspektor-gadget/inspektor-gadget => github.com/matthyx/inspektor-gadget v0.0.0-20260203101533-6ef87216d3dd | ||
|
|
||
| replace github.com/kubescape/storage => ../storage |
There was a problem hiding this comment.
Remove the local replace directive before merging to main.
replace github.com/kubescape/storage => ../storage is a filesystem-relative path that will cause build failures on any machine (including CI runners in incluster-comp-pr-merged.yaml) where ../storage does not exist. The unit-test step (go test -exec sudo -v ./...) has no checkout of the sibling storage directory, so the job will fail.
Either:
- Publish the new
kubescape/storageversion containingCompareExecArgsand bump the version ingo.mod, or - Keep this replace only in a local
.gitconfig-excluded file / developer.gitlocaloverlay rather than committing it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 463, Remove the local filesystem replace directive `replace
github.com/kubescape/storage => ../storage` from go.mod before merging: either
publish the updated github.com/kubescape/storage module (including
CompareExecArgs) and update the version in go.mod to that new release, or keep
the replace only in a developer-local overlay (e.g., untracked local config) so
it is not committed; ensure the committed go.mod references a resolvable module
version instead of a relative path.
| import ( | ||
| "context" | ||
|
|
||
| "github.com/inspektor-gadget/inspektor-gadget/pkg/datasource" | ||
| gadgetcontext "github.com/inspektor-gadget/inspektor-gadget/pkg/gadget-context" | ||
| "github.com/dorkamotorka/inspektor-gadget/pkg/operators" | ||
| ocihandler "github.com/inspektor-gadget/inspektor-gadget/pkg/operators/oci-handler" | ||
| "github.com/dorkamotorka/inspektor-gadget/pkg/operators/simple" | ||
| "github.com/inspektor-gadget/inspektor-gadget/pkg/runtime" | ||
| "github.com/kubescape/go-logger" | ||
| "github.com/kubescape/go-logger/helpers" | ||
| "github.com/kubescape/node-agent/pkg/config" | ||
| "github.com/kubescape/node-agent/pkg/containerwatcher" | ||
| "github.com/kubescape/node-agent/pkg/utils" | ||
| orasoci "oras.land/oras-go/v2/content/oci" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the actual imports in the file
cat -n pkg/containerwatcher/v2/tracers/kubelet_tls.go | head -20Repository: kubescape/node-agent
Length of output: 931
🏁 Script executed:
# Check for all uses of the dorkamotorka fork in the repository
rg -n 'dorkamotorka/inspektor-gadget' --type=go | head -30Repository: kubescape/node-agent
Length of output: 285
🏁 Script executed:
# Check go.mod to see if the fork is explicitly listed as a dependency
if [ -f go.mod ]; then
echo "=== Checking go.mod for dorkamotorka ===" && \
rg -A 2 -B 2 'dorkamotorka' go.mod
fiRepository: kubescape/node-agent
Length of output: 104
🏁 Script executed:
# Check what tracers exist and their imports pattern
fd -e go pkg/containerwatcher/v2/tracers/ | xargs -I {} sh -c 'echo "=== {} ===" && head -20 {}'Repository: kubescape/node-agent
Length of output: 497
🏁 Script executed:
# List all tracer files properly
fd --full-path 'pkg/containerwatcher/v2/tracers' -type f -name '*.go'Repository: kubescape/node-agent
Length of output: 234
🏁 Script executed:
# Check all imports in all tracer files
fd --full-path 'pkg/containerwatcher/v2/tracers' -type f -name '*.go' -exec head -20 {} +Repository: kubescape/node-agent
Length of output: 234
🏁 Script executed:
# Check go.mod content - full file or at least the imports section
head -50 go.modRepository: kubescape/node-agent
Length of output: 2120
🏁 Script executed:
# Search for any references to dorkamotorka in other files to understand context
rg 'dorkamotorka' --type=go --type=md --type=txt -B 2 -A 2Repository: kubescape/node-agent
Length of output: 838
🏁 Script executed:
# List all tracer files in the directory
find pkg/containerwatcher/v2/tracers -name '*.go' -type fRepository: kubescape/node-agent
Length of output: 2058
🏁 Script executed:
# Check the full go.mod for any indirect or replace directives that might handle the fork
cat go.mod | grep -i 'dorkamotorka\|replace\|indirect' | head -30Repository: kubescape/node-agent
Length of output: 1798
🏁 Script executed:
# Check if there's a go.sum file and if it has entries for dorkamotorka
if [ -f go.sum ]; then
rg 'dorkamotorka' go.sum || echo "No dorkamotorka entries in go.sum"
fiRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Check how other tracers import operators packages
rg 'github.com/inspektor-gadget/inspektor-gadget/pkg/operators' pkg/containerwatcher/v2/tracers/ -nRepository: kubescape/node-agent
Length of output: 10585
🏁 Script executed:
# Check the actual kubelet_tls.go to see how operators and simple are used
cat -n pkg/containerwatcher/v2/tracers/kubelet_tls.go | head -150Repository: kubescape/node-agent
Length of output: 4643
🏁 Script executed:
# Check if the fork packages are actually available or if they're supposed to be the same
# Look at what types are imported from the fork
rg 'operators\.' pkg/containerwatcher/v2/tracers/kubelet_tls.go -nRepository: kubescape/node-agent
Length of output: 283
🏁 Script executed:
# Check if simple.New is specific to the fork or if the official package also has it
rg 'simple\.New' pkg/containerwatcher/v2/tracers/ | head -10Repository: kubescape/node-agent
Length of output: 988
🏁 Script executed:
# See if there's a go.mod replace directive or vendor directory that might explain the fork
if [ -d vendor ]; then
ls -la vendor/ | grep -i dork || echo "No vendor directory for dorkamotorka"
fiRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Check if this is a known pattern - look for any comment explaining the fork use
rg -B 5 'dorkamotorka/inspektor-gadget' pkg/containerwatcher/v2/tracers/kubelet_tls.goRepository: kubescape/node-agent
Length of output: 436
🏁 Script executed:
# Double-check: verify these fork imports don't exist in the official inspektor-gadget package
# Check what version of inspektor-gadget is required
grep 'inspektor-gadget/inspektor-gadget' go.modRepository: kubescape/node-agent
Length of output: 274
🏁 Script executed:
# Check if the fork might be intentional through go work or other tooling
if [ -f go.work ]; then
cat go.work
fiRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Verify the official inspektor-gadget in go.mod actually provides operators and simple packages
# The replace directive shows it's being redirected to matthyx fork
# Let's check what other tracers are doing - they use official path but get matthyx via replace
grep -A 5 'inspektor-gadget' go.mod | head -20Repository: kubescape/node-agent
Length of output: 552
🏁 Script executed:
# Final check: confirm dorkamotorka is absolutely not listed anywhere in dependency files
grep -r 'dorkamotorka' go.mod go.sum 2>/dev/null || echo "dorkamotorka not found in go.mod or go.sum"Repository: kubescape/node-agent
Length of output: 106
🏁 Script executed:
# Verify the build would actually fail by checking if the import path is resolvable
# dorkamotorka imports are not in go.mod, so they would cause build failure
echo "=== Checking what packages are actually available ===" && \
rg 'require|replace' go.mod | grep -E 'inspektor-gadget|dorkamotorka|matthyx'Repository: kubescape/node-agent
Length of output: 244
🏁 Script executed:
# Final verification: confirm other tracers work with the official path that gets routed via replace
head -15 pkg/containerwatcher/v2/tracers/randomx.go | tail -10Repository: kubescape/node-agent
Length of output: 606
🏁 Script executed:
# Check if there's any indication this file was accidentally created with wrong imports
git log --oneline pkg/containerwatcher/v2/tracers/kubelet_tls.go 2>/dev/null | head -5 || echo "File history not available"Repository: kubescape/node-agent
Length of output: 120
Replace fork imports with official package path.
Lines 8 and 10 import operators and simple from github.com/dorkamotorka/inspektor-gadget, which is not declared in go.mod. The build will fail because Go cannot resolve this import path. All other tracers (randomx, syscall, unshare, etc.) correctly import from the official github.com/inspektor-gadget/inspektor-gadget path, which is already in go.mod with a replace directive pointing to github.com/matthyx/inspektor-gadget. Change lines 8 and 10 to use the official package path to match the rest of the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/containerwatcher/v2/tracers/kubelet_tls.go` around lines 3 - 18, The
imports for the operators packages are using a forked path; update the import
paths in kubelet_tls.go so they match the official module: replace
"github.com/dorkamotorka/inspektor-gadget/pkg/operators" with
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators" and replace
"github.com/dorkamotorka/inspektor-gadget/pkg/operators/simple" with
"github.com/inspektor-gadget/inspektor-gadget/pkg/operators/simple" so the
compiler will resolve the packages already declared in go.mod; keep all other
imports unchanged and ensure any references to the operators package (e.g.,
symbols in operators or simple) continue to compile.
| void* go_get_argument_by_reg(struct pt_regs *ctx, int index) { | ||
| switch (index) { | ||
| case 1: | ||
| return (void*)GO_PARAM1(ctx); | ||
| case 2: | ||
| return (void*)GO_PARAM2(ctx); | ||
| case 3: | ||
| return (void*)GO_PARAM3(ctx); | ||
| case 4: | ||
| return (void*)GO_PARAM4(ctx); | ||
| case 5: | ||
| return (void*)GO_PARAM5(ctx); | ||
| case 6: | ||
| return (void*)GO_PARAM6(ctx); | ||
| case 7: | ||
| return (void*)GO_PARAM7(ctx); | ||
| case 8: | ||
| return (void*)GO_PARAM8(ctx); | ||
| default: | ||
| return NULL; | ||
| } | ||
| } | ||
|
|
||
| void* go_get_argument_by_stack(struct pt_regs *ctx, int index) { | ||
| void* ptr = 0; | ||
| bpf_probe_read(&ptr, sizeof(ptr), (void *)(PT_REGS_SP(ctx)+(index*8))); | ||
| return ptr; | ||
| } | ||
|
|
||
| void* go_get_argument(struct pt_regs *ctx,bool is_register_abi, int index) { | ||
| if (is_register_abi) { | ||
| return go_get_argument_by_reg(ctx, index); | ||
| } | ||
| return go_get_argument_by_stack(ctx, index); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Functions in header should be static __always_inline.
go_get_argument_by_reg, go_get_argument_by_stack, and go_get_argument are defined in a header file without static or __always_inline. In eBPF, non-static functions in headers will cause issues if the header is ever included from multiple compilation units, and the BPF verifier generally requires inlining. This is also consistent with gotls_write/gotls_read in program.bpf.c which use static __always_inline.
Proposed fix
-void* go_get_argument_by_reg(struct pt_regs *ctx, int index) {
+static __always_inline void* go_get_argument_by_reg(struct pt_regs *ctx, int index) {
switch (index) {
case 1:-void* go_get_argument_by_stack(struct pt_regs *ctx, int index) {
+static __always_inline void* go_get_argument_by_stack(struct pt_regs *ctx, int index) {
void* ptr = 0;-void* go_get_argument(struct pt_regs *ctx,bool is_register_abi, int index) {
+static __always_inline void* go_get_argument(struct pt_regs *ctx, bool is_register_abi, int index) {
if (is_register_abi) {🧰 Tools
🪛 Clang (14.0.6)
[error] 93-93: use of undeclared identifier 'NULL'
(clang-diagnostic-error)
[error] 103-103: unknown type name 'bool'
(clang-diagnostic-error)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/ebpf/gadgets/kubelet_tls/go_argument.h` around lines 74 - 108, The three
functions go_get_argument_by_reg, go_get_argument_by_stack, and go_get_argument
in go_argument.h must be declared static __always_inline to avoid
multiple-definition linkage issues and to satisfy the eBPF verifier; update
their declarations/definitions to prepend static __always_inline (matching
gotls_write/gotls_read style) so they are inlined into callers and not emitted
as external symbols.
| char data[MAX_DATA_SIZE_OPENSSL]; | ||
| }; | ||
|
|
||
| GADGET_TRACER_MAP(events, 4096*4096); // 4096 in-flight pages |
There was a problem hiding this comment.
Perf buffer size 4096*4096 (16M entries) is almost certainly a mistake.
GADGET_TRACER_MAP(events, 4096*4096) allocates space for ~16 million in-flight events. Combined with the sizeof(struct event) (~16KiB each), this will attempt to reserve an enormous amount of memory. This looks like a debugging artifact; a typical value would be something like 1024 or 4096.
Proposed fix
-GADGET_TRACER_MAP(events, 4096*4096); // 4096 in-flight pages
+GADGET_TRACER_MAP(events, 4096);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| GADGET_TRACER_MAP(events, 4096*4096); // 4096 in-flight pages | |
| GADGET_TRACER_MAP(events, 4096); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/ebpf/gadgets/kubelet_tls/program.bpf.c` at line 36, The perf buffer size
is incorrectly set to GADGET_TRACER_MAP(events, 4096*4096) which would allocate
~16M entries (multiplied by sizeof(struct event)) — change the map declaration
to a reasonable size (e.g., GADGET_TRACER_MAP(events, 1024) or
GADGET_TRACER_MAP(events, 4096)) to avoid huge memory allocation; update the
entry count used in the GADGET_TRACER_MAP macro (referencing the events map and
struct event) to one of those realistic values.
| #pragma unroll | ||
| for (int i = 0; i < 256; i++) { | ||
| if (i >= event->data_len)break; | ||
|
|
||
| bpf_printk("data[%d]=%c", i, (u8)event->data[i]); | ||
| } | ||
| /* | ||
| if (ret < 0) { | ||
| gadget_discard_buf(&event); | ||
| bpf_printk("gotls_write bpf_probe_read_user_str failed, ret:%d, str:%d\n", ret, str); | ||
| bpf_printk("last return"); | ||
| return 0; | ||
| } | ||
| */ |
There was a problem hiding this comment.
Remove debug byte-printing loop and re-enable error handling before merging.
The #pragma unroll loop (Lines 75-79) emits a bpf_printk for every byte of captured data (up to 256 bytes per event). This will flood trace_pipe and significantly degrade performance in production. Similarly, the bpf_probe_read_user error check (Lines 80-87) is commented out, meaning corrupted or partial reads are silently submitted to userspace.
The same issues exist in the read path at Lines 141-153.
Proposed fix (write path; apply the same to read path)
int ret = bpf_probe_read_user(&event->data, event->data_len, (void *)str);
- `#pragma` unroll
- for (int i = 0; i < 256; i++) {
- if (i >= event->data_len)break;
-
- bpf_printk("data[%d]=%c", i, (u8)event->data[i]);
- }
- /*
if (ret < 0) {
- gadget_discard_buf(&event);
- bpf_printk("gotls_write bpf_probe_read_user_str failed, ret:%d, str:%d\n", ret, str);
- bpf_printk("last return");
+ gadget_discard_buf(event, sizeof(*event));
return 0;
}
- */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/ebpf/gadgets/kubelet_tls/program.bpf.c` around lines 74 - 87, Remove the
debug byte-printing loop that iterates with "#pragma unroll" over event->data
and instead restore the bpf_probe_read_user error handling in the write path:
after attempting to read into event->data (the
bpf_probe_read_user/bpf_probe_read_user_str call in the gotls_write handling),
check if ret < 0, call gadget_discard_buf(&event), emit a single bpf_printk
including the ret/error and context (e.g., "gotls_write bpf_probe_read_user_str
failed, ret:%d"), and return 0 to avoid submitting corrupted/partial data; apply
the same removal and error-check re-enabling to the read path handling (the
corresponding code around the read path that mirrors gotls_write).
| "github.com/kubescape/storage/pkg/registry/file/dynamicpathdetector" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for go.mod and look for replace directives
find . -name "go.mod" -type f | head -5Repository: kubescape/node-agent
Length of output: 72
🏁 Script executed:
# Search for CompareExecArgs function definition
rg -n "func CompareExecArgs" --type goRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Also look in the storage module path mentioned in the import
fd -type f -name "*.go" | xargs grep -l "CompareExecArgs" | head -10Repository: kubescape/node-agent
Length of output: 234
🏁 Script executed:
# Read the go.mod file to check for replace directives
cat go.mod | grep -A 2 -B 2 "storage"Repository: kubescape/node-agent
Length of output: 700
🏁 Script executed:
# Search more broadly for CompareExecArgs
rg -n "CompareExecArgs" --type-listRepository: kubescape/node-agent
Length of output: 453
🏁 Script executed:
# Try a simpler grep search
grep -r "CompareExecArgs" . --include="*.go" 2>/dev/null | head -20Repository: kubescape/node-agent
Length of output: 186
🏁 Script executed:
# Check if there's a local storage directory
ls -la | grep -i storageRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Check the file that imports the function
cat -n pkg/rulemanager/cel/libraries/applicationprofile/exec.go | sed -n '1,100p'Repository: kubescape/node-agent
Length of output: 3629
🏁 Script executed:
# Check if ../storage directory exists relative to go.mod
ls -la ../storage 2>/dev/null | head -20Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Also check parent directory structure
ls -la .. 2>/dev/null | grep -i storageRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Search in the repository for any documentation or references to CompareExecArgs
rg "CompareExecArgs" .Repository: kubescape/node-agent
Length of output: 186
🏁 Script executed:
# Check if there's any vendor directory or cached storage module
find . -path "*/storage/pkg/registry/file/dynamicpathdetector*" -type f 2>/dev/null | head -5Repository: kubescape/node-agent
Length of output: 46
🌐 Web query:
kubescape/storage CompareExecArgs dynamicpathdetector
💡 Result:
kubescape/storage: dynamicpathdetector
In the (storage) registry code, dynamicpathdetector is a helper package that normalizes paths/URLs by detecting “dynamic” segments (e.g., IDs) and replacing/marking them with a dynamic identifier (constant DynamicIdentifier, value "⋯"). It exposes helpers to analyze endpoints and open() calls and to merge/deduplicate results, including:
AnalyzeURL,AnalyzeOpen,AnalyzeOpensAnalyzeEndpoints,ProcessEndpointCompareDynamic(dynamicPath, regularPath) bool(checks whether a concrete path matches a dynamic-pattern path)MergeDuplicateEndpoints,MergeStrings(pkg.go.dev)
CompareExecArgs
I couldn’t find any symbol named CompareExecArgs in the publicly indexed kubescape/storage sources/docs (and GitHub code search requires sign-in from my environment), so I can’t reliably say what/where it is from just this query. (github.com)
If you paste a link to the file/line (or the snippet where CompareExecArgs appears), I can explain exactly how it relates to dynamicpathdetector and what it does.
Citations:
- 1: https://pkg.go.dev/github.com/Aryaman6492/storage%40v0.0.0-20250408210412-48aa2c976ccf/pkg/registry/file/dynamicpathdetector
- 2: https://github.com/kubescape/storage?utm_source=openai
🌐 Web query:
kubescape storage v0.0.239 CompareExecArgs
💡 Result:
In kubescape/storage v0.0.239, I can’t find any symbol/function named CompareExecArgs in the repository contents I was able to inspect; it’s not mentioned in the v0.0.239 changes either.
What did change in v0.0.239 (vs v0.0.237) is mainly:
- Kubernetes libraries bumped to
v0.35.0(plus broader dependency/toolchain updates) via PR Feature/demo #280. [2] - Large object storage handling tweaks via PR wait for storage when creating a client (other strategy) #279. [1]
If you’re seeing CompareExecArgs in an error, it’s likely coming from another module (possibly generated code or a dependency pulled in via the Kubernetes v0.35.0 bump), not from kubescape/storage itself in v0.0.239. [2]
Sources:
[1] v0.0.237…v0.0.239 release notes (lists storage PRs #279/#280) (newreleases.io)
[2] kubescape/storage PR #280 (“bump k8s version to v0.35.0”) (github.com)
Citations:
- 1: https://newreleases.io/project/github/kubescape/helm-charts/release/kubescape-operator-1.30.3
- 2: bump k8s version to v0.35.0 storage#280
CompareExecArgs does not exist in published kubescape/storage v0.0.239 — this code will fail to build when the local replace directive is removed.
The new import and usage on line 88 depends entirely on replace github.com/kubescape/storage => ../storage in go.mod. Web search confirms CompareExecArgs does not exist in the published v0.0.239 release; the dynamicpathdetector package has CompareDynamic but not CompareExecArgs. Removing the local replace and using the published version will cause a build failure.
Either:
- Add
CompareExecArgsto the kubescape/storage release, or - Implement the comparison logic locally instead of relying on an external function that doesn't exist in the dependency version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/rulemanager/cel/libraries/applicationprofile/exec.go` around lines 12 -
13, The code references dynamicpathdetector.CompareExecArgs which does not exist
in the published kubescape/storage v0.0.239 and will break builds when the local
replace is removed; update exec.go to avoid the missing symbol by implementing
the argument comparison locally (create a local helper like compareExecArgs that
reproduces the intended logic) or by switching to the existing
dynamicpathdetector.CompareDynamic if it provides the needed semantics; ensure
the call sites in exec.go that currently invoke CompareExecArgs are updated to
call the new local helper (or CompareDynamic) and include unit coverage for the
comparison logic so future dependency differences won't break the build.
| name: "storage" | ||
| image: | ||
| repository: quay.io/kubescape/storage | ||
| repository: ghcr.io/k8sstormcenter/storage |
There was a problem hiding this comment.
Image repositories point to a fork (k8sstormcenter) rather than upstream (kubescape).
Both storage.image.repository and nodeAgent.image.repository now reference ghcr.io/k8sstormcenter/.... If this PR targets the upstream kubescape/node-agent main branch, these should use the upstream organization's registry or be parameterized. Merging this as-is would make the test chart depend on a fork's container registry.
Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/chart/values.yaml` at line 35, The image repository values point to a
forked registry; update the values for storage.image.repository and
nodeAgent.image.repository to reference the upstream registry (e.g.,
ghcr.io/kubescape/...) or make them configurable via a new parameter (e.g.,
image.repositoryOverride) so tests don’t depend on the k8sstormcenter fork;
ensure both occurrences (storage.image.repository and
nodeAgent.image.repository) are changed consistently and document the new
parameter if added.
| alerts, err := testutils.GetAlerts(wl.Namespace) | ||
| require.NoError(t, err, "Failed to get alerts") | ||
| testutils.AssertNotContains(t, alerts, "Unexpected file open", "cat", "nginx", []bool{true}) | ||
| testutils.AssertContains(t, alerts, "Unexpected file open", "cat", "hostname", []bool{true}) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the test around lines 1636-1639 with context
sed -n '1568,1645p' tests/component_test.goRepository: kubescape/node-agent
Length of output: 2842
🏁 Script executed:
# Find the AssertContains function definition to understand parameters
rg -n 'func AssertContains' --type=goRepository: kubescape/node-agent
Length of output: 259
🏁 Script executed:
# Search for rule name definitions - look for "Unexpected file open" and "Files Access Anomalies"
rg -n 'Unexpected file open|Files Access Anomalies' --type=go | head -30Repository: kubescape/node-agent
Length of output: 474
🏁 Script executed:
# Search for file open related rules and their definitions
rg -n '"[^"]*[Ff]ile.*[Oo]pen|[Oo]pen.*[Ff]ile' --type=go -A2 -B2 | head -50Repository: kubescape/node-agent
Length of output: 3986
🏁 Script executed:
# Read the AssertContains implementation
sed -n '98,130p' tests/testutils/alertmanager.goRepository: kubescape/node-agent
Length of output: 1584
🏁 Script executed:
# Find other uses of AssertContains and AssertNotContains to see the pattern
rg -n 'AssertContains|AssertNotContains' tests/component_test.go -B2 -A1 | head -80Repository: kubescape/node-agent
Length of output: 3304
🏁 Script executed:
# Check what rule names exist in the test expectations
rg -n '".*Anomal|".*open' tests/component_test.go | grep -i 'assert\|rule'Repository: kubescape/node-agent
Length of output: 1852
🏁 Script executed:
# Search for rule definitions related to file access/open
rg -n 'Files Access|file.*open|open.*file' pkg/rulemanager --type=go -i | head -30Repository: kubescape/node-agent
Length of output: 2511
🏁 Script executed:
# Check the celprogram or rules that handle open syscalls
rg -n 'applicationprofile.*open|Open.*anomaly|anomaly.*open' --type=go -i | head -20Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Look at the test setup more carefully - what rule should trigger on unexpected file opens?
rg -n 'rule.*name.*=|RuleName.*=' pkg/rulemanager --type=go | grep -i 'file\|open'Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Search for where rule names are defined - look for the actual rule definitions
rg -n 'Unexpected file open' --type=goRepository: kubescape/node-agent
Length of output: 309
🏁 Script executed:
# Search for CEL programs or rule definitions that check file opens
rg -n 'was_path_opened|open.*in.*profile' --type=go -A2 | head -40Repository: kubescape/node-agent
Length of output: 3568
🏁 Script executed:
# Look for rule definitions in YAML or rule files
find . -name '*.yaml' -o -name '*.yml' -o -name '*rule*' | grep -v vendor | head -20Repository: kubescape/node-agent
Length of output: 822
🏁 Script executed:
# Search for where rules are actually defined or generated
rg -n 'rule_name.*=' tests/ --type=go -B2 -A2 | head -50Repository: kubescape/node-agent
Length of output: 3018
🏁 Script executed:
# Look for what happens when a file open is not in the profile - what alert is generated?
rg -n 'not in.*profile|deviation|anomal' pkg/rulemanager --type=go -i | head -30Repository: kubescape/node-agent
Length of output: 286
🏁 Script executed:
# Check the integration test to see what rule name is expected for file opens
sed -n '1,50p' pkg/rulemanager/cel/libraries/applicationprofile/integration_test.goRepository: kubescape/node-agent
Length of output: 1447
🏁 Script executed:
# Search for where CEL programs or rules check for unexpected file opens
rg -n 'was_path_opened.*false|!.*was_path_opened' --type=go | head -20Repository: kubescape/node-agent
Length of output: 770
🏁 Script executed:
# Look for rule definitions related to files - check if there's a rules directory or CEL program definitions
find . -type f -name '*rule*' -o -name '*celprogram*' | grep -v vendor | head -20Repository: kubescape/node-agent
Length of output: 1284
🏁 Script executed:
# Search for "Files Access" which was found earlier
rg -n '"Files Access' --type=go -B5 -A5Repository: kubescape/node-agent
Length of output: 1678
🏁 Script executed:
# Check if there are any CEL programs or rule definitions files in the repo
find . -type f -name '*.go' -path '*/rulecreator/*' | head -10Repository: kubescape/node-agent
Length of output: 207
🏁 Script executed:
# Search for where the actual rule names are defined - look at rule creator
rg -n 'Files Access Anomalies|Unexpected' pkg/rulemanager/rulecreator --type=go | head -20Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Look at the rule_manager to see what rules are created
sed -n '1,100p' pkg/rulemanager/rule_manager.goRepository: kubescape/node-agent
Length of output: 3941
🏁 Script executed:
# Search for where rules are actually instantiated/created to find the correct rule names
rg -n 'AddRule|CreateRule|RuleName' pkg/rulemanager/rulecreator --type=go | head -30Repository: kubescape/node-agent
Length of output: 1981
🏁 Script executed:
# Check the factory to understand how rules are created
cat pkg/rulemanager/rulecreator/factory.goRepository: kubescape/node-agent
Length of output: 4043
🏁 Script executed:
# Search for the actual CEL program definitions that would generate "Files Access Anomalies"
rg -n 'Files Access|anomal.*open|open.*anomal' pkg/ --type=go -i | grep -v test | head -20Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Find where RegisterRule is called to see where rules are actually defined
rg -n 'RegisterRule' --type=go | head -30Repository: kubescape/node-agent
Length of output: 363
🏁 Script executed:
# Search for where the rules list is populated - look for main or init files
rg -n 'Rules.*=.*\[\]|NewRuleCreator' --type=go | head -20Repository: kubescape/node-agent
Length of output: 208
🏁 Script executed:
# Search in main or initialization files
find . -name 'main.go' -o -name 'init.go' | xargs grep -l 'RegisterRule\|Rule' | head -5Repository: kubescape/node-agent
Length of output: 77
🏁 Script executed:
# Check main.go to see where rules are registered
sed -n '220,350p' cmd/main.goRepository: kubescape/node-agent
Length of output: 5280
🏁 Script executed:
# Also look at the imports to find where the rule definitions come from
rg -n 'import\|rules\.' cmd/main.go | head -40Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Check the ruleswatcher to see how rules are loaded
find . -name '*ruleswatcher*' -o -name '*rules_watcher*' | head -5Repository: kubescape/node-agent
Length of output: 94
🏁 Script executed:
# Search for where the actual rule definitions/names come from
rg -n 'OpenCalls|was_path_opened' --type=go pkg/rulemanager -B3 -A3 | head -50Repository: kubescape/node-agent
Length of output: 4391
🏁 Script executed:
# Since rules seem to be loaded from external source, let me check what the test expects
# Look at the actual error message when a test fails - check if there are any test logs or expected values
rg -n 'test.*file.*open|file.*open.*test' tests/ --type=go -i -A2 | head -40Repository: kubescape/node-agent
Length of output: 1166
🏁 Script executed:
# Let me verify the ruleswatcher to understand where rules come from
cat pkg/rulemanager/ruleswatcher/rules_watcher.go | head -100Repository: kubescape/node-agent
Length of output: 141
🏁 Script executed:
# Check if "Unexpected file open" is defined anywhere or if it's expected to be loaded from external rules
rg -n 'Unexpected file open|Files Access Anomalies' --type=go --type=yaml -i | grep -v test | grep -v component_testRepository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# List the ruleswatcher directory
ls -la pkg/rulemanager/ruleswatcher/Repository: kubescape/node-agent
Length of output: 429
🏁 Script executed:
# Search more broadly for rule name definitions
find pkg/ -name '*.go' | xargs grep -l 'rule.*Name\|RuleName' | head -10Repository: kubescape/node-agent
Length of output: 447
🏁 Script executed:
# Check what files exist in rulemanager
ls -la pkg/rulemanager/Repository: kubescape/node-agent
Length of output: 965
🏁 Script executed:
# List what's in ruleswatcher
ls -la pkg/rulemanager/ruleswatcher/Repository: kubescape/node-agent
Length of output: 429
🏁 Script executed:
# Check the watcher file to understand how rules are loaded
cat pkg/rulemanager/ruleswatcher/watcher.goRepository: kubescape/node-agent
Length of output: 5805
🏁 Script executed:
# Now let's verify the context around "Files Access Anomalies in container" to confirm it's the right rule name
sed -n '140,170p' tests/component_test.goRepository: kubescape/node-agent
Length of output: 1544
🏁 Script executed:
# Let's also check if there are any other file-access related test patterns we should be aware of
rg -n 'ExecIntoPod.*cat\|ExecIntoPod.*file' tests/component_test.go -B3 -A3Repository: kubescape/node-agent
Length of output: 46
🏁 Script executed:
# Final verification - check if there's any CEL program or rule definition that uses "Unexpected file open"
rg -n '"Unexpected file open"' --type=go --type=yamlRepository: kubescape/node-agent
Length of output: 309
Bug: Wrong container name and rule name in assertions — test will always fail.
On line 1639, "hostname" is passed as the expected container name, but the workload only has a container named "nginx" (created at line 1603). The string "hostname" is the file path being opened (/etc/hostname), not the container name. The AssertContains function searches for alerts matching the container_name label, so this assertion will never find a match.
Additionally, lines 1638–1639 use "Unexpected file open" as the rule name, but this rule does not exist in the codebase. A search of all Go and YAML files confirms it appears only in these two test lines. The correct rule name is "Files Access Anomalies in container" (as confirmed in the expectedAlerts and expectedFailOnProfile maps at lines 146 and 162).
Proposed fix
- testutils.AssertNotContains(t, alerts, "Unexpected file open", "cat", "nginx", []bool{true})
- testutils.AssertContains(t, alerts, "Unexpected file open", "cat", "hostname", []bool{true})
+ testutils.AssertNotContains(t, alerts, "Files Access Anomalies in container", "cat", "nginx", []bool{true})
+ testutils.AssertContains(t, alerts, "Files Access Anomalies in container", "cat", "nginx", []bool{true})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| alerts, err := testutils.GetAlerts(wl.Namespace) | |
| require.NoError(t, err, "Failed to get alerts") | |
| testutils.AssertNotContains(t, alerts, "Unexpected file open", "cat", "nginx", []bool{true}) | |
| testutils.AssertContains(t, alerts, "Unexpected file open", "cat", "hostname", []bool{true}) | |
| alerts, err := testutils.GetAlerts(wl.Namespace) | |
| require.NoError(t, err, "Failed to get alerts") | |
| testutils.AssertNotContains(t, alerts, "Files Access Anomalies in container", "cat", "nginx", []bool{true}) | |
| testutils.AssertContains(t, alerts, "Files Access Anomalies in container", "cat", "nginx", []bool{true}) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/component_test.go` around lines 1636 - 1639, The assertions use the
wrong rule and container names causing failures; update the two assertions that
call testutils.AssertNotContains and testutils.AssertContains to use the actual
rule "Files Access Anomalies in container" instead of "Unexpected file open" and
use the actual container name "nginx" (not "hostname"), keeping the same alerts
variable and wl.Namespace context so the label-based searches in
testutils.AssertContains/AssertNotContains can find the expected alerts.
| @@ -1,4 +1,4 @@ | |||
| #/bin/bash | |||
| curl -s https://raw.githubusercontent.com/kubescape/helm-charts/main/charts/kubescape-operator/values.yaml -o values.yaml | |||
| curl -s https://raw.githubusercontent.com/bob/test/localtestbuild/kubescape/values.yaml -o values.yaml | |||
There was a problem hiding this comment.
Debug/fork URL committed — will break CI.
The URL https://raw.githubusercontent.com/bob/test/localtestbuild/kubescape/values.yaml is from a personal fork (bob/test, branch localtestbuild) and was clearly left in from local debugging. This will either 404 or return the wrong values.yaml, breaking any CI job that calls this script to resolve the storage image tag.
Revert to the correct upstream URL (e.g., the official kubescape/helm-charts values.yaml).
🐛 Proposed fix
-curl -s https://raw.githubusercontent.com/bob/test/localtestbuild/kubescape/values.yaml -o values.yaml
+curl -s https://raw.githubusercontent.com/kubescape/helm-charts/main/charts/kubescape-operator/values.yaml -o values.yaml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -s https://raw.githubusercontent.com/bob/test/localtestbuild/kubescape/values.yaml -o values.yaml | |
| curl -s https://raw.githubusercontent.com/kubescape/helm-charts/main/charts/kubescape-operator/values.yaml -o values.yaml |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/scripts/storage-tag.sh` at line 2, The curl in
tests/scripts/storage-tag.sh is pointing to a personal fork URL
(https://raw.githubusercontent.com/bob/test/localtestbuild/kubescape/values.yaml);
replace that with the official upstream raw URL for the Kubescape chart values
(use the kubescape/helm-charts repo and the appropriate branch/tag) so the curl
command fetches the canonical values.yaml for resolving the storage image tag;
update the single curl invocation in the script accordingly and verify the
script still extracts the image tag as before.
2 participants
Overview
Summary by CodeRabbit
Release Notes
New Features
Infrastructure
Testing