Fix incorrect warning message for forced machine image updates#2775
Merged
Fix incorrect warning message for forced machine image updates#2775
Conversation
grolu
requested review from
holgerkoser,
klocke-io and
petersutter
as code owners
gardener-prow
Bot
added
do-not-merge/needs-kind
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughRefactors expiration/update logic: replaces severity strings with boolean flags (regularUpdate, forcedUpdate, noUpdate), adds utilities (machineImageHasUpdate, machineImageHasUpdateForAutoUpdateStrategy, getVersionExpirationWarning), and updates composables and components to compute and pass the new flags and version values. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as GShootMessages / GMachineImage
participant Composable as useShootMessages / useKubernetesVersions
participant Utils as utils (getVersionExpirationWarning, machineImageHasUpdateForAutoUpdateStrategy)
participant Message as GK8s/GWorkerGroupExpirationMessage
UI->>Composable: request expiration/update data
Composable->>Utils: machineImageHasUpdateForAutoUpdateStrategy(...)
Utils-->>Composable: updateAvailable / autoUpdatePossible
Composable->>Utils: getVersionExpirationWarning({isExpirationWarning, autoPatchEnabled, updateAvailable, autoUpdatePossible})
Utils-->>Composable: {severity, regularUpdate, forcedUpdate, noUpdate}
Composable-->>UI: payloads with version + boolean flags
UI->>Message: render with version, regularUpdate, forcedUpdate, noUpdate
Message-->>UI: rendered messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Member
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Member
Author
|
/cherry-pick hotfix-1.83 |
Contributor
|
@grolu: once the present PR merges, I will cherry-pick it on top of hotfix-1.83 in a new PR and assign it to you. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
petersutter
reviewed
Feb 13, 2026
|
@holgerkoser, @klocke-io You have pull request review open invite, please check |
gardener-prow
Bot
added
size/XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/package.json (1)
80-80: No security advisories affect undici@7.22.0; be aware of changed request de-duplication behavior.The update from ^7.18.2 to ^7.22.0 has no known CVEs. However, one behavioral change between these versions: undici v7.22.0 no longer deduplicates non-safe HTTP methods (POST, PUT, PATCH) by default. If your application relies on deduplication for these methods, test to ensure the behavior change is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/package.json` at line 80, Dependency undici was bumped to ^7.22.0 which changed deduplication behavior for non-safe HTTP methods; run integration tests that exercise POST/PUT/PATCH paths and search the codebase for usages of undici (the "undici" entry in package.json and any calls to Pool.request, Client, or undici.fetch) to confirm no code relies on implicit dedupe; if you do rely on it either pin the dependency back to ^7.18.2 in package.json or update the affected requests to use undici's explicit deduplication mechanism per its docs (or add explicit request-level options) so behavior remains correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/package.json`:
- Line 80: Dependency undici was bumped to ^7.22.0 which changed deduplication
behavior for non-safe HTTP methods; run integration tests that exercise
POST/PUT/PATCH paths and search the codebase for usages of undici (the "undici"
entry in package.json and any calls to Pool.request, Client, or undici.fetch) to
confirm no code relies on implicit dedupe; if you do rely on it either pin the
dependency back to ^7.18.2 in package.json or update the affected requests to
use undici's explicit deduplication mechanism per its docs (or add explicit
request-level options) so behavior remains correct.
d93a643 to
274624f
Compare
gardener-prow
Bot
added
size/L
Improve code readability by explicitly passing regularUpdate, forcedUpdate, and noUpdate parameters instead of using severity, which had implicit semantics Align Kubernetes machine image expiration message
grolu
force-pushed
the
bug/fix_expiration_messages
branch
from
274624f to
184ba16
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/composables/useCloudProfile/useKubernetesVersions.js (1)
138-141: Pre-existing bug: ref validation uses&&instead of||.The condition
!isRef(k8sVersion) && !isRef(k8sAutoPatch)only throws when both arguments are not refs. It should throw if either is not a ref, matching the validation pattern inuseAvailableKubernetesUpdates(line 83) anduseKubernetesVersionIsNotLatestPatch(line 117).This isn't introduced by this PR but could cause silent failures if only one argument is incorrectly passed.
Suggested fix
function useKubernetesVersionExpiration (k8sVersion, k8sAutoPatch) { - if (!isRef(k8sVersion) && !isRef(k8sAutoPatch)) { - throw Error('k8sVersion and k8sAutoPatch must be a ref!') + if (!isRef(k8sVersion) || !isRef(k8sAutoPatch)) { + throw Error('k8sVersion and k8sAutoPatch must be refs!') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/useCloudProfile/useKubernetesVersions.js` around lines 138 - 141, The ref validation in useKubernetesVersionExpiration currently throws only when both k8sVersion and k8sAutoPatch are non-refs; change the condition to throw when either is not a ref by replacing the logical AND with OR (i.e., use !isRef(k8sVersion) || !isRef(k8sAutoPatch)), keeping the Error message and leaving the rest of the function intact so it matches the validation pattern used in useAvailableKubernetesUpdates and useKubernetesVersionIsNotLatestPatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/composables/useCloudProfile/useKubernetesVersions.js`:
- Around line 138-141: The ref validation in useKubernetesVersionExpiration
currently throws only when both k8sVersion and k8sAutoPatch are non-refs; change
the condition to throw when either is not a ref by replacing the logical AND
with OR (i.e., use !isRef(k8sVersion) || !isRef(k8sAutoPatch)), keeping the
Error message and leaving the rest of the function intact so it matches the
validation pattern used in useAvailableKubernetesUpdates and
useKubernetesVersionIsNotLatestPatch.
…when both arguments are not refs. It should throw if either is not a ref, matching the validation pattern in useAvailableKubernetesUpdates (line 83) and useKubernetesVersionIsNotLatestPatch (line 117).
|
LGTM label has been added. DetailsGit tree hash: 4d1bb764d4df4976bb5fe5059e2d06b9a1121e5b |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petersutter The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
gardener-prow
Bot
added
the
approved
Contributor
|
@grolu: #2775 failed to apply on top of branch "hotfix-1.83": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve code readability by explicitly passing regularUpdate, forcedUpdate, and noUpdate parameters instead of using severity, which had implicit semantics Align Kubernetes machine image expiration message
What this PR does / why we need it:
Clusters that had no update for the
updateStrategy(e.g. minor) defined in cloud profile are incorrectly flagged aserroreven if there is a higher supported version available (e.g. major)This PR ensures correct handling for this case
Which issue(s) this PR fixes:
Fixes #2772
Special notes for your reviewer:
Release note:
Summary by CodeRabbit
New Features
Tests