-
Notifications
You must be signed in to change notification settings - Fork 69
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
feature/add-wl #632
feature/add-wl #632
Conversation
WalkthroughThis pull request updates package version strings and enhances configuration files. In the versions map, the commit hash for both the Changes
Sequence Diagram(s)sequenceDiagram
participant Configurator
participant ConfigMap as "cozy-dashboard ConfigMap"
participant KeycloakRealm
Configurator->>ConfigMap: Retrieve "locale" value
ConfigMap-->>Configurator: Return locale (if exists)
alt Locale defined
Configurator->>KeycloakRealm: Set displayNameHtml with locale value
else Locale missing
Configurator->>KeycloakRealm: Skip displayNameHtml configuration
end
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
5b1869a
to
6cd915b
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/system/keycloak-configure/templates/configure-kk.yaml (1)
32-33
: Add error handling for ConfigMap lookup.The white-label ConfigMap lookup should handle cases where the ConfigMap doesn't exist to prevent template rendering failures.
-{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} -{{- $locale := index $wlConfigmap.data "locale" }} +{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} +{{- $locale := "" }} +{{- if and $wlConfigmap $wlConfigmap.data }} + {{- $locale = index $wlConfigmap.data "locale" | default "" }} +{{- end }}packages/core/platform/bundles/paas-hosted.yaml (1)
183-211
: Optimize CSS selectors for better maintainability.The CSS customizations effectively hide UI elements, but consider using more specific selectors or CSS custom properties for better maintainability.
customStyle: | {{- $logoImage := dig "data" "logo" "" $wlConfigmap }} {{- if $logoImage }} .kubeapps-logo { background-image: {{ $logoImage }} } {{- end }} + /* Group related elements under a common parent selector */ + .kubeapps-dashboard { #serviceaccount-selector, .login-moreinfo, a[href="#/docs"], .login-group .clr-form-control .clr-control-label, .appview-separator div.appview-first-row div.center, .appview-separator div.appview-first-row section[aria-labelledby="app-secrets"] { display: none; } + } .appview-first-row section[aria-labelledby="access-urls-title"] { width: 100%; }packages/core/platform/bundles/paas-full.yaml (1)
248-287
: Consider extracting shared dashboard configuration.The dashboard configuration is duplicated between paas-hosted.yaml and paas-full.yaml. Consider extracting it into a shared template or values file.
Create a new file
_dashboard-shared.tpl
in the templates directory:{{- define "dashboard.config" -}} dashboard: image: registry: ghcr.io/aenix-io/cozystack repository: dashboard tag: v0.25.0 digest: "sha256:81e7b625c667bce5fc339eb97c8e115eafb82f66df4501550b3677ac53f6e234" {{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} {{- $locale := dig "data" "locale" "" $wlConfigmap }} {{- if $locale }} customLocale: "Kubeapps": {{ $locale }} {{- end }} customStyle: | {{- $logoImage := dig "data" "logo" "" $wlConfigmap }} {{- if $logoImage }} .kubeapps-logo { background-image: {{ $logoImage }} } {{- end }} /* Rest of the CSS */ {{- end -}}Then in both files:
values: kubeapps: - dashboard: - # ... duplicated config + {{- include "dashboard.config" . | nindent 4 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/apps/versions_map
(1 hunks)packages/core/platform/bundles/paas-full.yaml
(1 hunks)packages/core/platform/bundles/paas-hosted.yaml
(2 hunks)packages/system/dashboard/values.yaml
(0 hunks)packages/system/keycloak-configure/templates/configure-kk.yaml
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/system/dashboard/values.yaml
✅ Files skipped from review due to trivial changes (1)
- packages/apps/versions_map
🔇 Additional comments (1)
packages/system/keycloak-configure/templates/configure-kk.yaml (1)
89-91
: LGTM! Well-implemented white-label support.The conditional block for setting the realm's display name based on locale is clean and follows best practices.
dashboard: | ||
image: | ||
registry: ghcr.io/aenix-io/cozystack | ||
repository: dashboard | ||
tag: v0.25.0 | ||
digest: "sha256:81e7b625c667bce5fc339eb97c8e115eafb82f66df4501550b3677ac53f6e234" | ||
{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} |
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.
💡 Verification agent
🧩 Analysis chain
Verify the image digest for security.
The dashboard image is properly configured with a digest for immutability. However, let's verify the digest against the official repository.
🏁 Script executed:
#!/bin/bash
# Description: Verify the dashboard image digest
curl -s -H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
-H "Authorization: Bearer $(curl -s "https://ghcr.io/token?service=ghcr.io&scope=repository:aenix-io/cozystack/dashboard:pull" | jq -r .token)" \
https://ghcr.io/v2/aenix-io/cozystack/dashboard/manifests/v0.25.0 | jq -r '.config.digest'
Length of output: 450
Mismatch Found: Update the Dashboard Image Digest
The digest retrieved from the official repository for the dashboard image tag v0.25.0
is:
sha256:6ff65b7407e0ee4524df1591c0dc185fcaf52582bcc3d246540e380822a46d69
However, the file packages/core/platform/bundles/paas-hosted.yaml
currently specifies:
sha256:81e7b625c667bce5fc339eb97c8e115eafb82f66df4501550b3677ac53f6e234
Please update the digest in the YAML file to match the verified official digest.
- Location:
packages/core/platform/bundles/paas-hosted.yaml
(lines 171-177) - Action Required: Replace the current digest with
sha256:6ff65b7407e0ee4524df1591c0dc185fcaf52582bcc3d246540e380822a46d69
.
6cd915b
to
42f1dab
Compare
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.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/core/platform/bundles/paas-hosted.yaml (1)
171-176
:⚠️ Potential issueUpdate the dashboard image digest.
The image digest needs to be updated to match the official repository.
packages/core/platform/bundles/paas-full.yaml (2)
248-254
:⚠️ Potential issueUpdate the dashboard image digest.
The image digest needs to be updated to match the official repository.
254-259
: 🛠️ Refactor suggestionAdd error handling for ConfigMap lookups.
Both the white-label and auth config ConfigMap lookups lack error handling, similar to paas-hosted.yaml.
Also applies to: 288-295
🧹 Nitpick comments (1)
packages/core/platform/bundles/paas-hosted.yaml (1)
183-210
: Add documentation for CSS customizations.The CSS customizations lack documentation explaining the purpose of hiding each UI element.
Add comments to explain the purpose:
customStyle: | {{- $logoImage := dig "data" "logo" "" $wlConfigmap }} {{- if $logoImage }} .kubeapps-logo { background-image: {{ $logoImage }} } {{- end }} + /* Hide service account selector for security */ #serviceaccount-selector { display: none; } + /* Remove login page extra information */ .login-moreinfo { display: none; } + /* Hide documentation links */ a[href="#/docs"] { display: none; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/apps/versions_map
(1 hunks)packages/core/platform/bundles/paas-full.yaml
(1 hunks)packages/core/platform/bundles/paas-hosted.yaml
(2 hunks)packages/system/keycloak-configure/templates/configure-kk.yaml
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/apps/versions_map
- packages/system/keycloak-configure/templates/configure-kk.yaml
🔇 Additional comments (1)
packages/core/platform/bundles/paas-full.yaml (1)
260-287
: Add documentation for CSS customizations.The CSS customizations lack documentation explaining the purpose of hiding each UI element, similar to paas-hosted.yaml.
{{- $dashboardKCconfig := lookup "v1" "ConfigMap" "cozy-dashboard" "kubeapps-auth-config" }} | ||
{{- $dashboardKCValues := dig "data" "values.yaml" "" $dashboardKCconfig }} | ||
{{- if $dashboardKCValues }} | ||
valuesFrom: | ||
- kind: ConfigMap | ||
name: kubeapps-auth-config | ||
valuesKey: values.yaml | ||
{{- end }} |
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.
🛠️ Refactor suggestion
Add error handling for auth config ConfigMap.
Similar to the white-label ConfigMap, the auth config ConfigMap lookup lacks error handling.
Add error handling:
{{- $dashboardKCconfig := lookup "v1" "ConfigMap" "cozy-dashboard" "kubeapps-auth-config" }}
+{{- if not $dashboardKCconfig }}
+ {{- fail "ERROR: kubeapps-auth-config ConfigMap not found in cozy-dashboard namespace" }}
+{{- end }}
{{- $dashboardKCValues := dig "data" "values.yaml" "" $dashboardKCconfig }}
📝 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.
{{- $dashboardKCconfig := lookup "v1" "ConfigMap" "cozy-dashboard" "kubeapps-auth-config" }} | |
{{- $dashboardKCValues := dig "data" "values.yaml" "" $dashboardKCconfig }} | |
{{- if $dashboardKCValues }} | |
valuesFrom: | |
- kind: ConfigMap | |
name: kubeapps-auth-config | |
valuesKey: values.yaml | |
{{- end }} | |
{{- $dashboardKCconfig := lookup "v1" "ConfigMap" "cozy-dashboard" "kubeapps-auth-config" }} | |
{{- if not $dashboardKCconfig }} | |
{{- fail "ERROR: kubeapps-auth-config ConfigMap not found in cozy-dashboard namespace" }} | |
{{- end }} | |
{{- $dashboardKCValues := dig "data" "values.yaml" "" $dashboardKCconfig }} | |
{{- if $dashboardKCValues }} | |
valuesFrom: | |
- kind: ConfigMap | |
name: kubeapps-auth-config | |
valuesKey: values.yaml | |
{{- end }} |
{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} | ||
{{- $locale := dig "data" "locale" "" $wlConfigmap }} | ||
{{- if $locale }} | ||
customLocale: | ||
"Kubeapps": {{ $locale }} | ||
{{- end }} |
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.
🛠️ Refactor suggestion
Add error handling for ConfigMap lookup.
The white-label ConfigMap lookup lacks error handling. If the ConfigMap doesn't exist, the template will continue silently.
Add error handling:
{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }}
+{{- if not $wlConfigmap }}
+ {{- fail "ERROR: white-label ConfigMap not found in cozy-dashboard namespace" }}
+{{- end }}
{{- $locale := dig "data" "locale" "" $wlConfigmap }}
📝 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.
{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} | |
{{- $locale := dig "data" "locale" "" $wlConfigmap }} | |
{{- if $locale }} | |
customLocale: | |
"Kubeapps": {{ $locale }} | |
{{- end }} | |
{{- $wlConfigmap := lookup "v1" "ConfigMap" "cozy-dashboard" "white-label" }} | |
{{- if not $wlConfigmap }} | |
{{- fail "ERROR: white-label ConfigMap not found in cozy-dashboard namespace" }} | |
{{- end }} | |
{{- $locale := dig "data" "locale" "" $wlConfigmap }} | |
{{- if $locale }} | |
customLocale: | |
"Kubeapps": {{ $locale }} | |
{{- end }} |
Summary by CodeRabbit
Chores
New Features