Draft
Conversation
Introduces GGenericInputField Component that parses a DSL to render for specific vendor Use GGenericSecretDialog everywhere except for OpenStack Refactored OpenStack Dialog to also make use of GGenericInputFields so it becomes way easier Moved all provider config from store to dedicated config package
Refactored cloudprofiles fixtures for better re-use
…config Added missing onmetal cloud provider configuration
Changed useShootContext test suite to dynamically execute tests on all configured infras
gardener-prow
bot
added
do-not-merge/work-in-progress
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors credential dialogs and provider configuration from a provider-specific component approach to a data-driven, generic model. Provider-specific credential components are removed and replaced with generic input field components. Provider configurations are externalized into vendor data modules organized by type (infra, DNS). Composables and utilities are added to support structured text fields and vendor-based secret resolution. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Credential Dialog UI
participant GenericInput as GGenericInputField
participant SecretContext as useSecretContext
participant VendorConfig as Vendor Data Config
participant Helper as credential/helper.js
UI->>VendorConfig: Load vendorSecretConfiguration(providerType)
VendorConfig-->>UI: Return { fields, details, help }
UI->>GenericInput: Render field inputs with validators
GenericInput->>SecretContext: Update secretStringData
SecretContext->>SecretContext: Encode to base64
SecretContext-->>GenericInput: Sync state
UI->>Helper: Resolve secret details
Helper->>VendorConfig: Fetch provider config
VendorConfig-->>Helper: Return detail definitions
Helper-->>UI: Display decoded secret values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can get early access to new features in CodeRabbit.Enable the |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
[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
![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 (1)
frontend/src/components/Credentials/GSecretDialogOpenstack.vue (1)
169-175:⚠️ Potential issue | 🟠 MajorRestore the auth mode and existing non-secret fields in edit mode.
Lines 173 and 291-295 currently reopen every existing secret as
USERand only repopulatedomainName/tenantName. Editing an application-credential secret therefore shows the wrong form and drops values likeauthURL,username,applicationCredentialID, andapplicationCredentialNamefrom the edit flow. DeriveauthenticationMethodfrom the stored keys before rendering, then repopulate the matching non-secret fields duringreset(). Based on learnings: dialog components underfrontend/src/components/Credentials/GSecretDialogWrapper.vueare recreated on each open viav-if="visibleDialog", so this default is reapplied every time.Also applies to: 279-296
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Credentials/GSecretDialogOpenstack.vue` around lines 169 - 175, When opening the dialog for edit, don't default authenticationMethod to 'USER'—detect and set authenticationMethod from the stored secret keys (e.g. presence of applicationCredentialID/applicationCredentialName vs username) before the component renders and before reset() runs, and update reset() to repopulate the corresponding non-secret fields (authURL, username, applicationCredentialID, applicationCredentialName, domainName/tenantName) for the selected authenticationMethod instead of only restoring domain/tenantName; locate and change the authenticationMethod default in data() and the reset() implementation so the correct form shows and all non-secret values are restored in edit mode.
🧹 Nitpick comments (7)
frontend/src/data/vendors/infra/stackit.js (1)
8-9: Consider capitalizingdisplayNamefor consistency.The
displayNameis'stackit'(lowercase) while other providers use proper capitalization for UI display (e.g.,'AWS','Hetzner Cloud','OnMetal'). Consider using'StackIT'or'STACKIT'to match the branding.📝 Suggested change
export default { name: 'stackit', - displayName: 'stackit', + displayName: 'StackIT', weight: 1100,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/data/vendors/infra/stackit.js` around lines 8 - 9, displayName for the provider object is lowercase ('stackit') and should follow the capitalization used by other providers; update the provider object's displayName field (the 'displayName' property alongside 'name' in this module) to a properly capitalized form such as 'StackIT' so UI displays match branding and other providers.frontend/__tests__/composables/credential.helper.spec.js (1)
18-35: Include PowerDNSapiKeyin the shared fixture.The new provider adds an
apiKeydetail path, but this fixture only setsserver, so the snapshot never exercises the masked API key case.✅ Suggested test update
Zone: encode('example.org.'), server: encode('https://powerdns.example.org'), + apiKey: encode('example-powerdns-api-key'), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/composables/credential.helper.spec.js` around lines 18 - 35, The test fixture function createSecretData() is missing the PowerDNS apiKey, so add an encoded apiKey entry (e.g., apiKey: encode('example-powerdns-api-key')) to the returned object so the snapshot covers the masked API key path; update the object alongside the existing server property to ensure tests exercise the new provider's apiKey behavior.frontend/src/components/GGenericInputField.vue (2)
194-225: Consider usingref()for thetouchedflag.The
touchedvariable is a plain mutableletbinding. While this works for synchronous read/write within the component, usingref()would make the reactive intent clearer and align with Vue 3 conventions.✏️ Suggested change
// ----- structured fields (yaml/json) ----- -let touched = false +const touched = ref(false) const { rawText, setRawTextWithObject, parseRawTextToObject, } = useStructuredTextField(computed(() => props.field.type)) const structuredRawText = computed({ get: () => rawText.value, set: value => { - touched = true + touched.value = true rawText.value = value // ... }, }) watch( () => props.modelValue, value => { - if (isStructuredLike.value && !touched) { + if (isStructuredLike.value && !touched.value) { setRawTextWithObject(value) } }, { immediate: true }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/GGenericInputField.vue` around lines 194 - 225, Replace the plain let touched = false with a Vue ref (e.g., const touched = ref(false)) and update all usages to use touched.value (set touched.value = true inside the structuredRawText setter and check if (!touched.value) in the watch callback) so the touched flag is reactive and follows Vue 3 conventions; update any other references to touched accordingly (e.g., in structuredRawText setter, watch, or elsewhere relying on that flag like setRawTextWithObject).
251-251: TODO: Address incomplete YAML error handling.The TODO comment indicates that invalid YAML may not properly update modelValue on parse errors. This could lead to stale data being submitted.
Would you like me to help implement proper error handling for invalid YAML/JSON that ensures modelValue reflects the parse state?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/GGenericInputField.vue` at line 251, The TODO notes that in the switch branch for 'isValidObject' the component doesn't update modelValue when YAML/JSON parsing fails, leaving stale data; update the 'isValidObject' handling in GGenericInputField.vue so that when parse fails you set a clear parse error state and also emit/update modelValue with a canonical representation (e.g., null or the raw text) to reflect the invalid parse, and ensure you call the existing emit/model update method used elsewhere (the same function that sets modelValue and emits 'update:modelValue') so consumers get the parse-state change; keep the error message and parse result in the component's validation/error properties so the UI can show the YAML/JSON error.frontend/src/data/vendors/infra/vsphere.js (1)
88-91: Consider using English locale for VMware documentation link.The VMware vSphere documentation link uses the German locale (
vmware.com/de/). For an international audience, consider using the English locale or a locale-neutral URL.✏️ Suggested change
<a href="https://docs.vmware.com/de/VMware-vSphere/index.html"> + <a href="https://docs.vmware.com/en/VMware-vSphere/index.html">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/data/vendors/infra/vsphere.js` around lines 88 - 91, Update the VMware documentation link to use an English or locale-neutral URL by replacing the current href "https://docs.vmware.com/de/VMware-vSphere/index.html" (the anchor with text "VMware vSphere Documentation") with the English version (e.g., "https://docs.vmware.com/en/VMware-vSphere/index.html") or a locale-neutral root URL; locate the anchor element in frontend/src/data/vendors/infra/vsphere.js and change only the href value to the preferred URL.frontend/src/components/Credentials/GSecretDialogGeneric.vue (1)
29-40: RenderhelpJSONTemplateindependently ofhelpHtml.Lines 29-40 nest the code block under
v-if="helpHtml", so a provider that only supplieshelpJSONTemplatefalls back to the generic help text instead of showing its example payload.♻️ Proposed adjustment
- <template v-if="helpHtml"> + <template v-if="helpHtml || helpJSONTemplate"> <div + v-if="helpHtml" class="markdown" v-html="helpHtml" /> <g-code-block v-if="helpJSONTemplate"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Credentials/GSecretDialogGeneric.vue` around lines 29 - 40, The g-code-block is currently nested inside the template guarded by v-if="helpHtml", so providers that only provide helpJSONTemplate don't render the example; change the template structure so g-code-block is rendered based solely on v-if="helpJSONTemplate" (move it out of the element controlled by v-if="helpHtml" or create a sibling block), keeping the existing props (class, max-height, lang, :content) and the conditional v-if="helpJSONTemplate" on the g-code-block itself.frontend/src/components/GGenericInputFields.vue (1)
68-72: KeepfieldDatasynced when the parent replacesmodelValue.Lines 68-72 only hydrate once. After the first non-empty assignment, parent-side resets/replacements are ignored, so the rendered inputs can drift from the source object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/GGenericInputFields.vue` around lines 68 - 72, The watcher on props.modelValue only hydrates fieldData once because it only assigns when fieldData is empty; update the watcher for props.modelValue so it keeps fieldData in sync on subsequent parent replacements (not just initial hydration). In practice, change the watch handler for props.modelValue to overwrite/update fieldData.value whenever the incoming value changes (use a proper equality/clone strategy if you need to preserve reactivity or avoid mutating parent objects), referencing the existing watch(() => props.modelValue, ...) and the fieldData ref/isEmpty logic to locate and replace the assignment.
🤖 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/__fixtures__/cloudprofiles.js`:
- Around line 9-25: createMachineType currently keeps the caller's storage
object by reference which causes shared mutation across fixtures (e.g., in
createOpenstackMachineTypes); change createMachineType so that when storage is
provided it is cloned (e.g., shallow-copy the storage into a new object) before
attaching it to the returned object, by replacing the current ...(storage ? {
storage } : {}) with a cloned storage assignment (e.g., ...(storage ? { storage:
{ ...storage } } : {})) or an equivalent safe clone method.
- Around line 63-74: The default kubernetes and machine image defaults are only
shallow-copied, causing nested version/image objects to be shared; in
createCloudProfile update the kubernetes and machineImagesProfile parameter
defaults to deep-clone the fixtures (e.g., deep clone kubernetesVersions and
machineImages) so each profile gets its own nested objects—use
structuredClone(kubernetesVersions) or a fallback deep-clone
(JSON.parse(JSON.stringify(...)) or _.cloneDeep) when assigning the default
values inside createCloudProfile to avoid shared mutable state.
In `@frontend/src/components/GGenericInputField.vue`:
- Around line 251-260: The isValidObject validator incorrectly allows non-empty
raw text to pass when parsing failed because it only checks isEmpty(value) OR
Object.keys(props.modelValue).length > 0; update the validator (symbol:
compiledValidators[validatorName] in the isValidObject case) to require that
props.modelValue is a successfully parsed object before using its keys — e.g.
replace the second condition with a check that props.modelValue is a non-null
object (typeof === 'object') and has keys; this ensures the validator fails if
the raw input is non-empty but parsing did not produce a valid object; keep the
existing YAML/JSON error messages (isYAML, isJSON).
In `@frontend/src/composables/credential/helper.js`:
- Around line 174-177: The function secretDetails dereferences secret.data
before any guard, causing a crash when secret is undefined; update secretDetails
to first check that secret is truthy (or use optional chaining) before accessing
secret.data and computing secretData, e.g. move the const secretData = ...
assignment inside the try after verifying secret or change it to const
secretData = secret?.data || {}; ensure any subsequent logic that uses
providerConfig (from cloudProviderConfigByName.get(providerType)) still runs
only when secret is present so the function returns undefined consistently when
secret is missing.
In `@frontend/src/composables/credential/useSecretContext.js`:
- Around line 112-133: The getter currently JSON.parse() any decoded string
which coerces plain values like "0","false","null" to non-string types and the
setter uses truthy checks that drop falsy values; update the getter in
useSecretContext.js to only attempt JSON.parse when the decoded string (after
trimming) starts with '{' or '[' (i.e., object/array payloads) and keep the
existing fallback to the raw decoded string, and change the null-check guards in
both the getter and setter from a truthy check (!v) to a nullish check (v ==
null) so values like 0, false, and "" are preserved; reference symbols:
secretData.value, decodeBase64, encodeBase64, mapValues.
In `@frontend/src/data/vendors/dns/google-clouddns.js`:
- Around line 27-30: The Cloud DNS anchor tag is malformed (uses `href==`) in
the HTML snippet containing the URL "https://cloud.google.com/dns/docs/zones";
fix it by changing `href==` to a single equals `href=` so the anchor reads `<a
href="https://cloud.google.com/dns/docs/zones">...` ensuring the link works
correctly.
In `@frontend/src/data/vendors/dns/netlify.js`:
- Around line 36-38: Fix the inconsistent example token in the help text by
making the token value consistent between the prose and the example command:
replace either the prose token '1234567890123456' or the echo argument in the
<pre> block currently showing '1234567890123456789' so they match (prefer using
the 16‑char value '1234567890123456' in both places) and ensure the displayed
echo command is $ echo -n '1234567890123456' | base64.
- Around line 13-18: The details entry for the Netlify DNS provider is missing a
key mapping, so the secret won't bind to the UI; update the details object (the
one with label 'API Key') to include a key property (e.g., key: 'apiKey' or key:
['API Key','apiKey']) so the UI can correctly map and display the secret value.
In `@frontend/src/data/vendors/dns/powerdns.js`:
- Around line 18-21: The PowerDNS secret details entry is missing the detail
key, so add the key that maps to secret.data.apiKey to the object with label
'API Key' (the entry that currently has label: 'API Key' and hidden: true) by
adding key: 'apiKey' so secret.details can resolve and display the masked API
key in the PowerDNS details view.
In `@frontend/src/data/vendors/dns/rfc2136.js`:
- Around line 120-125: Fix the spelling mistake in the provider help text by
updating the 'help' property in rfc2136.js: replace "authoritive" with
"authoritative" inside the template string used for the DNS provider help
description so the sentence reads "...create and manage DNS entries for
authoritative DNS server supporting dynamic updates...".
- Around line 30-31: Fix the typo in the UI label string used for the DNS
RFC2136 vendor config: update the 'label' value that currently reads "label:
'<host>:<port> of the authorive DNS server'" to use the correct spelling
"authoritative" so the label becomes "<host>:<port> of the authoritative DNS
server"; locate the 'label' property in the RFC2136 vendor definition
(frontend/src/data/vendors/dns/rfc2136.js) and replace the misspelled word only.
In `@frontend/src/data/vendors/infra/azure.js`:
- Around line 68-77: The label string for the config entry with key 'tenantId'
contains an extraneous trailing double-quote; update the object where key:
'tenantId' to change the label property from 'Tenant Id"' to 'Tenant Id' (remove
the stray quote) so the UI displays the correct label.
In `@frontend/src/data/vendors/infra/gcp.js`:
- Around line 92-95: Replace the incorrect anchor attribute `url="..."` in the
paragraph containing the link text "Service Account Documentation" with a proper
`href="..."` so the link becomes clickable; also locate the similar malformation
`href==` in the other vendor file (the anchor with `href==` in the Google Cloud
DNS snippet) and correct it to a single equals `href="..."`. Ensure both anchors
use standard `href` attributes and valid URL strings.
- Around line 56-60: The projectID validator uses pattern
/^[a-z][a-z0-9-]{4,28}[a-z0-9]+$/, where the trailing + lets IDs exceed the
intended max length; update the regex in the projectID field (key: 'project_id'
in gcp.js) to require exactly one final alphanumeric char so the total length is
capped (e.g. change the trailing [a-z0-9]+ to [a-z0-9]), i.e. use
/^[a-z][a-z0-9-]{4,28}[a-z0-9]$/ so gcp.secret.fields and any consumers like
google-clouddns inherit the tightened validation.
In `@frontend/src/data/vendors/infra/hcloud.js`:
- Around line 44-53: The hcloudToken form field currently uses type: 'text'
which exposes the credential; update the field object for key 'hcloudToken' to
use type: 'password' (leave validators and label unchanged) so the input is
masked in the UI; verify other provider token fields (e.g., secretAccessKey) are
consistent and run a quick UI check to ensure masking works for the
'hcloudToken' input component.
In `@frontend/src/data/vendors/infra/vsphere.js`:
- Around line 68-77: The NSXTPassword field is using type: 'text' which exposes
the secret; update the field definition for key 'NSXTPassword' to use type:
'password' (matching the existing vSpherePassword pattern) so the input is
masked; locate the object with key 'NSXTPassword' and change its type property
from 'text' to 'password'.
In `@frontend/src/utils/shoot.js`:
- Around line 83-85: getKubernetesTemplate currently returns the vendor template
object by reference (via
resolveShootVendor(providerType)?.shoot?.templates?.kubernetes), which allows
callers to mutate global vendor defaults; change getKubernetesTemplate to return
a deep clone of the template (null-safe) instead of the original object (use
structuredClone if available or JSON.parse(JSON.stringify(...)) as a fallback)
and apply the same cloning fix to the analogous returns in getProviderTemplate
and getNetworkingTemplate so no live vendor config objects are leaked.
---
Outside diff comments:
In `@frontend/src/components/Credentials/GSecretDialogOpenstack.vue`:
- Around line 169-175: When opening the dialog for edit, don't default
authenticationMethod to 'USER'—detect and set authenticationMethod from the
stored secret keys (e.g. presence of
applicationCredentialID/applicationCredentialName vs username) before the
component renders and before reset() runs, and update reset() to repopulate the
corresponding non-secret fields (authURL, username, applicationCredentialID,
applicationCredentialName, domainName/tenantName) for the selected
authenticationMethod instead of only restoring domain/tenantName; locate and
change the authenticationMethod default in data() and the reset() implementation
so the correct form shows and all non-secret values are restored in edit mode.
---
Nitpick comments:
In `@frontend/__tests__/composables/credential.helper.spec.js`:
- Around line 18-35: The test fixture function createSecretData() is missing the
PowerDNS apiKey, so add an encoded apiKey entry (e.g., apiKey:
encode('example-powerdns-api-key')) to the returned object so the snapshot
covers the masked API key path; update the object alongside the existing server
property to ensure tests exercise the new provider's apiKey behavior.
In `@frontend/src/components/Credentials/GSecretDialogGeneric.vue`:
- Around line 29-40: The g-code-block is currently nested inside the template
guarded by v-if="helpHtml", so providers that only provide helpJSONTemplate
don't render the example; change the template structure so g-code-block is
rendered based solely on v-if="helpJSONTemplate" (move it out of the element
controlled by v-if="helpHtml" or create a sibling block), keeping the existing
props (class, max-height, lang, :content) and the conditional
v-if="helpJSONTemplate" on the g-code-block itself.
In `@frontend/src/components/GGenericInputField.vue`:
- Around line 194-225: Replace the plain let touched = false with a Vue ref
(e.g., const touched = ref(false)) and update all usages to use touched.value
(set touched.value = true inside the structuredRawText setter and check if
(!touched.value) in the watch callback) so the touched flag is reactive and
follows Vue 3 conventions; update any other references to touched accordingly
(e.g., in structuredRawText setter, watch, or elsewhere relying on that flag
like setRawTextWithObject).
- Line 251: The TODO notes that in the switch branch for 'isValidObject' the
component doesn't update modelValue when YAML/JSON parsing fails, leaving stale
data; update the 'isValidObject' handling in GGenericInputField.vue so that when
parse fails you set a clear parse error state and also emit/update modelValue
with a canonical representation (e.g., null or the raw text) to reflect the
invalid parse, and ensure you call the existing emit/model update method used
elsewhere (the same function that sets modelValue and emits 'update:modelValue')
so consumers get the parse-state change; keep the error message and parse result
in the component's validation/error properties so the UI can show the YAML/JSON
error.
In `@frontend/src/components/GGenericInputFields.vue`:
- Around line 68-72: The watcher on props.modelValue only hydrates fieldData
once because it only assigns when fieldData is empty; update the watcher for
props.modelValue so it keeps fieldData in sync on subsequent parent replacements
(not just initial hydration). In practice, change the watch handler for
props.modelValue to overwrite/update fieldData.value whenever the incoming value
changes (use a proper equality/clone strategy if you need to preserve reactivity
or avoid mutating parent objects), referencing the existing watch(() =>
props.modelValue, ...) and the fieldData ref/isEmpty logic to locate and replace
the assignment.
In `@frontend/src/data/vendors/infra/stackit.js`:
- Around line 8-9: displayName for the provider object is lowercase ('stackit')
and should follow the capitalization used by other providers; update the
provider object's displayName field (the 'displayName' property alongside 'name'
in this module) to a properly capitalized form such as 'StackIT' so UI displays
match branding and other providers.
In `@frontend/src/data/vendors/infra/vsphere.js`:
- Around line 88-91: Update the VMware documentation link to use an English or
locale-neutral URL by replacing the current href
"https://docs.vmware.com/de/VMware-vSphere/index.html" (the anchor with text
"VMware vSphere Documentation") with the English version (e.g.,
"https://docs.vmware.com/en/VMware-vSphere/index.html") or a locale-neutral root
URL; locate the anchor element in frontend/src/data/vendors/infra/vsphere.js and
change only the href value to the preferred URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 90a757c2-ff9c-4588-bcba-01b252af5847
⛔ Files ignored due to path filters (3)
frontend/__tests__/composables/__snapshots__/credential.helper.spec.js.snapis excluded by!**/*.snapfrontend/__tests__/composables/__snapshots__/useShootContext.spec.js.snapis excluded by!**/*.snapfrontend/__tests__/stores/__snapshots__/credential.spec.js.snapis excluded by!**/*.snap
📒 Files selected for processing (53)
frontend/__fixtures__/cloudprofiles.jsfrontend/__tests__/composables/credential.helper.spec.jsfrontend/__tests__/composables/useShootContext.spec.jsfrontend/src/components/Credentials/GSecretDialog.vuefrontend/src/components/Credentials/GSecretDialogAlicloud.vuefrontend/src/components/Credentials/GSecretDialogAws.vuefrontend/src/components/Credentials/GSecretDialogAzure.vuefrontend/src/components/Credentials/GSecretDialogCloudflare.vuefrontend/src/components/Credentials/GSecretDialogDDns.vuefrontend/src/components/Credentials/GSecretDialogGcp.vuefrontend/src/components/Credentials/GSecretDialogGeneric.vuefrontend/src/components/Credentials/GSecretDialogHCloud.vuefrontend/src/components/Credentials/GSecretDialogInfoblox.vuefrontend/src/components/Credentials/GSecretDialogMetal.vuefrontend/src/components/Credentials/GSecretDialogNetlify.vuefrontend/src/components/Credentials/GSecretDialogOpenstack.vuefrontend/src/components/Credentials/GSecretDialogPowerdns.vuefrontend/src/components/Credentials/GSecretDialogVSphere.vuefrontend/src/components/Credentials/GSecretDialogWrapper.vuefrontend/src/components/GGenericInputField.vuefrontend/src/components/GGenericInputFields.vuefrontend/src/composables/credential/helper.jsfrontend/src/composables/credential/useSecretContext.jsfrontend/src/composables/useStructuredTextField.jsfrontend/src/data/vendors/dns/alicloud-dns.jsfrontend/src/data/vendors/dns/aws-route53.jsfrontend/src/data/vendors/dns/azure-dns.jsfrontend/src/data/vendors/dns/azure-private-dns.jsfrontend/src/data/vendors/dns/cloudflare.jsfrontend/src/data/vendors/dns/google-clouddns.jsfrontend/src/data/vendors/dns/index.jsfrontend/src/data/vendors/dns/infoblox.jsfrontend/src/data/vendors/dns/netlify.jsfrontend/src/data/vendors/dns/openstack-designate.jsfrontend/src/data/vendors/dns/powerdns.jsfrontend/src/data/vendors/dns/rfc2136.jsfrontend/src/data/vendors/infra/alicloud.jsfrontend/src/data/vendors/infra/aws.jsfrontend/src/data/vendors/infra/azure.jsfrontend/src/data/vendors/infra/gcp.jsfrontend/src/data/vendors/infra/hcloud.jsfrontend/src/data/vendors/infra/index.jsfrontend/src/data/vendors/infra/ironcore.jsfrontend/src/data/vendors/infra/local.jsfrontend/src/data/vendors/infra/metal.jsfrontend/src/data/vendors/infra/onmetal.jsfrontend/src/data/vendors/infra/openstack.jsfrontend/src/data/vendors/infra/stackit.jsfrontend/src/data/vendors/infra/vsphere.jsfrontend/src/data/vendors/machineImage/index.jsfrontend/src/store/config.jsfrontend/src/utils/index.jsfrontend/src/utils/shoot.js
💤 Files with no reviewable changes (12)
- frontend/src/components/Credentials/GSecretDialogDDns.vue
- frontend/src/components/Credentials/GSecretDialogCloudflare.vue
- frontend/src/components/Credentials/GSecretDialogInfoblox.vue
- frontend/src/components/Credentials/GSecretDialogMetal.vue
- frontend/src/components/Credentials/GSecretDialogPowerdns.vue
- frontend/src/components/Credentials/GSecretDialogVSphere.vue
- frontend/src/components/Credentials/GSecretDialogNetlify.vue
- frontend/src/components/Credentials/GSecretDialogGcp.vue
- frontend/src/components/Credentials/GSecretDialogHCloud.vue
- frontend/src/components/Credentials/GSecretDialogAlicloud.vue
- frontend/src/components/Credentials/GSecretDialogAzure.vue
- frontend/src/components/Credentials/GSecretDialogAws.vue
Comment on lines
+63
to
+74
| const createCloudProfile = ({ | ||
| metadataName, | ||
| displayName, | ||
| type, | ||
| seedNames, | ||
| providerConfig, | ||
| machineTypes, | ||
| regions, | ||
| volumeTypes, | ||
| kubernetes = { versions: [...kubernetesVersions] }, | ||
| machineImagesProfile = [...machineImages], | ||
| }) => ({ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd cloudprofiles.js --type fRepository: gardener/dashboard
Length of output: 174
🏁 Script executed:
cat -n frontend/__fixtures__/cloudprofiles.js | head -100Repository: gardener/dashboard
Length of output: 2659
🏁 Script executed:
# Get context around lines 63-74 and understand the fixture structure
cat -n frontend/__fixtures__/cloudprofiles.js | sed -n '1,80p'Repository: gardener/dashboard
Length of output: 2048
🏁 Script executed:
# Check if kubernetesVersions and machineImages are defined and what they contain
cat -n frontend/__fixtures__/cloudprofiles.js | grep -A 50 "kubernetesVersions\|machineImages" | head -150Repository: gardener/dashboard
Length of output: 4510
🏁 Script executed:
# Search for usages of createCloudProfile to understand mutation patterns
rg "createCloudProfile" --type js -B 2 -A 5Repository: gardener/dashboard
Length of output: 6575
🏁 Script executed:
# Check if there are mutations on kubernetes or machineImages in tests
rg "kubernetes.*=|machineImages.*=" --type js -B 2 -A 2Repository: gardener/dashboard
Length of output: 20082
Deep-clone the default Kubernetes and machine image fixtures.
Lines 72-73 only clone the outer arrays. The nested version/image objects are still shared by every profile that uses the defaults, so a mutation in one provider fixture can leak into the others and into the exported base fixtures.
Proposed fix
+const cloneKubernetes = ({ versions }) => ({
+ versions: versions.map(version => ({ ...version })),
+})
+
+const cloneMachineImages = images => images.map(image => ({
+ ...image,
+ versions: image.versions.map(version => ({
+ ...version,
+ ...(version.cri
+ ? {
+ cri: version.cri.map(cri => ({
+ ...cri,
+ ...(cri.containerRuntimes
+ ? {
+ containerRuntimes: cri.containerRuntimes.map(runtime => ({ ...runtime })),
+ }
+ : {}),
+ })),
+ }
+ : {}),
+ ...(version.architectures ? { architectures: [...version.architectures] } : {}),
+ })),
+}))
+
const createCloudProfile = ({
metadataName,
displayName,
type,
seedNames,
providerConfig,
machineTypes,
regions,
volumeTypes,
- kubernetes = { versions: [...kubernetesVersions] },
- machineImagesProfile = [...machineImages],
+ kubernetes = cloneKubernetes({ versions: kubernetesVersions }),
+ machineImagesProfile = cloneMachineImages(machineImages),
}) => ({📝 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
| const createCloudProfile = ({ | |
| metadataName, | |
| displayName, | |
| type, | |
| seedNames, | |
| providerConfig, | |
| machineTypes, | |
| regions, | |
| volumeTypes, | |
| kubernetes = { versions: [...kubernetesVersions] }, | |
| machineImagesProfile = [...machineImages], | |
| }) => ({ | |
| const cloneKubernetes = ({ versions }) => ({ | |
| versions: versions.map(version => ({ ...version })), | |
| }) | |
| const cloneMachineImages = images => images.map(image => ({ | |
| ...image, | |
| versions: image.versions.map(version => ({ | |
| ...version, | |
| ...(version.cri | |
| ? { | |
| cri: version.cri.map(cri => ({ | |
| ...cri, | |
| ...(cri.containerRuntimes | |
| ? { | |
| containerRuntimes: cri.containerRuntimes.map(runtime => ({ ...runtime })), | |
| } | |
| : {}), | |
| })), | |
| } | |
| : {}), | |
| ...(version.architectures ? { architectures: [...version.architectures] } : {}), | |
| })), | |
| })) | |
| const createCloudProfile = ({ | |
| metadataName, | |
| displayName, | |
| type, | |
| seedNames, | |
| providerConfig, | |
| machineTypes, | |
| regions, | |
| volumeTypes, | |
| kubernetes = cloneKubernetes({ versions: kubernetesVersions }), | |
| machineImagesProfile = cloneMachineImages(machineImages), | |
| }) => ({ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/__fixtures__/cloudprofiles.js` around lines 63 - 74, The default
kubernetes and machine image defaults are only shallow-copied, causing nested
version/image objects to be shared; in createCloudProfile update the kubernetes
and machineImagesProfile parameter defaults to deep-clone the fixtures (e.g.,
deep clone kubernetesVersions and machineImages) so each profile gets its own
nested objects—use structuredClone(kubernetesVersions) or a fallback deep-clone
(JSON.parse(JSON.stringify(...)) or _.cloneDeep) when assigning the default
values inside createCloudProfile to avoid shared mutable state.
Member
Author
There was a problem hiding this comment.
right now this is not an issue in our test suite. I think it can stay like this for now as we dont mutate these things
There was a problem hiding this comment.
@grolu, understood — if the fixtures are treated as read-only throughout the test suite, the shallow spread is perfectly fine. I'll note this for future reviews.
✏️ Learnings added
Learnt from: grolu
Repo: gardener/dashboard PR: 2824
File: frontend/__fixtures__/cloudprofiles.js:63-74
Timestamp: 2026-03-11T17:51:28.917Z
Learning: In `frontend/__fixtures__/cloudprofiles.js` (gardener/dashboard), the `createCloudProfile` factory uses shallow spreads for `kubernetes` and `machineImagesProfile` defaults (`[...kubernetesVersions]` / `[...machineImages]`). This is intentional — the fixture objects are treated as read-only in the test suite, so deep-cloning is not required and should not be flagged.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Comment on lines
+56
to
+60
| projectID: { | ||
| type: 'hasObjectProp', | ||
| key: 'project_id', | ||
| pattern: /^[a-z][a-z0-9-]{4,28}[a-z0-9]+$/, | ||
| }, |
There was a problem hiding this comment.
Tighten the project_id regex.
Line 59 ends with [a-z0-9]+, so the {4,28} middle segment no longer caps the overall length. Overlong project IDs will pass here, and google-clouddns inherits the same validator via gcp.secret.fields.
🐛 Proposed fix
- pattern: /^[a-z][a-z0-9-]{4,28}[a-z0-9]+$/,
+ pattern: /^[a-z][a-z0-9-]{4,28}[a-z0-9]$/,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/data/vendors/infra/gcp.js` around lines 56 - 60, The projectID
validator uses pattern /^[a-z][a-z0-9-]{4,28}[a-z0-9]+$/, where the trailing +
lets IDs exceed the intended max length; update the regex in the projectID field
(key: 'project_id' in gcp.js) to require exactly one final alphanumeric char so
the total length is capped (e.g. change the trailing [a-z0-9]+ to [a-z0-9]),
i.e. use /^[a-z][a-z0-9-]{4,28}[a-z0-9]$/ so gcp.secret.fields and any consumers
like google-clouddns inherit the tightened validation.
Comment on lines
83
to
85
| export function getKubernetesTemplate (providerType) { | ||
| switch (providerType) { | ||
| case 'metal': | ||
| return { | ||
| kubeControllerManager: { | ||
| nodeCIDRMaskSize: 23, | ||
| }, | ||
| kubelet: { | ||
| maxPods: 510, | ||
| }, | ||
| } | ||
| } | ||
| return resolveShootVendor(providerType)?.shoot?.templates?.kubernetes | ||
| } |
There was a problem hiding this comment.
Return clones here, not live vendor config objects.
Unlike getProviderTemplate and getNetworkingTemplate, Lines 84 and 228 hand back the registry object by reference. Any caller that mutates the returned template/provider config will mutate the global vendor defaults and leak state into later shoot creations.
🐛 Proposed fix
+import cloneDeep from 'lodash/cloneDeep'
+
export function getKubernetesTemplate (providerType) {
- return resolveShootVendor(providerType)?.shoot?.templates?.kubernetes
+ return cloneDeep(resolveShootVendor(providerType)?.shoot?.templates?.kubernetes)
}
@@
export function getWorkerProviderConfig (providerType) {
- return resolveShootVendor(providerType)?.shoot?.workerProviderConfig
+ return cloneDeep(resolveShootVendor(providerType)?.shoot?.workerProviderConfig)
}Also applies to: 227-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/utils/shoot.js` around lines 83 - 85, getKubernetesTemplate
currently returns the vendor template object by reference (via
resolveShootVendor(providerType)?.shoot?.templates?.kubernetes), which allows
callers to mutate global vendor defaults; change getKubernetesTemplate to return
a deep clone of the template (null-safe) instead of the original object (use
structuredClone if available or JSON.parse(JSON.stringify(...)) as a fallback)
and apply the same cloning fix to the analogous returns in getProviderTemplate
and getNetworkingTemplate so no live vendor config objects are leaked.
Member
Author
There was a problem hiding this comment.
yeah I know but this is current limitation AFAIK that we have to set here for this provider 🤷
There was a problem hiding this comment.
@grolu, understood — thanks for the context! Would you like me to open a follow-up GitHub issue to track this so it doesn't get lost? That way the mutation risk in getKubernetesTemplate and getWorkerProviderConfig can be addressed in a future PR when the limitation is lifted.
🧠 Learnings used
Learnt from: grolu
Repo: gardener/dashboard PR: 2824
File: frontend/__fixtures__/cloudprofiles.js:63-74
Timestamp: 2026-03-11T17:51:28.917Z
Learning: In `frontend/__fixtures__/cloudprofiles.js` (gardener/dashboard), the `createCloudProfile` factory uses shallow spreads for `kubernetes` and `machineImagesProfile` defaults (`[...kubernetesVersions]` / `[...machineImages]`). This is intentional — the fixture objects are treated as read-only in the test suite, so deep-cloning is not required and should not be flagged.
Fixed OpenStack Secret Dialog Defaulting Default autocomplete to 'off' for password fields FIx object references in cloudprofiles fixtures Some Cleanup (mainly comments) Fixed some text=>password field type errors
gardener-prow
bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions 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. |
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
1 participant
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:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor