Skip to content

Extend the defaults accepted via configuration#2476

Open
benedikt-haug wants to merge 37 commits into
gardener:masterfrom
benedikt-haug:feature_add_dashboard_defaults
Open

Extend the defaults accepted via configuration#2476
benedikt-haug wants to merge 37 commits into
gardener:masterfrom
benedikt-haug:feature_add_dashboard_defaults

Conversation

@benedikt-haug

@benedikt-haug benedikt-haug commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
This PR is part of the Hackaton.
This PR extend the defaults gardener dashboard accepts via configuration.
Additionally, it moves the default declaration to the config store so they are declared in the same spot.
Lastly, it also adds the example folder describing the new additions made possible by the gardener-dashboard-frontend configmap.

Which issue(s) this PR fixes:

Special notes for your reviewer:
/cc @marc1404 @klocke-io

Release note:

**New `shootDefaults` configuration introduced:** A new top-level configuration property, `shootDefaults`, has been added. Existing shoot default settings have been moved under this new namespace:

- `controlPlaneHighAvailabilityHelp` → `shootDefaults.controlPlaneHighAvailabilityHelp`
- `defaultHibernationSchedule` → `shootDefaults.hibernationSchedule`
- `defaultNodesCIDR` → `shootDefaults.nodesCIDR`

The previous configuration locations are now deprecated. Please update your deployments to use the new `shootDefaults` configuration paths.
Improved shoot creation defaults: The Gardener Dashboard now allows predefined default values when creating new shoot resources

Summary by CodeRabbit

  • New Features
    • Added unified shootDefaults support for hibernation/maintenance, autoscaling, workerless defaults, container runtime, purposes, networking, and help text, while keeping legacy compatibility.
    • Updated dashboard ConfigMap templating to emit shootDefaults and validate controlPlaneHighAvailabilityHelp.text.
  • Bug Fixes
    • Improved sanitization of help text to remove unsafe HTML/script and preserve expected legacy/new behavior.
    • Refined wildcard/custom-wildcard selection state and emitted update events.
    • Fixed conditional validation and field wiring for provider-specific infrastructure inputs (including workerless behavior).
  • Documentation
    • Refreshed examples and local setup guidance for the new shootDefaults structure.
  • Tests
    • Expanded coverage for sanitization, default precedence, templating validation, and composable/component behaviors.

@ghost ghost added the needs/review Needs review label Jun 4, 2025
@gardener-robot-ci-2

Copy link
Copy Markdown
Contributor

Thank you @benedikt-haug for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

@ghost

ghost commented Jun 4, 2025

Copy link
Copy Markdown

@benedikt-haug Thank you for your contribution.

@ghost ghost added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 4, 2025
@klocke-io

Copy link
Copy Markdown
Member

/ok-to-test

@ghost ghost added the needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 4, 2025
@grolu

grolu commented Jun 4, 2025

Copy link
Copy Markdown
Member

Nice!
Please also have a look at this PR: #2467
Especially the defaultInfrastructures configuration may cause conflicts.

@klocke-io klocke-io added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 4, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 4, 2025
@grolu

grolu commented Jun 4, 2025

Copy link
Copy Markdown
Member

Maybe it would be better to group the default options under a key in the config, e.g., shootDefaults. This way, we can ensure the config remains clear and all shoot default settings are grouped together. These settings are also likely to grow, so this approach gives us better control.

Additionally, these values need to be added to the Helm charts. Right now, the Helm charts are the only documentation for the dashboard configuration. We should consider introducing a dedicated documentation page for the configuration — but that’s a separate task and not related to this PR.

@klocke-io klocke-io added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 4, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 4, 2025
@ghost ghost added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs/second-opinion Needs second review by someone else and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 4, 2025
@benedikt-haug

benedikt-haug commented Jun 4, 2025

Copy link
Copy Markdown
Contributor Author

Nice! Please also have a look at this PR: #2467 Especially the defaultInfrastructures configuration may cause conflicts.

Sure <3
On quick glance it looks like all it would take to allow user overrides for your new knownInfraVendors array would be to move the defaulting function to store/config.js, just like this PR does for all the other defaults.

Maybe it would be better to group the default options under a key in the config, e.g., shootDefaults. This way, we can ensure the config remains clear and all shoot default settings are grouped together. These settings are also likely to grow, so this approach gives us better control.

Seems reasonable :) Incorporated that feedback. Also moved the "old" variables there, while including a fallback for the users of the old naming scheme so things don't suddenly break.

Additionally, these values need to be added to the Helm charts. Right now, the Helm charts are the only documentation for the dashboard configuration. We should consider introducing a dedicated documentation page for the configuration — but that’s a separate task and not related to this PR.

Added an example like the other repos have it. Also tried to extend the helm charts to feature the new variable. Do you know how to test the charts locally, helm template . ain't working for me?

@grolu

grolu commented Jun 5, 2025

Copy link
Copy Markdown
Member

Do you know how to test the charts locally, helm template . ain't working for me?

We implemented tests for the help charts. You can adapt them and run them with yarn test in the charts directory

Comment thread frontend/src/store/config.js Outdated
@klocke-io klocke-io added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 6, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 6, 2025
@ghost

