feat: Add support for namespacedcloudprofiles#2691
Open
Conversation
klocke-io
requested review from
grolu,
holgerkoser and
petersutter
as code owners
gardener-robot
added
needs/review
klocke-io
force-pushed
the
feature/namespaced-cloud-profile-support
branch
3 times, most recently
from
a561dff to
674686f
Compare
|
@holgerkoser, @grolu You have pull request review open invite, please check |
grolu
added
area/ipcei
petersutter
reviewed
Dec 8, 2025
backend/lib/routes/index.js
Outdated
| '/projects': projectsRoute, | ||
| '/namespaces/:namespace/shoots': shootsRoute, | ||
| '/namespaces/:namespace/tickets': ticketsRoute, | ||
| '/namespaces/:namespace/cloudprofiles': namespacedCloudProfilesRoute, |
Member
There was a problem hiding this comment.
let's align closer to the gardener API (/apis/core.gardener.cloud/v1beta1/namespaces/garden-core/namespacedcloudprofiles)
Suggested change
| '/namespaces/:namespace/cloudprofiles': namespacedCloudProfilesRoute, | |
| '/namespaces/:namespace/namespacedcloudprofiles': namespacedCloudProfilesRoute, |
Contributor
|
The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
You can:
/lifecycle stale |
gardener-robot
added
the
lifecycle/stale
Contributor
|
The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
You can:
/lifecycle rotten |
gardener-robot
added
lifecycle/rotten
grolu
added
lifecycle/stale
Member
Author
|
/remove-lifecycle rotten |
klocke-io
removed
the
lifecycle/frozen
gardener-prow
bot
added
the
do-not-merge/work-in-progress
klocke-io
force-pushed
the
feature/namespaced-cloud-profile-support
branch
from
674686f to
a3a62e7
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
gardener-prow
bot
added
cla: yes
klocke-io
force-pushed
the
feature/namespaced-cloud-profile-support
branch
from
23fa0b5 to
6d52a64
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces comprehensive support for NamespacedCloudProfile resources, a new Kubernetes resource enabling namespace-scoped cloud profile configurations. Changes span backend API routes, services, caching, authorization, frontend UI components, and state management, with integrated diff-based profile specification handling via jsondiffpatch. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
gardener-prow
bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
frontend/src/composables/useShootMessages.js (1)
49-49: Prefer a more specific JSDoc shape thanobjectfor better DX.
Ref<object>works, but a minimal structural type would preserve IntelliSense while still covering both profile kinds.♻️ Optional JSDoc refinement
- * `@param` {Ref<object>} cloudProfile - A Vue ref containing a CloudProfile or NamespacedCloudProfile object + * `@param` {Ref<{ kind?: string, spec?: object }>} cloudProfile - A Vue ref containing a CloudProfile-like object🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/useShootMessages.js` at line 49, The JSDoc for the cloudProfile parameter is too generic (Ref<object>) which hurts IntelliSense; update the JSDoc on the cloudProfile param in useShootMessages (the cloudProfile param of the function exported from frontend/src/composables/useShootMessages.js) to a minimal structural type that covers both CloudProfile and NamespacedCloudProfile (for example a Ref with metadata and optional namespace fields or an explicit union like Ref<CloudProfile|NamespacedCloudProfile>) so editors can infer properties like metadata.name/namespace while still accepting both kinds.backend/__tests__/acceptance/api.namespacedCloudprofiles.spec.cjs (1)
33-37: Add an explicit 200 assertion in the success path.This test already validates content type and body; adding
.expect(200)makes the contract stricter and clearer.✅ Suggested tweak
const res = await agent .get(`/api/namespaces/${namespace}/namespacedcloudprofiles`) .set('cookie', await user.cookie) + .expect(200) .expect('content-type', /json/)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/__tests__/acceptance/api.namespacedCloudprofiles.spec.cjs` around lines 33 - 37, The test's request chain lacks an explicit HTTP status assertion, so update the agent GET call to assert a 200 OK response by adding .expect(200) in the request chain (e.g., on the agent.get(`/api/namespaces/${namespace}/namespacedcloudprofiles`) call that currently sets 'cookie' and expects 'content-type' and inspects res.body); ensure .expect(200) is placed before or alongside the existing .expect('content-type', /json/) to make the success contract explicit.frontend/src/composables/useApi/api.js (1)
239-242: Consider makingdiffconfigurable in the API helper.Hardcoding
diff=truecouples all callers to diff computation. Exposing a parameter (default as needed) keeps this endpoint reusable and avoids unnecessary backend work.♻️ Proposed refactor
-export function getNamespacedCloudProfiles ({ namespace }) { +export function getNamespacedCloudProfiles ({ namespace, diff = true }) { namespace = encodeURIComponent(namespace) - return getResource(`/api/namespaces/${namespace}/namespacedcloudprofiles?diff=true`) + const search = new URLSearchParams({ diff: String(diff) }).toString() + return getResource(`/api/namespaces/${namespace}/namespacedcloudprofiles?${search}`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/useApi/api.js` around lines 239 - 242, Update getNamespacedCloudProfiles to accept a diff option instead of hardcoding diff=true: modify the function signature (getNamespacedCloudProfiles) to take an options object (e.g., { namespace, diff = true }) or an extra parameter, url-encode namespace as before, build the query string to include diff only when provided (using the diff value as default true if desired), and call getResource with `/api/namespaces/${namespace}/namespacedcloudprofiles?diff=${diff}` (or conditionally append the diff param) so callers can override or omit diff to avoid forcing diff computation.backend/lib/utils/index.js (1)
150-185: Consider makingsimplifyCloudProfilenon-mutating.At Line 168 and Line 182, the function mutates the passed object. Cloning internally would reduce side-effect risk when future callers pass shared references.
♻️ Suggested refactor
function simplifyCloudProfile (profile) { + profile = _.cloneDeep(profile) const kind = _.get(profile, ['kind']) let entrypath = ['spec'] if (kind === 'NamespacedCloudProfile') { entrypath = ['status', 'cloudProfileSpec']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/lib/utils/index.js` around lines 150 - 185, simplifyCloudProfile currently mutates its input by calling simplifyObjectMetadata(profile) and _.set(profile, [...entrypath, 'providerConfig'], strippedProviderConfig); to make it non-mutating, create a deep clone of the incoming profile at the start (e.g., cloneProfile = _.cloneDeep(profile)) and operate on that clone throughout (pass cloneProfile into simplifyObjectMetadata and set the providerConfig on cloneProfile instead of profile), and when calling stripProviderConfig ensure you also clone providerConfig before passing it if stripProviderConfig mutates its input; return the cloned/modified object (cloneProfile) and leave the original profile unchanged, referencing the simplifyCloudProfile function, profile variable, providerConfig variable, simplifyObjectMetadata and stripProviderConfig helpers, and the _.set usage to locate the changes.frontend/src/composables/useShootHelper.js (1)
102-107: Avoidnamespaceshadowing indefaultCloudProfileRef.The local
namespaceat Line 102 shadows the outernamespacecomputed ref, which makes this block harder to read.🧹 Small readability tweak
- const namespace = get(defaultCloudProfile, ['metadata', 'namespace']) + const profileNamespace = get(defaultCloudProfile, ['metadata', 'namespace']) return { name, - kind: namespace ? 'NamespacedCloudProfile' : 'CloudProfile', - ...(namespace && { namespace }), + kind: profileNamespace ? 'NamespacedCloudProfile' : 'CloudProfile', + ...(profileNamespace && { namespace: profileNamespace }), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/useShootHelper.js` around lines 102 - 107, The local variable named "namespace" inside the object returned from defaultCloudProfile (const namespace = get(defaultCloudProfile, ['metadata', 'namespace'])) shadows the outer computed ref named "namespace" (defaultCloudProfileRef), hurting readability; rename the local to something like "profileNamespace" (or "cpNamespace") and update its usage in the returned object (kind: profileNamespace ? 'NamespacedCloudProfile' : 'CloudProfile', ...(profileNamespace && { namespace: profileNamespace })) so the outer computed ref remains unshadowed and the code is clearer.backend/__tests__/services.namespacedCloudProfiles.spec.js (1)
227-230: Prefer name-based assertions over index-based assertions in these tests.Using profile identity (e.g.,
metadata.name) instead of fixed positions will make tests less brittle to ordering changes.Also applies to: 269-272
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/__tests__/services.namespacedCloudProfiles.spec.js` around lines 227 - 230, Replace brittle index-based assertions on result[0]/result[1] with name-based assertions that check the set of profile names; for example, map result to an array of metadata.name and use something like expect(names).toEqual(expect.arrayContaining(['custom-cloudprofile-1','custom-cloudprofile-2'])) (or toContain for each name). Apply the same change to the other test block that asserts result[0]/result[1] (the second occurrence referenced) so both tests assert on metadata.name values rather than fixed positions.backend/lib/services/namespacedCloudProfiles.js (1)
24-39: Consider memoizing parent-spec simplification in diff mode.When multiple NamespacedCloudProfiles share a parent,
getCloudProfile+simplifyCloudProfileare recomputed for each item. Caching by parent name in the request scope would cut repeated work.Also applies to: 70-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/lib/services/namespacedCloudProfiles.js` around lines 24 - 39, Multiple NamespacedCloudProfiles recompute getCloudProfile + simplifyCloudProfile for the same parent; add a request-scoped cache (e.g., a Map keyed by parentName) inside computeDiff to avoid repeated work: before calling getCloudProfile/simplifyCloudProfile check the cache for parentName, if present reuse the cached simplified parent or parentSpec, otherwise call getCloudProfile and simplifyCloudProfile, store the simplified result in the cache, then proceed to compute jsondiffpatch.diff; apply the same caching pattern to the other identical call site around lines 70-72 so both places reuse the request-scoped memoized simplified parent by parentName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/__tests__/composables/useCloudProfile/useMachineTypes.spec.js`:
- Line 134: The test creates the computed from useFilteredMachineTypes(region,
architecture) but never evaluates it; change the test to capture the returned
computed (e.g., const filtered = useFilteredMachineTypes(region, architecture))
and then read or assign filtered.value so the computed callback runs and the
defaulting/mutation-sensitive logic executes; ensure any assertions use
filtered.value afterward to validate results.
In `@frontend/src/components/GSelectCloudProfile.vue`:
- Around line 38-49: The custom selection slot currently renders item.raw.title
directly which yields a blank label when the selected value isn't in
selectItems; update the selection template (the template `#selection`="{ item }"
slot) to use a safe fallback chain (e.g. item.raw.title || item.title ||
item.text || item.value) so a meaningful label is shown whenever item.raw.title
is empty or undefined and preserve the existing v-chip logic for
item.raw.namespaced.
In `@frontend/src/router/guards.js`:
- Line 73: The variable assigned to namespace (currently using
to.params.namespace ?? to.query.namespace) must be normalized to a single string
because to.query.namespace can be a string[]; update the assignment in guards.js
so that if to.query.namespace is an array you extract a single string (e.g.,
first element) or join/flatten it, and then fallback to to.params.namespace;
ensure the final namespace is a string (use Array.isArray on to.query.namespace
and coerce/trim as needed) before any validation or API calls that use
namespace.
In `@frontend/src/store/cloudProfile/index.js`:
- Around line 69-75: fetchNamespacedCloudProfilesForNamespaces currently only
adds missing namespaces but never removes stale entries, leaving
namespacedList/allCloudProfiles polluted; update this function to prune cache
entries for namespaces that are no longer in scope: compute the canonical set of
requested namespaces (uniq/filter(Boolean)), call the existing
fetchNamespacedCloudProfiles for any missing ones as before, and then remove any
keys from namespacedList and any entries in allCloudProfiles that belong to
namespaces not in that canonical set (use the store's mutation or helper that
manages namespacedList and allCloudProfiles so state stays consistent). Ensure
you reference and update namespacedList and allCloudProfiles within
fetchNamespacedCloudProfilesForNamespaces so removed namespaces are cleared
after the fetch step.
---
Nitpick comments:
In `@backend/__tests__/acceptance/api.namespacedCloudprofiles.spec.cjs`:
- Around line 33-37: The test's request chain lacks an explicit HTTP status
assertion, so update the agent GET call to assert a 200 OK response by adding
.expect(200) in the request chain (e.g., on the
agent.get(`/api/namespaces/${namespace}/namespacedcloudprofiles`) call that
currently sets 'cookie' and expects 'content-type' and inspects res.body);
ensure .expect(200) is placed before or alongside the existing
.expect('content-type', /json/) to make the success contract explicit.
In `@backend/__tests__/services.namespacedCloudProfiles.spec.js`:
- Around line 227-230: Replace brittle index-based assertions on
result[0]/result[1] with name-based assertions that check the set of profile
names; for example, map result to an array of metadata.name and use something
like
expect(names).toEqual(expect.arrayContaining(['custom-cloudprofile-1','custom-cloudprofile-2']))
(or toContain for each name). Apply the same change to the other test block that
asserts result[0]/result[1] (the second occurrence referenced) so both tests
assert on metadata.name values rather than fixed positions.
In `@backend/lib/services/namespacedCloudProfiles.js`:
- Around line 24-39: Multiple NamespacedCloudProfiles recompute getCloudProfile
+ simplifyCloudProfile for the same parent; add a request-scoped cache (e.g., a
Map keyed by parentName) inside computeDiff to avoid repeated work: before
calling getCloudProfile/simplifyCloudProfile check the cache for parentName, if
present reuse the cached simplified parent or parentSpec, otherwise call
getCloudProfile and simplifyCloudProfile, store the simplified result in the
cache, then proceed to compute jsondiffpatch.diff; apply the same caching
pattern to the other identical call site around lines 70-72 so both places reuse
the request-scoped memoized simplified parent by parentName.
In `@backend/lib/utils/index.js`:
- Around line 150-185: simplifyCloudProfile currently mutates its input by
calling simplifyObjectMetadata(profile) and _.set(profile, [...entrypath,
'providerConfig'], strippedProviderConfig); to make it non-mutating, create a
deep clone of the incoming profile at the start (e.g., cloneProfile =
_.cloneDeep(profile)) and operate on that clone throughout (pass cloneProfile
into simplifyObjectMetadata and set the providerConfig on cloneProfile instead
of profile), and when calling stripProviderConfig ensure you also clone
providerConfig before passing it if stripProviderConfig mutates its input;
return the cloned/modified object (cloneProfile) and leave the original profile
unchanged, referencing the simplifyCloudProfile function, profile variable,
providerConfig variable, simplifyObjectMetadata and stripProviderConfig helpers,
and the _.set usage to locate the changes.
In `@frontend/src/composables/useApi/api.js`:
- Around line 239-242: Update getNamespacedCloudProfiles to accept a diff option
instead of hardcoding diff=true: modify the function signature
(getNamespacedCloudProfiles) to take an options object (e.g., { namespace, diff
= true }) or an extra parameter, url-encode namespace as before, build the query
string to include diff only when provided (using the diff value as default true
if desired), and call getResource with
`/api/namespaces/${namespace}/namespacedcloudprofiles?diff=${diff}` (or
conditionally append the diff param) so callers can override or omit diff to
avoid forcing diff computation.
In `@frontend/src/composables/useShootHelper.js`:
- Around line 102-107: The local variable named "namespace" inside the object
returned from defaultCloudProfile (const namespace = get(defaultCloudProfile,
['metadata', 'namespace'])) shadows the outer computed ref named "namespace"
(defaultCloudProfileRef), hurting readability; rename the local to something
like "profileNamespace" (or "cpNamespace") and update its usage in the returned
object (kind: profileNamespace ? 'NamespacedCloudProfile' : 'CloudProfile',
...(profileNamespace && { namespace: profileNamespace })) so the outer computed
ref remains unshadowed and the code is clearer.
In `@frontend/src/composables/useShootMessages.js`:
- Line 49: The JSDoc for the cloudProfile parameter is too generic (Ref<object>)
which hurts IntelliSense; update the JSDoc on the cloudProfile param in
useShootMessages (the cloudProfile param of the function exported from
frontend/src/composables/useShootMessages.js) to a minimal structural type that
covers both CloudProfile and NamespacedCloudProfile (for example a Ref with
metadata and optional namespace fields or an explicit union like
Ref<CloudProfile|NamespacedCloudProfile>) so editors can infer properties like
metadata.name/namespace while still accepting both kinds.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
.yarn/cache/@dmsnell-diff-match-patch-npm-1.1.0-021308d314-8547bf4a62.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/jsondiffpatch-npm-0.7.3-5579555f56-fae49073e5.zipis excluded by!**/.yarn/**,!**/*.zipbackend/__tests__/acceptance/__snapshots__/api.namespacedCloudprofiles.spec.cjs.snapis excluded by!**/*.snapcharts/__tests__/gardener-dashboard/application/dashboard/__snapshots__/clusterrole.spec.js.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
.pnp.cjsbackend/__fixtures__/index.cjsbackend/__fixtures__/namespacedcloudprofiles.cjsbackend/__mocks__/jsondiffpatch.cjsbackend/__tests__/acceptance/api.cloudprofiles.spec.cjsbackend/__tests__/acceptance/api.namespacedCloudprofiles.spec.cjsbackend/__tests__/cache.spec.cjsbackend/__tests__/services.namespacedCloudProfiles.spec.jsbackend/jest.config.cjsbackend/jest.setup.cjsbackend/lib/cache/index.jsbackend/lib/hooks.jsbackend/lib/routes/index.jsbackend/lib/routes/namespacedCloudProfiles.jsbackend/lib/services/authorization.jsbackend/lib/services/cloudprofiles.jsbackend/lib/services/index.jsbackend/lib/services/namespacedCloudProfiles.jsbackend/lib/utils/index.jsbackend/package.jsonbackend/vitest.config.jscharts/gardener-dashboard/charts/application/templates/dashboard/clusterrole.yamlfrontend/__tests__/composables/useCloudProfile/useMachineTypes.spec.jsfrontend/__tests__/stores/cloudProfile.spec.jsfrontend/__tests__/utils/index.spec.jsfrontend/package.jsonfrontend/src/components/GSelectCloudProfile.vuefrontend/src/composables/helper.jsfrontend/src/composables/useApi/api.jsfrontend/src/composables/useCloudProfile/useDefaultNodesCIDR.jsfrontend/src/composables/useCloudProfile/useKubernetesVersions.jsfrontend/src/composables/useCloudProfile/useMachineImages.jsfrontend/src/composables/useCloudProfile/useMachineTypes.jsfrontend/src/composables/useCloudProfile/useMetalConstraints.jsfrontend/src/composables/useCloudProfile/useOpenStackConstraints.jsfrontend/src/composables/useCloudProfile/useRegions.jsfrontend/src/composables/useCloudProfile/useVolumeTypes.jsfrontend/src/composables/useShootHelper.jsfrontend/src/composables/useShootMessages.jsfrontend/src/router/guards.jsfrontend/src/store/cloudProfile/index.jsfrontend/src/store/seed.jsfrontend/src/utils/index.jspackages/kube-client/lib/resources/GardenerCore.js
|
|
||
| const region = ref('region2') | ||
| const architecture = ref(undefined) | ||
| useFilteredMachineTypes(region, architecture) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the composable returns a lazy computed and the test doesn't dereference it.
rg -n -C2 "function useFilteredMachineTypes|return computed" frontend/src/composables/useCloudProfile/useMachineTypes.jsRepository: gardener/dashboard
Length of output: 661
🏁 Script executed:
#!/bin/bash
rg -n -C4 "should not mutate cloud profile machine types when defaulting architecture|useFilteredMachineTypes\(|\\.value" frontend/__tests__/composables/useCloudProfile/useMachineTypes.spec.jsRepository: gardener/dashboard
Length of output: 3281
Evaluate the computed value in this test to ensure the defaulting logic executes.
Line 134 creates the computed but never accesses it, so the mutation-sensitive logic inside the computed callback won't run. The test should assign and dereference .value to trigger the lazy-evaluated function.
🧪 Suggested fix
- useFilteredMachineTypes(region, architecture)
+ const dashboardMachineTypes = useFilteredMachineTypes(region, architecture)
+ void dashboardMachineTypes.value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useFilteredMachineTypes(region, architecture) | |
| const dashboardMachineTypes = useFilteredMachineTypes(region, architecture) | |
| void dashboardMachineTypes.value |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/__tests__/composables/useCloudProfile/useMachineTypes.spec.js` at
line 134, The test creates the computed from useFilteredMachineTypes(region,
architecture) but never evaluates it; change the test to capture the returned
computed (e.g., const filtered = useFilteredMachineTypes(region, architecture))
and then read or assign filtered.value so the computed callback runs and the
defaulting/mutation-sensitive logic executes; ensure any assertions use
filtered.value afterward to validate results.
Comment on lines
+38
to
+49
| <template #selection="{ item }"> | ||
| <span>{{ item.raw.title }}</span> | ||
| <v-chip | ||
| v-if="item.raw.namespaced" | ||
| size="x-small" | ||
| color="primary" | ||
| variant="tonal" | ||
| class="ml-2" | ||
| > | ||
| Namespaced | ||
| </v-chip> | ||
| </template> |
There was a problem hiding this comment.
Selection label fallback is bypassed in the custom slot.
If the selected value is not present in selectItems, item.raw.title can be empty, so the field renders a blank selection label.
💡 Suggested fix
- <template `#selection`="{ item }">
- <span>{{ item.raw.title }}</span>
+ <template `#selection`="{ item }">
+ <span>{{ item.title }}</span>
<v-chip
- v-if="item.raw.namespaced"
+ v-if="item.raw.namespaced ?? item.raw.kind === 'NamespacedCloudProfile'"
size="x-small"
color="primary"
variant="tonal"
class="ml-2"
>
Namespaced
</v-chip>
</template>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/GSelectCloudProfile.vue` around lines 38 - 49, The
custom selection slot currently renders item.raw.title directly which yields a
blank label when the selected value isn't in selectItems; update the selection
template (the template `#selection`="{ item }" slot) to use a safe fallback chain
(e.g. item.raw.title || item.title || item.text || item.value) so a meaningful
label is shown whenever item.raw.title is empty or undefined and preserve the
existing v-chip logic for item.raw.namespaced.
| } | ||
|
|
||
| try { | ||
| const namespace = to.params.namespace ?? to.query.namespace |
There was a problem hiding this comment.
Normalize namespace to a single string before using it.
to.query.namespace may be string[]; forwarding it directly can cause invalid namespace checks and API calls.
💡 Suggested fix
- const namespace = to.params.namespace ?? to.query.namespace
+ const rawNamespace = to.params.namespace ?? to.query.namespace
+ const namespace = Array.isArray(rawNamespace) ? rawNamespace[0] : rawNamespace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/router/guards.js` at line 73, The variable assigned to namespace
(currently using to.params.namespace ?? to.query.namespace) must be normalized
to a single string because to.query.namespace can be a string[]; update the
assignment in guards.js so that if to.query.namespace is an array you extract a
single string (e.g., first element) or join/flatten it, and then fallback to
to.params.namespace; ensure the final namespace is a string (use Array.isArray
on to.query.namespace and coerce/trim as needed) before any validation or API
calls that use namespace.
Comment on lines
+69
to
+75
| async function fetchNamespacedCloudProfilesForNamespaces (namespaces = []) { | ||
| const namespacesToFetch = uniq(namespaces) | ||
| .filter(Boolean) | ||
| .filter(namespace => !hasNamespacedCloudProfilesForNamespace(namespace)) | ||
|
|
||
| await Promise.all(namespacesToFetch.map(namespace => fetchNamespacedCloudProfiles(namespace))) | ||
| } |
There was a problem hiding this comment.
Prune stale namespace cache entries when namespace scope shrinks.
Current logic only fetches missing namespaces; it never removes namespaces that are no longer in scope. That leaves stale namespaced cloud profiles in namespacedList / allCloudProfiles.
💡 Suggested fix
async function fetchNamespacedCloudProfilesForNamespaces (namespaces = []) {
- const namespacesToFetch = uniq(namespaces)
+ const allowedNamespaces = uniq(namespaces)
.filter(Boolean)
- .filter(namespace => !hasNamespacedCloudProfilesForNamespace(namespace))
+
+ // Drop cached namespaces that are no longer in scope
+ namespacedListsByNamespace.value = Object.fromEntries(
+ Object.entries(namespacedListsByNamespace.value)
+ .filter(([namespace]) => allowedNamespaces.includes(namespace)),
+ )
+ namespacedList.value = flatten(Object.values(namespacedListsByNamespace.value))
+
+ const namespacesToFetch = allowedNamespaces
+ .filter(namespace => !hasNamespacedCloudProfilesForNamespace(namespace))
await Promise.all(namespacesToFetch.map(namespace => fetchNamespacedCloudProfiles(namespace)))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function fetchNamespacedCloudProfilesForNamespaces (namespaces = []) { | |
| const namespacesToFetch = uniq(namespaces) | |
| .filter(Boolean) | |
| .filter(namespace => !hasNamespacedCloudProfilesForNamespace(namespace)) | |
| await Promise.all(namespacesToFetch.map(namespace => fetchNamespacedCloudProfiles(namespace))) | |
| } | |
| async function fetchNamespacedCloudProfilesForNamespaces (namespaces = []) { | |
| const allowedNamespaces = uniq(namespaces) | |
| .filter(Boolean) | |
| // Drop cached namespaces that are no longer in scope | |
| namespacedListsByNamespace.value = Object.fromEntries( | |
| Object.entries(namespacedListsByNamespace.value) | |
| .filter(([namespace]) => allowedNamespaces.includes(namespace)), | |
| ) | |
| namespacedList.value = flatten(Object.values(namespacedListsByNamespace.value)) | |
| const namespacesToFetch = allowedNamespaces | |
| .filter(namespace => !hasNamespacedCloudProfilesForNamespace(namespace)) | |
| await Promise.all(namespacesToFetch.map(namespace => fetchNamespacedCloudProfiles(namespace))) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/store/cloudProfile/index.js` around lines 69 - 75,
fetchNamespacedCloudProfilesForNamespaces currently only adds missing namespaces
but never removes stale entries, leaving namespacedList/allCloudProfiles
polluted; update this function to prune cache entries for namespaces that are no
longer in scope: compute the canonical set of requested namespaces
(uniq/filter(Boolean)), call the existing fetchNamespacedCloudProfiles for any
missing ones as before, and then remove any keys from namespacedList and any
entries in allCloudProfiles that belong to namespaces not in that canonical set
(use the store's mutation or helper that manages namespacedList and
allCloudProfiles so state stays consistent). Ensure you reference and update
namespacedList and allCloudProfiles within
fetchNamespacedCloudProfilesForNamespaces so removed namespaces are cleared
after the fetch step.
grolu
reviewed
Mar 9, 2026
Member
There was a problem hiding this comment.
use camel case for file like you did with the other files
Member
Author
There was a problem hiding this comment.
done, thanks for catching it :)
grolu
reviewed
Mar 9, 2026
| @@ -50,6 +50,13 @@ rules: | |||
| verbs: | |||
Member
There was a problem hiding this comment.
we also need to add this to gardener operator
Member
Author
There was a problem hiding this comment.
https://github.com/gardener/gardener/pull/13500/changes
Done and merged, or did you mean something else :)
petersutter
reviewed
Mar 10, 2026
| const parentCloudProfile = getCloudProfile(parentName) | ||
|
|
||
| if (!parentCloudProfile) { | ||
| return null |
Member
There was a problem hiding this comment.
Should this case throw an exception?
Comment on lines
+51
to
+58
| const result = _.cloneDeep(profile) | ||
| result.status = { | ||
| ...result.status, | ||
| cloudProfileSpecDiff: diff || null, | ||
| } | ||
| delete result.status.cloudProfileSpec | ||
|
|
||
| return result |
Member
There was a problem hiding this comment.
Suggested change
| const result = _.cloneDeep(profile) | |
| result.status = { | |
| ...result.status, | |
| cloudProfileSpecDiff: diff || null, | |
| } | |
| delete result.status.cloudProfileSpec | |
| return result | |
| const { status = {}, ...profileRest } = profile | |
| const { cloudProfileSpec, ...statusRest } = status | |
| return { | |
| ...profileRest, | |
| status: { | |
| ...statusRest, | |
| cloudProfileSpecDiff: diff ?? null, | |
| }, | |
| } |
| "test": "NODE_PATH=./dist cross-env NODE_ENV=test node --experimental-vm-modules $(yarn bin jest)", | ||
| "test-coverage": "yarn test --coverage" | ||
| "test-coverage": "yarn test --coverage", | ||
| "test-vitest": "vitest run" |
Member
There was a problem hiding this comment.
test-vitest is currently not run viamake verify, or?
| const parentName = profile.spec?.parent?.name | ||
| const parent = find(list.value, ['metadata.name', parentName]) | ||
| if (!parent) { | ||
| return profile |
Member
There was a problem hiding this comment.
isn't this an exception / should be handled as error case?
| ) ?? null | ||
| } | ||
| // Fallback: only return when name is unique across namespaces | ||
| const matches = filter(namespacedList.value, ['metadata.name', cloudProfileRef.name]) |
Member
There was a problem hiding this comment.
when namespace is not set on cloudProfileRef, the namespace of the parent object (e.g. the shoot) has to be taken
Comment on lines
+44
to
+46
| const isNamespacedInitial = computed(() => { | ||
| return Object.keys(namespacedListsByNamespace.value).length === 0 | ||
| }) |
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
5 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.
What this PR does / why we need it:
Add support for namespacedcloudprofiles
Which issue(s) this PR fixes:
Fixes # #1971
Special notes for your reviewer:
NSCP in create cluster dialog
No more warnings because the cloudprofile is not understood
Release note:
Summary by CodeRabbit
Release Notes