Bump k8s.io/* deps to v1.35#489
Conversation
github-actions
Bot
added
size/XXL
documentation
📝 WalkthroughWalkthroughUpdates Kubernetes API code generation, dependency management, and controller-manager logic: bumps Go tooling versions, upgrades Kubernetes dependencies to v0.35.3, refactors client-go extraction APIs with subresource support, migrates controllers to server-side apply configurations, adds OpenAPI model name generation, changes Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/api-reference/core.md (1)
4053-4064:⚠️ Potential issue | 🟡 MinorDocs are stale w.r.t. the
NetworkPolicyPort.Porttype change.The API now declares
Port *int32withomitempty(optional), but this reference doc still showsportasint32without an(Optional)marker, and the description doesn't mention the "unset matches all ports" semantics that the Go comment now encodes. Please regenerate the API reference so optionality is reflected here (and similarly verify other entries derived from the updated types).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/api-reference/core.md` around lines 4053 - 4064, The docs entry for NetworkPolicyPort.Port is stale: update the API reference generation so the field reflects the new type Port *int32 with omitempty by marking the `port` field as Optional (e.g., “int32 (Optional)”) and amend the description to state that if the port is unset it matches all ports (e.g., “If this field is not provided, this matches all port names and numbers; if present, only traffic on the specified protocol AND port will be matched.”); regenerate the reference for NetworkPolicyPort.Port and similarly re-run the doc generation for other entries derived from updated Go types to ensure optional fields are annotated consistently.client-go/applyconfigurations/core/v1alpha1/loadbalancerrouting.go (1)
38-71:⚠️ Potential issue | 🟠 MajorRestore
ExtractLoadBalancerRoutingStatusto maintain API backward compatibility.The exported
ExtractLoadBalancerRoutingStatusfunction that existed onmainhas been removed, breaking downstream callers. Add it back as a thin wrapper toExtractLoadBalancerRoutingFrom(..., "status").Required wrapper
func ExtractLoadBalancerRouting(loadBalancerRouting *corev1alpha1.LoadBalancerRouting, fieldManager string) (*LoadBalancerRoutingApplyConfiguration, error) { return ExtractLoadBalancerRoutingFrom(loadBalancerRouting, fieldManager, "") } + +// ExtractLoadBalancerRoutingStatus extracts the applied configuration owned by fieldManager from +// loadBalancerRouting for the status subresource. +func ExtractLoadBalancerRoutingStatus(loadBalancerRouting *corev1alpha1.LoadBalancerRouting, fieldManager string) (*LoadBalancerRoutingApplyConfiguration, error) { + return ExtractLoadBalancerRoutingFrom(loadBalancerRouting, fieldManager, "status") +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client-go/applyconfigurations/core/v1alpha1/loadbalancerrouting.go` around lines 38 - 71, A exported wrapper function ExtractLoadBalancerRoutingStatus was removed and must be restored to keep API compatibility; add back a function named ExtractLoadBalancerRoutingStatus with signature matching ExtractLoadBalancerRouting (i.e., func ExtractLoadBalancerRoutingStatus(loadBalancerRouting *corev1alpha1.LoadBalancerRouting, fieldManager string) (*LoadBalancerRoutingApplyConfiguration, error)) that simply calls and returns ExtractLoadBalancerRoutingFrom(loadBalancerRouting, fieldManager, "status"); place it alongside the other extractor functions so callers expecting this symbol compile again.client-go/applyconfigurations/core/v1alpha1/nattable.go (1)
39-72:⚠️ Potential issue | 🟠 MajorAdd back
ExtractNATTableStatusas a compatibility wrapper.Removing the exported
ExtractNATTableStatusfunction is a source-breaking public API change. Callers cannot migrate silently; they face a compile-time error. To maintain backward compatibility while preserving the new architecture, restore it as a wrapper delegating toExtractNATTableFrom(..., "status"):func ExtractNATTable(nATTable *corev1alpha1.NATTable, fieldManager string) (*NATTableApplyConfiguration, error) { return ExtractNATTableFrom(nATTable, fieldManager, "") } + +// ExtractNATTableStatus extracts the applied configuration owned by fieldManager from +// nATTable for the status subresource. +func ExtractNATTableStatus(nATTable *corev1alpha1.NATTable, fieldManager string) (*NATTableApplyConfiguration, error) { + return ExtractNATTableFrom(nATTable, fieldManager, "status") +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client-go/applyconfigurations/core/v1alpha1/nattable.go` around lines 39 - 72, Restore the exported compatibility wrapper ExtractNATTableStatus to avoid a breaking API change: add a new function ExtractNATTableStatus(nATTable *corev1alpha1.NATTable, fieldManager string) (*NATTableApplyConfiguration, error) that simply calls and returns ExtractNATTableFrom(nATTable, fieldManager, "status"); place it alongside ExtractNATTable and ExtractNATTableFrom so callers can continue using ExtractNATTableStatus while the implementation delegates to the new ExtractNATTableFrom(..., "status").cmd/controller-manager/main.go (1)
208-249:⚠️ Potential issue | 🔴 CriticalAdd events.k8s.io RBAC permissions to controller-manager role.
The manager-role in
config/controller/rbac/role.yamlis missing event permissions entirely. The reconcilers initializingEventRecorderviaGetEventRecorder()at lines 208 and 249 require RBAC permissions to create/patch events. After migration to controller-runtime v0.23,GetEventRecorder()returns anevents.EventRecorderthat writes to theevents.k8s.ioAPI group, not the core group. Add the following rule toconfig/controller/rbac/role.yaml:- apiGroups: - events.k8s.io resources: - events verbs: - create - patchOtherwise, event emission from NATGatewayReconciler, SchedulerReconciler, and other reconcilers will fail at runtime with permission denied errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/controller-manager/main.go` around lines 208 - 249, The controller managers use EventRecorder (via mgr.GetEventRecorder()) in reconcilers like controllers.NATGatewayReconciler and controllers.SchedulerReconciler which now emit events to the events.k8s.io API group; update the controller-manager RBAC role (config/controller/rbac/role.yaml) to grant create and patch permissions for the events resource in the events.k8s.io apiGroup so these reconcilers can successfully create/patch Kubernetes events at runtime.apinetlet/controllers/network_controller.go (1)
153-160:⚠️ Potential issue | 🔴 CriticalGuard the pending-status update when apply returns no APINet network.
applyAPINetNetworkreturnsnil, erron apply/get failures, but the error path passes that nil intoupdateNetworkStatus, which dereferencesapiNetNetwork.Statusand can panic instead of requeueing.🐛 Proposed fix
apiNetNetwork, err := r.applyAPINetNetwork(ctx, log, network) if err != nil { - if network.Status.State != networkingv1alpha1.NetworkStateAvailable { + if apiNetNetwork != nil && network.Status.State != networkingv1alpha1.NetworkStateAvailable { if err := r.updateNetworkStatus(ctx, log, network, apiNetNetwork, networkingv1alpha1.NetworkStatePending); err != nil { log.Error(err, "Error updating network state") } + } else if apiNetNetwork == nil && network.Status.State != networkingv1alpha1.NetworkStatePending { + networkBase := network.DeepCopy() + network.Status.State = networkingv1alpha1.NetworkStatePending + if err := r.Status().Patch(ctx, network, client.MergeFrom(networkBase)); err != nil { + log.Error(err, "Error updating network state") + } } return ctrl.Result{}, fmt.Errorf("error applying APINet network: %w", err) }Also applies to: 257-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apinetlet/controllers/network_controller.go` around lines 153 - 160, The error path calls updateNetworkStatus with apiNetNetwork which can be nil when applyAPINetNetwork returned (nil, err); guard that call by checking apiNetNetwork != nil before accessing its Status (i.e., only call updateNetworkStatus(apiNetNetwork, ...) when apiNetNetwork is non-nil), and for the nil case either skip the update or invoke updateNetworkStatus with a safe default/zeroed APINetNetwork that does not require dereferencing Status; apply the same nil-check pattern for the other occurrence (the block around lines that reference applyAPINetNetwork and updateNetworkStatus later in the file).
🧹 Nitpick comments (2)
go.mod (1)
11-13: Consider tagging ironcore-dev deps to a release before merging.The three
github.com/ironcore-dev/*deps are pinned to pseudo-versions (commit SHAs from April 2026). If tagged releases corresponding to the k8s v1.35 bump exist (or are planned), prefer those for reproducibility and downstream consumability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 11 - 13, The go.mod currently pins github.com/ironcore-dev/controller-utils, github.com/ironcore-dev/ironcore, and github.com/ironcore-dev/metalnet to pseudo-versions (commit SHAs); update those module lines to use corresponding released semver tags (e.g., vX.Y.Z) if available for the k8s v1.35 bump to improve reproducibility. Edit the entries for github.com/ironcore-dev/controller-utils, github.com/ironcore-dev/ironcore, and github.com/ironcore-dev/metalnet to the proper release tags and then run module resolution (go get <module>@<tag> and go mod tidy) to ensure the go.sum and dependency graph are consistent. Ensure the chosen tags are compatible with the k8s v1.35 changes and CI builds.internal/controllers/natgateway_controller.go (1)
362-366: Minor: collapse the section/IP append loops into variadic calls.
WithSections/WithIPsare variadic, so the intermediateforloops can be replaced with spread for clarity.♻️ Proposed refactor
- natIPConf := corev1alpha1apply.NATIP().WithIP(ip) - for _, sectionConf := range sectionConfs { - natIPConf.WithSections(sectionConf) - } - natTableIPs = append(natTableIPs, natIPConf) + natTableIPs = append(natTableIPs, corev1alpha1apply.NATIP().WithIP(ip).WithSections(sectionConfs...)) @@ - for _, natIPConf := range natTableIPs { - natTableApplyConfig.WithIPs(natIPConf) - } + natTableApplyConfig.WithIPs(natTableIPs...)Also applies to: 382-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/controllers/natgateway_controller.go` around lines 362 - 366, The code uses explicit loops to add sections and IPs to NATIP and NATTable entries though WithSections and WithIPs are variadic; replace the per-item for-loops by passing the whole slice via variadic expansion (e.g., call corev1alpha1apply.NATIP().WithIP(ip).WithSections(sectionConfs...) and similarly use WithIPs(someSlice...) when building natTableIPs) so you remove the intermediate loops and directly append the configured object to natTableIPs (refer to natIPConf, corev1alpha1apply.NATIP(), WithSections, WithIPs, and natTableIPs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/core/v1alpha1/doc.go`:
- Around line 4-7: The file api/core/v1alpha1/zz_generated.defaults.go is a
stale autogenerated artifact (its RegisterDefaults function is empty and not
referenced) left after removing +k8s:defaulter-gen; delete
zz_generated.defaults.go and any references/imports to it, remove or update any
build tags or go:generate lines that expect a defaulter in the api/core/v1alpha1
package, and ensure there are no remaining usages of RegisterDefaults so
compilation and imports remain clean.
In `@api/core/v1alpha1/networkpolicy_types.go`:
- Line 40: The public API NetworkPolicyPort.Port was changed from int32 to
*int32 which breaks callers that take the address of an int field; update the
affected conversion at the site that sets `Port: &port.Port` (in
apinetlet/controllers/conversion.go) to create a proper *int32 from the external
value (e.g. capture the int32 into a local variable `p := port.Port` then use
`Port: &p` or use your pointer helper like `Port: generic.Pointer(port.Port)`),
and scan the repo for other direct `&...Port` usages to similarly convert to a
*int32; ensure nil-checks where appropriate.
In `@apinetlet/controllers/networkpolicy_controller.go`:
- Around line 419-428: The code dereferences apiNetNetworkPolicy.Spec.Priority
unconditionally when calling WithPriority, which can cause an NPE if the pointer
is nil; change the construction of networkPolicyRuleApplyCfg to only call
WithPriority when apiNetNetworkPolicy.Spec.Priority is non-nil (e.g., check if p
:= apiNetNetworkPolicy.Spec.Priority; p != nil then chain .WithPriority(*p)),
leaving all other chained calls (WithLabels, WithNetworkRef, WithTargets,
WithIngressRules, WithEgressRules, WithOwnerReferences) intact so the cfg is
built defensively.
In `@client-go/applyconfigurations/core/v1alpha1/loadbalancerspec.go`:
- Line 26: The comment on the Selector field of LoadBalancerSpec is incorrect
and misleading; update the Selector comment in type LoadBalancerSpec to reflect
load balancer context and pluralize "Instance" (e.g., "Selector selects all
Instances that are targeted/associated with this LoadBalancer" or similar),
ensuring the comment references LoadBalancer rather than "daemon set" and uses
correct pluralization for Instance/Instances.
In `@client-go/applyconfigurations/core/v1alpha1/networkpolicy.go`:
- Around line 62-64: The generated Godoc comment in networkpolicy.go is awkward
and duplicates wording (e.g., "no managed fields were found for because other
field managers..." and "never owned fields any fields"); edit the public comment
that begins with "APIVersion and Kind populated." to remove the duplicated words
and fix grammar so it reads clearly (e.g., "It is possible that no managed
fields were found because other field managers have taken ownership, or because
the fieldManager never owned any fields. APIVersion and Kind are populated.").
Ensure you update the comment associated with the generated extractor in
networkpolicy.go so the public documentation is concise and grammatically
correct.
- Around line 69-70: Add a deprecated compatibility wrapper named
ExtractNetworkPolicyStatus that preserves the original public API and simply
delegates to the new generic helper ExtractNetworkPolicyFrom: implement a
function ExtractNetworkPolicyStatus(networkPolicy *corev1alpha1.NetworkPolicy,
fieldManager string) (*NetworkPolicyApplyConfiguration, error) which calls and
returns ExtractNetworkPolicyFrom(networkPolicy, fieldManager, "status"); include
a deprecation comment on ExtractNetworkPolicyStatus to indicate it forwards to
ExtractNetworkPolicyFrom and must be kept for source compatibility.
In `@client-go/applyconfigurations/core/v1alpha1/topologyspreadconstraint.go`:
- Around line 32-33: Fix the grammatical typo in the WhenUnsatisfiable field
comment: change "a instance" to "an instance" in the comment for
WhenUnsatisfiable in topologyspreadconstraint.go (the documentation directly
above the WhenUnsatisfiable field in the TopologySpreadConstraint
apply-configuration type).
In `@client-go/applyconfigurations/internal/internal.go`:
- Around line 29-178: The generated internal schema lists many resources (e.g.,
DaemonSet, IP, Instance, Network, Node, etc.) as scalar: untyped which breaks
field-level managed-fields extraction used by the Extract*From helpers that call
internal.Parser().Type(...); regenerate
client-go/applyconfigurations/internal/internal.go using applyconfiguration-gen
from the corrected OpenAPI definitions so each resource has full field schema
(not scalar: untyped), then re-run codegen so Extract*From functions reference
the proper parsed types; finally, add extraction regression tests that exercise
Extract<Kind>From for several concrete resources to ensure field ownership and
managed-fields metadata are preserved.
In `@docs/api-reference/core.md`:
- Around line 4243-4244: The API reference generator is emitting full
import-path anchors for types like NetworkPeeringStatus from
github.com/ironcore-dev/ironcore-net/api/core/v1alpha1 because
hack/api-reference/config.json lacks a rule for api/ packages; add a new
typeMatchPrefix entry (regex ^github\.com/ironcore-dev/ironcore-net/api) to the
config.json rules so the generator treats nested api/ package types the same as
other domain-based packages and produces the proper domain-based anchor format
for types such as NetworkPeeringStatus.
In `@hack/update-codegen.sh`:
- Line 24: The script installs two EXIT traps so the later one overrides the
earlier rm -rf "$VIRTUAL_GOPATH" cleanup; fix by consolidating cleanup actions
into a single EXIT handler (e.g., define a cleanup function and call rm -rf
"$VIRTUAL_GOPATH" plus the other cleanup tasks inside it) and replace both
standalone trap '...' EXIT invocations with one trap that references that
cleanup function; ensure VIRTUAL_GOPATH is referenced in that consolidated
handler so the temporary GOPATH is always removed.
In `@internal/apis/core/doc.go`:
- Line 6: The openapi-model-package directive value is pointing to a
non-existent import path; update the directive value from
"com.github.ironcore-dev.ironcore-net.api.core.core" to the correct Go import
path "com.github.ironcore-dev.ironcore-net.internal.apis.core" in the doc.go
file (or remove the // +k8s:openapi-model-package line entirely) so generated
OpenAPI models reference the actual hub package used by your types (ensure you
modify the line containing the openapi-model-package directive).
In `@internal/controllers/loadbalancer_controller.go`:
- Around line 80-96: The DaemonSet constructor call is using the namespace as
the first argument and the name as the second; swap the arguments so
corev1alpha1apply.DaemonSet receives the generated name first (use
v1alpha1.LoadBalancerDaemonSetName(loadBalancer.Name) as the name) and
loadBalancer.Namespace as the second argument, and add WithController(true) to
the OwnerReference chain (the v1.OwnerReference() builder) so the created
OwnerReference marks the LoadBalancer as the controller for proper ownership and
reconciliation with .Owns(&v1alpha1.DaemonSet{}).
In `@internal/controllers/natgateway_controller.go`:
- Around line 323-328: The owner reference on the NATTable created in
NATTable(natGateway.Name, natGateway.Namespace) currently omits Controller and
BlockOwnerDeletion flags; update the OwnerReference builder (the
v1.OwnerReference() chain used when constructing natTableApplyConfig) to include
.WithController(true).WithBlockOwnerDeletion(true) so the NATTable has
Controller=true and BlockOwnerDeletion=true like other reconcilers—this restores
event routing for Owns(&v1alpha1.NATTable{}) and ensures proper cascading
deletion behavior.
- Around line 349-360: The comparator passed to slices.SortFunc for sectionConfs
(elements of type corev1alpha1apply.NATIPSectionApplyConfiguration) must
establish a total order when Port may be nil; change the lambda used in
slices.SortFunc so it treats nil consistently (e.g., consider nil > any non-nil
or nil < any non-nil), by first checking both nils (return 0), then handling
one-nil cases deterministically (return 1 or -1), and only then comparing
*a.Port and *b.Port; update the comparator used in slices.SortFunc to implement
this deterministic ordering so SortFunc receives a strict weak ordering.
In `@internal/controllers/scheduler_controller.go`:
- Around line 470-472: The Eventf call incorrectly puts the full message into
the action parameter and the instance name into the note; update the r.Eventf
invocation so action is a short verb like "SchedulingFailed" (or "Scheduling"),
note is a format string such as "No nodes available to schedule %s on", and pass
the namespaced key (namespace/name) of the instance as the argument to format
the note (use inst.Namespace and inst.Name) so the emitted event reads "No nodes
available to schedule <namespace/name> on" and follows EventRecorder semantics;
locate and change the call to r.Eventf in the scheduler controller where inst is
used.
In `@metalnetlet/controllers/metalnetnode_controller.go`:
- Line 137: The server-side apply call using r.Apply(ctx, nodeApplyCfg,
PartitionFieldOwner(r.PartitionName)) must include client.ForceOwnership so
pre-existing fields owned by other managers are taken over; update the call to
pass client.ForceOwnership alongside PartitionFieldOwner (e.g., r.Apply(ctx,
nodeApplyCfg, client.ForceOwnership, PartitionFieldOwner(r.PartitionName))) and
ensure the client symbol is available in scope; reference r.Apply, nodeApplyCfg,
PartitionFieldOwner, and client.ForceOwnership when making the change.
In `@metalnetlet/controllers/networkinterface_controller.go`:
- Around line 243-248: The switch that dereferences port.Protocol must be
guarded to avoid nil panic: wrap the existing switch that uses *port.Protocol
with an if port.Protocol != nil { switch *port.Protocol { ... } } else { treat
as the TCP branch (i.e., apply the same logic as the ProtocolTCP case) }; update
references inside the switch to use the same branch logic for TCP, UDP, SCTP as
before and ensure baseFirewallRule.ProtocolMatch is set consistently when
port.Protocol is nil by defaulting to TCP.
---
Outside diff comments:
In `@apinetlet/controllers/network_controller.go`:
- Around line 153-160: The error path calls updateNetworkStatus with
apiNetNetwork which can be nil when applyAPINetNetwork returned (nil, err);
guard that call by checking apiNetNetwork != nil before accessing its Status
(i.e., only call updateNetworkStatus(apiNetNetwork, ...) when apiNetNetwork is
non-nil), and for the nil case either skip the update or invoke
updateNetworkStatus with a safe default/zeroed APINetNetwork that does not
require dereferencing Status; apply the same nil-check pattern for the other
occurrence (the block around lines that reference applyAPINetNetwork and
updateNetworkStatus later in the file).
In `@client-go/applyconfigurations/core/v1alpha1/loadbalancerrouting.go`:
- Around line 38-71: A exported wrapper function
ExtractLoadBalancerRoutingStatus was removed and must be restored to keep API
compatibility; add back a function named ExtractLoadBalancerRoutingStatus with
signature matching ExtractLoadBalancerRouting (i.e., func
ExtractLoadBalancerRoutingStatus(loadBalancerRouting
*corev1alpha1.LoadBalancerRouting, fieldManager string)
(*LoadBalancerRoutingApplyConfiguration, error)) that simply calls and returns
ExtractLoadBalancerRoutingFrom(loadBalancerRouting, fieldManager, "status");
place it alongside the other extractor functions so callers expecting this
symbol compile again.
In `@client-go/applyconfigurations/core/v1alpha1/nattable.go`:
- Around line 39-72: Restore the exported compatibility wrapper
ExtractNATTableStatus to avoid a breaking API change: add a new function
ExtractNATTableStatus(nATTable *corev1alpha1.NATTable, fieldManager string)
(*NATTableApplyConfiguration, error) that simply calls and returns
ExtractNATTableFrom(nATTable, fieldManager, "status"); place it alongside
ExtractNATTable and ExtractNATTableFrom so callers can continue using
ExtractNATTableStatus while the implementation delegates to the new
ExtractNATTableFrom(..., "status").
In `@cmd/controller-manager/main.go`:
- Around line 208-249: The controller managers use EventRecorder (via
mgr.GetEventRecorder()) in reconcilers like controllers.NATGatewayReconciler and
controllers.SchedulerReconciler which now emit events to the events.k8s.io API
group; update the controller-manager RBAC role
(config/controller/rbac/role.yaml) to grant create and patch permissions for the
events resource in the events.k8s.io apiGroup so these reconcilers can
successfully create/patch Kubernetes events at runtime.
In `@docs/api-reference/core.md`:
- Around line 4053-4064: The docs entry for NetworkPolicyPort.Port is stale:
update the API reference generation so the field reflects the new type Port
*int32 with omitempty by marking the `port` field as Optional (e.g., “int32
(Optional)”) and amend the description to state that if the port is unset it
matches all ports (e.g., “If this field is not provided, this matches all port
names and numbers; if present, only traffic on the specified protocol AND port
will be matched.”); regenerate the reference for NetworkPolicyPort.Port and
similarly re-run the doc generation for other entries derived from updated Go
types to ensure optional fields are annotated consistently.
---
Nitpick comments:
In `@go.mod`:
- Around line 11-13: The go.mod currently pins
github.com/ironcore-dev/controller-utils, github.com/ironcore-dev/ironcore, and
github.com/ironcore-dev/metalnet to pseudo-versions (commit SHAs); update those
module lines to use corresponding released semver tags (e.g., vX.Y.Z) if
available for the k8s v1.35 bump to improve reproducibility. Edit the entries
for github.com/ironcore-dev/controller-utils, github.com/ironcore-dev/ironcore,
and github.com/ironcore-dev/metalnet to the proper release tags and then run
module resolution (go get <module>@<tag> and go mod tidy) to ensure the go.sum
and dependency graph are consistent. Ensure the chosen tags are compatible with
the k8s v1.35 changes and CI builds.
In `@internal/controllers/natgateway_controller.go`:
- Around line 362-366: The code uses explicit loops to add sections and IPs to
NATIP and NATTable entries though WithSections and WithIPs are variadic; replace
the per-item for-loops by passing the whole slice via variadic expansion (e.g.,
call corev1alpha1apply.NATIP().WithIP(ip).WithSections(sectionConfs...) and
similarly use WithIPs(someSlice...) when building natTableIPs) so you remove the
intermediate loops and directly append the configured object to natTableIPs
(refer to natIPConf, corev1alpha1apply.NATIP(), WithSections, WithIPs, and
natTableIPs).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15be1f6f-9974-48d7-8755-9c3112430488
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (104)
Makefileapi/core/v1alpha1/doc.goapi/core/v1alpha1/networkpolicy_types.goapi/core/v1alpha1/zz_generated.deepcopy.goapi/core/v1alpha1/zz_generated.model_name.goapinetlet/controllers/controllers_suite_test.goapinetlet/controllers/loadbalancer_controller.goapinetlet/controllers/natgateway_controller.goapinetlet/controllers/network_controller.goapinetlet/controllers/networkpolicy_controller.goclient-go/applyconfigurations/core/v1alpha1/daemonset.goclient-go/applyconfigurations/core/v1alpha1/daemonsetspec.goclient-go/applyconfigurations/core/v1alpha1/instance.goclient-go/applyconfigurations/core/v1alpha1/instanceaffinityterm.goclient-go/applyconfigurations/core/v1alpha1/instanceantiaffinity.goclient-go/applyconfigurations/core/v1alpha1/instancespec.goclient-go/applyconfigurations/core/v1alpha1/ip.goclient-go/applyconfigurations/core/v1alpha1/ipaddress.goclient-go/applyconfigurations/core/v1alpha1/ipblock.goclient-go/applyconfigurations/core/v1alpha1/loadbalancer.goclient-go/applyconfigurations/core/v1alpha1/loadbalancerdestination.goclient-go/applyconfigurations/core/v1alpha1/loadbalancerip.goclient-go/applyconfigurations/core/v1alpha1/loadbalancerport.goclient-go/applyconfigurations/core/v1alpha1/loadbalancerrouting.goclient-go/applyconfigurations/core/v1alpha1/loadbalancerspec.goclient-go/applyconfigurations/core/v1alpha1/loadbalancerstatus.goclient-go/applyconfigurations/core/v1alpha1/loadbalancertargetref.goclient-go/applyconfigurations/core/v1alpha1/localuidreference.goclient-go/applyconfigurations/core/v1alpha1/natgateway.goclient-go/applyconfigurations/core/v1alpha1/natgatewayautoscaler.goclient-go/applyconfigurations/core/v1alpha1/natgatewayautoscalerspec.goclient-go/applyconfigurations/core/v1alpha1/natgatewayip.goclient-go/applyconfigurations/core/v1alpha1/natgatewayspec.goclient-go/applyconfigurations/core/v1alpha1/natgatewaystatus.goclient-go/applyconfigurations/core/v1alpha1/natip.goclient-go/applyconfigurations/core/v1alpha1/natipsection.goclient-go/applyconfigurations/core/v1alpha1/nattable.goclient-go/applyconfigurations/core/v1alpha1/nattableiptargetref.goclient-go/applyconfigurations/core/v1alpha1/network.goclient-go/applyconfigurations/core/v1alpha1/networkid.goclient-go/applyconfigurations/core/v1alpha1/networkinterface.goclient-go/applyconfigurations/core/v1alpha1/networkinterfacenat.goclient-go/applyconfigurations/core/v1alpha1/networkinterfacenatclaimref.goclient-go/applyconfigurations/core/v1alpha1/networkinterfacepublicip.goclient-go/applyconfigurations/core/v1alpha1/networkinterfacespec.goclient-go/applyconfigurations/core/v1alpha1/networkinterfacestatus.goclient-go/applyconfigurations/core/v1alpha1/networkpeering.goclient-go/applyconfigurations/core/v1alpha1/networkpeeringstatus.goclient-go/applyconfigurations/core/v1alpha1/networkpolicy.goclient-go/applyconfigurations/core/v1alpha1/networkpolicyegressrule.goclient-go/applyconfigurations/core/v1alpha1/networkpolicyingressrule.goclient-go/applyconfigurations/core/v1alpha1/networkpolicypeer.goclient-go/applyconfigurations/core/v1alpha1/networkpolicyport.goclient-go/applyconfigurations/core/v1alpha1/networkpolicyrule.goclient-go/applyconfigurations/core/v1alpha1/networkpolicyspec.goclient-go/applyconfigurations/core/v1alpha1/networkspec.goclient-go/applyconfigurations/core/v1alpha1/networkstatus.goclient-go/applyconfigurations/core/v1alpha1/node.goclient-go/applyconfigurations/core/v1alpha1/nodeselector.goclient-go/applyconfigurations/core/v1alpha1/nodeselectorrequirement.goclient-go/applyconfigurations/core/v1alpha1/nodeselectorterm.goclient-go/applyconfigurations/core/v1alpha1/objectip.goclient-go/applyconfigurations/core/v1alpha1/objectselector.goclient-go/applyconfigurations/core/v1alpha1/pciaddress.goclient-go/applyconfigurations/core/v1alpha1/peeringprefix.goclient-go/applyconfigurations/core/v1alpha1/rule.goclient-go/applyconfigurations/core/v1alpha1/tapdevice.goclient-go/applyconfigurations/core/v1alpha1/targetnetworkinterface.goclient-go/applyconfigurations/core/v1alpha1/topologyspreadconstraint.goclient-go/applyconfigurations/internal/internal.goclient-go/informers/externalversions/core/v1alpha1/daemonset.goclient-go/informers/externalversions/core/v1alpha1/instance.goclient-go/informers/externalversions/core/v1alpha1/ip.goclient-go/informers/externalversions/core/v1alpha1/ipaddress.goclient-go/informers/externalversions/core/v1alpha1/loadbalancer.goclient-go/informers/externalversions/core/v1alpha1/loadbalancerrouting.goclient-go/informers/externalversions/core/v1alpha1/natgateway.goclient-go/informers/externalversions/core/v1alpha1/natgatewayautoscaler.goclient-go/informers/externalversions/core/v1alpha1/nattable.goclient-go/informers/externalversions/core/v1alpha1/network.goclient-go/informers/externalversions/core/v1alpha1/networkid.goclient-go/informers/externalversions/core/v1alpha1/networkinterface.goclient-go/informers/externalversions/core/v1alpha1/networkpolicy.goclient-go/informers/externalversions/core/v1alpha1/networkpolicyrule.goclient-go/informers/externalversions/core/v1alpha1/node.goclient-go/informers/externalversions/factory.goclient-go/ironcorenet/versioned/fake/clientset_generated.goclient-go/openapi/zz_generated.openapi.gocmd/controller-manager/main.godocs/api-reference/core.mdgo.modhack/api-reference/config.jsonhack/update-codegen.shinternal/apis/core/doc.gointernal/apis/core/v1alpha1/zz_generated.conversion.gointernal/app/app_suite_test.gointernal/controllers/controllers_suite_test.gointernal/controllers/loadbalancer_controller.gointernal/controllers/natgateway_controller.gointernal/controllers/scheduler_controller.gometalnetlet/controllers/controllers_suite_test.gometalnetlet/controllers/metalnetnode_controller.gometalnetlet/controllers/networkinterface_controller.gometalnetlet/controllers/networkinterface_controller_test.go
| // +k8s:deepcopy-gen=package | ||
| // +k8s:openapi-gen=true | ||
| // +k8s:defaulter-gen=TypeMeta | ||
| // +k8s:protobuf-gen=package | ||
| // +k8s:openapi-model-package=com.github.ironcore-dev.ironcore-net.api.core.v1alpha1 | ||
| // +groupName=core.apinet.ironcore.dev |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'zz_generated.defaults.go|generated.pb.go|generated.proto' api/core/v1alpha1
rg -nP 'defaulter-gen|go-to-protobuf|protobuf-gen' hack/ -g '!**/vendor/**'Repository: ironcore-dev/ironcore-net
Length of output: 111
🏁 Script executed:
cat -n api/core/v1alpha1/zz_generated.defaults.go | head -30Repository: ironcore-dev/ironcore-net
Length of output: 788
🏁 Script executed:
rg -n 'zz_generated|defaults\.' --type go api/core/v1alpha1/ | head -20Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
rg -rn 'from.*zz_generated\.defaults|import.*defaults' . --type go | grep -v vendor | head -20Repository: ironcore-dev/ironcore-net
Length of output: 51
Delete the stale zz_generated.defaults.go artifact.
The file api/core/v1alpha1/zz_generated.defaults.go exists but is a stale autogenerated stub—its RegisterDefaults() function is empty and the file is never imported or referenced anywhere in the codebase. Since the +k8s:defaulter-gen marker has been removed from doc.go and code generation no longer occurs (confirmed by absence of defaulter-gen directives in hack/ scripts), this file should be deleted in this PR to keep the repository clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/core/v1alpha1/doc.go` around lines 4 - 7, The file
api/core/v1alpha1/zz_generated.defaults.go is a stale autogenerated artifact
(its RegisterDefaults function is empty and not referenced) left after removing
+k8s:defaulter-gen; delete zz_generated.defaults.go and any references/imports
to it, remove or update any build tags or go:generate lines that expect a
defaulter in the api/core/v1alpha1 package, and ensure there are no remaining
usages of RegisterDefaults so compilation and imports remain clean.
| // all port names and numbers. | ||
| // If present, only traffic on the specified protocol AND port will be matched. | ||
| Port int32 `json:"port,omitempty"` | ||
| Port *int32 `json:"port,omitempty"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all uses of NetworkPolicyPort.Port field access or struct literals
rg -nP -C3 '\bNetworkPolicyPort\b' --type=go
echo "---"
rg -nP -C2 '\.Port\b' --type=go -g '!**/zz_generated*' -g '!vendor/**' | rg -n 'networkpolicy|NetworkPolicyPort|policy\.Port|ports\[' | head -100
echo "---"
# Check conversion / validation files
fd -t f -e go | xargs rg -nP -l 'NetworkPolicyPort' 2>/dev/nullRepository: ironcore-dev/ironcore-net
Length of output: 41547
🏁 Script executed:
# Check the validation code more carefully - this has pointer comparisons
cat -n internal/apis/core/validation/networkpolicy.go | sed -n '113,135p'Repository: ironcore-dev/ironcore-net
Length of output: 1113
🏁 Script executed:
# Check the apinetlet test code assignments
cat -n apinetlet/controllers/networkpolicy_controller_test.go | sed -n '93,105p'
cat -n apinetlet/controllers/networkpolicy_controller_test.go | sed -n '128,140p'
cat -n apinetlet/controllers/networkpolicy_controller_test.go | sed -n '389,405p'
cat -n apinetlet/controllers/networkpolicy_controller_test.go | sed -n '425,440p'Repository: ironcore-dev/ironcore-net
Length of output: 2460
🏁 Script executed:
# Check both apinetlet and metalnetlet controller usages
cat -n apinetlet/controllers/networkpolicy_controller.go | sed -n '236,276p'
cat -n metalnetlet/controllers/networkinterface_controller.go | sed -n '1,50p' | head -20Repository: ironcore-dev/ironcore-net
Length of output: 2300
🏁 Script executed:
# Find the apinetv1alpha1 NetworkPolicyPort definition
fd -t f -e go -path '*/apinetv1alpha1*' | xargs rg -l 'type NetworkPolicyPort' 2>/dev/null | head -5Repository: ironcore-dev/ironcore-net
Length of output: 402
🏁 Script executed:
# Check what Port type is in the target struct (apinetv1alpha1)
rg -A5 'type NetworkPolicyPort struct' api/ --type=go | head -20Repository: ironcore-dev/ironcore-net
Length of output: 591
🏁 Script executed:
# Verify the metalnetlet controller properly uses pointers
cat -n metalnetlet/controllers/networkinterface_controller_test.go | sed -n '134,165p'Repository: ironcore-dev/ironcore-net
Length of output: 1224
🏁 Script executed:
# Find apinetv1alpha1 NetworkPolicyPort definition
fd -t f -name '*.go' | xargs rg -l 'apinetv1alpha1' | head -5Repository: ironcore-dev/ironcore-net
Length of output: 529
🏁 Script executed:
# Look for api/core/v1alpha1 Rule and NetworkPolicyPort types
rg -A8 'type Rule struct' api/core/v1alpha1/ --type goRepository: ironcore-dev/ironcore-net
Length of output: 858
🏁 Script executed:
# Check the conversion code to understand the transformation
cat -n apinetlet/controllers/conversion.go | sed -n '183,195p'Repository: ironcore-dev/ironcore-net
Length of output: 604
🏁 Script executed:
# Check if the test code actually compiles - look for Port field type issues
cat -n apinetlet/controllers/networkpolicy_controller_test.go | sed -n '96,110p'Repository: ironcore-dev/ironcore-net
Length of output: 627
🏁 Script executed:
# Look at the full validation function to see all the issues
cat -n internal/apis/core/validation/networkpolicy.go | sed -n '113,145p'Repository: ironcore-dev/ironcore-net
Length of output: 1402
🏁 Script executed:
# Check the generated conversion functions
cat -n internal/apis/core/v1alpha1/zz_generated.conversion.go | sed -n '2638,2665p'Repository: ironcore-dev/ironcore-net
Length of output: 1754
🏁 Script executed:
# Check the source type in apinetlet controller - what is np.Spec.Ingress.Ports element type?
cat -n apinetlet/controllers/networkpolicy_controller.go | sed -n '236,245p'
# Get more context on what type np is
cat -n apinetlet/controllers/networkpolicy_controller.go | sed -n '225,240p'Repository: ironcore-dev/ironcore-net
Length of output: 1009
🏁 Script executed:
# Check internal/apis/core NetworkPolicyPort definition to see Port type there
cat -n internal/apis/core/networkpolicy_types.go | sed -n '28,42p'Repository: ironcore-dev/ironcore-net
Length of output: 714
🏁 Script executed:
# Check what type apinetv1alpha1.NetworkPolicyPort.Port is
fd -t f -name '*.go' -path '*/api/core/v1alpha1/*' | xargs grep -l 'type NetworkPolicyPort' | head -1 | xargs cat | grep -A10 'type NetworkPolicyPort struct'Repository: ironcore-dev/ironcore-net
Length of output: 299
🏁 Script executed:
# Verify line 128 in validation is actually comparing pointer to value (compilation error)
cat -n internal/apis/core/validation/networkpolicy.go | sed -n '123,132p'Repository: ironcore-dev/ironcore-net
Length of output: 526
🏁 Script executed:
# Find api/core/v1alpha1 NetworkPolicyPort to see if Port is pointer there
find . -name "networkpolicy_types.go" -path "*/api/core/v1alpha1/*" -exec cat {} \; | head -50Repository: ironcore-dev/ironcore-net
Length of output: 2406
🏁 Script executed:
# Check the apinetlet conversion.go to understand the Port field handling
cat -n apinetlet/controllers/conversion.go | sed -n '183,194p'Repository: ironcore-dev/ironcore-net
Length of output: 604
🏁 Script executed:
# Check apinetlet controller imports to understand what apinetv1alpha1 is
head -30 apinetlet/controllers/networkpolicy_controller.go | grep -E 'import|apinetv1alpha1'Repository: ironcore-dev/ironcore-net
Length of output: 251
🏁 Script executed:
# Search for where apinetv1alpha1.NetworkPolicyPort is defined
rg 'type NetworkPolicyPort struct' --type go | grep -v zz_generated | grep -v applyconfigurationsRepository: ironcore-dev/ironcore-net
Length of output: 215
🏁 Script executed:
# Check line 188 in conversion.go more carefully - this looks like a bug
cat -n apinetlet/controllers/conversion.go | sed -n '183,195p'Repository: ironcore-dev/ironcore-net
Length of output: 604
🏁 Script executed:
# Verify what the issue is - Port is now *int32 so &port.Port would be **int32
# Let me check the full context of the conversion function
cat -n apinetlet/controllers/conversion.go | sed -n '175,200p'Repository: ironcore-dev/ironcore-net
Length of output: 948
🏁 Script executed:
# Check what networkingv1alpha1 is - this is the public API v1alpha1
head -30 apinetlet/controllers/conversion.go | grep -E 'networkingv1alpha1|import' -A20Repository: ironcore-dev/ironcore-net
Length of output: 1079
🏁 Script executed:
# Verify the exact type mismatch at line 188 - Port should already be *int32
cat -n apinetlet/controllers/conversion.go | sed -n '1,30p' | grep -E 'import|networkingv1alpha1'Repository: ironcore-dev/ironcore-net
Length of output: 376
🏁 Script executed:
# Now let me check: is networkingv1alpha1 (external ironcore) Port value or pointer?
# Look at the conversion registration to understand the type mappings
rg 'Convert_.*NetworkPolicyPort' internal/apis/core/v1alpha1/zz_generated.conversion.go | head -5Repository: ironcore-dev/ironcore-net
Length of output: 696
🏁 Script executed:
# Check if the test code compiles - look at Port assignment with literal int32
cat -n apinetlet/controllers/networkpolicy_controller_test.go | sed -n '95,106p'
# Compare with metalnetlet which uses generic.Pointer
cat -n metalnetlet/controllers/networkinterface_controller_test.go | sed -n '137,143p'Repository: ironcore-dev/ironcore-net
Length of output: 740
🏁 Script executed:
# Let me check what the actual Port field type is in the external ironcore package
# by checking if there's any ironcore dependency definition or checking the actual error
rg 'type NetworkPolicyPort' --type go -B2 -A8 | grep -E 'api/core/v1alpha1|internal/apis/core' -A10Repository: ironcore-dev/ironcore-net
Length of output: 2955
🏁 Script executed:
# Check if there are any build errors or test failures mentioned
fd -t f -name 'go.mod' | xargs cat | grep ironcoreRepository: ironcore-dev/ironcore-net
Length of output: 299
🏁 Script executed:
# Verify the actual consumers of api/core/v1alpha1.NetworkPolicyPort - look at apinetlet controller again
# The key is: does it use api/core/v1alpha1 or networkingv1alpha1?
rg 'np \*.*NetworkPolicy|np := ' apinetlet/controllers/networkpolicy_controller.go -B5 | head -30Repository: ironcore-dev/ironcore-net
Length of output: 934
🏁 Script executed:
# Verify which NetworkPolicyPort the validation function uses
cat -n internal/apis/core/validation/networkpolicy.go | sed -n '1,20p'Repository: ironcore-dev/ironcore-net
Length of output: 821
🏁 Script executed:
# Check if there's also a validation file for the public API
find . -name "*.go" -path "*/validation/*" | xargs grep -l 'NetworkPolicyPort' 2>/dev/nullRepository: ironcore-dev/ironcore-net
Length of output: 117
🏁 Script executed:
# Compare the two Port field types side by side
echo "=== internal/apis/core ===" && \
cat internal/apis/core/networkpolicy_types.go | grep -A2 'Port ' | head -5 && \
echo "=== api/core/v1alpha1 ===" && \
cat api/core/v1alpha1/networkpolicy_types.go | grep -A2 'Port ' | head -5Repository: ironcore-dev/ironcore-net
Length of output: 525
🏁 Script executed:
# Identify the exact issues - check what happens with the conversion between internal and api types
cat -n internal/apis/core/v1alpha1/zz_generated.conversion.go | sed -n '2638,2665p'Repository: ironcore-dev/ironcore-net
Length of output: 1754
🏁 Script executed:
# Specifically check line 2640 - what does Convert_Pointer_int32_To_int32 do?
# This shows the conversion from api (pointer) to internal (value) type
# So the internal/apis/core still has Port int32 - the public API changed!
# Verify this is the actual current state
grep -A1 'Port ' internal/apis/core/networkpolicy_types.go | head -6
grep -A1 'Port ' api/core/v1alpha1/networkpolicy_types.go | head -6Repository: ironcore-dev/ironcore-net
Length of output: 469
Breaking API change: Port in public API api/core/v1alpha1/networkpolicy_types.go is now a pointer.
Changing Port int32 → Port *int32 in the public API is a source-breaking change for consumers of api/core/v1alpha1.NetworkPolicyPort. However, the internal API type (internal/apis/core.NetworkPolicyPort) still uses Port int32 and is unaffected; proper conversion functions are generated to bridge the two.
The breaking change affects:
- Direct struct literal construction in public API code (consumers must use
generic.Pointer(int32(...))) - Code accessing the field must now dereference or nil-check
The in-tree consumers show mixed migration status:
metalnetlet/controllers/networkinterface_controller_test.go✓ correctly usesgeneric.Pointer(int32(...))apinetlet/controllers/networkpolicy_controller_test.gouses externalnetworkingv1alpha1.NetworkPolicyPort(unaffected by this change)apinetlet/controllers/conversion.goline 188 may have a type issue:Port: &port.Portwhereport.Portappears to be from the external packageinternal/apis/core/validation/networkpolicy.gois unaffected (uses internal value typePort int32)
Confirm that serialization behavior is intentional: with omitempty, a *int32 pointing to 0 will be omitted (same as before), so port 0 cannot round-trip, but this is likely acceptable since port 0 is typically invalid anyway.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/core/v1alpha1/networkpolicy_types.go` at line 40, The public API
NetworkPolicyPort.Port was changed from int32 to *int32 which breaks callers
that take the address of an int field; update the affected conversion at the
site that sets `Port: &port.Port` (in apinetlet/controllers/conversion.go) to
create a proper *int32 from the external value (e.g. capture the int32 into a
local variable `p := port.Port` then use `Port: &p` or use your pointer helper
like `Port: generic.Pointer(port.Port)`), and scan the repo for other direct
`&...Port` usages to similarly convert to a *int32; ensure nil-checks where
appropriate.
| networkPolicyRuleApplyCfg := apinetv1alpha1ac.NetworkPolicyRule(apiNetNetworkPolicy.Name, apiNetNetworkPolicy.Namespace). | ||
| WithLabels(apinetletclient.SourceLabels(r.Scheme(), r.RESTMapper(), networkPolicy)). | ||
| WithNetworkRef(apinetv1alpha1ac.LocalUIDReference(). | ||
| WithName(network.Name). | ||
| WithUID(network.UID)). | ||
| WithPriority(*apiNetNetworkPolicy.Spec.Priority). | ||
| WithTargets(targetCfgs...). | ||
| WithIngressRules(ingressRuleCfgs...). | ||
| WithEgressRules(egressRuleCfgs...). | ||
| WithOwnerReferences(ownerRef) |
There was a problem hiding this comment.
Guard against nil Spec.Priority before dereferencing.
apiNetNetworkPolicy.Spec.Priority is an *int32 and is dereferenced unconditionally at line 424. Although applyAPINetNetworkPolicy sets a default of 1000, a future change (or a partial server response) could leave it nil and crash the reconciler with a NPE.
🛡️ Proposed defensive fix
- WithPriority(*apiNetNetworkPolicy.Spec.Priority).
+ WithPriority(ptr.Deref(apiNetNetworkPolicy.Spec.Priority, 1000)).(or inline a short if p := apiNetNetworkPolicy.Spec.Priority; p != nil { cfg = cfg.WithPriority(*p) })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apinetlet/controllers/networkpolicy_controller.go` around lines 419 - 428,
The code dereferences apiNetNetworkPolicy.Spec.Priority unconditionally when
calling WithPriority, which can cause an NPE if the pointer is nil; change the
construction of networkPolicyRuleApplyCfg to only call WithPriority when
apiNetNetworkPolicy.Spec.Priority is non-nil (e.g., check if p :=
apiNetNetworkPolicy.Spec.Priority; p != nil then chain .WithPriority(*p)),
leaving all other chained calls (WithLabels, WithNetworkRef, WithTargets,
WithIngressRules, WithEgressRules, WithOwnerReferences) intact so the cfg is
built defensively.
| // Ports are the ports the load balancer should allow. | ||
| // If empty, the load balancer allows all ports. | ||
| Ports []LoadBalancerPortApplyConfiguration `json:"ports,omitempty"` | ||
| // Selector selects all Instance that are managed by this daemon set. |
There was a problem hiding this comment.
Fix incorrect selector comment context on Line 26.
“managed by this daemon set” appears unrelated to LoadBalancerSpec, and “Instance” should be pluralized. This is misleading API documentation.
Suggested doc fix
- // Selector selects all Instance that are managed by this daemon set.
+ // Selector selects all Instances matching the given labels.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client-go/applyconfigurations/core/v1alpha1/loadbalancerspec.go` at line 26,
The comment on the Selector field of LoadBalancerSpec is incorrect and
misleading; update the Selector comment in type LoadBalancerSpec to reflect load
balancer context and pluralize "Instance" (e.g., "Selector selects all Instances
that are targeted/associated with this LoadBalancer" or similar), ensuring the
comment references LoadBalancer rather than "daemon set" and uses correct
pluralization for Instance/Instances.
| // APIVersion and Kind populated. It is possible that no managed fields were found for because other | ||
| // field managers have taken ownership of all the fields previously owned by fieldManager, or because | ||
| // the fieldManager never owned fields any fields. |
There was a problem hiding this comment.
Clean up the generated extractor Godoc grammar.
This public Godoc currently reads awkwardly and has duplicated wording.
📝 Proposed generated-doc fix
-// APIVersion and Kind populated. It is possible that no managed fields were found for because other
+// APIVersion and Kind populated. It is possible that no managed fields were found because other
// field managers have taken ownership of all the fields previously owned by fieldManager, or because
-// the fieldManager never owned fields any fields.
+// the fieldManager never owned any fields.📝 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.
| // APIVersion and Kind populated. It is possible that no managed fields were found for because other | |
| // field managers have taken ownership of all the fields previously owned by fieldManager, or because | |
| // the fieldManager never owned fields any fields. | |
| // APIVersion and Kind populated. It is possible that no managed fields were found because other | |
| // field managers have taken ownership of all the fields previously owned by fieldManager, or because | |
| // the fieldManager never owned any fields. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client-go/applyconfigurations/core/v1alpha1/networkpolicy.go` around lines 62
- 64, The generated Godoc comment in networkpolicy.go is awkward and duplicates
wording (e.g., "no managed fields were found for because other field
managers..." and "never owned fields any fields"); edit the public comment that
begins with "APIVersion and Kind populated." to remove the duplicated words and
fix grammar so it reads clearly (e.g., "It is possible that no managed fields
were found because other field managers have taken ownership, or because the
fieldManager never owned any fields. APIVersion and Kind are populated.").
Ensure you update the comment associated with the generated extractor in
networkpolicy.go so the public documentation is concise and grammatically
correct.
| natTableApplyConfig := corev1alpha1apply.NATTable(natGateway.Name, natGateway.Namespace). | ||
| WithOwnerReferences(v1.OwnerReference(). | ||
| WithAPIVersion(v1alpha1.SchemeGroupVersion.String()). | ||
| WithKind("NATGateway"). | ||
| WithName(natGateway.Name). | ||
| WithUID(natGateway.UID)) |
There was a problem hiding this comment.
Missing Controller/BlockOwnerDeletion on NATTable owner reference — behavioral regression.
Every other reconciler in this PR sets WithController(true).WithBlockOwnerDeletion(true) on its metav1apply.OwnerReference, and the previous implementation almost certainly used ctrl.SetControllerReference (which sets both). Two concrete consequences here:
SetupWithManageruses.Owns(&v1alpha1.NATTable{})(line 478).Ownsroutes events to the parent via the controller owner reference — withoutController=true, NATTable changes will not re-enqueue the owningNATGateway.- Without
BlockOwnerDeletion, the NATTable can be GC'd before/separately from the NATGateway during cascading delete, inverting prior guarantees.
🛠️ Proposed fix
natTableApplyConfig := corev1alpha1apply.NATTable(natGateway.Name, natGateway.Namespace).
WithOwnerReferences(v1.OwnerReference().
WithAPIVersion(v1alpha1.SchemeGroupVersion.String()).
WithKind("NATGateway").
WithName(natGateway.Name).
- WithUID(natGateway.UID))
+ WithUID(natGateway.UID).
+ WithController(true).
+ WithBlockOwnerDeletion(true))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| natTableApplyConfig := corev1alpha1apply.NATTable(natGateway.Name, natGateway.Namespace). | |
| WithOwnerReferences(v1.OwnerReference(). | |
| WithAPIVersion(v1alpha1.SchemeGroupVersion.String()). | |
| WithKind("NATGateway"). | |
| WithName(natGateway.Name). | |
| WithUID(natGateway.UID)) | |
| natTableApplyConfig := corev1alpha1apply.NATTable(natGateway.Name, natGateway.Namespace). | |
| WithOwnerReferences(v1.OwnerReference(). | |
| WithAPIVersion(v1alpha1.SchemeGroupVersion.String()). | |
| WithKind("NATGateway"). | |
| WithName(natGateway.Name). | |
| WithUID(natGateway.UID). | |
| WithController(true). | |
| WithBlockOwnerDeletion(true)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/controllers/natgateway_controller.go` around lines 323 - 328, The
owner reference on the NATTable created in NATTable(natGateway.Name,
natGateway.Namespace) currently omits Controller and BlockOwnerDeletion flags;
update the OwnerReference builder (the v1.OwnerReference() chain used when
constructing natTableApplyConfig) to include
.WithController(true).WithBlockOwnerDeletion(true) so the NATTable has
Controller=true and BlockOwnerDeletion=true like other reconcilers—this restores
event routing for Owns(&v1alpha1.NATTable{}) and ensures proper cascading
deletion behavior.
| slices.SortFunc(sectionConfs, func(a, b *corev1alpha1apply.NATIPSectionApplyConfiguration) int { | ||
| if a.Port == nil || b.Port == nil { | ||
| return 0 | ||
| } | ||
| if *a.Port < *b.Port { | ||
| return -1 | ||
| } | ||
| if *a.Port > *b.Port { | ||
| return 1 | ||
| } | ||
| return 0 | ||
| }) |
There was a problem hiding this comment.
SortFunc comparator violates the total-order contract when exactly one Port is nil.
Returning 0 whenever either operand is nil makes the comparator non-transitive: e.g. with items A(Port=1), B(Port=nil), C(Port=2), cmp(A,B)=0 and cmp(B,C)=0 but cmp(A,C)=-1. slices.SortFunc requires a strict weak ordering; this produces nondeterministic output and can cause spurious diffs in the server-side-apply payload (churn/ownership flips). Sort nils to a consistent side.
🛠️ Proposed fix
slices.SortFunc(sectionConfs, func(a, b *corev1alpha1apply.NATIPSectionApplyConfiguration) int {
- if a.Port == nil || b.Port == nil {
- return 0
- }
- if *a.Port < *b.Port {
- return -1
- }
- if *a.Port > *b.Port {
- return 1
- }
- return 0
+ switch {
+ case a.Port == nil && b.Port == nil:
+ return 0
+ case a.Port == nil:
+ return 1
+ case b.Port == nil:
+ return -1
+ }
+ return cmp.Compare(*a.Port, *b.Port)
})(add "cmp" to imports)
📝 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.
| slices.SortFunc(sectionConfs, func(a, b *corev1alpha1apply.NATIPSectionApplyConfiguration) int { | |
| if a.Port == nil || b.Port == nil { | |
| return 0 | |
| } | |
| if *a.Port < *b.Port { | |
| return -1 | |
| } | |
| if *a.Port > *b.Port { | |
| return 1 | |
| } | |
| return 0 | |
| }) | |
| slices.SortFunc(sectionConfs, func(a, b *corev1alpha1apply.NATIPSectionApplyConfiguration) int { | |
| switch { | |
| case a.Port == nil && b.Port == nil: | |
| return 0 | |
| case a.Port == nil: | |
| return 1 | |
| case b.Port == nil: | |
| return -1 | |
| } | |
| return cmp.Compare(*a.Port, *b.Port) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/controllers/natgateway_controller.go` around lines 349 - 360, The
comparator passed to slices.SortFunc for sectionConfs (elements of type
corev1alpha1apply.NATIPSectionApplyConfiguration) must establish a total order
when Port may be nil; change the lambda used in slices.SortFunc so it treats nil
consistently (e.g., consider nil > any non-nil or nil < any non-nil), by first
checking both nils (return 0), then handling one-nil cases deterministically
(return 1 or -1), and only then comparing *a.Port and *b.Port; update the
comparator used in slices.SortFunc to implement this deterministic ordering so
SortFunc receives a strict weak ordering.
| if len(nodes) == 0 { | ||
| r.Event(inst, corev1.EventTypeNormal, outOfCapacity, "No nodes available to schedule instance on") | ||
| r.Eventf(inst, nil, corev1.EventTypeNormal, outOfCapacity, "No nodes available to schedule %v on", inst.Name) | ||
| return ctrl.Result{}, nil |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
k8s.io/client-go/tools/events EventRecorder Eventf action note parameter semantics
💡 Result:
In k8s.io/client-go/tools/events, the EventRecorder interface (specifically implementations like recorderImpl and recorderImplLogger, and the EventBroadcasterAdapter via tools/record) provides an Eventf method with signature: func Eventf(regarding runtime.Object, related runtime.Object, eventtype, reason, action, note string, args ...interface{}) This method formats 'note' using fmt.Sprintf(note, args...) to produce the event message, constructs an events.k8s.io/v1 Event object, and asynchronously records it via the broadcaster (as watch.Added). Parameter semantics, grounded in source code and events/v1 API docs: - regarding: Primary object the event concerns (populates Event.Regarding ObjectReference). - related: Optional secondary object (populates Event.Related ObjectReference). - eventtype: Event type, typically "Normal" or "Warning" (populates Event.Type; validated via util.ValidateEventType). - reason: Machine-readable reason/code for the event (UpperCamelCase; populates Event.Reason; max 128 chars). - action: Machine-readable description of the action taken/failed on the regarding object (e.g., "Created", "Updated", "Deleted"; populates Event.Action directly; max 128 chars, required/non-empty for new events). - note: Human-readable format string for the event description (formatted to Event.Note; max 1kB, prepare for 64kB). The event also sets EventTime (microsecond precision first observation time), ReportingController/Instance, and generates a unique name/namespace from regarding. This contrasts with the simpler tools/record.EventRecorder.Eventf(object, eventtype, reason, messageFmt string, args...), which lacks action/note/related and targets core/v1 Events (where message is unstructured). The events package targets the newer structured events.k8s.io/v1 API.
Citations:
- 1: https://github.com/kubernetes/client-go/blob/master/tools/events/event_recorder.go
- 2: https://pkg.go.dev/k8s.io/client-go/tools/record
- 3: https://github.com/kubernetes/client-go/blob/v0.35.3/tools/record/event.go
- 4: https://github.com/kubernetes/client-go/blob/master/tools/record/event.go
- 5: https://kubernetes.io/docs/reference/kubernetes-api/cluster-resources/event-v1
- 6: https://github.com/kubernetes/api/blob/master/events/v1beta1/types.go
- 7: https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/383-new-event-api-ga-graduation/README.md
🏁 Script executed:
# First, locate the scheduler_controller.go file
find . -name "scheduler_controller.go" -type fRepository: ironcore-dev/ironcore-net
Length of output: 115
🏁 Script executed:
# Check the specific lines mentioned in the review
sed -n '460,480p' ./internal/controllers/scheduler_controller.go | cat -nRepository: ironcore-dev/ironcore-net
Length of output: 790
🏁 Script executed:
# Check imports to see which EventRecorder is being used
head -50 ./internal/controllers/scheduler_controller.go | cat -nRepository: ironcore-dev/ironcore-net
Length of output: 2070
🏁 Script executed:
# Search for all Eventf calls in the scheduler_controller.go file
rg "Eventf" ./internal/controllers/scheduler_controller.go -nRepository: ironcore-dev/ironcore-net
Length of output: 184
🏁 Script executed:
# Check the v1alpha1.Instance type definition to verify it has Namespace and Name fields
find . -path "*/api/core/v1alpha1/*instance*" -type f -name "*.go" | head -5Repository: ironcore-dev/ironcore-net
Length of output: 106
🏁 Script executed:
# Check the Instance type definition for Namespace and Name fields
head -100 ./api/core/v1alpha1/instance_types.go | cat -nRepository: ironcore-dev/ironcore-net
Length of output: 5329
🏁 Script executed:
# Find the Instance struct definition
rg "type Instance struct" ./api/core/v1alpha1/instance_types.go -A 20Repository: ironcore-dev/ironcore-net
Length of output: 679
Eventf arguments are misaligned with the k8s.io/client-go/tools/events.EventRecorder signature — instance name is not interpolated into the message.
The EventRecorder.Eventf signature from k8s.io/client-go/tools/events is:
Eventf(regarding, related runtime.Object, eventtype, reason, action, note string, args ...interface{})
where action is a short UpperCamelCase verb (e.g., "Scheduling", "Updated"), note is a format string, and args are substituted into note via fmt.Sprintf(note, args...).
The current call at line 471:
r.Eventf(inst, nil, corev1.EventTypeNormal, outOfCapacity, "No nodes available to schedule %v on", inst.Name)
incorrectly passes:
action:"No nodes available to schedule %v on"(a full sentence, not a verb)note:inst.Name(a plain string, not a format string)args: (empty)
This causes two problems:
- The
%vplaceholder is never substituted; the emitted event message is literally just the instance name instead of"No nodes available to schedule <name> on". actionsemantics are violated (expected to be a short verb, not a full sentence), breaking event filtering and classification.
Additionally, using the namespaced key namespace/name is more useful in scheduler contexts than the bare name alone.
Proposed fix
- r.Eventf(inst, nil, corev1.EventTypeNormal, outOfCapacity, "No nodes available to schedule %v on", inst.Name)
+ r.Eventf(inst, nil, corev1.EventTypeNormal, outOfCapacity, "Scheduling", "No nodes available to schedule %s/%s on", inst.Namespace, inst.Name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/controllers/scheduler_controller.go` around lines 470 - 472, The
Eventf call incorrectly puts the full message into the action parameter and the
instance name into the note; update the r.Eventf invocation so action is a short
verb like "SchedulingFailed" (or "Scheduling"), note is a format string such as
"No nodes available to schedule %s on", and pass the namespaced key
(namespace/name) of the instance as the argument to format the note (use
inst.Namespace and inst.Name) so the emitted event reads "No nodes available to
schedule <namespace/name> on" and follows EventRecorder semantics; locate and
change the call to r.Eventf in the scheduler controller where inst is used.
| r.NodeLabels, | ||
| )) | ||
|
|
||
| if err := r.Apply(ctx, nodeApplyCfg, PartitionFieldOwner(r.PartitionName)); err != nil { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "metalnetnode_controller.go" | head -5Repository: ironcore-dev/ironcore-net
Length of output: 119
🏁 Script executed:
find . -name "metalnetnode_controller.go" -type fRepository: ironcore-dev/ironcore-net
Length of output: 121
🏁 Script executed:
cat -n metalnetlet/controllers/metalnetnode_controller.go | sed -n '1,20p'Repository: ironcore-dev/ironcore-net
Length of output: 926
🏁 Script executed:
cat -n metalnetlet/controllers/metalnetnode_controller.go | sed -n '120,145p'Repository: ironcore-dev/ironcore-net
Length of output: 905
🏁 Script executed:
cat -n metalnetlet/controllers/metalnetnode_controller.go | sed -n '1,50p'Repository: ironcore-dev/ironcore-net
Length of output: 2427
🏁 Script executed:
rg "func.*Apply" metalnetlet/controllers/metalnetnode_controller.go -A 3Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
rg "type.*Reconciler" metalnetlet/controllers/metalnetnode_controller.go -A 5Repository: ironcore-dev/ironcore-net
Length of output: 209
🏁 Script executed:
rg "func PartitionFieldOwner" metalnetlet/controllers/ -B 2 -A 5Repository: ironcore-dev/ironcore-net
Length of output: 586
🏁 Script executed:
rg "PartitionFieldOwner" metalnetlet/controllers/metalnetnode_controller.goRepository: ironcore-dev/ironcore-net
Length of output: 158
🏁 Script executed:
rg "func PartitionFieldOwner" . -B 2 -A 5Repository: ironcore-dev/ironcore-net
Length of output: 602
🏁 Script executed:
git log --oneline -n 20 -- metalnetlet/controllers/metalnetnode_controller.goRepository: ironcore-dev/ironcore-net
Length of output: 104
🏁 Script executed:
git show HEAD:metalnetlet/controllers/metalnetnode_controller.go 2>/dev/null | sed -n '120,145p'Repository: ironcore-dev/ironcore-net
Length of output: 723
🏁 Script executed:
rg "func.*Apply.*\(" metalnetlet/controllers/ -B 2 -A 8 | head -50Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
rg "\.Apply\(" metalnetlet/controllers/metalnetnode_controller.go -B 2 -A 2Repository: ironcore-dev/ironcore-net
Length of output: 234
🏁 Script executed:
rg "func.*Apply" . --type go -B 1 -A 5 | grep -A 5 "func.*Apply"Repository: ironcore-dev/ironcore-net
Length of output: 50382
🏁 Script executed:
fd -t f "*.go" | xargs rg "func.*Apply.*ctx.*ApplyConfiguration" | head -20Repository: ironcore-dev/ironcore-net
Length of output: 352
🏁 Script executed:
fd -t f "*.go" -p "*clientutils*"Repository: ironcore-dev/ironcore-net
Length of output: 169
🏁 Script executed:
rg "package clientutils" . -A 20Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
rg "func Apply" . --type go -B 2 -A 8Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
rg "\.Apply\(" metalnetlet/controllers/ -B 1 -A 1Repository: ironcore-dev/ironcore-net
Length of output: 379
🏁 Script executed:
git log --all --source -S "ForceOwnership" -- metalnetlet/controllers/ | head -20Repository: ironcore-dev/ironcore-net
Length of output: 422
🏁 Script executed:
rg "ForceOwnership" . --type go | head -20Repository: ironcore-dev/ironcore-net
Length of output: 1355
🏁 Script executed:
rg "client\.(Apply|ForceOwnership)" . --type goRepository: ironcore-dev/ironcore-net
Length of output: 1355
🏁 Script executed:
rg "func.*Apply.*ctx.*ApplyConfiguration.*FieldOwner" . --type go -A 3 | head -30Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
cat -n metalnetlet/controllers/common.go | sed -n '1,30p'Repository: ironcore-dev/ironcore-net
Length of output: 1150
Add missing client.ForceOwnership to the server-side apply call.
Line 137 applies without client.ForceOwnership, unlike all other Apply calls across the codebase (natgateway_controller, loadbalancer_controller, networkpolicy_controller, etc.). Without force ownership, pre-existing fields owned by another manager will cause conflicts and block reconciliation.
🐛 Proposed fix
- if err := r.Apply(ctx, nodeApplyCfg, PartitionFieldOwner(r.PartitionName)); err != nil {
+ if err := r.Apply(ctx, nodeApplyCfg, PartitionFieldOwner(r.PartitionName), client.ForceOwnership); err != nil {📝 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.
| if err := r.Apply(ctx, nodeApplyCfg, PartitionFieldOwner(r.PartitionName)); err != nil { | |
| if err := r.Apply(ctx, nodeApplyCfg, PartitionFieldOwner(r.PartitionName), client.ForceOwnership); err != nil { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@metalnetlet/controllers/metalnetnode_controller.go` at line 137, The
server-side apply call using r.Apply(ctx, nodeApplyCfg,
PartitionFieldOwner(r.PartitionName)) must include client.ForceOwnership so
pre-existing fields owned by other managers are taken over; update the call to
pass client.ForceOwnership alongside PartitionFieldOwner (e.g., r.Apply(ctx,
nodeApplyCfg, client.ForceOwnership, PartitionFieldOwner(r.PartitionName))) and
ensure the client symbol is available in scope; reference r.Apply, nodeApplyCfg,
PartitionFieldOwner, and client.ForceOwnership when making the change.
| if port.Port != nil { | ||
| if direction == metalnetv1alpha1.FirewallRuleDirectionIngress { | ||
| baseFirewallRule.ProtocolMatch.PortRange = &metalnetv1alpha1.PortMatch{SrcPort: &port.Port} | ||
| baseFirewallRule.ProtocolMatch.PortRange = &metalnetv1alpha1.PortMatch{SrcPort: port.Port} | ||
| } else { | ||
| baseFirewallRule.ProtocolMatch.PortRange = &metalnetv1alpha1.PortMatch{DstPort: &port.Port} | ||
| baseFirewallRule.ProtocolMatch.PortRange = &metalnetv1alpha1.PortMatch{DstPort: port.Port} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'Protocol\s+\*?\s*corev1\.Protocol|Protocol\s+\*corev1\.Protocol' --type=go
ast-grep --pattern 'type NetworkPolicyPort struct { $$$ }'Repository: ironcore-dev/ironcore-net
Length of output: 4500
🏁 Script executed:
cd metalnetlet/controllers && sed -n '230,250p' networkinterface_controller.goRepository: ironcore-dev/ironcore-net
Length of output: 1044
🏁 Script executed:
cd metalnetlet/controllers && sed -n '200,240p' networkinterface_controller.goRepository: ironcore-dev/ironcore-net
Length of output: 1716
🏁 Script executed:
cd metalnetlet/controllers && sed -n '200,220p' networkinterface_controller.goRepository: ironcore-dev/ironcore-net
Length of output: 777
🏁 Script executed:
cd api/core/v1alpha1 && grep -n "type Rule struct" -A 20 *.goRepository: ironcore-dev/ironcore-net
Length of output: 1516
🏁 Script executed:
rg -n "SetDefaults|func.*default" --type=go metalnetlet/controllers/networkinterface_controller.goRepository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
find . -name "*.go" -type f | xargs grep -l "SetDefaults.*NetworkPolicyPort\|Protocol.*default" | head -20Repository: ironcore-dev/ironcore-net
Length of output: 112
🏁 Script executed:
find api -name "*webhook*" -o -name "*default*" -o -name "*validation*" | head -20Repository: ironcore-dev/ironcore-net
Length of output: 111
🏁 Script executed:
grep -n "NetworkPolicyPort\|Protocol" api/core/v1alpha1/zz_generated.defaults.go | head -30Repository: ironcore-dev/ironcore-net
Length of output: 51
🏁 Script executed:
grep -A 15 "type NetworkPolicyPort struct" api/core/v1alpha1/networkpolicy_types.goRepository: ironcore-dev/ironcore-net
Length of output: 809
Add nil-check for port.Protocol before the switch statement to prevent potential panic.
The concern is valid: Protocol is an optional field (*corev1.Protocol with omitempty JSON tag) per the API definition, but the switch statement at line 235 dereferences it without checking for nil. While the API comment states the field "defaults to TCP if not specified," no defaulting logic is implemented in the codebase. If Protocol is nil, the code will panic. This needs the same nil-guard protection that Port now has:
if port.Protocol != nil {
switch *port.Protocol {
// ... existing cases
}
} else {
// handle or default to TCP
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@metalnetlet/controllers/networkinterface_controller.go` around lines 243 -
248, The switch that dereferences port.Protocol must be guarded to avoid nil
panic: wrap the existing switch that uses *port.Protocol with an if
port.Protocol != nil { switch *port.Protocol { ... } } else { treat as the TCP
branch (i.e., apply the same logic as the ProtocolTCP case) }; update references
inside the switch to use the same branch logic for TCP, UDP, SCTP as before and
ensure baseFirewallRule.ProtocolMatch is set consistently when port.Protocol is
nil by defaulting to TCP.
|
Current blocker metalnet |
2 participants
Proposed Changes
Summary by CodeRabbit
Release Notes
Updates
New Features
Enhancements