Skip to content

OCPBUGS-85826: rebase to pick up dependency updates#237

Open
danwinship wants to merge 18 commits into
openshift:mainfrom
danwinship:rebase-20260515
Open

OCPBUGS-85826: rebase to pick up dependency updates#237
danwinship wants to merge 18 commits into
openshift:mainfrom
danwinship:rebase-20260515

Conversation

@danwinship
Copy link
Copy Markdown
Contributor

@danwinship danwinship commented May 15, 2026

This picks up the last six months of upstream work. Not too much of interest besides the dependency updates, which include a fix for a CVE that doesn't affect the Linux plugins but might affect the Windows plugins. (I didn't bother to figure out.)

/assign @bpickard22

Summary by CodeRabbit

  • New Features

    • Port mapping: added MasqAll masquerading option.
    • Source-based routing: added static gateway config and optional source-address hinting.
  • Bug Fixes

    • Clearer IP address error messages.
    • Bandwidth plugin skips checks for empty configs.
    • Improved IPv6 route handling when moving interfaces into VRFs.
  • Tests

    • Added/expanded tests for MasqAll, SBR, IPAM ECMP, and static IPAM gateway preservation.
  • Chores

    • Bumped Go/toolchain, dependencies, CI images, Docker build bases, and linter/workflow configs.

squeed and others added 16 commits December 16, 2025 17:31
Somehow we missed this case; if CHECK is called with no bandwidth
configuration, we segfault. Oops.

Fixes: #1221

Signed-off-by: Casey Callendrello <c1@caseyc.net>
The previous implementation filtered out routes without an explicit
source address (route.Src == nil), which incorrectly removed routes
added by IPAM plugins. IPAM plugins typically configure routes without
setting a source address, causing those routes to be lost when the
interface was moved to the VRF.

The SCOPE_UNIVERSE filter already excludes local and connected routes
that are automatically recreated by the kernel, so the additional
route.Src filter was both unnecessary and harmful.

Fixes #1223

Signed-off-by: Marcelo Guerrero <marcegue@cisco.com>
The "test" prefix on hostVethName exceeds the 15-char Linux interface
name limit (IFNAMSIZ), causing ERANGE instead of EEXIST. Shorten the
prefix so the test exercises the intended error path.

Signed-off-by: Marcelo Guerrero <marcegue@cisco.com>
Signed-off-by: sbiradar10 <sbiradar@redhat.com>
Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 8 to 9.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v8...v9)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-version: '9'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
This PR includes the IP in the `AddrAdd` error message which could help debug better with any issues while adding the IP address to the bridge and improves the readability.

Signed-off-by: Amulyam24 <amulmek1@in.ibm.com>
This reflects the version used in the GitHub workflows.

Signed-off-by: Mads Jensen <atombrella@users.noreply.github.com>
Signed-off-by: Mads Jensen <atombrella@users.noreply.github.com>
Signed-off-by: Tomofumi Hayashi <tohayash@redhat.com>
Bumps the golang group with 7 updates in the / directory:

