-
Notifications
You must be signed in to change notification settings - Fork 112
Extend the defaults accepted via configuration #2476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Extend the defaults accepted via configuration #2476
Conversation
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. |
@benedikt-haug Thank you for your contribution. |
/ok-to-test |
Nice! |
Maybe it would be better to group the default options under a key in the config, e.g., 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. |
Sure <3
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.
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, |
We implemented tests for the help charts. You can adapt them and run them with |
@@ -230,19 +230,19 @@ export const useConfigStore = defineStore('config', () => { | |||
}) | |||
|
|||
const controlPlaneHighAvailabilityHelp = computed(() => { | |||
return state.value?.shootDefaults.controlPlaneHighAvailabilityHelp ?? state.value?.controlPlaneHighAvailabilityHelp | |||
return state.value?.shootDefaults?.controlPlaneHighAvailabilityHelp ?? state.value?.controlPlaneHighAvailabilityHelp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should introduce a shootDefaults
computed that defaults to {}
to reduce optional chaining.
@holgerkoser, @klocke-io You have pull request review open invite, please check |
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: