Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds multi-host-type key formats and a ProfileIdentifier abstraction; replaces single-format key builders with K8s/ECS/Host builders/parsers; propagates Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Processor as ContainerProfileProcessor
participant Storage as ContainerProfileStorage
participant DB as SQLiteBackend
Client->>Processor: send timeseries / container event
Processor->>Processor: determine HostType & build armotypes.ProfileIdentifier (id)
Processor->>Storage: BuildContainerProfileKey(id, kind) / Save or Update using K8s/ECS/Host path
Storage->>DB: read/write using constructed key path
DB-->>Storage: return data / ack
Storage-->>Processor: aggregated/saved profile result
Processor-->>Client: emit consolidated slug / acknowledgement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 unit tests (beta)
📝 Coding Plan
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/registry/file/sqlite_test.go (1)
103-110:⚠️ Potential issue | 🟠 MajorTest assertions may fail due to path segment mismatch.
The test paths are 5-segment format (e.g.,
/spdx.../applicationprofiles/default/name), butK8sPathToKeysexpects 6-segment paths with cluster. For the test path on line 87:/spdx.softwarecomposition.kubescape.io/applicationprofiles/default/replicaset-collection-85f89d8b47
K8sPathToKeyswill parse this as:
kind= "applicationprofiles" ✓cluster= "default" (wrong - this is namespace)namespace= "replicaset-collection-85f89d8b47" (wrong - this is name)name= "" (padded empty)But the test expects
namespace="default"andname="replicaset-collection-85f89d8b47".Either update the test paths to include an empty cluster segment, or use
ParseContainerProfileKeywhich handles both formats.🐛 Proposed fix: Update test paths to 6-segment format
tests := []struct { test string path string kind string namespace string name string }{ { test: "single", - path: "/spdx.softwarecomposition.kubescape.io/applicationprofiles/default/replicaset-collection-85f89d8b47", + path: "/spdx.softwarecomposition.kubescape.io/applicationprofiles//default/replicaset-collection-85f89d8b47", kind: "applicationprofiles", namespace: "default", name: "replicaset-collection-85f89d8b47", }, { test: "namespace", - path: "/spdx.softwarecomposition.kubescape.io/applicationprofiles/default", + path: "/spdx.softwarecomposition.kubescape.io/applicationprofiles//default", kind: "applicationprofiles", namespace: "default", }, { test: "cluster", - path: "/spdx.softwarecomposition.kubescape.io/applicationprofiles", + path: "/spdx.softwarecomposition.kubescape.io/applicationprofiles/", kind: "applicationprofiles", }, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/registry/file/sqlite_test.go` around lines 103 - 110, Tests are failing because K8sPathToKeys expects a 6-segment path (including cluster) but test inputs are 5-segment; update the test to either add an empty cluster segment to each path (e.g., insert "" between kind and namespace) or replace the K8sPathToKeys call with ParseContainerProfileKey which accepts both 5- and 6-segment formats; change the test loop in sqlite_test.go to use ParseContainerProfileKey (or adjust the test paths) and assert against the returned namespace/name/kind accordingly.pkg/registry/file/containerprofile_storage.go (1)
116-130:⚠️ Potential issue | 🟠 MajorUse
BuildContainerProfileKeyto support all ProfileIdentifier host types.The methods hardcode
K8sKeysToPathfor key construction, ignoringid.HostType. This will generate incorrect storage keys for ECS and Host ProfileIdentifiers:
- For ECS types (EcsEc2, EcsFargate):
id.Namespaceis empty; should useid.AWSAccountIDandid.Region- For Host types:
id.Namespaceis empty; should useid.HostIDAdditionally, assigning
ap.Namespace = id.Namespacewill be incorrect for non-K8s types.Use
BuildContainerProfileKey(id, "applicationprofiles")andBuildContainerProfileKey(id, "networkneighborhoods")instead of manually constructing keys withK8sKeysToPath. This function already exists and correctly handles all host types. Apply the same fix toUpdateNetworkNeighborhood.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/registry/file/containerprofile_storage.go` around lines 116 - 130, UpdateApplicationProfile (and similarly UpdateNetworkNeighborhood) currently builds keys with K8sKeysToPath and assigns ap.Namespace = id.Namespace, which fails for ECS/Host ProfileIdentifier host types; replace the manual key construction calls with BuildContainerProfileKey(id, "applicationprofiles") (and "networkneighborhoods" in the other method) to ensure AWS/Host fields (AWSAccountID, Region, HostID) are used, and stop unconditionally setting ap.Namespace = id.Namespace for non-K8s hosts (derive namespace from BuildContainerProfileKey behavior or set appropriately only for K8s host types); update both UpdateApplicationProfile and UpdateNetworkNeighborhood to use BuildContainerProfileKey and adjust ap.Namespace handling accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 72: The go.mod lists the vulnerable indirect dependency
"github.com/docker/cli v28.3.3+incompatible"; update that module to v29.2.0 or
later (e.g. replace the version entry with "github.com/docker/cli v29.2.0" or
run a direct upgrade via go get github.com/docker/cli@v29.2.0) and then run go
mod tidy to propagate the change and update the go.sum; if the dependency is
pulled in transitively, bump the parent module that requires
github.com/docker/cli or add an explicit require/replace for
github.com/docker/cli@v29.2.0 to ensure the fixed version is used.
In `@pkg/config/config.go`:
- Around line 17-19: LoadConfig should validate and normalize the HostType field
after mapstructure unmarshalling: if HostType is empty set it to
armotypes.HostTypeKubernetes, and if it is set validate it against the supported
enum values (e.g. armotypes.HostTypeKubernetes, armotypes.HostTypeECS, etc.) and
return an error for any unsupported value so invalid strings cannot flow into
HostKeysToPath; update LoadConfig to perform this check and adjust error returns
accordingly and add unit tests in pkg/config/config_test.go that assert behavior
for Kubernetes, ECS and an invalid value.
In `@pkg/registry/file/containerprofile_processor.go`:
- Line 118: The PreSave method currently hardcodes K8s key format via
K8sKeysToPath; replace those hardcoded keys with BuildContainerProfileKey by
creating a ProfileIdentifier populated with the processor's HostType
(a.HostType), cluster/region, namespace (or region) and the profile name, and
call BuildContainerProfileKey(id, "containerprofile") to generate the key;
update both places where K8sKeysToPath is used in PreSave (the key at the start
and the one used when ReportSeriesIdMetadataKey is present) so keys match
ParseContainerProfileKey and consolidateKeyTimeSeries which expect the processor
HostType.
---
Outside diff comments:
In `@pkg/registry/file/containerprofile_storage.go`:
- Around line 116-130: UpdateApplicationProfile (and similarly
UpdateNetworkNeighborhood) currently builds keys with K8sKeysToPath and assigns
ap.Namespace = id.Namespace, which fails for ECS/Host ProfileIdentifier host
types; replace the manual key construction calls with
BuildContainerProfileKey(id, "applicationprofiles") (and "networkneighborhoods"
in the other method) to ensure AWS/Host fields (AWSAccountID, Region, HostID)
are used, and stop unconditionally setting ap.Namespace = id.Namespace for
non-K8s hosts (derive namespace from BuildContainerProfileKey behavior or set
appropriately only for K8s host types); update both UpdateApplicationProfile and
UpdateNetworkNeighborhood to use BuildContainerProfileKey and adjust
ap.Namespace handling accordingly.
In `@pkg/registry/file/sqlite_test.go`:
- Around line 103-110: Tests are failing because K8sPathToKeys expects a
6-segment path (including cluster) but test inputs are 5-segment; update the
test to either add an empty cluster segment to each path (e.g., insert ""
between kind and namespace) or replace the K8sPathToKeys call with
ParseContainerProfileKey which accepts both 5- and 6-segment formats; change the
test loop in sqlite_test.go to use ParseContainerProfileKey (or adjust the test
paths) and assert against the returned namespace/name/kind accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6daa18c1-d0b4-4954-bdc2-9312dc950da5
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
go.modpkg/config/config.gopkg/registry/file/applicationprofile_processor.gopkg/registry/file/containerprofile_aggregator_test.gopkg/registry/file/containerprofile_processor.gopkg/registry/file/containerprofile_processor_test.gopkg/registry/file/containerprofile_storage.gopkg/registry/file/containerprofile_storage_interface.gopkg/registry/file/sqlite.gopkg/registry/file/sqlite_test.gopkg/registry/file/storage.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/registry/file/containerprofile_processor.go (1)
314-339:⚠️ Potential issue | 🟠 MajorPass full
ProfileIdentifieror host-type-aware payload to downstream consumers.The
ConsolidatedSlugDatapayload loses critical profile identity information for non-Kubernetes hosts. For ECS/EC2 profiles,Namespaceis empty whileAWSAccountID,Region, andClustercontain the actual identity. Downstream consumers receiving onlyNameand emptyNamespacecannot properly route or aggregate profiles across accounts/regions. Either serialize the completeProfileIdentifieror add host-type-aware fields to the payload.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/registry/file/containerprofile_processor.go` around lines 314 - 339, The ConsolidatedSlugData being sent from sendConsolidatedSlugToChannel loses non-Kubernetes identity (e.g., AWSAccountID, Region, Cluster) so downstream consumers can't route ECS/EC2 profiles; update the payload to carry the full ProfileIdentifier (or add host-type-aware fields) and populate it from the id argument before sending. Concretely, modify the ConsolidatedSlugData type to include either a ProfileIdentifier field or explicit fields for HostType/AWSAccountID/Region/Cluster (while preserving Name/Namespace for backwards compatibility), then in sendConsolidatedSlugToChannel set those fields from the id parameter (e.g., id.AWSAccountID, id.Region, id.Cluster, id.Namespace, id.HostType) so consumers receive complete identity information. Ensure any code that reads ConsolidatedSlugData is updated to use the new field or fall back to existing Namespace when appropriate.
♻️ Duplicate comments (1)
pkg/registry/file/containerprofile_processor.go (1)
114-146:⚠️ Potential issue | 🟠 MajorPreSave still hardcodes K8s lookup keys.
Both lookups use
K8sKeysToPath(..., "", ...), so they bypass the new host-type-aware key format and also drop the cluster segment. That meansPreSavecan miss the existing consolidated profile/SBOM for ECS and for cluster-qualified K8s keys. Please build these keys with the same host-type-specific helpers used elsewhere (BuildContainerProfileKeyfor the container profile, and the equivalent builder for SBOM) so lookups match save/parse paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/registry/file/containerprofile_processor.go` around lines 114 - 146, PreSave currently builds lookup keys with K8sKeysToPath("", ..., ...) which hardcodes K8s semantics and drops the cluster segment; change the two lookups in containerprofile_processor.go to use the host-aware builders instead (use BuildContainerProfileKey for the container profile lookup that feeds GetContainerProfileMetadata and use the SBOM key builder used elsewhere for the SBOM lookup before calling GetSbom), ensuring you pass the same host-type, cluster/namespace and name parameters as used by the save/parse paths (replace the K8sKeysToPath calls surrounding the GetContainerProfileMetadata and GetSbom usages, and keep names.ImageInfoToSlug for sbom name generation).
🤖 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/registry/file/containerprofile_processor.go`:
- Around line 50-60: The constructor NewContainerProfileProcessor currently
normalizes an empty cfg.HostType to armotypes.HostTypeKubernetes but accepts any
other value; validate cfg.HostType against the supported armotypes.HostType*
constants before storing it in ContainerProfileProcessor (e.g., allow only the
known constants such as armotypes.HostTypeKubernetes and other defined HostType
values), set the default when cfg.HostType == "" as you already do, and fail
fast for invalid values (e.g., panic or log.Fatalf with a clear message) so that
incorrect HostType strings are rejected instead of being treated as host
profiles.
---
Outside diff comments:
In `@pkg/registry/file/containerprofile_processor.go`:
- Around line 314-339: The ConsolidatedSlugData being sent from
sendConsolidatedSlugToChannel loses non-Kubernetes identity (e.g., AWSAccountID,
Region, Cluster) so downstream consumers can't route ECS/EC2 profiles; update
the payload to carry the full ProfileIdentifier (or add host-type-aware fields)
and populate it from the id argument before sending. Concretely, modify the
ConsolidatedSlugData type to include either a ProfileIdentifier field or
explicit fields for HostType/AWSAccountID/Region/Cluster (while preserving
Name/Namespace for backwards compatibility), then in
sendConsolidatedSlugToChannel set those fields from the id parameter (e.g.,
id.AWSAccountID, id.Region, id.Cluster, id.Namespace, id.HostType) so consumers
receive complete identity information. Ensure any code that reads
ConsolidatedSlugData is updated to use the new field or fall back to existing
Namespace when appropriate.
---
Duplicate comments:
In `@pkg/registry/file/containerprofile_processor.go`:
- Around line 114-146: PreSave currently builds lookup keys with
K8sKeysToPath("", ..., ...) which hardcodes K8s semantics and drops the cluster
segment; change the two lookups in containerprofile_processor.go to use the
host-aware builders instead (use BuildContainerProfileKey for the container
profile lookup that feeds GetContainerProfileMetadata and use the SBOM key
builder used elsewhere for the SBOM lookup before calling GetSbom), ensuring you
pass the same host-type, cluster/namespace and name parameters as used by the
save/parse paths (replace the K8sKeysToPath calls surrounding the
GetContainerProfileMetadata and GetSbom usages, and keep names.ImageInfoToSlug
for sbom name generation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3dadb274-0dff-4586-8cfa-50440ae0e900
📒 Files selected for processing (1)
pkg/registry/file/containerprofile_processor.go
shanyl9
force-pushed
the
support-ecs-host-storage
branch
3 times, most recently
from
b2ccbef to
89530a9
Compare
|
Summary:
|
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
pkg/config/config.go (1)
17-19:⚠️ Potential issue | 🟠 MajorValidate
HostTypevalues inLoadConfigto reject unsupported enum values.The
HostTypefield usesmapstructurewhich accepts any string without validation. Invalid values will silently flow through to storage key generation, potentially causing unexpected behavior. Consider adding validation after unmarshalling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/config/config.go` around lines 17 - 19, After unmarshalling in LoadConfig, validate the HostType field (armotypes.HostType) on the config struct and return an error for any unsupported value instead of allowing arbitrary strings to pass through; update the LoadConfig function to check cfg.HostType (or use a helper isValidHostType/ switch over known armotypes.HostType constants) and return a descriptive error when the value is not one of the supported enum values so invalid HostType cannot silently flow into storage key generation.go.mod (1)
72-72:⚠️ Potential issue | 🟠 MajorIndirect dependency
github.com/docker/cli v28.3.3+incompatiblehas a known vulnerability (GHSA-p436-gjf2-799p).This is a local privilege escalation on Windows. Since this is an indirect dependency, upgrading may require bumping a parent module that pulls it in, or adding an explicit replace directive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 72, The go.mod lists an indirect dependency github.com/docker/cli v28.3.3+incompatible which has a known vulnerability; fix by either identifying the direct parent module that pulls it in (use go list -m all or go mod graph) and update that parent to a version that depends on a non-vulnerable docker/cli, or add an explicit requirement/replace in go.mod to a patched docker/cli version (e.g., run go get github.com/docker/cli@<safe-version> or add a replace directive to the safe version), then run go mod tidy and verify with go list -m all that github.com/docker/cli is bumped to the secure release.
🤖 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/registry/file/applicationprofile_processor.go`:
- Around line 63-64: The lookup builds a path using K8sKeysToPath that uses the
wrong kind segment "sbomsyft" so a.storageImpl.GetSbom never finds stored SBOMs
(which are saved under "sbomsyfts"); update the path construction in the code
that calls K8sKeysToPath (where key := K8sKeysToPath("",
"spdx.softwarecomposition.kubescape.io", "sbomsyft", "", a.defaultNamespace,
sbomName)) to use the plural kind "sbomsyfts" before calling
a.storageImpl.GetSbom(ctx, key) so the key matches how SBOMs are written and
sbomSet will be populated.
In `@pkg/registry/file/containerprofile_processor.go`:
- Around line 145-146: The lookup for the SBOM is using the wrong resource kind
string so ContainerProfileStorage.GetSbom never finds the stored SBOM; update
the key construction in the code that builds "key" (call site uses
K8sKeysToPath) to use the correct kind "sbomsyfts" instead of "sbomsyft" so the
K8sKeysToPath(...) call (and subsequent ContainerProfileStorage.GetSbom(ctx,
key)) points to the actual stored SBOM path.
In `@pkg/registry/file/containerprofile_storage.go`:
- Around line 116-121: The current key construction in UpdateApplicationProfile
uses K8sKeysToPath and drops host-specific fields from
armotypes.ProfileIdentifier (losing id.Cluster, AWS account/region, HostID
etc.), so implement a helper (e.g., buildProfileKey) that switches on
id.HostType and returns the correct path using K8sKeysToPath, ECSKeysToPath or
HostKeysToPath (and includes id.Cluster, id.AWSAccountID/id.Region, or id.HostID
as appropriate), then replace the existing K8sKeysToPath(...) calls in
UpdateApplicationProfile (and the other similar method around the 168-172
region) to call buildProfileKey(prefix, root, "applicationprofiles", id, slug)
(or the appropriate kind/name) so keys preserve host-specific identity fields.
In `@pkg/registry/file/sqlite.go`:
- Around line 90-100: BuildContainerProfileKey now emits cluster/ECS/host-aware
paths (via ECSKeysToPath, K8sKeysToPath, HostKeysToPath) but SQLite storage
currently persists only kind/namespace/name causing collisions and loss of
distinguishing fields; update the SQLite persistence logic so it stores and
reads the full key shape used by BuildContainerProfileKey (either persist the
full generated path string instead of only kind/namespace/name, or extend the
schema to include cluster/AWSAccountID/region/hostID fields and preserve which
path-builder was used), and update the corresponding read/delete code paths that
reconstruct keys to use the stored full key or the stored distinguishing fields
so keys round-trip correctly (apply this change consistently for the other
affected builders referenced in the diff).
- Around line 77-84: K8sPathToKeys currently uses strings.SplitN(path, "/", 6)
and pads which shifts legacy paths (like "/root/kind/ns/name") into the wrong
slots; update K8sPathToKeys to use strings.Split(path, "/") and then handle
segments explicitly: if the split has a leading empty element and exactly 5
parts (legacy form), map cluster="" and assign kind=parts[1],
namespace=parts[2], name=parts[3] (leaving the remaining slots empty) otherwise
normalize/pad to 6 elements and return them in the existing return order so
existing full paths still map to cluster, kind, namespace, name, etc., without
shifting.
---
Duplicate comments:
In `@go.mod`:
- Line 72: The go.mod lists an indirect dependency github.com/docker/cli
v28.3.3+incompatible which has a known vulnerability; fix by either identifying
the direct parent module that pulls it in (use go list -m all or go mod graph)
and update that parent to a version that depends on a non-vulnerable docker/cli,
or add an explicit requirement/replace in go.mod to a patched docker/cli version
(e.g., run go get github.com/docker/cli@<safe-version> or add a replace
directive to the safe version), then run go mod tidy and verify with go list -m
all that github.com/docker/cli is bumped to the secure release.
In `@pkg/config/config.go`:
- Around line 17-19: After unmarshalling in LoadConfig, validate the HostType
field (armotypes.HostType) on the config struct and return an error for any
unsupported value instead of allowing arbitrary strings to pass through; update
the LoadConfig function to check cfg.HostType (or use a helper isValidHostType/
switch over known armotypes.HostType constants) and return a descriptive error
when the value is not one of the supported enum values so invalid HostType
cannot silently flow into storage key generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 90ae2f8d-e534-4163-ad44-364ce09d0b9d
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
go.modpkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy.gopkg/apis/softwarecomposition/networkpolicy/v2/networkpolicy_test.gopkg/config/config.gopkg/registry/file/applicationprofile_processor.gopkg/registry/file/cleanup.gopkg/registry/file/containerprofile_aggregator_test.gopkg/registry/file/containerprofile_processor.gopkg/registry/file/containerprofile_processor_test.gopkg/registry/file/containerprofile_storage.gopkg/registry/file/containerprofile_storage_interface.gopkg/registry/file/generatednetworkpolicy_test.gopkg/registry/file/sqlite.gopkg/registry/file/sqlite_test.gopkg/registry/file/storage.gopkg/registry/file/storage_test.gopkg/registry/file/vulnerabilitysummarystorage_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/registry/file/containerprofile_aggregator_test.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
go.mod (1)
72-72:⚠️ Potential issue | 🟠 MajorThe docker/cli security vulnerability remains unresolved.
github.com/docker/cli v28.3.3+incompatibleis still affected by GHSA-p436-gjf2-799p (CVE-2025-15558), a local privilege escalation vulnerability on Windows. This issue was previously flagged and has not been addressed. The fix is available in v29.2.0 and later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 72, The go.mod still pins github.com/docker/cli at v28.3.3 which is vulnerable; update the module requirement for github.com/docker/cli to v29.2.0 or later (remove the +incompatible if applicable), then run the Go tooling to update modules (e.g., go get github.com/docker/cli@v29.2.0 and go mod tidy) so go.mod and go.sum are regenerated to reference the fixed version.
🧹 Nitpick comments (1)
go.mod (1)
31-31: OpenTelemetry ecosystem updated to v1.42.0.The OpenTelemetry core and related instrumentation libraries have been upgraded from v1.40.0 to v1.42.0 (and corresponding v0.62.0 for instrumentation). This is a coordinated ecosystem update that should maintain backward compatibility.
Ensure that observability (tracing, metrics, logging) functions correctly after this update, particularly if the application relies on specific OpenTelemetry behaviors.
Also applies to: 172-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 31, The dependency upgrade to go.opentelemetry.io/otel v1.42.0 requires verifying and adapting our OpenTelemetry initialization and instrumentation usage: check functions like setupTracing, initOpenTelemetry, NewTracerProvider (and any places importing go.opentelemetry.io/otel, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/metric or instrumentation packages) for any API changes and update calls accordingly, bump related instrumentation modules to the matching v0.62.0, run go get ./... and go mod tidy to update go.sum, and run the app’s tracing/metrics smoke tests to confirm traces/metrics/log correlation still work and adjust exporter configuration or resource attributes if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Line 72: The go.mod still pins github.com/docker/cli at v28.3.3 which is
vulnerable; update the module requirement for github.com/docker/cli to v29.2.0
or later (remove the +incompatible if applicable), then run the Go tooling to
update modules (e.g., go get github.com/docker/cli@v29.2.0 and go mod tidy) so
go.mod and go.sum are regenerated to reference the fixed version.
---
Nitpick comments:
In `@go.mod`:
- Line 31: The dependency upgrade to go.opentelemetry.io/otel v1.42.0 requires
verifying and adapting our OpenTelemetry initialization and instrumentation
usage: check functions like setupTracing, initOpenTelemetry, NewTracerProvider
(and any places importing go.opentelemetry.io/otel,
go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/metric or
instrumentation packages) for any API changes and update calls accordingly, bump
related instrumentation modules to the matching v0.62.0, run go get ./... and go
mod tidy to update go.sum, and run the app’s tracing/metrics smoke tests to
confirm traces/metrics/log correlation still work and adjust exporter
configuration or resource attributes if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd7da9d3-2380-43c0-9efd-364ce222bcf2
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
There was a problem hiding this comment.
♻️ Duplicate comments (1)
go.mod (1)
72-72:⚠️ Potential issue | 🟠 MajorUpgrade
docker/clito v29.2.0 or later before merging.The indirect dependency
github.com/docker/cli v28.3.3+incompatibleremains vulnerable to GHSA-p436-gjf2-799p (CVE-2025-15558), a local privilege escalation issue on Windows. This was previously flagged in past reviews and remains unresolved.The fix is available in v29.2.0 or later. Update this dependency by upgrading its parent or adding an explicit require/replace directive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 72, The go.mod currently pins the indirect dependency github.com/docker/cli at v28.3.3+incompatible which is vulnerable; update go.mod to require github.com/docker/cli at v29.2.0 or later (or add a replace directive) so the resolved version is >= v29.2.0, or alternatively bump the parent module that transitively requires github.com/docker/cli to a version that pulls v29.2.0+; ensure you run go mod tidy / go get github.com/docker/cli@v29.2.0 (or newer) and commit the updated go.mod and go.sum.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Line 72: The go.mod currently pins the indirect dependency
github.com/docker/cli at v28.3.3+incompatible which is vulnerable; update go.mod
to require github.com/docker/cli at v29.2.0 or later (or add a replace
directive) so the resolved version is >= v29.2.0, or alternatively bump the
parent module that transitively requires github.com/docker/cli to a version that
pulls v29.2.0+; ensure you run go mod tidy / go get
github.com/docker/cli@v29.2.0 (or newer) and commit the updated go.mod and
go.sum.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61cc6caf-0dee-42cf-9383-75f9f303d908
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
shanyl9
force-pushed
the
support-ecs-host-storage
branch
2 times, most recently
from
573afd3 to
5412960
Compare
|
Summary:
|
|
Summary:
|
1 similar comment
|
Summary:
|
shanyl9
commented
Mar 18, 2026
|
Summary:
|
|
Summary:
|
shanyl9
force-pushed
the
support-ecs-host-storage
branch
2 times, most recently
from
e085667 to
9fb4fb0
Compare
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com> Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com> Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com> Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
Signed-off-by: shanyl9 <shanyl@armosec.io>
shanyl9
force-pushed
the
support-ecs-host-storage
branch
from
9fb4fb0 to
cfe9e46
Compare
matthyx
approved these changes
Mar 19, 2026
|
Summary:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sorry, we do not accept changes directly against this repository. Please see
CONTRIBUTING.md for information on where and how to contribute instead.
Summary by CodeRabbit