Repository navigation
[dashboard] Add branding config - #1460
Conversation
Signed-off-by: Andrei Kvapil <[email protected]>
WalkthroughIntroduces dynamic branding in the dashboard ConfigMap by looking up data from a cozystack-branding ConfigMap and setting template variables with defaults. Updates the Makefile to switch the tenant text update from yq to sed for in-place substitution in the template. No other targets or files are changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Make as Makefile
participant Git as Template Files
participant Helm as Helm Templater
participant K8s as Kubernetes API
rect rgba(240,248,255,0.6)
Note over Dev,Make: Update tenant text during build
Dev->>Make: make update-tenant-text TAG=<value>
Make->>Git: sed in-place<br/>(replace $$tenantText default)
Git-->>Make: Updated template
end
rect rgba(245,255,240,0.6)
Note over Helm,K8s: Render dashboard ConfigMap
Dev->>Helm: helm template/apply
Helm->>K8s: lookup ConfigMap<br/>cozy-system/cozystack-branding
alt Branding exists
K8s-->>Helm: brandingConfig.data{tenantText,footerText,...}
Helm->>Helm: dig with defaults<br/>(tenantText, footerText, titleText,<br/>logoText, logoSvg, iconSvg)
else Branding missing
Helm->>Helm: Use template defaults
end
Helm-->>Dev: Rendered ConfigMap with data fields
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate 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. Comment |
Summary of ChangesHello Andrei Kvapil (@kvaps), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Cozystack Dashboard by introducing a flexible and dynamic branding mechanism. It allows for the customization of key visual elements such as titles, footers, and logos through a Kubernetes ConfigMap. This change provides greater adaptability for different deployment environments or white-labeling requirements, ensuring that the dashboard can reflect specific organizational branding while maintaining a consistent user experience with built-in defaults. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces branding configuration for the dashboard, allowing customization through a cozystack-branding ConfigMap. The changes use Helm templates to provide default values if the ConfigMap is not present. My review identified a critical issue in the Makefile where the sed command for updating the tenant text is incorrect and will not function as intended. Additionally, I've pointed out a medium-severity maintainability concern regarding large, embedded SVG strings in the configmap.yaml template.
|
|
||
| update-tenant-text: | ||
| yq -i '.data.TENANT_TEXT = "$(TAG)"' ./templates/configmap.yaml | ||
| sed -i 's|\($$tenantText := "\)[^"]\+|\1$(TAG)|' templates/configmap.yaml |
There was a problem hiding this comment.
The sed command in the update-tenant-text target is incorrect and will not update the tenantText variable in templates/configmap.yaml. The regular expression does not match the structure of the Helm template file, and the replacement is also malformed. This will cause the tenant text to always be the default 'latest' instead of the provided $(TAG), breaking the intended functionality of this make target.
sed -i 's|\({{- \$$tenantText := "\)[^\"]*"|\1$(TAG)"|' templates/configmap.yaml
| {{- $logoSvg := "PHN2ZyB3aWR0aD0iMTUwIiBoZWlnaHQ9IjMwIiB2aWV3Qm94PSIwIDAgMTUwIDMwIiBmaWxsPSJub25lIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciPgogIDxwYXRoIGQ9Ik0xMzMuMzMxMDkgMjIuMzczOTg3VjQuNzk1Mjc2OWgyLjA0NDUyVjEyLjc3Mzg5aC4wNDk5bDguNTI3NzEtNy45NzkxNjcyaDIuNjE4MjdsLTkuNzc0NjQgOS4xMDExNTgyLjAyNDktMS4wOTcwNTkgMTAuMjk4MjQgOS41NzQ4ODloLTIuNjkyNzlsLTkuMDAxNjktOC4yNzgzNjNoLS4wNDk5djguMjc4MzYzem0tOS4xNzIzNi4yMjQzOTljLTEuNzI4NjkuMC0zLjIwODA1LS4zNjU2ODYtNC40MzgxLTEuMDk3MDU5LTEuMjMwNTktLjczMTM3My0yLjE3ODA0LTEuNzcwMjU0LTIuODQyOTMtMy4xMTY2NDUtLjY0ODI3LTEuMzQ2NjY3LS45NzIzOS0yLjk1MDk3OC0uOTcyMzktNC44MTI2NTQuMC0xLjg2MTY3Ny4zMjQxMi0zLjQ1NzM5OS45NzIzOS00Ljc4NzQ0NS42NjQ4OS0xLjM0NjM5MDYgMS42MTIzNC0yLjM4NTI3MjUgMi44NDI2Ni0zLjExNjkyMjQgMS4yMzAwMy0uNzMxMzcyOSAyLjcwOTQxLTEuMDk3MDU5MiA0LjQzODM3LTEuMDk3MDU5MiAxLjIxMzQyLjAgMi4zMzU0MS4xOTExNTQzIDMuMzY2MjYuNTczNDYyOCAxLjAzMDU3LjM4MjMwODUgMS44OTQ5My45MzkxNDkxIDIuNTkzMDUgMS42NzA1MjE5bC0uNzk3ODYgMS42NzA1MjE5Yy0uNzY0NjItLjcxNDc1LTEuNTYyNDgtMS4yMzAwMzYtMi4zOTM1OS0xLjU0NTg1NjEtLjgxNDQ3LS4zMzI0NDIxLTEuNzIwMzgtLjQ5ODY2MzItMi43MTc3MS0uNDk4NjYzMi0xLjk3ODU5LjAtMy40OTEyLjYyMzMyOS00LjUzODM4IDEuODY5OTg4My0xLjA0NzIxIDEuMjQ2NjU3LTEuNTcwOCAzLjAwMDU2Ni0xLjU3MDggNS4yNjE0NTEuMCAyLjI2MDYwNi41MjM1OSA0LjAyMjU0OSAxLjU3MDggNS4yODYxMDggMS4wNDcxOCAxLjI0NjY1NyAyLjU1OTc5IDEuODY5OTg2IDQuNTM4MDkgMS44Njk5ODYuOTk3MzQuMCAxLjkwMzI0LS4xNTc5MDkgMi43MTc3My0uNDczNzMuODMxMzgtLjMzMjQ0MiAxLjYyOTI0LS44NTYwMzggMi4zOTM4Ni0xLjU3MDc4OWwuNzk3ODYgMS42NzA1MjJjLS42OTgxMi43MTQ3NTEtMS41NjI0OCAxLjI3MTg2OS0yLjU5MzA1IDEuNjcwNzk5LTEuMDMwNTguMzgyMzA5LTIuMTUyNTcuNTczNDYyLTMuMzY1OTcuNTczNDYyek05Ni45ODQ2MzEgMjIuMzczOTg3IDEwNC43Mzk0IDQuNzk0OTk5OWgxLjc0NTMzbDcuNzU0NzYgMTcuNTc4OTg3MWgtMi4xMTkzMmwtMi4xNjkxOS01LjAxMTg0MS45OTczMy41MjM1OTVoLTEwLjcyMjA5bDEuMDIyMjctLjUyMzU5NS0yLjE0NDI1NCA1LjAxMTg0MXptOC42MDI0OTktMTUuMTg1NDAzNC00LjAxNDI0IDkuNDUwNTAwNC0uNTk4NC0uNDczNzNoOS4yMjU1NWwtLjU0ODUzLjQ3MzczLTQuMDE0MjUtOS40NTA1MDA0ek04OS44OTM5MjEgMjIuMzczOTg3VjYuNTY1NTMxNkg4My41MTAyMDJWNC43OTUyNzY5aDE0LjgzNjMzVjYuNTY1NTMxNkg5MS45NjMzNjZWMjIuMzc0MjY1eiIgZmlsbD17dG9rZW4uY29sb3JUZXh0fT48L3BhdGg+CiAgPHBhdGggZD0ibTY3Ljg1NDM4NSA0Ljc3MzExNDJoMTQuMDgwODd2MS43NjAwMDQyaC0xNC4wODA4N3ptMCAxNS44NDA4Njk4aDE0LjA4MDg3djEuNzYwMDAzaC0xNC4wODA4N3ptMTQuMDgwODctNy45MjA0MzVoLTE0LjA4MDg3djEuNzYwMDA1aDE0LjA4MDg3eiIgZmlsbD17dG9rZW4uY29sb3JUZXh0fT48L3BhdGg+CiAgPHBhdGggZD0ibTU3LjY2NDQ3MyAyMi4zNzM5ODd2LTkuMTAxMTU4bC40NDg4MDQgMS40MjExOTEtNy4xODEzMDktOS44OTkwMjAxaDIuMzkzODYxbDUuNjYwMTA5IDcuODI5NTY3MWgtLjUyMzYwNmw1LjY2MDM5MS03LjgyOTU2NzFoMi4zMTg3ODdsLTcuMTU2MzczIDkuODk4NzQyMS40MjQxNC0xLjQyMTE4OXY5LjEwMTE1OHptLTIwLjE4ODEwMi4wVjIwLjg1MzA2NUw0OC4xNDg1OTYgNS43NjczOTMydi43OTc4NjEzSDM3LjQ3NjM3MVY0Ljc5NDk5OTlINTAuMDY4NDVWNi4zMTU5MjI4TDM5LjM5NjUwMSAyMS4zNzY2NjF2LS43NzI5MjdoMTEuMDIxMjl2MS43NzAyNTN6bS0xMC4zOTYyOTguMjI0Mzk5Yy0xLjIxMzQxNC4wLTIuMzE5MDYyLS4yMDc3NzYtMy4zMTYzODktLjYyMzMyOC0uOTk3MzI3LS40MzIxNzUtMS44NDUwNTUtMS4wMzg4ODItMi41NDMxODQtMS44MjAxMjEtLjY5ODQwNi0uNzgxMjM5LTEuMjM4NjI0LTEuNzI4Ny0xLjYyMDkzMy0yLjg0MjY1OC0uMzY1Njg2LTEuMTEzNjgyLS41NDg1MjktMi4zNjAzMzktLjU0ODUyOS0zLjc0MDI1MS4wLTEuMzk2MjU3LjE4Mjg0My0yLjY0MjkxNy41NDg1MjktMy43NDAyNTIuMzgyMzA5LTEuMTEzNjgyLjkyMjUyNy0yLjA1MjgzMDkgMS42MjA2NTYtMi44MTc0NDc1LjY5ODEyOS0uNzgxMjM5MiAxLjUzNzU0NS0xLjM3OTkxMjEgMi41MTgyNS0xLjc5NTQ2NDguOTk3NjA0LS40MzIxNzQ5IDIuMTExMjg3LS42NDgyNjIzIDMuMzQxNi0uNjQ4MjYyMyAxLjI0NjY1Ny4wIDIuMzYwMzM5LjIwNzc3NjMgMy4zNDEwNDQuNjIzMzI5MS45OTczMjcuNDE1NTUyNyAxLjg0NTA1NSAxLjAxMzk0ODYgMi41NDM0NTkgMS43OTUxODc3LjcxNDc1MS43ODEyMzk4IDEuMjU0OTY5IDEuNzI4Njk5OCAxLjYyMDY1NSAyLjg0MjY1NzguMzgyMzEgMS4wOTcwNTkuNTczNDY0IDIuMzM1NDA2LjU3MzQ2NCAzLjcxNTA0Mi4wIDEuMzk2NTMzLS4xOTExNTQgMi42NTE3NzktLjU3MzQ2NCAzLjc2NTQ2MS0uMzgyMzA4IDEuMTEzNjgyLS45MjI1MjYgMi4wNjExNDItMS42MjA2NTUgMi44NDIzODFzLTEuNTQ1ODU2IDEuMzg3OTQ2LTIuNTQzMTgzIDEuODIwMzk4Yy0uOTgwNzA0LjQxNTU1Mi0yLjA5NDY2My42MjMzMjgtMy4zNDEzMi42MjMzMjh6bTAtMS44MjAxMmMxLjI2MzI3OS4wIDIuMzI3MDk0LS4yODI1NzYgMy4xOTE0NDMtLjg0NzcyOC44ODA5NzMtLjU2NTE1MiAxLjU1NDE2OS0xLjM4Nzk0NiAyLjAxOTU4OC0yLjQ2ODY2LjQ2NTY5Ni0xLjA4MDQzNy42OTg0MDYtMi4zNzY5NjEuNjk4NDA2LTMuODg5ODUuMC0xLjUyOTIzNC0uMjMyNzEtMi44MjU3NTgtLjY5ODEyOS0zLjg4OTg1MS0uNDY1NDE5LTEuMDYzODE1LTEuMTM4NjE0LTEuODc4Mjk3OC0yLjAxOTU4Ny0yLjQ0MzQ1LS44NjQzNS0uNTY1MTUxOC0xLjkyODQ0Mi0uODQ3NzI3Ni0zLjE5MTcyMS0uODQ3NzI3Ni0xLjIzMDAzOC4wLTIuMjg1ODE4LjI4MjU3NTgtMy4xNjY3OTEuODQ3NzI3Ni0uODY0MzUuNTY1MTUyMi0xLjUyOTIzNCAxLjM4Nzk0Ni0xLjk5NDY1MyAyLjQ2ODM4My0uNDY1NDE5IDEuMDYzODE1LS42OTgxMjkgMi4zNTIwMjktLjY5ODEyOSAzLjg2NDY0MS4wIDEuNTEyODg5LjIzMjcxIDIuODA5NjkuNjk4MTI5IDMuODkwMTI3LjQ2NTQxOSAxLjA2MzgxNSAxLjEzMDMwMyAxLjg4NjYwOSAxLjk5NDY1MyAyLjQ2ODM4My44ODA5NzMuNTY1MTUxIDEuOTM2NDc4Ljg0NzcyNyAzLjE2NjUxMy44NDc3Mjd6bS0xNi4wNDE3MjQgMS44MjAxMmMtMS43Mjg2OTgxLjAtMy4yMDgzNDE3LS4zNjU2ODYtNC40Mzg2NTYxLTEuMDk3MDU5QzUuMzY5NjU3MSAyMC43Njk5NTQgNC40MjIxOTY2IDE5LjczMTA3MyAzLjc1NzMxMzEgMTguMzg0NjgyYy0uNjQ4MjYzLTEuMzQ2NjY3LS45NzIzOTM2LTIuOTUwOTc4LS45NzIzOTM2LTQuODEyNjU0LjAtMS44NjE2NzcuMzI0MTMwNi0zLjQ1NzM5OS45NzIzOTM2LTQuNzg3NDQ1LjY2NDg4MzUtMS4zNDYzOTA2IDEuNjEyMzQ0LTIuMzg1MjcyNSAyLjg0MjM3OTgtMy4xMTY5MjI0QzcuODI5NzI5OCA0LjkzNjI4NzcgOS4zMDkzNzM0IDQuNTcwNjAxNCAxMS4wMzgzNDkgNC41NzA2MDE0YzEuMjEzNDE0LjAgMi4zMzU0MDguMTkxMTU0MyAzLjM2NTk3OC41NzM0NjI4IDEuMDMwNTcyLjM4MjMwODUgMS44OTQ5MjIuOTM5MTQ5MSAyLjU5MzMyNiAxLjY3MDUyMTlMMTYuMTk5NzkxIDguNDg1MTA4QzE1LjQzNTE3NSA3Ljc3MDM1OCAxNC42MzczMTMgNy4yNTUwNzIgMTMuODA2MjA5IDYuOTM5MjUxOSAxMi45OTE0NDcgNi42MDY4MDk4IDEyLjA4NTU0MyA2LjQ0MDU4ODcgMTEuMDg4MjE2IDYuNDQwNTg4N2MtMS45NzgwMzA0LjAtMy40OTA5MTg4LjYyMzMyOS00LjUzODM4OTIgMS44Njk5ODgzLTEuMDQ3MTkyNyAxLjI0NjY1Ny0xLjU3MDc4ODYgMy4wMDA1NjYtMS41NzA3ODg2IDUuMjYxNDUxLjAgMi4yNjA2MDYuNTIzNTk1OSA0LjAyMjU0OSAxLjU3MDc4ODYgNS4yODYxMDggMS4wNDcxOTI5IDEuMjQ2NjU3IDIuNTYwMDgyMyAxLjg2OTk4NiA0LjUzODM4OTIgMS44Njk5ODYuOTk3MzI3LjAgMS45MDMyMzEtLjE1NzkwOSAyLjcxNzcxNS0uNDczNzMuODMxMTA2LS4zMzI0NDIgMS42Mjg5NjgtLjg1NjAzOCAyLjM5MzU4NC0xLjU3MDc4OWwuNzk4MTM4IDEuNjcwNTIyYy0uNjk4MTI4LjcxNDc1MS0xLjU2MjQ3OCAxLjI3MTg2OS0yLjU5MzA0OCAxLjY3MDc5OS0xLjAzMDU3Mi4zODIzMDktMi4xNTI4NDIuNTczNDYyLTMuMzY2MjU2LjU3MzQ2MnoiIGZpbGw9e3Rva2VuLmNvbG9yVGV4dH0+PC9wYXRoPgo8L3N2Zz4K" }} | ||
| {{- $iconSvg := "PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiIHN0YW5kYWxvbmU9Im5vIj8+CjxzdmcKICAgd2lkdGg9IjkxIgogICBoZWlnaHQ9IjkxIgogICB2aWV3Qm94PSIwIDAgNjguMjUwMDI0IDY4LjI1MDAyNCIKICAgcHJlc2VydmVBc3BlY3RSYXRpbz0ieE1pZFlNaWQgbWVldCIKICAgdmVyc2lvbj0iMS4xIgogICBpZD0ic3ZnODI2IgogICBzb2RpcG9kaTpkb2NuYW1lPSJmYXZpY29uLnN2ZyIKICAgaW5rc2NhcGU6dmVyc2lvbj0iMS4xLjEgKGMzMDg0ZWYsIDIwMjEtMDktMjIpIgogICB4bWxuczppbmtzY2FwZT0iaHR0cDovL3d3dy5pbmtzY2FwZS5vcmcvbmFtZXNwYWNlcy9pbmtzY2FwZSIKICAgeG1sbnM6c29kaXBvZGk9Imh0dHA6Ly9zb2RpcG9kaS5zb3VyY2Vmb3JnZS5uZXQvRFREL3NvZGlwb2RpLTAuZHRkIgogICB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciCiAgIHhtbG5zOnN2Zz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciPgogIDxkZWZzCiAgICAgaWQ9ImRlZnM4MzAiIC8+CiAgPHNvZGlwb2RpOm5hbWVkdmlldwogICAgIGlkPSJuYW1lZHZpZXc4MjgiCiAgICAgcGFnZWNvbG9yPSIjZmZmZmZmIgogICAgIGJvcmRlcmNvbG9yPSIjNjY2NjY2IgogICAgIGJvcmRlcm9wYWNpdHk9IjEuMCIKICAgICBpbmtzY2FwZTpwYWdlc2hhZG93PSIyIgogICAgIGlua3NjYXBlOnBhZ2VvcGFjaXR5PSIwLjAiCiAgICAgaW5rc2NhcGU6cGFnZWNoZWNrZXJib2FyZD0iMCIKICAgICBzaG93Z3JpZD0iZmFsc2UiCiAgICAgaW5rc2NhcGU6em9vbT0iMC43NzAzNTc0MSIKICAgICBpbmtzY2FwZTpjeD0iNDM2LjgxMDIzIgogICAgIGlua3NjYXBlOmN5PSI1NDEuOTU2MjMiCiAgICAgaW5rc2NhcGU6d2luZG93LXdpZHRoPSIxNzIwIgogICAgIGlua3NjYXBlOndpbmRvdy1oZWlnaHQ9IjEzODciCiAgICAgaW5rc2NhcGU6d2luZG93LXg9IjE3MjAiCiAgICAgaW5rc2NhcGU6d2luZG93LXk9IjI1IgogICAgIGlua3NjYXBlOndpbmRvdy1tYXhpbWl6ZWQ9IjAiCiAgICAgaW5rc2NhcGU6Y3VycmVudC1sYXllcj0ic3ZnODI2IiAvPgogIDxyZWN0CiAgICAgc3R5bGU9ImZpbGw6IzAwMDAwMDtzdHJva2Utd2lkdGg6MC42MTc5MDUiCiAgICAgaWQ9InJlY3Q5MzgiCiAgICAgd2lkdGg9IjY4LjI1MDAyMyIKICAgICBoZWlnaHQ9IjY4LjI1MDAyMyIKICAgICB4PSIwIgogICAgIHk9Ii0xLjc3NjM1NjhlLTE1IiAvPgogIDxwYXRoCiAgICAgZmlsbC1ydWxlPSJldmVub2RkIgogICAgIGNsaXAtcnVsZT0iZXZlbm9kZCIKICAgICBkPSJtIDE2LjYwMTU5OCwxMy45MjY5MSBjIDAsLTEuMjgwMTE1IDEuMDE1MDUxLC0yLjMxNzg2MSAyLjI2NzQyNCwtMi4zMTc4NjEgaCAzMS43NDM5MzcgYyAxLjI1MjM3OCwwIDIuMjY3NDIsMS4wMzc3NDYgMi4yNjc0MiwyLjMxNzg2MSB2IDAgYyAwLDEuMjgwMTMzIC0xLjAxNTA0MiwyLjMxNzg3IC0yLjI2NzQyLDIuMzE3ODcgSCAxOC44NjkwMjIgYyAtMS4yNTIzNzMsMCAtMi4yNjc0MjQsLTEuMDM3NzM3IC0yLjI2NzQyNCwtMi4zMTc4NyB6IG0gMCw0MS43MjE1NzIgYyAwLC0xLjI4MDA4MSAxLjAxNTA1MSwtMi4zMTc4NjUgMi4yNjc0MjQsLTIuMzE3ODY1IGggMzEuNzQzOTM3IGMgMS4yNTIzNzgsMCAyLjI2NzQyLDEuMDM3Nzg0IDIuMjY3NDIsMi4zMTc4NjUgdiAwIGMgMCwxLjI4MDE1OCAtMS4wMTUwNDIsMi4zMTc4NjYgLTIuMjY3NDIsMi4zMTc4NjYgSCAxOC44NjkwMjIgYyAtMS4yNTIzNzMsMCAtMi4yNjc0MjQsLTEuMDM3NzA4IC0yLjI2NzQyNCwtMi4zMTc4NjYgeiBtIDM2LjI3ODc4MSwtMjAuODYwOCBjIDAsLTEuMjgwMDggLTEuMDE1MDQyLC0yLjMxNzg2NSAtMi4yNjc0MiwtMi4zMTc4NjUgSCAxOC44NjkwMjIgYyAtMS4yNTIzNzMsMCAtMi4yNjc0MjQsMS4wMzc3ODUgLTIuMjY3NDI0LDIuMzE3ODY1IHYgMCBjIDAsMS4yODAxNTggMS4wMTUwNTEsMi4zMTc4NjYgMi4yNjc0MjQsMi4zMTc4NjYgaCAzMS43NDM5MzcgYyAxLjI1MjM3OCwwIDIuMjY3NDIsLTEuMDM3NzA4IDIuMjY3NDIsLTIuMzE3ODY2IHoiCiAgICAgZmlsbD0iI2ZmZmZmZiIKICAgICBpZD0icGF0aDg0MCIKICAgICBzdHlsZT0ic3Ryb2tlLXdpZHRoOjAuNzY0MTYzIiAvPgo8L3N2Zz4K" }} |
There was a problem hiding this comment.
Embedding large SVG strings directly into the YAML template makes the file difficult to read and maintain. For better modularity and readability, consider moving these SVGs to separate files within the chart (e.g., in a files/ directory) and loading them using the .Files.Get helper function.
Example:
{{- /* Assuming files/logo.svg and files/icon.svg exist */ -}}
{{- $logoSvg := .Files.Get "files/logo.svg" }}
{{- $iconSvg := .Files.Get "files/icon.svg" }}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/system/dashboard/Makefile (1)
70-70: Harden replacement against special chars in TAG and improve portability.
- If $(TAG) contains '&', sed will inject the whole match. Escape it via Make’s subst.
- BSD/macOS sed needs
-i ''. If contributors run this locally, consider documenting or using a portable approach.Example hardening (GNU sed/Linux CI assumed):
- sed -i -E 's|(\$$tenantText := ")[^"]+|\1$(TAG)|' templates/configmap.yaml + sed -i -E 's|(\$$tenantText := ")[^"]+|\1$(subst &,\&,$(TAG))|' templates/configmap.yamlpackages/system/dashboard/templates/configmap.yaml (1)
22-24: Clean up extra spaces after colons (lint warning).Not functionally relevant, but removes YAMLlint noise.
- TITLE_TEXT: {{ $brandingConfig | dig "data" "titleText" $titleText | quote }} - LOGO_TEXT: {{ $brandingConfig | dig "data" "logoText" $logoText | quote }} - LOGO_SVG: {{ $brandingConfig | dig "data" "logoSvg" $logoSvg | quote }} + TITLE_TEXT: {{ $brandingConfig | dig "data" "titleText" $titleText | quote }} + LOGO_TEXT: {{ $brandingConfig | dig "data" "logoText" $logoText | quote }} + LOGO_SVG: {{ $brandingConfig | dig "data" "logoSvg" $logoSvg | quote }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/system/dashboard/Makefile(1 hunks)packages/system/dashboard/templates/configmap.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/system/dashboard/templates/configmap.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 22-22: too many spaces after colon
(colons)
[warning] 23-23: too many spaces after colon
(colons)
[warning] 24-24: too many spaces after colon
(colons)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
packages/system/dashboard/templates/configmap.yaml (3)
19-24: Data mapping and quoting are correct.Keys map 1:1 to brandingConfig.data and quote ensures valid YAML scalars even for long/base64 strings.
1-1: YAMLlint “syntax error” at Line 1 is expected for Helm templates.The
{{- ... }}Go‑template trim marker confuses plain YAML linters. Consider excluding Helm templates from YAMLlint or using helm lint for this file.
1-9: Ensure cozystack-branding ConfigMap is provisionedTemplates reference this ConfigMap via lookup but do not define it. Confirm it’s created in this chart or documented as an external dependency.
|
|
||
| update-tenant-text: | ||
| yq -i '.data.TENANT_TEXT = "$(TAG)"' ./templates/configmap.yaml | ||
| sed -i 's|\($$tenantText := "\)[^"]\+|\1$(TAG)|' templates/configmap.yaml |
There was a problem hiding this comment.
Fix sed pattern to match literal $tenantText (currently won’t match).
In sed, $ is an end‑of‑line anchor. After Make reduces $$ to $, the pattern ($$tenantText …) becomes ($tenantText …) and won’t match the literal $tenantText. Escape the dollar for sed by writing \$$ in the Make recipe.
-update-tenant-text:
- sed -i 's|\($$tenantText := "\)[^"]\+|\1$(TAG)|' templates/configmap.yaml
+update-tenant-text:
+ sed -i -E 's|(\$$tenantText := ")[^"]+|\1$(TAG)|' templates/configmap.yaml📝 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.
| sed -i 's|\($$tenantText := "\)[^"]\+|\1$(TAG)|' templates/configmap.yaml | |
| update-tenant-text: | |
| sed -i -E 's|(\$$tenantText := ")[^"]+|\1$(TAG)|' templates/configmap.yaml |
🤖 Prompt for AI Agents
In packages/system/dashboard/Makefile around line 70 the sed pattern uses
($$tenantText := "\)[^"]\+ which after Make variable expansion becomes
($tenantText …) and fails to match the literal $tenantText; fix it by escaping
the dollar so sed sees a literal $ (use \$$tenantText in the pattern) — update
the sed invocation to use \$$tenantText instead of $$tenantText so the
replacement matches the literal $tenantText in templates/configmap.yaml.
1 participant
Signed-off-by: Andrei Kvapil [email protected]
What this PR does
Release note
Summary by CodeRabbit