WIP: CORENET-7045: INFW K8s rebase to 1.36.1#713
Open
vinnie1110 wants to merge 2 commits into
Open
Conversation
Signed-off-by: Vincenzo Palmieri <287618728+vinnie1110@users.noreply.github.com>
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@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. |
Contributor
|
[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 |
|
Warning Review limit reached
More reviews will be available in 12 minutes and 27 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR upgrades the ingress-node-firewall operator from version 4.22.0 to 5.0.0, including Go 1.25 → 1.26, operator-sdk 1.33 → 1.42, and controller-tools v0.20 → v0.21. The changes span version strings across manifests, build tooling updates, eBPF map field refactoring, webhook registration pattern modernization, and widespread code cleanup including deprecated API replacements and explicit error handling. Version Bump and Build Tooling
CRD and Operator Manifests
Core Code Refactoring
Code Modernization and Testing Infrastructure
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
|
|
||
| # pin golangci-lint version to 1.54.2 | ||
| VERSION=v1.54.2 | ||
| # pin golangci-lint version to 1.64.8#1.54.2 |
There was a problem hiding this comment.
Fix the malformed version comment.
The comment includes an accidental concatenation (1.64.8#1.54.2), which is misleading during maintenance.
Suggested fix
-# pin golangci-lint version to 1.64.8#1.54.2
+# pin golangci-lint version to 1.64.8📝 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.
Suggested change
| # pin golangci-lint version to 1.64.8#1.54.2 | |
| # pin golangci-lint version to 1.64.8 |
🤖 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 `@hack/lint.sh` at 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).
Comment on lines
+374
to
+375
| 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.
Comment on lines
+147
to
148
| _ = conn.Close() | ||
| return nil |
There was a problem hiding this comment.
Propagate the TLS close error from the readiness probe.
If conn.Close() starts failing here, Eventually still returns nil and hides the signal. Returning the close error keeps the probe honest.
🔧 Small fix
- _ = conn.Close()
- return nil
+ return conn.Close()As per coding guidelines "Never ignore error returns".
📝 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.
Suggested change
| _ = conn.Close() | |
| return nil | |
| return conn.Close() |
🤖 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 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.
Comment on lines
+71
to
73
| 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.
Comment on lines
+95
to
+96
| defer func() { _ = f.Close() }() | ||
| _, _ = fmt.Fprintf(f, "-----------------------------------\n") |
There was a problem hiding this comment.
Close files per iteration; avoid defer in the outer loop body.
These defer calls execute only when the whole function returns, so many files can stay open at once and exhaust descriptors on large pod sets.
💡 Suggested fix
for _, pod := range pods.Items {
f, err := logFileFor(r.reportPath, dirName, pod.Namespace+"-pods_specs")
if err != nil {
fmt.Fprintf(os.Stderr, "failed to open pods_specs file: %v\n", dirName)
return
}
- defer func() { _ = f.Close() }()
- _, _ = fmt.Fprintf(f, "-----------------------------------\n")
- j, err := json.MarshalIndent(pod, "", " ")
- if err != nil {
- fmt.Println("Failed to marshal pods", err)
- return
- }
- _, _ = fmt.Fprintln(f, string(j))
+ _, _ = fmt.Fprintf(f, "-----------------------------------\n")
+ j, err := json.MarshalIndent(pod, "", " ")
+ if err != nil {
+ _ = f.Close()
+ fmt.Println("Failed to marshal pods", err)
+ return
+ }
+ _, _ = fmt.Fprintln(f, string(j))
+ _ = f.Close()
} for _, pod := range pods.Items {
...
f, err := logFileFor(r.reportPath, dirName, pod.Namespace+"-pods_logs")
if err != nil {
fmt.Fprintf(os.Stderr, "failed to open pods_logs file: %v\n", dirName)
return
}
- defer func() { _ = f.Close() }()
containersToLog := make([]corev1.Container, 0)
...
for _, container := range containersToLog {
logs, err := r.clients.Pods(pod.Namespace).GetLogs(pod.Name, &corev1.PodLogOptions{Container: container.Name}).DoRaw(context.Background())
if err == nil {
_, _ = fmt.Fprintf(f, "-----------------------------------\n")
_, _ = fmt.Fprintf(f, "Dumping logs for pod %s-%s-%s\n", pod.Namespace, pod.Name, container.Name)
_, _ = fmt.Fprintln(f, string(logs))
}
}
+ _ = f.Close()
}Also applies to: 144-145
🤖 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/k8sreporter/reporter.go` around lines 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.
| }, 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.
Suggested change
| 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
Author
|
/retest |
Contributor
|
@vinnie1110: all tests passed! 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. |
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
3 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.
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