ghost commented Jun 7, 2025

Copy link
Copy Markdown

@holgerkoser, @klocke-io You have pull request review open invite, please check

@grolu
grolu force-pushed the feature_add_dashboard_defaults branch from 92e832a to 07d8038 Compare July 17, 2026 16:06
@grolu
grolu requested a review from a team as a code owner July 17, 2026 16:06
@gardener-prow gardener-prow Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dd5fca88-f08d-4456-b01c-d82bb2bc9c1c

📥 Commits

Reviewing files that changed from the base of the PR and between 8ae368a and 71537a8.

📒 Files selected for processing (2)
  • frontend/__tests__/components/GNewShootInfrastructureDetails.spec.js
  • frontend/src/components/NewShoot/GNewShootInfrastructureDetails.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/src/components/NewShoot/GNewShootInfrastructureDetails.vue

📝 Walkthrough

Walkthrough

The change consolidates frontend shoot defaults under shootDefaults, applies configurable values during shoot creation, updates workerless and wildcard behavior, and adds sanitization coverage for legacy and nested high-availability help text.

Changes

Shoot defaults and configuration

Layer / File(s) Summary
Nested shoot defaults configuration
charts/gardener-dashboard/..., charts/__tests__/..., docs/..., example/...
Chart templates, compatibility mappings, tests, and examples now use nested frontend.shootDefaults values.
Config store default resolution
frontend/src/store/config.js, frontend/src/store/gardenerExtension.js, frontend/src/composables/useShootHelper.js, frontend/src/utils/index.js, frontend/__tests__/stores/*, frontend/__tests__/utils/*
Configurable default getters, sorted networking types, purpose selection, and maintenance-hour fallback behavior are added.
Shoot creation default application
frontend/src/composables/useShootContext.js, frontend/__tests__/composables/useShootContext.spec.js
Shoot creation applies configured workerless, networking, provider, maintenance, high-availability, runtime, zone, and autoscaler defaults.
Workerless and wildcard UI behavior
frontend/src/components/NewShoot/GNewShootInfrastructureDetails.vue, frontend/src/components/GWildcardSelect.vue, related tests
Workerless mode hides and relaxes provider infrastructure inputs, while custom wildcard selection records prefix state and emits the selected value.
Frontend configuration sanitization
backend/lib/routes/config.js, backend/__tests__/routes.config.spec.js
Legacy and nested high-availability help text is sanitized, with injection-removal and immutability tests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested labels: kind/enhancement, area/ipcei, approved, lgtm

Suggested reviewers: holgerkoser, klocke-io, petersutter

Sequence Diagram(s)

sequenceDiagram
  participant FrontendConfig
  participant useConfigStore
  participant gardenerExtensionStore
  participant useShootContext
  FrontendConfig->>useConfigStore: load shootDefaults
  useConfigStore->>gardenerExtensionStore: provide defaultNetworkingType
  useConfigStore->>useShootContext: provide configured defaults
  gardenerExtensionStore->>useShootContext: provide sortedNetworkingTypes
  useShootContext->>useShootContext: createShootManifest
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: expanding configuration-accepted defaults.
Description check ✅ Passed The description covers the purpose, reviewer notes, and release note, but it omits the template's categorization block.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/utils/index.js`:
- Around line 521-525: Update randomMaintenanceBegin to handle an explicitly
empty hours array before calling sample, returning the established safe
maintenance-window fallback instead of interpolating undefined. Preserve the
current random selection and formatting behavior when hours contains values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 16e32087-8a69-494b-8cbe-90ee6484be5c

📥 Commits

Reviewing files that changed from the base of the PR and between e467be2 and 07d8038.

⛔ Files ignored due to path filters (1)
  • charts/__tests__/gardener-dashboard/runtime/dashboard/__snapshots__/configmap.spec.js.snap is excluded by !**/*.snap
📒 Files selected for processing (17)
  • backend/__tests__/routes.config.spec.js
  • backend/lib/routes/config.js
  • charts/__tests__/gardener-dashboard/runtime/dashboard/configmap.spec.js
  • charts/gardener-dashboard/charts/runtime/templates/dashboard/configmap.yaml
  • charts/gardener-dashboard/values.yaml
  • docs/development/local-setup.md
  • example/00-configmap-gardener-dashboard-frontend.yaml
  • frontend/__tests__/components/GWildcardSelect.spec.js
  • frontend/__tests__/composables/useShootContext.spec.js
  • frontend/__tests__/stores/config.spec.js
  • frontend/src/components/GWildcardSelect.vue
  • frontend/src/components/NewShoot/GNewShootInfrastructureDetails.vue
  • frontend/src/composables/useShootContext.js
  • frontend/src/composables/useShootHelper.js
  • frontend/src/store/config.js
  • frontend/src/store/gardenerExtension.js
  • frontend/src/utils/index.js

Comment thread frontend/src/utils/index.js Outdated
@gardener-prow gardener-prow Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/rebase Needs git rebase needs/review Needs review needs/second-opinion Needs second review by someone else size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/accepted Issue was accepted as something we need to work on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants