Bump k8s.io/* to v1.35
#602
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughUpdated tooling and Kubernetes dependencies, regenerated CRD metadata annotations, migrated webhook validators from runtime.Object to strongly-typed parameters, switched some controller resource updates from Patch to Apply, simplified probe implementations to return literal slices, and bumped test binary versions to 1.35.0. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🤖 Fix all issues with AI agents
In `@internal/controller/server_controller.go`:
- Around line 625-671: The OwnerReference construction using
bootConfig.APIVersion and bootConfig.Kind must be replaced with hardcoded values
to avoid SSA validation failures: create a single ownerRef (replace both
duplicated metav1apply.OwnerReference() blocks) that sets APIVersion to
"metal.ironcore.dev/v1alpha1" and Kind to "ServerBootConfiguration", keep the
WithName/WithUID/WithController/WithBlockOwnerDeletion calls as-is, and reuse
that ownerRef for both sshSecretApply and ignitionSecretApply to remove the
duplicated blocks.
In `@internal/webhook/v1alpha1/biossettings_webhook.go`:
- Around line 91-95: The error string mistakenly says "BMC" instead of "Server";
update the fmt.Errorf call that builds the duplicate reference message (the one
using settings.Spec.ServerRef.Name, settings.Name, bs.Spec.ServerRef.Name,
bs.Name) to refer to "Server" or "ServerRef" (e.g., "Server (%s) referred in %s
is duplicate of Server (%s) referred in %s") so the message correctly reflects
BIOSSettings' ServerRef usage.
🧹 Nitpick comments (2)
internal/webhook/v1alpha1/endpoint_webhook.go (1)
96-96: Inconsistent field path casing between create and update validations.Line 96 uses
"MACAddress"while Line 113 uses"macAddress". Consider aligning both to use consistent casing (typicallymacAddressto match JSON field naming conventions).🔧 Suggested fix
for _, e := range endpoints.Items { if e.Spec.MACAddress == spec.MACAddress { - allErrs = append(allErrs, field.Duplicate(field.NewPath("spec").Child("MACAddress"), e.Spec.MACAddress)) + allErrs = append(allErrs, field.Duplicate(field.NewPath("spec").Child("macAddress"), e.Spec.MACAddress)) } }Also applies to: 113-113
internal/webhook/v1alpha1/bmcversion_webhook.go (1)
75-92: RedundantGetcall - the object is already provided as parameter.The
ValidateDeletemethod fetches theBMCVersionviav.Client.Get(), but the admission request already provides the complete object inobj. This is unnecessary and could introduce race conditions if the object changes between the admission request and theGetcall.Other webhook files in this PR (e.g.,
biosversion_webhook.go,biossettings_webhook.go) directly useobj.Status.Statewithout re-fetching.♻️ Suggested simplification
func (v *BMCVersionCustomValidator) ValidateDelete(ctx context.Context, obj *metalv1alpha1.BMCVersion) (admission.Warnings, error) { bmcversionlog.Info("Validation for BMCVersion upon deletion", "name", obj.GetName()) - bv := &metalv1alpha1.BMCVersion{} - err := v.Client.Get(ctx, client.ObjectKey{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - }, bv) - if err != nil { - return nil, fmt.Errorf("failed to get BMCVersion: %w", err) - } - - if bv.Status.State == metalv1alpha1.BMCVersionStateInProgress && !ShouldAllowForceDeleteInProgress(obj) { + if obj.Status.State == metalv1alpha1.BMCVersionStateInProgress && !ShouldAllowForceDeleteInProgress(obj) { return nil, apierrors.NewBadRequest("Unable to delete BMCVersion as it is in progress") } return nil, nil }
afritzler
force-pushed
the
enh/k8s-1.35
branch
from
e8a0270 to
f11b9e6
Compare
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/webhook/v1alpha1/endpoint_webhook.go (1)
86-114: Keep MAC address error paths consistent across create/update.The
ValidateMACAddressCreatefunction reportsspec.MACAddress(uppercase), whileValidateMACAddressUpdatereportsspec.macAddress(lowercase). Additionally, both functions ignore thepathparameter passed to them and create a new path instead. Usepath.Child("macAddress")in both functions to maintain consistency and properly utilize the path context.🔧 Proposed fix
- allErrs = append(allErrs, field.Duplicate(field.NewPath("spec").Child("MACAddress"), e.Spec.MACAddress)) + allErrs = append(allErrs, field.Duplicate(path.Child("macAddress"), e.Spec.MACAddress))- allErrs = append(allErrs, field.Duplicate(field.NewPath("spec").Child("macAddress"), e.Spec.MACAddress)) + allErrs = append(allErrs, field.Duplicate(path.Child("macAddress"), e.Spec.MACAddress))
🤖 Fix all issues with AI agents
In `@docs/api-reference/api.md`:
- Around line 452-455: The table contains bare URLs (e.g., the RFC link in the
`data` row and the Kubernetes links in the `type` row) that trigger markdownlint
MD034; update those bare URLs to use Markdown autolinks (wrap each URL in angle
brackets, e.g. <https://...>) so they are recognized as links by the linter
while preserving the existing text and links for the `data` and `type` table
rows.
In `@internal/webhook/v1alpha1/bmcsettings_webhook.go`:
- Around line 64-66: The apierrors.NewInvalid call constructs schema.GroupKind
using mixed sources (newObj.GroupVersionKind().Group and oldObj.Kind); change it
so both Group and Kind come from the same object (use newObj). Replace the Kind
argument (oldObj.Kind) with newObj.GroupVersionKind().Kind (or newObj's
equivalent) in the schema.GroupKind constructor inside the return statement in
bmcsettings_webhook.go where NewInvalid is called.
🧹 Nitpick comments (1)
internal/webhook/v1alpha1/bmcversion_webhook.go (1)
78-89: Redundant API fetch in ValidateDelete.The
objparameter already contains the BMCVersion being deleted. Re-fetching it from the API server (lines 78-85) is unnecessary, adds latency, and introduces a potential race condition if the object state changes between the admission request and the re-fetch.Compare with
bmcsettings_webhook.gowhich correctly usesobjdirectly:if obj.Status.State == metalv1alpha1.BMCSettingsStateInProgress && !ShouldAllowForceDeleteInProgress(obj) {Proposed fix to use obj directly
func (v *BMCVersionCustomValidator) ValidateDelete(ctx context.Context, obj *metalv1alpha1.BMCVersion) (admission.Warnings, error) { bmcversionlog.Info("Validation for BMCVersion upon deletion", "name", obj.GetName()) - bv := &metalv1alpha1.BMCVersion{} - err := v.Client.Get(ctx, client.ObjectKey{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - }, bv) - if err != nil { - return nil, fmt.Errorf("failed to get BMCVersion: %w", err) - } - - if bv.Status.State == metalv1alpha1.BMCVersionStateInProgress && !ShouldAllowForceDeleteInProgress(obj) { + if obj.Status.State == metalv1alpha1.BMCVersionStateInProgress && !ShouldAllowForceDeleteInProgress(obj) { return nil, apierrors.NewBadRequest("Unable to delete BMCVersion as it is in progress") } return nil, nil }
afritzler
force-pushed
the
enh/k8s-1.35
branch
from
f11b9e6 to
1ecd99a
Compare
afritzler
force-pushed
the
enh/k8s-1.35
branch
from
5824705 to
c890911
Compare
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.