| Package | From | To |
| --- | --- | --- |
| [github.com/buger/jsonparser](https://github.com/buger/jsonparser) | `1.1.1` | `1.1.2` |
| [github.com/coreos/go-systemd/v22](https://github.com/coreos/go-systemd) | `22.6.0` | `22.7.0` |
| [github.com/godbus/dbus/v5](https://github.com/godbus/dbus) | `5.1.0` | `5.2.2` |
| [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) | `2.25.1` | `2.28.1` |
| [github.com/opencontainers/selinux](https://github.com/opencontainers/selinux) | `1.13.0` | `1.13.1` |
| [github.com/safchain/ethtool](https://github.com/safchain/ethtool) | `0.6.2` | `0.7.0` |
| [sigs.k8s.io/knftables](https://github.com/kubernetes-sigs/knftables) | `0.0.18` | `0.0.21` |



Updates `github.com/buger/jsonparser` from 1.1.1 to 1.1.2
- [Release notes](https://github.com/buger/jsonparser/releases)
- [Commits](buger/jsonparser@v1.1.1...v1.1.2)

Updates `github.com/coreos/go-systemd/v22` from 22.6.0 to 22.7.0
- [Release notes](https://github.com/coreos/go-systemd/releases)
- [Commits](coreos/go-systemd@v22.6.0...v22.7.0)

Updates `github.com/godbus/dbus/v5` from 5.1.0 to 5.2.2
- [Release notes](https://github.com/godbus/dbus/releases)
- [Commits](godbus/dbus@v5.1.0...v5.2.2)

Updates `github.com/onsi/ginkgo/v2` from 2.25.1 to 2.28.1
- [Release notes](https://github.com/onsi/ginkgo/releases)
- [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md)
- [Commits](onsi/ginkgo@v2.25.1...v2.28.1)

Updates `github.com/onsi/gomega` from 1.38.1 to 1.39.0
- [Release notes](https://github.com/onsi/gomega/releases)
- [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md)
- [Commits](onsi/gomega@v1.38.1...v1.39.0)

Updates `github.com/opencontainers/selinux` from 1.13.0 to 1.13.1
- [Release notes](https://github.com/opencontainers/selinux/releases)
- [Commits](opencontainers/selinux@v1.13.0...v1.13.1)

Updates `github.com/safchain/ethtool` from 0.6.2 to 0.7.0
- [Release notes](https://github.com/safchain/ethtool/releases)
- [Commits](safchain/ethtool@v0.6.2...v0.7.0)

Updates `golang.org/x/sys` from 0.35.0 to 0.40.0
- [Commits](golang/sys@v0.35.0...v0.40.0)

Updates `sigs.k8s.io/knftables` from 0.0.18 to 0.0.21
- [Changelog](https://github.com/kubernetes-sigs/knftables/blob/master/CHANGELOG.md)
- [Commits](kubernetes-sigs/knftables@v0.0.18...v0.0.21)

---
updated-dependencies:
- dependency-name: github.com/buger/jsonparser
  dependency-version: 1.1.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
- dependency-name: github.com/coreos/go-systemd/v22
  dependency-version: 22.7.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: github.com/godbus/dbus/v5
  dependency-version: 5.2.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: github.com/onsi/ginkgo/v2
  dependency-version: 2.28.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: github.com/onsi/gomega
  dependency-version: 1.39.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: github.com/opencontainers/selinux
  dependency-version: 1.13.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
- dependency-name: github.com/safchain/ethtool
  dependency-version: 0.7.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: golang.org/x/sys
  dependency-version: 0.40.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: sigs.k8s.io/knftables
  dependency-version: 0.0.21
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.69.0 to 1.79.3.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.69.0...v1.79.3)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-version: 1.79.3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
- add static IPAM spec proving multiple routes with the same dst keep distinct GWs
- add ConfigureIface ECMP test that inspects netlink routes for both gateways

Signed-off-by: Yusho Yamaguchi <ys-yamaguchi@kddi.com>
Bumps the golang group with 3 updates: [github.com/Microsoft/hcsshim](https://github.com/Microsoft/hcsshim), [github.com/onsi/gomega](https://github.com/onsi/gomega) and [golang.org/x/sys](https://github.com/golang/sys).


Updates `github.com/Microsoft/hcsshim` from 0.13.0 to 0.14.0
- [Release notes](https://github.com/Microsoft/hcsshim/releases)
- [Commits](microsoft/hcsshim@v0.13.0...v0.14.0)

Updates `github.com/onsi/gomega` from 1.39.0 to 1.39.1
- [Release notes](https://github.com/onsi/gomega/releases)
- [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md)
- [Commits](onsi/gomega@v1.39.0...v1.39.1)

Updates `golang.org/x/sys` from 0.40.0 to 0.42.0
- [Commits](golang/sys@v0.40.0...v0.42.0)

---
updated-dependencies:
- dependency-name: github.com/Microsoft/hcsshim
  dependency-version: 0.14.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
- dependency-name: github.com/onsi/gomega
  dependency-version: 1.39.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
- dependency-name: golang.org/x/sys
  dependency-version: 0.42.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: golang
...

Signed-off-by: dependabot[bot] <support@github.com>
Add two configuration options:

1. "gateways" ([]string): Static gateway IPs that override prevResult.
   Supports dual-stack (one IPv4 and/or one IPv6 address).

2. "addSourceHints" (bool): Preserves subnet routes in the main table
   with source IP hints, enabling destination-based routing to work
   alongside source-based routing

Example:
{
  "type": "sbr",
  "gateways": ["10.0.0.1"],
  "addSourceHints": true
}

Signed-off-by: David Whyte-Gray <40244437+dagrayvid@users.noreply.github.com>
- Use 0.0.0.0/0 or ::/0 as source address when MasqAll is true for full traffic match

Signed-off-by: l1b0k <libokang.lbk@alibaba-inc.com>
@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 15, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@danwinship: This pull request references Jira Issue OCPBUGS-83942, which is invalid:

  • expected the vulnerability to target either version "5.0." or "openshift-5.0.", but it targets "4.22.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This picks up the last six months of upstream work. Not too much of interest besides the dependency updates, which include a fix for a CVE that doesn't affect the Linux plugins but might affect the Windows plugins. (I didn't bother to figure out.)

/assign @bpickard22

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.

@openshift-ci openshift-ci Bot requested review from bpickard22 and dougbtv May 15, 2026 18:29
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6f852e6c-4e7a-457d-8d24-3df5e602ea76

📥 Commits

Reviewing files that changed from the base of the PR and between 0e0b6ef and 8be148b.

📒 Files selected for processing (2)
  • Dockerfile
  • Dockerfile.microshift
🚧 Files skipped from review as they are similar to previous changes (2)
  • Dockerfile
  • Dockerfile.microshift

Walkthrough

Bumps Go/tooling and CI images, refreshes module deps, adds MasqAll portmap behavior with tests, extends SBR with static gateways and optional main-table source-hint routing plus tests, adds ECMP/multi-gateway tests, and applies minor networking fixes.

Changes

Dependency and Toolchain Modernization

Layer / File(s) Summary
Go toolchain and module dependencies
.github/go-version, go.mod
Bumped Go toolchain; updated module go directive and refreshed direct and indirect dependency versions.
CI/images and lint config
Dockerfile, Dockerfile.microshift, .ci-operator.yaml, .github/workflows/test.yaml, .golangci.yml
Updated builder/base image tags and CI build_root_image; golangci-lint action bumped to v9 and linter to v2.10.0; revive linter extended with var-naming and extra bad package names.

Networking and Routing Enhancements

Layer / File(s) Summary
Portmap MasqAll implementation & tests
plugins/meta/portmap/portmap_nftables.go, plugins/meta/portmap/portmap_nftables_test.go
Add MasqAll behavior: masquerade source becomes 0.0.0.0/0 or ::/0 when enabled; IPv4 localhost rule suppressed when MasqAll=true; adjust expected rule counts and add tests for IPv4/IPv6 and rule-count checks.
SBR static gateways and source-hint routing
plugins/meta/sbr/main.go, plugins/meta/sbr/sbr_linux_test.go
Add gateways and addSourceHints fields to PluginConf; parse/validate static gateways, resolve per-IP gateway precedence, set default routes with resolved gateway, and optionally replace main-table subnet routes with Src hints. Tests added for IPv4/IPv6, dual-stack, validation, and addSourceHints scenarios.
ECMP multi-gateway tests
pkg/ipam/ipam_linux_test.go, plugins/ipam/static/static_test.go
Add ConfigureIface test and static IPAM test verifying dual default gateways/ECMP behavior and that both gateway routes are preserved/returned.
VRF route snapshotting
plugins/meta/vrf/vrf.go
Stop filtering out connected IPv6 routes when snapshotting global routes before moving an interface into a VRF.
Minor fixes and tests
pkg/ip/ipmasq_iptables_linux.go, pkg/ip/link_linux_test.go, plugins/main/bridge/bridge.go, plugins/meta/bandwidth/main.go
Insert comment separators in iptables file, shorten host veth test prefix, include IP in bridge ensureAddr error message, and early-exit in bandwidth cmdCheck when no bandwidth configured.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Ipv6 And Disconnected Network Test Compatibility ⚠️ Warning New tests have IPv4-only assumptions: ipam_linux ECMP uses FAMILY_V4 only, static ECMP uses IPv4-only addresses, portmap hardcodes 127.0.0.1 without ::1. Add IPv6 ECMP variants to ipam_linux and static tests. Add ::1 IPv6 localhost to portmap. Test IPv6: /payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-metal-ipi-ovn-ipv6
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main purpose of the PR: a rebase to incorporate dependency updates, which is the primary change across all modified files (Go versions, dependencies, and related toolchain updates).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All test names in modified test files are stable and deterministic. No dynamic content (pod names, timestamps, UUIDs, node names, IP addresses) found in test titles.
Test Structure And Quality ✅ Passed Tests follow quality standards: single-responsibility (each validates one behavior), proper setup/cleanup via BeforeEach and helpers, meaningful assertion messages, consistent with codebase patterns.
Microshift Test Compatibility ✅ Passed New Ginkgo tests are unit/integration tests for CNI plugins, not OpenShift e2e tests. They use netlink/syscall at Linux level and contain no OpenShift API references. MicroShift check not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed CNI plugins repository. Tests are unit tests for networking plugins, not OpenShift e2e tests. SNO check not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed Not applicable to this PR. This is a CNI plugins library rebase; no Kubernetes deployment manifests, operator code, or controllers are modified. No scheduling constraints are introduced.
Ote Binary Stdout Contract ✅ Passed No OTE Binary Stdout Contract violations found. All main() and init() functions properly use the CNI framework. Test suite setup correctly uses GinkgoWriter. All logging is in test blocks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@danwinship danwinship changed the title OCPBUGS-83942: rebase to pick up dependency updates OCPBUGS-85826: rebase to pick up dependency updates May 15, 2026
@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 15, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@danwinship: This pull request references Jira Issue OCPBUGS-85826, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This picks up the last six months of upstream work. Not too much of interest besides the dependency updates, which include a fix for a CVE that doesn't affect the Linux plugins but might affect the Windows plugins. (I didn't bother to figure out.)

/assign @bpickard22

Summary by CodeRabbit

  • New Features

  • Port mapping plugin: Added MasqAll masquerading behavior for configurable port forwarding rules.

  • Source-Based Routing plugin: Added static gateway configuration and source address hinting capabilities.

  • Bug Fixes

  • Enhanced error messages for IP address configuration failures.

  • Optimized bandwidth plugin checks for empty configurations.

  • Improved IPv6 route handling in VRF configurations.

  • Chores

  • Updated Go toolchain and dependencies.

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.

@danwinship
Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@danwinship: This pull request references Jira Issue OCPBUGS-85826, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

/jira refresh

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
plugins/meta/bandwidth/main.go (1)

302-305: ⚡ Quick win

Make the no-bandwidth short-circuit truly early.

At Line 302, this return is correct, but it happens after PrevResult/netns/host-interface work. Move this check earlier in cmdCheck so zero/absent bandwidth cannot fail unrelated validations.

Proposed refactor
 func cmdCheck(args *skel.CmdArgs) error {
 	bwConf, err := parseConfig(args.StdinData)
 	if err != nil {
 		return err
 	}
 
+	bandwidth := getBandwidth(bwConf)
+	// No bandwidth config; nothing to validate.
+	if bandwidth == nil || bandwidth.isZero() {
+		return nil
+	}
+
 	if bwConf.PrevResult == nil {
 		return fmt.Errorf("must be called as a chained plugin")
 	}
@@
-	bandwidth := getBandwidth(bwConf)
-
-	// No bandwidth config; nothing to do.
-	if bandwidth == nil || bandwidth.isZero() {
-		return nil
-	}
-
 	if bandwidth.IngressRate > 0 && bandwidth.IngressBurst > 0 {
🤖 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 `@plugins/meta/bandwidth/main.go` around lines 302 - 305, The code currently
checks "if bandwidth == nil || bandwidth.isZero()" too late in cmdCheck after
running PrevResult/netns/host-interface work; move that nil/zero-bandwidth
short-circuit to the very start of cmdCheck (before any PrevResult handling,
netns setup, or host-interface validations) so that when bandwidth is absent or
zero no further validations or network namespace/host-interface actions are
performed; update cmdCheck to return nil immediately on that condition and
ensure any downstream code that assumes bandwidth is non-nil is still guarded.
plugins/meta/sbr/main.go (2)

408-408: 💤 Low value

Comment is misleading.

The routes slice was fetched from all tables at line 247. The deletion loop removes routes from their original tables, not specifically "from main table."

📝 Suggested comment fix
-		// Delete non-subnet routes from main table (gateway routes, etc.)
+		// Delete non-subnet routes from their original tables (gateway routes, etc.)
🤖 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 `@plugins/meta/sbr/main.go` at line 408, The comment above the deletion loop is
misleading because the slice named routes (populated earlier) contains routes
from all tables, and the loop removes each entry from its original table rather
than specifically the main table; update the comment to accurately describe the
action (e.g., "Delete non‑subnet routes from their original route tables" or
"Remove non‑subnet routes from their source tables") so it refers to the routes
slice and the deletion loop that iterates over routes and deletes them from each
routeEntry's table.

401-405: 💤 Low value

Silent failure on RouteReplace may leave routing in inconsistent state.

When RouteReplace fails, only a warning is logged, but the function continues. If the subnet route with source hint cannot be added, destination-based routing won't work as expected, yet the plugin reports success. Consider returning an error for consistency with the error handling in the else branch (lines 424-427).

🛠️ Suggestion: propagate error or document the rationale

If warning-only is intentional (e.g., best-effort behavior), consider adding a comment explaining why. Otherwise:

 			err := netlink.RouteReplace(&route)
 			if err != nil {
-				log.Printf("Warning: Failed to add subnet route to main table: %v", err)
-				// Don't fail completely, just warn
+				return fmt.Errorf("failed to add subnet route with source hint to main table: %v", 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 `@plugins/meta/sbr/main.go` around lines 401 - 405, The current
netlink.RouteReplace(&route) call swallows failures by only logging a warning
which can leave routing inconsistent; update the error handling where
netlink.RouteReplace and the route variable are used so that on error you return
the error (e.g., wrap with context like "failed to replace subnet route" and the
route info) instead of continuing, to match the behavior in the existing else
branch that propagates errors; if this warning-only behavior is intentional, add
a clear comment above the netlink.RouteReplace call explaining the best-effort
rationale and why the function may still return success.
plugins/meta/sbr/sbr_linux_test.go (2)

950-951: 💤 Low value

Mask comparison is unconventional.

Using .Mask.String() == "ffffff00" works but is less readable than comparing mask lengths. Consider using a clearer approach.

📝 Alternative mask comparison
-			if route.Dst != nil && route.Dst.IP.Equal(net.IPv4(192, 168, 1, 0)) &&
-				route.Dst.Mask.String() == "ffffff00" {
+			if route.Dst != nil && route.Dst.IP.Equal(net.IPv4(192, 168, 1, 0)) {
+				ones, _ := route.Dst.Mask.Size()
+				if ones == 24 {
🤖 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 `@plugins/meta/sbr/sbr_linux_test.go` around lines 950 - 951, Replace the
opaque string comparison of the mask with a clear mask length check: instead of
checking route.Dst.Mask.String() == "ffffff00", call route.Dst.Mask.Size() (or
use net.CIDRMask/bytes.Count) to assert the prefix length equals 24; e.g. check
that route.Dst != nil && route.Dst.IP.Equal(net.IPv4(192, 168, 1, 0)) && ones ==
24 where ones, _ := route.Dst.Mask.Size(). This makes the intent explicit and
more readable while still using route.Dst.Mask and route.Dst.IP.Equal from the
test.

848-890: 💤 Low value

Consider adding test for multiple IPv6 gateways rejection.

The implementation at main.go:270-273 rejects multiple IPv6 gateways with a similar error, but there's no corresponding test case.

🤖 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 `@plugins/meta/sbr/sbr_linux_test.go` around lines 848 - 890, Add a new test
mirroring "Rejects multiple IPv4 gateways" that verifies multiple IPv6 gateways
are rejected: create a test (e.g., "Rejects multiple IPv6 gateways") in
sbr_linux_test.go that uses ifname, targetNs, setup(targetNs,
createDefaultStatus()), builds a conf JSON like the IPv4 case but with
"gateways": ["2001:db8::1","2001:db8::2"] and a prevResult "ips" entry with
"version": "6" and an IPv6 address (e.g., "2001:db8::5/64"), call
testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) }), and assert
err occurs and err.Error() contains the same IPv6-specific message used in
main.go (the "multiple IPv6" rejection text).
🤖 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 @.github/go-version:
- Line 1: The CI Go version in .github/go-version (currently "1.26") does not
match the module target declared in go.mod ("go 1.25.0"); update the CI
configuration so it includes or targets Go 1.25 to match consumers — either
change the value in .github/go-version to "1.25" or expand to a matrix that runs
both "1.25" and "1.26" to ensure compatibility with the go.mod setting.

In `@plugins/ipam/static/static_test.go`:
- Line 102: Update the mismatched test comment in static_test.go: replace the
current line that claims "Result must preserve both default gateways in order to
prove static IPAM keeps ECMP routes." with a concise comment that describes what
the test actually asserts (e.g., that the result preserves the expected gateway
or preserves order/selection being tested). Locate the comment near the
assertion(s) in the test (in plugins/ipam/static/static_test.go) and make the
wording match the concrete assertion being checked so the comment accurately
reflects the test's expectation.

---

Nitpick comments:
In `@plugins/meta/bandwidth/main.go`:
- Around line 302-305: The code currently checks "if bandwidth == nil ||
bandwidth.isZero()" too late in cmdCheck after running
PrevResult/netns/host-interface work; move that nil/zero-bandwidth short-circuit
to the very start of cmdCheck (before any PrevResult handling, netns setup, or
host-interface validations) so that when bandwidth is absent or zero no further
validations or network namespace/host-interface actions are performed; update
cmdCheck to return nil immediately on that condition and ensure any downstream
code that assumes bandwidth is non-nil is still guarded.

In `@plugins/meta/sbr/main.go`:
- Line 408: The comment above the deletion loop is misleading because the slice
named routes (populated earlier) contains routes from all tables, and the loop
removes each entry from its original table rather than specifically the main
table; update the comment to accurately describe the action (e.g., "Delete
non‑subnet routes from their original route tables" or "Remove non‑subnet routes
from their source tables") so it refers to the routes slice and the deletion
loop that iterates over routes and deletes them from each routeEntry's table.
- Around line 401-405: The current netlink.RouteReplace(&route) call swallows
failures by only logging a warning which can leave routing inconsistent; update
the error handling where netlink.RouteReplace and the route variable are used so
that on error you return the error (e.g., wrap with context like "failed to
replace subnet route" and the route info) instead of continuing, to match the
behavior in the existing else branch that propagates errors; if this
warning-only behavior is intentional, add a clear comment above the
netlink.RouteReplace call explaining the best-effort rationale and why the
function may still return success.

In `@plugins/meta/sbr/sbr_linux_test.go`:
- Around line 950-951: Replace the opaque string comparison of the mask with a
clear mask length check: instead of checking route.Dst.Mask.String() ==
"ffffff00", call route.Dst.Mask.Size() (or use net.CIDRMask/bytes.Count) to
assert the prefix length equals 24; e.g. check that route.Dst != nil &&
route.Dst.IP.Equal(net.IPv4(192, 168, 1, 0)) && ones == 24 where ones, _ :=
route.Dst.Mask.Size(). This makes the intent explicit and more readable while
still using route.Dst.Mask and route.Dst.IP.Equal from the test.
- Around line 848-890: Add a new test mirroring "Rejects multiple IPv4 gateways"
that verifies multiple IPv6 gateways are rejected: create a test (e.g., "Rejects
multiple IPv6 gateways") in sbr_linux_test.go that uses ifname, targetNs,
setup(targetNs, createDefaultStatus()), builds a conf JSON like the IPv4 case
but with "gateways": ["2001:db8::1","2001:db8::2"] and a prevResult "ips" entry
with "version": "6" and an IPv6 address (e.g., "2001:db8::5/64"), call
testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) }), and assert
err occurs and err.Error() contains the same IPv6-specific message used in
main.go (the "multiple IPv6" rejection text).
🪄 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: 4a661a5f-da7d-4577-b366-c3628ac222a5

📥 Commits

Reviewing files that changed from the base of the PR and between 747ad66 and 72d0219.

⛔ Files ignored due to path filters (285)
  • go.sum is excluded by !**/*.sum
  • vendor/cyphar.com/go-pathrs/.golangci.yml is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/COPYING is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/handle_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/internal/fdutils/fd_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/internal/libpathrs/error_unix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/internal/libpathrs/libpathrs_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/procfs/procfs_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/root_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/cyphar.com/go-pathrs/utils_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/.golangci.yml is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/hcn/hcnerrors.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/log/hook.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/oc/exporter.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/protocol/guestrequest/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/safefile/safeopen.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/security/grantvmgroupaccess.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/wclayer/importlayer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/winapi/cimfs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/winapi/devices.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/Microsoft/hcsshim/internal/winapi/zsyscall_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/buger/jsonparser/.travis.yml is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/buger/jsonparser/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/buger/jsonparser/bytes.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/buger/jsonparser/parser.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/containerd/typeurl/v2/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/containerd/typeurl/v2/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/containerd/typeurl/v2/types_gogo.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/coreos/go-systemd/v22/activation/files.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/coreos/go-systemd/v22/activation/files_stub.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/coreos/go-systemd/v22/activation/files_unix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/coreos/go-systemd/v22/activation/listeners.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/COPYING.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/LICENSE.BSD is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/LICENSE.MPL-2.0 is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/internal/consts/consts.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/assert/assert.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/errors_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/at_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/fd.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/fd_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/mount_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/fd/openat2_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_errors_go120.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_errors_unsupported.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_generics_go121.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gocompat/gocompat_generics_unsupported.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gopathrs/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gopathrs/lookup_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gopathrs/mkdir_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gopathrs/open_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/gopathrs/openat2_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/kernelversion/kernel_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/linux/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/linux/mount_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/linux/openat2_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/procfs/procfs_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/internal/procfs/procfs_lookup_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/mkdir.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/mkdir_libpathrs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/mkdir_purego.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/open.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/open_libpathrs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/open_purego.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/procfs/procfs_libpathrs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/cyphar/filepath-securejoin/pathrs-lite/procfs/procfs_purego.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/.cirrus.yml is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/.golangci.yml is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/SECURITY.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/auth.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_default_other.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_default_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/auth_sha1_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/call.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/conn.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_darwin.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_other.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_unix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/conn_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/dbus.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/decoder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/default_handler.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/encoder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/export.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/homedir.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/match.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/message.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/object.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/sequential_handler.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/server_interfaces.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/sig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_nonce_tcp.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_freebsd.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/transport_unixcred_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/variant.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/variant_lexer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/godbus/dbus/v5/variant_parser.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/google/pprof/profile/profile.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/google/pprof/profile/proto.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/core_dsl.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/decorator_dsl.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/automaxprocs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroup.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cgroups2.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/cpu_quota_unsupported.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/errors.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/mountpoint.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/runtime.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/automaxprocs/subsys.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/command/command.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/profiles_and_reports.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/internal/run.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/main.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo/run/run_command.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/ginkgo_t_dsl.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/focus.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/group.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/node.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/ordering.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_event_writer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/reporters/gojson_reporter.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/suite.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/internal/testingtproxy/testing_t_proxy.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/reporters/default_reporter.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/reporters/gojson_report.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/reporters/junit_report.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/reporters/teamcity_report.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/reporting_dsl.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/table_dsl.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/types/config.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/types/errors.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/types/semver_filter.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/types/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/ginkgo/v2/types/version.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/format/format.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/gomega_dsl.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/matchers.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/matchers/have_key_matcher.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/matchers/have_key_with_value_matcher.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/matchers/match_error_strictly_matcher.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/onsi/gomega/matchers/support/goraph/edge/edge.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/opencontainers/selinux/go-selinux/selinux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/opencontainers/selinux/go-selinux/selinux_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/opencontainers/selinux/go-selinux/selinux_stub.go is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/.codecov.yml is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/.gitignore is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/CODE_OF_CONDUCT.md is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/CONTRIBUTING.md is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/LICENSE is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/Makefile is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/README.md is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/automaxprocs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/internal/cgroups/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/maxprocs/maxprocs.go is excluded by !**/vendor/**, !vendor/**
  • vendor/go.uber.org/automaxprocs/maxprocs/version.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/mod/LICENSE is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/mod/PATENTS is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/mod/semver/semver.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/net/html/escape.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/net/html/parse.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/net/html/render.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sync/errgroup/errgroup.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/affinity_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/fdset.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/ifreq_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/ioctl_signed.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/ioctl_unsigned.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/mkall.sh is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/mkerrors.sh is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/syscall_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/syscall_netbsd.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/syscall_solaris.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/syscall_unix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_386.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_amd64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_arm.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_arm64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_loong64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mips.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mips64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mips64le.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_mipsle.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_ppc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_ppc64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_ppc64le.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_riscv64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_s390x.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zerrors_linux_sparc64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zsyscall_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/zsyscall_solaris_amd64.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_linux.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/windows/aliases.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/windows/registry/key.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/windows/registry/zsyscall_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/windows/syscall_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/windows/types_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/sys/windows/zsyscall_windows.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/japanese/eucjp.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/japanese/iso2022jp.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/japanese/shiftjis.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/korean/euckr.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/simplifiedchinese/gbk.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/simplifiedchinese/hzgb2312.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/traditionalchinese/big5.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/text/encoding/unicode/unicode.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/ast/inspector/cursor.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/gcexportdata/gcexportdata.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/gcexportdata/importer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/external.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/golist.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/golist_overlay.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/loadmode_string.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/packages.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/packages/visit.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/types/objectpath/objectpath.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/types/typeutil/callee.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/types/typeutil/imports.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/types/typeutil/map.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/types/typeutil/methodsetcache.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/go/types/typeutil/ui.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/aliases/aliases.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/aliases/aliases_go122.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/core/event.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/core/export.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/core/fast.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/event.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/keys/keys.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/keys/standard.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/keys/util.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/event/label/label.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/bimport.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/exportdata.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/gcimporter.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/iexport.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/iimport.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/predeclared.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/support.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gcimporter/ureader_yes.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gocommand/invoke.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gocommand/invoke_notunix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gocommand/invoke_unix.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gocommand/vendor.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/gocommand/version.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/packagesinternal/packages.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/codes.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/decoder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/doc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/encoder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/flags.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/reloc.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/support.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/sync.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/syncmarker_string.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/pkgbits/version.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/stdlib/deps.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/stdlib/import.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/stdlib/manifest.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/stdlib/stdlib.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/typeparams/common.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/typeparams/coretype.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/typeparams/free.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/typeparams/normalize.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/typeparams/termlist.go is excluded by !**/vendor/**, !vendor/**
  • vendor/golang.org/x/tools/internal/typeparams/typeterm.go is excluded by !**/vendor/**, !vendor/**
📒 Files selected for processing (15)
  • .github/go-version
  • .github/workflows/test.yaml
  • .golangci.yml
  • go.mod
  • pkg/ip/ipmasq_iptables_linux.go
  • pkg/ip/link_linux_test.go
  • pkg/ipam/ipam_linux_test.go
  • plugins/ipam/static/static_test.go
  • plugins/main/bridge/bridge.go
  • plugins/meta/bandwidth/main.go
  • plugins/meta/portmap/portmap_nftables.go
  • plugins/meta/portmap/portmap_nftables_test.go
  • plugins/meta/sbr/main.go
  • plugins/meta/sbr/sbr_linux_test.go
  • plugins/meta/vrf/vrf.go

Comment thread .github/go-version
Comment thread plugins/ipam/static/static_test.go
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 18, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: danwinship
Once this PR has been reviewed and has the lgtm label, please ask for approval from bpickard22. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
Dockerfile (1)

3-3: ⚡ Quick win

Replace ADD with COPY for local source tree staging.

ADD . /usr/src/plugins is unnecessary here and has extra semantics (e.g., archive/URL handling). Use COPY for predictable behavior and to satisfy the DL3020 lint error.

Proposed fix
-ADD . /usr/src/plugins
+COPY . /usr/src/plugins

Apply the same replacement at Line 3, Line 11, and Line 19.

Also applies to: 11-11, 19-19

🤖 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 3, Replace the three uses of the ADD instruction with
COPY to avoid ADD's extra semantics; specifically, change the ADD occurrences
that stage the local tree (the lines containing "ADD . /usr/src/plugins" and the
similar two occurrences shown in the diff) to "COPY . /usr/src/plugins" so all
three spots use COPY for predictable local file copying and to satisfy DL3020.
🤖 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.

Nitpick comments:
In `@Dockerfile`:
- Line 3: Replace the three uses of the ADD instruction with COPY to avoid ADD's
extra semantics; specifically, change the ADD occurrences that stage the local
tree (the lines containing "ADD . /usr/src/plugins" and the similar two
occurrences shown in the diff) to "COPY . /usr/src/plugins" so all three spots
use COPY for predictable local file copying and to satisfy DL3020.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b07b6f00-08c5-4847-867b-1bb073695c90

📥 Commits

Reviewing files that changed from the base of the PR and between 72d0219 and 925556d.

📒 Files selected for processing (2)
  • .ci-operator.yaml
  • Dockerfile
✅ Files skipped from review due to trivial changes (1)
  • .ci-operator.yaml

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 3: Replace the Dockerfile ADD instructions with COPY for plain local file
copies to address DL3020: change every occurrence of "ADD . /usr/src/plugins"
(and the other ADD occurrences at the same pattern around lines 11 and 19) to
"COPY . /usr/src/plugins" so the Dockerfile uses the clearer, safer COPY
instruction for local source context.

In `@Dockerfile.microshift`:
- Line 3: Replace the Dockerfile instruction that adds the repo contents with
COPY instead of ADD to satisfy best-practices/DL3020; update the instruction in
Dockerfile.microshift where the line reads ADD . /usr/src/plugins to use COPY .
/usr/src/plugins so only a plain file copy is performed (no archive extraction
or URL handling).
🪄 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: d4e69f15-c733-4b6e-a403-e4afe068696a

📥 Commits

Reviewing files that changed from the base of the PR and between 925556d and 0e0b6ef.

📒 Files selected for processing (3)
  • .ci-operator.yaml
  • Dockerfile
  • Dockerfile.microshift
✅ Files skipped from review due to trivial changes (1)
  • .ci-operator.yaml

Comment thread Dockerfile Outdated
Comment thread Dockerfile.microshift Outdated
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 18, 2026

@danwinship: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.