bugfix/fix-nil-pointer #643
Conversation
dosubot
bot
added
size:M
WalkthroughThis pull request removes the dashboard image configuration from both the paas-full and paas-hosted bundles, updates the logic for setting the dashboard’s locale and style using a new branding variable, and removes the customStyle property from the dashboard values. Additionally, the keycloak configuration has been updated to introduce a new variable for branding and modify related display fields in the realm specification. Changes
Sequence Diagram(s)sequenceDiagram
participant D as Dashboard Template
participant TE as Template Engine
participant CM as ConfigMap (cozystack-branding)
D->>TE: Render dashboard configuration
TE->>CM: Lookup branding information
CM-->>TE: Return branding data
TE->>D: Update customLocale and customStyle with branding
sequenceDiagram
participant KT as Keycloak Template
participant TE as Template Engine
participant CM as ConfigMap (cozystack-branding)
KT->>TE: Render keycloak configuration
TE->>CM: Retrieve branding details
CM-->>TE: Return branding info
TE->>KT: Populate display fields using branding
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/system/keycloak-configure/templates/configure-kk.yaml (1)
33-36: Conditional Locale Assignment and Indentation WarningInitializing
$localewith an empty string and then conditionally assigning it using the$wlConfigmapis an effective safeguard against nil pointer exceptions. However, YAMLlint has flagged an indentation warning on line 35. Consider adjusting the indentation of this template directive to align with the YAML formatting expectations, ensuring that linting tools do not raise unnecessary warnings.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 35-35: wrong indentation: expected 0 but found 2
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/dashboard/values.yaml(0 hunks)packages/system/keycloak-configure/templates/configure-kk.yaml(3 hunks)
💤 Files with no reviewable changes (3)
- packages/system/dashboard/values.yaml
- packages/core/platform/bundles/paas-full.yaml
- packages/core/platform/bundles/paas-hosted.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/keycloak-configure/templates/configure-kk.yaml
[warning] 35-35: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (2)
packages/system/keycloak-configure/templates/configure-kk.yaml (2)
10-10: Addition of $wlConfigmap VariableThe new variable
$wlConfigmapretrieves the "white-label" ConfigMap from the "cozy-dashboard" namespace. This retrieval is key to conditionally determining the locale later in the template, which helps avoid nil pointer issues when the ConfigMap is missing.
91-94: Updated Display Name Fields in ClusterKeycloakRealmThe conditional block that adds the new
displayHtmlNameanddisplayNamefields based on the value of$localeis well implemented. Ensure that any downstream components referencing the old field (displayNameHtml) are updated accordingly so that the renaming does not introduce integration issues.
klinch0
force-pushed
the
bugfix/fix-wl
branch
from
180834e to
b393235
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/apps/tenant/templates/quota.yaml (1)
1-1: YAMLlint Warning: Likely a False Positive Due to Helm Templating
The linter complains about a syntax error on line 1, but this is expected since Helm templates use special syntax (e.g.{{- if .Values.resources }}). If this warning becomes noisy in CI, consider adding a YAML lint disable comment or configuring the linter to ignore Helm template directives.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/keycloak-configure/templates/configure-kk.yaml (1)
33-36: Conditional Locale Assignment: Protecting Against Nil Pointers
The snippet initializes$localeas an empty string and then conditionally assigns it from$wlConfigmap.dataonly if$wlConfigmapexists. This prevents nil pointer dereferences when the ConfigMap is missing.
Note: The extra indentation on line 35 causes a YAMLlint warning ("wrong indentation: expected 0 but found 2"). Since this is part of Helm templating and does not affect the rendered output, you may either ignore it or adjust the spacing if your linting setup allows.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 35-35: wrong indentation: expected 0 but found 2
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/apps/tenant/Chart.yaml(1 hunks)packages/apps/tenant/README.md(1 hunks)packages/apps/tenant/templates/quota.yaml(1 hunks)packages/apps/tenant/values.schema.json(1 hunks)packages/apps/tenant/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)packages/core/platform/bundles/paas-full.yaml(0 hunks)packages/core/platform/bundles/paas-hosted.yaml(0 hunks)packages/system/dashboard/values.yaml(0 hunks)packages/system/keycloak-configure/templates/configure-kk.yaml(3 hunks)
💤 Files with no reviewable changes (3)
- packages/system/dashboard/values.yaml
- packages/core/platform/bundles/paas-hosted.yaml
- packages/core/platform/bundles/paas-full.yaml
✅ Files skipped from review due to trivial changes (1)
- packages/apps/tenant/Chart.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/apps/tenant/templates/quota.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/keycloak-configure/templates/configure-kk.yaml
[warning] 35-35: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (8)
packages/apps/tenant/README.md (1)
61-61: Well-documented parameter!The new
resourcesparameter is properly documented in the README with a clear description and default value. This addition is consistent with the formatting of other parameters in the table.packages/apps/tenant/values.schema.json (1)
34-39: Proper schema definition for the new resources parameter!The schema definition for the
resourcesparameter is correctly implemented as an object type with an appropriate description and default value. This matches the implementation in values.yaml and the documentation in README.md.packages/apps/tenant/values.yaml (2)
9-9: Parameter documentation follows conventions!The documentation format for the new
resourcesparameter follows the established pattern of other parameters in the file.
16-23: Great implementation with helpful examples!The
resourcesparameter is correctly initialized with an empty object as the default value, and the commented examples provide clear guidance on how to structure CPU and memory limits/requests. This helps prevent nil pointer errors when the configuration is accessed.packages/apps/versions_map (1)
107-108: Version mapping updated appropriately!The version mapping has been correctly updated to lock version 1.7.0 to a specific commit hash and introduce the new 1.7.1 version pointing to HEAD. This follows the established pattern in the file and allows for stable references while development continues.
packages/apps/tenant/templates/quota.yaml (1)
1-17: ResourceQuota Template: Correct Conditional Rendering
The template cleanly checks for the existence of.Values.resourcesand further nests conditionals for resource requests and limits. This design provides flexible configuration for tenant quotas. The structure is clear and aligns well with Helm templating practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/keycloak-configure/templates/configure-kk.yaml (2)
10-10: White-Label ConfigMap Retrieval: Preventing Nil Pointer Dereference
The new variable assignment usinglookupto fetch thewhite-labelConfigMap ensures that subsequent references (such as when assigning$locale) don’t attempt to index a nil object. This is an effective measure to fix potential nil pointer issues.
91-94: Updated Keycloak Realm Display Fields: Consistent with Locale Configuration
Changing fromdisplayNameHtmltodisplayHtmlNameand adding a new fielddisplayName(both using the$localevariable) is a solid update. This conditional inclusion ensures that these display fields are only set when a locale value is provided.
klinch0
force-pushed
the
bugfix/fix-wl
branch
from
b393235 to
5fdcfc6
Compare
klinch0
force-pushed
the
bugfix/fix-wl
branch
from
5fdcfc6 to
d0d62e8
Compare
dosubot
bot
added
size:XS
dosubot
bot
added
size:M
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/system/keycloak-configure/templates/configure-kk.yaml (1)
33-36: Initialize Branding Safely & Adjust Indentation
Here the variable$brandingis initialized with an empty string and conditionally set if$cozystackBrandingexists. This prevents nil pointer dereferences when accessing the config’s data.Note: YAML lint flagged line 35 for unexpected indentation. Consider aligning the action with the surrounding template code for consistency. For example:
- {{- $branding = index $cozystackBranding.data "branding" }} +{{- $branding = index $cozystackBranding.data "branding" }}🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 35-35: wrong indentation: expected 0 but found 2
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/platform/bundles/paas-full.yaml(1 hunks)packages/core/platform/bundles/paas-hosted.yaml(1 hunks)packages/system/dashboard/values.yaml(0 hunks)packages/system/keycloak-configure/templates/configure-kk.yaml(3 hunks)
💤 Files with no reviewable changes (1)
- packages/system/dashboard/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/keycloak-configure/templates/configure-kk.yaml
[warning] 35-35: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (6)
packages/system/keycloak-configure/templates/configure-kk.yaml (2)
10-10: Introducing the Branding ConfigMap Lookup
The new variable$cozystackBrandingis correctly looked up from the "cozy-system" namespace. The downstream nil-checks (in the block below) help avoid nil pointer issues.
92-95: Apply Branding to Keycloak Realm Spec
The conditional block applies$brandingto bothdisplayHtmlNameanddisplayNamein theClusterKeycloakRealmspec. This ensures that if branding is available it will be used to update the display fields and prevents a nil pointer when absent.packages/core/platform/bundles/paas-full.yaml (2)
249-254: Integrate Dashboard Branding for Custom Locale
This block retrieves the branding configuration using thelookupanddigfunctions and conditionally applies it to thecustomLocalefield. The use ofdigwith a default value protects against nil pointers if the ConfigMap does not exist.Example rendering:
customLocale: "Kubeapps": <branding_value>
255-261: Set Custom Style with Dynamic Logo Image
The updated customStyle block conditionally inserts a CSS rule for the.kubeapps-logobased on the presence of a logo image in the branding ConfigMap. This approach helps avoid nil pointer issues and ensures that the style is applied only when available.packages/core/platform/bundles/paas-hosted.yaml (2)
172-177: Consistent Branding Configuration for Hosted Dashboard
The branding lookup and conditional injection intocustomLocalemirrors the approach used inpaas-full.yaml. Usingdigensures that if the key is missing, a default empty string is returned, thus preventing nil pointer dereferences.This consistency across bundles is key for maintainability.
178-184: Apply Custom Style Dynamically with Logo Extraction
The customStyle block is updated to include the.kubeapps-logostyle only when a logo image exists in the branding ConfigMap. This conditional injection is both safe and efficient.
Summary by CodeRabbit
New Features
Style