WIP: CORENET-7045: INFW K8s rebase to 1.36.1#713
Conversation
Signed-off-by: Vincenzo Palmieri <287618728+vinnie1110@users.noreply.github.com>
|
@vinnie1110: This pull request references CORENET-7045 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vinnie1110 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThe change upgrades the project to Go 1.26 and version 5.0.0, refreshes Kubernetes and controller tooling, updates release manifests, adjusts controller and eBPF map handling, and modernizes diagnostics and end-to-end test helpers. ChangesGo 1.26 and tooling
Runtime behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
pkg/webhook/webhook.go (1)
37-45: ⚡ Quick winUse the typed validator path instead of adding new deprecation suppressions.
webhook.CustomValidatorandWebhookBuilder.WithCustomValidatorare already deprecated in controller-runtime v0.24.1, and the typed replacement isadmission.Validator[T]wired withWithValidator. Carrying fresh//nolint:staticchecksuppressions into the rebase makes the next controller-runtime bump harder for no real gain. (pkg.go.dev)♻️ Suggested direction
- _ webhook.CustomValidator = &IngressNodeFirewallWebhook{ingressnodefwv1alpha1.IngressNodeFirewall{}} //nolint:staticcheck + _ admission.Validator[*ingressnodefwv1alpha1.IngressNodeFirewall] = &IngressNodeFirewallWebhook{} ... - return ctrl.NewWebhookManagedBy(mgr, &ingressnodefwv1alpha1.IngressNodeFirewall{}). //nolint:staticcheck - WithCustomValidator(&IngressNodeFirewallWebhook{}). //nolint:staticcheck + return ctrl.NewWebhookManagedBy(mgr, &ingressnodefwv1alpha1.IngressNodeFirewall{}). + WithValidator(&IngressNodeFirewallWebhook{}). Complete()You'll also need to switch
ValidateCreate,ValidateUpdate, andValidateDeleteto the typed*ingressnodefwv1alpha1.IngressNodeFirewallsignatures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/webhook/webhook.go` around lines 37 - 45, Replace the deprecated untyped webhook wiring: remove the webhook.CustomValidator assignment and the WithCustomValidator calls in SetupWebhookWithManager and instead implement the typed admission.Validator[ingressnodefwv1alpha1.IngressNodeFirewall] on IngressNodeFirewallWebhook and register it via ctrl.NewWebhookManagedBy(mgr, &ingressnodefwv1alpha1.IngressNodeFirewall{}).WithValidator(&IngressNodeFirewallWebhook{}).Complete(); update the method signatures for ValidateCreate, ValidateUpdate, and ValidateDelete to accept *ingressnodefwv1alpha1.IngressNodeFirewall (typed versions) so they satisfy admission.Validator[T], and drop the //nolint:staticcheck suppressions related to CustomValidator/WithCustomValidator.pkg/webhook/webhook_suite_test.go (1)
585-600: ⚡ Quick winFail fast on unsupported protocols in this test helper.
This still silently builds a TCP-shaped rule for any unexpected
protocolvalue. A typo or future enum addition will produce a misleading fixture instead of failing the spec at the callsite.🧪 Suggested guard
- switch protocol { - case ingressnodefwv1alpha1.ProtocolTypeUDP: + switch protocol { + case ingressnodefwv1alpha1.ProtocolTypeTCP: + // keep the preinitialized TCP rule + case ingressnodefwv1alpha1.ProtocolTypeUDP: rule = ingressnodefwv1alpha1.IngressNodeFirewallRules{ SourceCIDRs: []string{cidr}, FirewallProtocolRules: []ingressnodefwv1alpha1.IngressNodeFirewallProtocolRule{ getUDPRule(order, protocol, ports, action), }, } case ingressnodefwv1alpha1.ProtocolTypeSCTP: rule = ingressnodefwv1alpha1.IngressNodeFirewallRules{ SourceCIDRs: []string{cidr}, FirewallProtocolRules: []ingressnodefwv1alpha1.IngressNodeFirewallProtocolRule{ getSCTPRule(order, protocol, ports, action), }, } + default: + Fail(fmt.Sprintf("unsupported transport protocol %q", protocol)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/webhook/webhook_suite_test.go` around lines 585 - 600, The test helper silently builds a TCP-shaped rule for unknown protocols; update the switch on variable protocol (which currently handles ingressnodefwv1alpha1.ProtocolTypeUDP and ProtocolTypeSCTP and constructs rule as ingressnodefwv1alpha1.IngressNodeFirewallRules using getUDPRule/getSCTPRule) to include a default case that fails fast (e.g., t.Fatalf or panic) reporting the unexpected protocol value so typos or new enum values cause the test to fail rather than produce a misleading fixture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Line 2: Replace tag-based base images with digest-pinned references: change
the builder FROM golang:1.26 to the corresponding digest-pinned form
(golang:1.26@sha256:...) and the final stage FROM
gcr.io/distroless/static:nonroot to its digest-pinned form
(gcr.io/distroless/static:nonroot@sha256:...); obtain the canonical sha256
digests using a registry inspect command (e.g., docker pull + docker image
inspect or skopeo inspect) for each image and update the Dockerfile to use those
`@sha256` digests to make builds reproducible and reduce supply-chain risk.
In `@Dockerfile.daemon`:
- Line 1: Replace the two unpinned base images in Dockerfile.daemon — the FROM
golang:1.26 stage and the FROM quay.io/centos/centos:stream8 stage — with
digest-pinned references (e.g., FROM golang:1.26@sha256:<digest> and FROM
quay.io/centos/centos:stream8@sha256:<digest>); fetch the exact sha256 digests
from the respective registries (Docker Hub for golang, Quay for quay.io/centos)
or by pulling the images and inspecting manifest digests, then update the
Dockerfile lines to use those `@sha256` digests so the builds are reproducible.
In `@hack/lint.sh`:
- Line 5: The comment string "# pin golangci-lint version to 1.64.8#1.54.2" is
malformed; replace the concatenated version token "1.64.8#1.54.2" with the
intended single version (e.g., "1.64.8") so the comment reads "# pin
golangci-lint version to 1.64.8" (update the comment text wherever the exact
malformed token appears).
In `@Makefile`:
- Around line 374-375: The curl invocation that downloads the operator-sdk
binary uses the insecure -k flag; update the Makefile command that references
OPERATOR_SDK_VERSION and writes to $(OPERATOR_SDK) to remove -k and use a safer
curl invocation (e.g., keep -L, add --fail and --show-error) so TLS cert
verification is enforced and failures are visible; ensure the subsequent chmod
u+x $(OPERATOR_SDK) step remains unchanged.
In `@pkg/webhook/webhook_suite_test.go`:
- Around line 147-148: The readiness probe closure currently ignores the return
value of conn.Close() (using "_ = conn.Close()" and returning nil), which hides
errors from the Eventually check; update the closure used by the readiness probe
(the function passed to Eventually in webhook_suite_test.go) to propagate the
TLS close error by returning conn.Close() (or checking the error and returning
it) instead of discarding it so Eventually receives and fails on any Close()
error.
In `@README.md`:
- Around line 71-73: README lists mismatched tool versions: it shows
"operator-sdk 1.42.2" and "controller-gen v0.20.1+" while the Makefile was
bumped to v0.21.0; update the README entry for "controller-gen v0.20.1+" to
"controller-gen v0.21.0" so the README's prerequisite versions match the
Makefile defaults.
In `@test/e2e/k8sreporter/reporter.go`:
- Around line 95-96: The defer f.Close() inside the loop keeps many files open
until function exit; change to close each file immediately after writing:
replace "defer func() { _ = f.Close() }()" with a direct close call (e.g., err
:= f.Close(); if err != nil { /* log or handle */ }) right after the fmt.Fprintf
calls that write to f (the variable f used with fmt.Fprintf), and do the same
fix for the other occurrence (the writes around lines 144-145) so each opened
file is closed per iteration and errors from Close are handled or logged.
In `@test/e2e/namespaces/namespaces.go`:
- Line 68: Replace the fmt.Errorf call that currently formats the error with %v
(the call containing the message "failed to delete pods" and the variable err)
so it wraps the underlying error using %w (e.g., fmt.Errorf("failed to delete
pods: %w", err)); update the return in the function in namespaces.go to use %w
to preserve error-chain semantics when returning err.
---
Nitpick comments:
In `@pkg/webhook/webhook_suite_test.go`:
- Around line 585-600: The test helper silently builds a TCP-shaped rule for
unknown protocols; update the switch on variable protocol (which currently
handles ingressnodefwv1alpha1.ProtocolTypeUDP and ProtocolTypeSCTP and
constructs rule as ingressnodefwv1alpha1.IngressNodeFirewallRules using
getUDPRule/getSCTPRule) to include a default case that fails fast (e.g.,
t.Fatalf or panic) reporting the unexpected protocol value so typos or new enum
values cause the test to fail rather than produce a misleading fixture.
In `@pkg/webhook/webhook.go`:
- Around line 37-45: Replace the deprecated untyped webhook wiring: remove the
webhook.CustomValidator assignment and the WithCustomValidator calls in
SetupWebhookWithManager and instead implement the typed
admission.Validator[ingressnodefwv1alpha1.IngressNodeFirewall] on
IngressNodeFirewallWebhook and register it via ctrl.NewWebhookManagedBy(mgr,
&ingressnodefwv1alpha1.IngressNodeFirewall{}).WithValidator(&IngressNodeFirewallWebhook{}).Complete();
update the method signatures for ValidateCreate, ValidateUpdate, and
ValidateDelete to accept *ingressnodefwv1alpha1.IngressNodeFirewall (typed
versions) so they satisfy admission.Validator[T], and drop the
//nolint:staticcheck suppressions related to
CustomValidator/WithCustomValidator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2dfd9c4e-d13e-41cf-90c8-d0f376b014f7
⛔ Files ignored due to path filters (249)
go.sumis excluded by!**/*.sumvendor/github.com/emicklei/go-restful/v3/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/CHANGES.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/curly.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/custom_verb.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/jsr311.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/route.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.cirrus.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.editorconfigis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.gitattributesis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_fen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_inotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_kqueue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_other.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/backend_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/fsnotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_dragonfly.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_freebsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_kqueue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_linux.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_netbsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_openbsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_solaris.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/debug_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/freebsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/unix2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/internal/windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/mkdoc.zshis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/shared.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/staticcheck.confis excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/system_bsd.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fsnotify/fsnotify/system_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/bytestring.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/common.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/encode_map_go117.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/omitzero_pre_go124.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/simplevalue.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/structfields.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/fxamacker/cbor/v2/tag.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-logr/logr/.golangci.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/CONTRIBUTORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/clone.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/custom_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/discard.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/duration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/duration_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/encode_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/equal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/extensions_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/lib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/lib_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/message_set.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_reflect.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_reflect_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_unsafe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/pointer_unsafe_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/properties.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/properties_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/skip_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_marshal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_marshal_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_merge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_unmarshal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/table_unmarshal_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/text.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/text_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/text_parser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/timestamp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/timestamp_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/wrappers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/proto/wrappers_gogo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/sortkeys/sortkeys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/CONTRIBUTORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/buffer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/discard.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/properties.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/proto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/text_decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/text_encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/wire.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/wrappers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/any.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/any/any.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/duration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/duration/duration.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/timestamp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/ptypes/timestamp/timestamp.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/google/btree/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/btree/btree.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/btree/btree_generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/compiler/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/extensions/extension.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/extensions/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/models.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/reader.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/jsonschema/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/OpenAPIv2.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/OpenAPIv2.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv2/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/OpenAPIv3.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/OpenAPIv3.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/annotations.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/annotations.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gnostic-models/openapiv3/document.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/google/gofuzz/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/compression.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/conn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/tls_handshake.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/tls_handshake_116.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gorilla/websocket/x_net_proxy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/connection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/PATENTSis excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/dictionary.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/read.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/moby/spdystream/spdy/write.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/modern-go/reflect2/safe_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/flowrate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/io.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mxk/go-flowrate/flowrate/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/format/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/gomega_dsl.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/internal/async_assertion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/be_comparable_to_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_key_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_error_strictly_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/match_yaml_matcher.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/onsi/gomega/matchers/support/goraph/edge/edge.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/pmezard/go-difflib/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/pmezard/go-difflib/difflib/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/desc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/difflib.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/internal/go_runtime_metrics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_mem_nocgo_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/process_collector_procfsenabled.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/promhttp/instrument_server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/testutil/testutil.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/vec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/client_golang/prometheus/wrap.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/expfmt.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/fuzz.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/openmetrics_create.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/text_create.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/expfmt/text_parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/labels.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/labelset.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/metric.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value_histogram.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/common/model/value_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/Makefile.commonis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/arp.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/buddyinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cmdline.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_armx.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_loong64.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_mipsx.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_others.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_ppcx.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_riscvx.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_s390x.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/cpuinfo_x86.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/crypto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fs_statfs_notype.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fs_statfs_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/fscache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/fs/fs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/readfile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/sysreadfile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/sysreadfile_compat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/internal/util/valueparser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/ipvs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/kernel_hung.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/kernel_random.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/loadavg.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/mdstat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/meminfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/mountinfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/mountstats.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_conntrackstat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_dev.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_dev_snmp6.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_ip_socket.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_protocols.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/prometheus/procfs/net_route.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (51)
.ci-operator.yamlDockerfileDockerfile.daemonDockerfile.daemon.openshiftDockerfile.openshiftMakefileREADME.mdapi/v1alpha1/groupversion_info.gobundle.Dockerfilebundle/manifests/ingress-node-firewall.clusterserviceversion.yamlbundle/manifests/ingressnodefirewall.openshift.io_ingressnodefirewallconfigs.yamlbundle/manifests/ingressnodefirewall.openshift.io_ingressnodefirewallnodestates.yamlbundle/manifests/ingressnodefirewall.openshift.io_ingressnodefirewalls.yamlbundle/metadata/annotations.yamlconfig/crd/bases/ingressnodefirewall.openshift.io_ingressnodefirewallconfigs.yamlconfig/crd/bases/ingressnodefirewall.openshift.io_ingressnodefirewallnodestates.yamlconfig/crd/bases/ingressnodefirewall.openshift.io_ingressnodefirewalls.yamlconfig/manifests/bases/ingress-node-firewall.clusterserviceversion.yamlconfig/olm-install/install-resources.yamlcontrollers/ingressnodefirewall_controller.gocontrollers/ingressnodefirewall_controller_rules_test.gocontrollers/ingressnodefirewall_controller_test.gocontrollers/ingressnodefirewallconfig_controller.gocontrollers/ingressnodefirewallnodestate_controller.gogo.modhack/generators.Dockerfilehack/lint.shmanifests/art.yamlmanifests/ingress-node-firewall.package.yamlmanifests/stable/ingress-node-firewall.clusterserviceversion.yamlmanifests/stable/ingressnodefirewall.openshift.io_ingressnodefirewallconfigs.yamlmanifests/stable/ingressnodefirewall.openshift.io_ingressnodefirewallnodestates.yamlmanifests/stable/ingressnodefirewall.openshift.io_ingressnodefirewalls.yamlopenshift-ci/wait_for_csv.shpkg/bpf-mgr/lib-bpfman.gopkg/ebpf/ingress_node_firewall_events.gopkg/ebpf/ingress_node_firewall_loader.gopkg/ebpfsyncer/ebpfsyncer.gopkg/ebpfsyncer/ebpfsyncer_test.gopkg/render/render.gopkg/utils/utils.gopkg/version/version.gopkg/webhook/webhook.gopkg/webhook/webhook_suite_test.gotest/e2e/events/events.gotest/e2e/exec/exec.gotest/e2e/functional/tests/e2e.gotest/e2e/ingress-node-firewall/ingress-node-firewall.gotest/e2e/k8sreporter/reporter.gotest/e2e/namespaces/namespaces.gotest/e2e/validation/tests/validation.go
| @@ -1,5 +1,5 @@ | |||
| # Build the manager binary | |||
| FROM golang:1.25 as builder | |||
| FROM golang:1.26 as builder | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify unpinned non-Red Hat base images in Dockerfiles
rg -n '^\s*FROM\s+(golang|gcr\.io|docker\.io|ghcr\.io)/[^@[:space:]]+:[^[:space:]]+' --iglob 'Dockerfile*'Repository: openshift/ingress-node-firewall
Length of output: 127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate Dockerfiles/Containerfiles
echo "== Dockerfiles =="
fd -a -t f -g 'Dockerfile*' -g 'Containerfile*' . | sort
echo
echo "== FROM lines (all Dockerfile* and Containerfile*) =="
fd -a -t f -g 'Dockerfile*' -g 'Containerfile*' . -x \
rg -n --no-heading '^\s*FROM\s+' | sort
echo
echo "== FROM golang:* (all Dockerfile* and Containerfile*) =="
fd -a -t f -g 'Dockerfile*' -g 'Containerfile*' . -x \
rg -n --no-heading '^\s*FROM\s+golang:[^@[:space:]]+' --hidden | sort
echo
echo "== Initial targeted regex again (for mutable tags) =="
rg -n '^\s*FROM\s+(golang|gcr\.io|docker\.io|ghcr\.io)/[^@[:space:]]+:[^[:space:]]+' --iglob 'Dockerfile*' || trueRepository: openshift/ingress-node-firewall
Length of output: 1178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a -t f -g 'Dockerfile*' . | sort | while read -r f; do
echo "== $f =="
nl -ba "$f" | rg -n '^\s*FROM\s+' || true
echo
done
echo "== Focus: golang:1.26 FROM lines (all Dockerfiles) =="
fd -a -t f -g 'Dockerfile*' . | sort | xargs -I{} sh -c 'rg -n "^\s*FROM\s+golang:1\.26\b" "{}" || true'Repository: openshift/ingress-node-firewall
Length of output: 750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Occurrences of exact lowercase variant =="
rg -n --with-filename '^\s*FROM\s+golang:1\.26\s+as\s+builder\b' Dockerfile* Dockerfile* 2>/dev/null || true
rg -n --with-filename '^\s*FROM\s+golang:1\.26\s+as\s+builder\b' . -g 'Dockerfile*' || true
echo
echo "== Occurrences of exact uppercase variant =="
rg -n --with-filename '^\s*FROM\s+golang:1\.26\s+AS\s+builder\b' . -g 'Dockerfile*' || trueRepository: openshift/ingress-node-firewall
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ./Dockerfile: line 1-40 =="
awk 'NR>=1 && NR<=40 {printf "%d:%s\n", NR, $0}' Dockerfile
echo
echo "== ./Dockerfile: FROM lines (full) =="
awk 'match($0,/^\s*FROM/){print NR ":" $0}' Dockerfile
echo
echo "== ./Dockerfile.daemon: FROM lines (full) =="
awk 'match($0,/^\s*FROM/){print NR ":" $0}' Dockerfile.daemonRepository: openshift/ingress-node-firewall
Length of output: 1319
Review comment
File: Dockerfile
Lines: 2-2
Snippet showing the final state of code at these lines
FROM golang:1.26 as builder
Comment
Pin non-Red Hat base images by digest.
golang:1.26 (builder, line 2) and gcr.io/distroless/static:nonroot (final stage, line 24) are non-Red Hat tag-based images; pin them by @sha256:... to keep builds reproducible and reduce supply-chain risk.
As per coding guidelines for **/{Dockerfile,Containerfile}*: "non-RH images: pin by digest".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` at line 2, Replace tag-based base images with digest-pinned
references: change the builder FROM golang:1.26 to the corresponding
digest-pinned form (golang:1.26@sha256:...) and the final stage FROM
gcr.io/distroless/static:nonroot to its digest-pinned form
(gcr.io/distroless/static:nonroot@sha256:...); obtain the canonical sha256
digests using a registry inspect command (e.g., docker pull + docker image
inspect or skopeo inspect) for each image and update the Dockerfile to use those
`@sha256` digests to make builds reproducible and reduce supply-chain risk.
| @@ -1,4 +1,4 @@ | |||
| FROM golang:1.25 AS builder | |||
| FROM golang:1.26 AS builder | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List all FROM lines so pinned vs unpinned bases can be checked quickly
rg -n '^\s*FROM\s+' --iglob 'Dockerfile*'Repository: openshift/ingress-node-firewall
Length of output: 750
Pin non-RH FROM images in Dockerfile.daemon by digest
Dockerfile.daemon:1usesgolang:1.26without a digest.Dockerfile.daemon:7usesquay.io/centos/centos:stream8without a digest.
Update both toFROM ...@sha256:<digest>as required for non-RH images.
🧰 Tools
🪛 Trivy (0.69.3)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile.daemon` at line 1, Replace the two unpinned base images in
Dockerfile.daemon — the FROM golang:1.26 stage and the FROM
quay.io/centos/centos:stream8 stage — with digest-pinned references (e.g., FROM
golang:1.26@sha256:<digest> and FROM
quay.io/centos/centos:stream8@sha256:<digest>); fetch the exact sha256 digests
from the respective registries (Docker Hub for golang, Quay for quay.io/centos)
or by pulling the images and inspecting manifest digests, then update the
Dockerfile lines to use those `@sha256` digests so the builds are reproducible.
| curl -Lk https://github.com/operator-framework/operator-sdk/releases/download/$(OPERATOR_SDK_VERSION)/operator-sdk_linux_amd64 > $(OPERATOR_SDK) ;\ | ||
| chmod u+x $(OPERATOR_SDK) ;\ |
There was a problem hiding this comment.
Remove curl -k when downloading operator-sdk binary.
Using -k disables TLS cert verification for a supply-chain download path and enables MITM risk in CI/dev environments.
Suggested fix
- curl -Lk https://github.com/operator-framework/operator-sdk/releases/download/$(OPERATOR_SDK_VERSION)/operator-sdk_linux_amd64 > $(OPERATOR_SDK) ;\
+ curl -L https://github.com/operator-framework/operator-sdk/releases/download/$(OPERATOR_SDK_VERSION)/operator-sdk_linux_amd64 > $(OPERATOR_SDK) ;\🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 374 - 375, The curl invocation that downloads the
operator-sdk binary uses the insecure -k flag; update the Makefile command that
references OPERATOR_SDK_VERSION and writes to $(OPERATOR_SDK) to remove -k and
use a safer curl invocation (e.g., keep -L, add --fail and --show-error) so TLS
cert verification is enforced and failures are visible; ensure the subsequent
chmod u+x $(OPERATOR_SDK) step remains unchanged.
| operator-sdk 1.42.2 | ||
|
|
||
| controller-gen v0.20.1+ |
There was a problem hiding this comment.
Keep tool prerequisite versions aligned with Makefile defaults.
README now lists operator-sdk 1.42.2, but still advertises controller-gen v0.20.1+ while Makefile was bumped to v0.21.0. Please update this line to avoid contributor environment drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 71 - 73, README lists mismatched tool versions: it
shows "operator-sdk 1.42.2" and "controller-gen v0.20.1+" while the Makefile was
bumped to v0.21.0; update the README entry for "controller-gen v0.20.1+" to
"controller-gen v0.21.0" so the README's prerequisite versions match the
Makefile defaults.
| }, metav1.ListOptions{}) | ||
| if err != nil { | ||
| return fmt.Errorf("Failed to delete pods %v", err) | ||
| return fmt.Errorf("failed to delete pods %v", err) |
There was a problem hiding this comment.
Wrap the underlying error with %w to keep error-chain semantics.
Using %v loses structured error unwrapping for callers.
💡 Suggested fix
- return fmt.Errorf("failed to delete pods %v", err)
+ return fmt.Errorf("failed to delete pods: %w", err)📝 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.
| return fmt.Errorf("failed to delete pods %v", err) | |
| return fmt.Errorf("failed to delete pods: %w", err) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e/namespaces/namespaces.go` at line 68, Replace the fmt.Errorf call
that currently formats the error with %v (the call containing the message
"failed to delete pods" and the variable err) so it wraps the underlying error
using %w (e.g., fmt.Errorf("failed to delete pods: %w", err)); update the return
in the function in namespaces.go to use %w to preserve error-chain semantics
when returning err.
…new firewall rule is created
|
/retest |
Signed-off-by: Vincenzo Palmieri <287618728+vinnie1110@users.noreply.github.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@manifests/art.yaml`:
- Around line 9-10: Update the skipRange replacement in the manifest so its
lower bound remains aligned with the pre-5.0.0 range, matching existing 4.x
CSVs; only use FULL_VER for the upper bound and preserve normal upgrades to
5.0.0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| - search: "olm.skipRange: '>=5.0.0-0 <{MAJOR}.{MINOR}.0'" | ||
| replace: "olm.skipRange: '>=5.0.0-0 <{FULL_VER}'" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the pre-5.0.0 upgrade range.
Changing the search value to >=5.0.0-0 no longer matches the current CSV’s olm.skipRange: '>=4.11.0 <5.0.0'. For FULL_VER=5.0.0, it would also generate the empty range >=5.0.0-0 <5.0.0, preventing normal upgrades from existing 4.x versions. Keep the search lower bound aligned with the previous release range.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@manifests/art.yaml` around lines 9 - 10, Update the skipRange replacement in
the manifest so its lower bound remains aligned with the pre-5.0.0 range,
matching existing 4.x CSVs; only use FULL_VER for the upper bound and preserve
normal upgrades to 5.0.0.
Signed-off-by: Vincenzo Palmieri <287618728+vinnie1110@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Dockerfile (1)
2-2: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin non-Red Hat base images by digest.
As per path instructions for
**/{Dockerfile,Containerfile}*, non-Red Hat images must be pinned by digest. Please append the@sha256:...digest togolang:1.26to ensure reproducible builds and reduce supply chain risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 2, Update the builder stage’s golang:1.26 base image reference in the FROM instruction to include its full `@sha256` digest, preserving the existing image tag and stage alias while ensuring the non-Red Hat base image is pinned immutably.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 99: Update the indirect go.opentelemetry.io/otel/sdk dependency from
v1.40.0 to a patched version that resolves GHSA-hfvc-g4fc-pqhx, keeping the
module declaration consistent with the corresponding OpenTelemetry dependency
versions.
In `@test/e2e/exec/exec.go`:
- Around line 76-80: Update the goroutine around io.Copy in the stdin streaming
flow to capture its error and propagate it through writer.CloseWithError instead
of discarding it. Preserve normal writer closure when io.Copy succeeds, and
handle the close result without ignoring errors.
---
Duplicate comments:
In `@Dockerfile`:
- Line 2: Update the builder stage’s golang:1.26 base image reference in the
FROM instruction to include its full `@sha256` digest, preserving the existing
image tag and stage alias while ensuring the non-Red Hat base image is pinned
immutably.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.40.0 // indirect | ||
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.40.0 // indirect | ||
| go.opentelemetry.io/otel/metric v1.44.0 // indirect | ||
| go.opentelemetry.io/otel/sdk v1.40.0 // indirect |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Address known vulnerability in go.opentelemetry.io/otel/sdk.
The updated version 1.40.0 is flagged for a high-severity vulnerability (GHSA-hfvc-g4fc-pqhx) where the BSD kenv command does not use an absolute path, enabling PATH hijacking. As per path instructions, please update this indirect dependency to a patched version to resolve the known CVE.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 99-99: go.opentelemetry.io/otel/sdk 1.40.0: opentelemetry-go: BSD kenv command not using absolute path enables PATH hijacking
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 99, Update the indirect go.opentelemetry.io/otel/sdk
dependency from v1.40.0 to a patched version that resolves GHSA-hfvc-g4fc-pqhx,
keeping the module declaration consistent with the corresponding OpenTelemetry
dependency versions.
Sources: Path instructions, Linters/SAST tools
| go func() { | ||
| defer writer.Close() | ||
| defer func() { _ = writer.Close() }() | ||
| buf := bytes.NewBufferString(stdin) | ||
| _, _ = io.Copy(writer, buf) | ||
| }() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Propagate the io.Copy error to the pipe writer.
The current implementation explicitly ignores the return values of io.Copy and writer.Close(). If writing to the remote command's stdin stream fails, the error is suppressed, which can lead to opaque test hangs or unexpected behavior. As per path instructions, never ignore error returns.
Instead of discarding the error, capture it and propagate it to the stream reader using writer.CloseWithError().
💡 Proposed fix
- go func() {
- defer func() { _ = writer.Close() }()
- buf := bytes.NewBufferString(stdin)
- _, _ = io.Copy(writer, buf)
- }()
+ go func() {
+ buf := bytes.NewBufferString(stdin)
+ _, err := io.Copy(writer, buf)
+ _ = writer.CloseWithError(err)
+ }()📝 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.
| go func() { | |
| defer writer.Close() | |
| defer func() { _ = writer.Close() }() | |
| buf := bytes.NewBufferString(stdin) | |
| _, _ = io.Copy(writer, buf) | |
| }() | |
| go func() { | |
| buf := bytes.NewBufferString(stdin) | |
| _, err := io.Copy(writer, buf) | |
| _ = writer.CloseWithError(err) | |
| }() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e/exec/exec.go` around lines 76 - 80, Update the goroutine around
io.Copy in the stdin streaming flow to capture its error and propagate it
through writer.CloseWithError instead of discarding it. Preserve normal writer
closure when io.Copy succeeds, and handle the close result without ignoring
errors.
Source: Path instructions
|
@vinnie1110: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
INFW K8s rebase to 1.36.1
Updated go mods:
Go: 1.25.0 -> 1.26.0
K8s: 1.32.3 -> 1.36.1
Updated tools:
Controller tools: 0.20.1 -> 0.21.0
Operator SDK: 1.33.0 -> 1.42.2
Golangci linter: 1.54.2 -> 1.64.8
Reviewed a few tests which were failing after updating the linter version.
Updated manifests to reflect the above references.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation