Skip to content

feat(base-cluster/flux): use centralised helmRepositories template#1845

Merged
cwrau merged 1 commit intomainfrom
feat/base-cluster/use-centralised-helmrepositories-template
Dec 5, 2025
Merged

feat(base-cluster/flux): use centralised helmRepositories template#1845
cwrau merged 1 commit intomainfrom
feat/base-cluster/use-centralised-helmrepositories-template

Conversation

@cwrau
Copy link
Member

@cwrau cwrau commented Dec 1, 2025

Summary by CodeRabbit

  • Refactor

    • Simplified Helm repository template generation logic by consolidating manual resource creation into a template include.
  • Chores

    • Standardized schema validation by replacing inline definitions with shared external schema references for improved configuration consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 1, 2025 09:04
@cwrau cwrau enabled auto-merge December 1, 2025 09:04
@gemini-code-assist
Copy link

Summary of Changes

Hello @cwrau, 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 refactors the base-cluster chart to leverage centralized Helm template logic and schema definitions. By replacing extensive inline definitions with references to a common chart, it streamlines the management of Helm repositories and conditions, promoting consistency and reducing code duplication across the project's Helm charts.

Highlights

  • Centralized Helm Repository Template: The helmRepositories.yaml template in base-cluster now utilizes a shared template from common.helm.repositories, replacing a large block of inline logic.
  • Centralized Helm Repository Schema: The values.schema.json for base-cluster has been updated to reference a centralized schema definition for helmRepositories from the common chart, rather than defining it inline.
  • Centralized Condition Schema: The values.schema.json also now references a centralized schema definition for condition from the common chart, promoting consistency.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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-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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

The pull request refactors HelmRepository generation and schema validation by replacing local, manual implementations with references to external common templates and schemas. The helmRepositories template simplifies from iterative logic to a single include call, while schema definitions are externalized via $refs to the shared common charts schema.

Changes

Cohort / File(s) Summary
Template simplification
charts/base-cluster/templates/global/helmRepositories.yaml
Replaces manual HelmRepository/GitRepository generation logic with a single include call to common.helm.repositories template, eliminating iterative conditional creation and nested chart handling.
Schema externalization
charts/base-cluster/values.schema.json
Replaces inline schema definitions for global.helmRepositories and root-level condition with $ref pointers to external definitions in the common charts schema, delegating validation to a centralized source.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring attention:
    • Verify that the external common template (common.helm.repositories) produces identical output to the replaced manual logic, particularly for conditional creation and nested chart handling
    • Confirm the $ref URLs are stable and correctly resolve to the intended schema definitions
    • Ensure no breaking changes in the schema validation behavior due to the externalized references
    • Cross-reference with related PRs (#1756, #1757, #1758) to understand the broader refactoring context

Possibly related PRs

  • PR #1756: Adds autoscaler entry to global.helmRepositories, which will be rendered by the new common.helm.repositories include
  • PR #1758: Modifies HelmRepository resource generation and repository naming, directly affected by this template consolidation
  • PR #1757: Changes the common.helm.repositories template being referenced, establishing the underlying implementation

Suggested labels

common

Suggested reviewers

  • tasches
  • marvinWolff
  • teutonet-bot

Poem

🐇 With templates unified and schemas aligned,
Common grounds found, loose ends refined!
What once was scattered now gently includes—
A refactoring dance in the best attitudes. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing manual HelmRepository generation logic with a centralized template include.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/base-cluster/use-centralised-helmrepositories-template

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 and usage tips.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to centralize the logic for creating Helm repository resources by moving it into a common template and schema. This is a good refactoring practice for maintainability. However, the implementation appears to be incomplete. The referenced common template (common.helm.repositories) and JSON schema definitions (helmRepositories, condition) seem to be missing from the common chart files included in this pull request. This will cause failures during chart rendering and schema validation. My review includes critical feedback to address these missing components.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to refactor the base-cluster chart to use centralized helm repository definitions from the common chart. The changes aim to reduce code duplication by replacing inline schema definitions and template logic with references to shared components. However, the PR references schemas and templates that do not yet exist in the common chart.

Key changes:

  • Replace inline helmRepositories schema (171 lines) with external reference to common chart
  • Replace inline condition schema definition with external reference to common chart
  • Replace 47 lines of template logic with single template include call

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
charts/base-cluster/values.schema.json Replaces inline helmRepositories and condition schema definitions with external references to common chart schemas that don't exist yet
charts/base-cluster/templates/global/helmRepositories.yaml Replaces inline template logic with call to common.helm.repositories template that doesn't exist yet

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
charts/base-cluster/values.schema.json (1)

1-1702: Assess offline validation impact and CI/CD resilience.

External schema references via GitHub URLs introduce runtime dependencies. If GitHub is unavailable or the URLs change, Helm value validation could fail during CI/CD pipelines or in offline environments.

Consider:

  1. Schema caching strategy: Document how teams should cache external schemas for offline use (e.g., via Helm plugin or CI/CD artifacts).
  2. Fallback mechanism: If feasible, provide a fallback or embedded copy of external schemas to reduce availability risk.
  3. Monitoring: Add alerting for broken schema URLs in CI/CD pipelines.
  4. Documentation: Update chart README to explain the external schema dependency and any prerequisites for schema validation.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae36393 and 8abc27e.

📒 Files selected for processing (2)
  • charts/base-cluster/templates/global/helmRepositories.yaml (1 hunks)
  • charts/base-cluster/values.schema.json (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-24T09:56:41.380Z
Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts PR: 1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's `valuesFrom` feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.

Applied to files:

  • charts/base-cluster/values.schema.json
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/global/helmRepositories.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

⏰ 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: Agent

@cwrau cwrau added this pull request to the merge queue Dec 5, 2025
Merged via the queue into main with commit 60658ed Dec 5, 2025
36 of 39 checks passed
@cwrau cwrau deleted the feat/base-cluster/use-centralised-helmrepositories-template branch December 5, 2025 12:38
github-merge-queue bot pushed a commit that referenced this pull request Dec 11, 2025
🤖 I have created a release *beep* *boop*
---


##
[11.0.0](base-cluster-v10.1.2...base-cluster-v11.0.0)
(2025-12-10)


### ⚠ BREAKING CHANGES

* **base-cluster/monitoring:** grafana-tempo-distributed would need s3
([#1875](#1875))

### Features

* **base-cluster/flux:** use centralised helmRepositories template
([#1845](#1845))
([60658ed](60658ed))


### Bug Fixes

* **base-cluster/monitoring:** grafana-tempo-distributed would need s3
([#1875](#1875))
([df5c372](df5c372))
* **base-cluster/monitoring:** increase limit of grafana sidecar
([#1878](#1878))
([fb1ebc6](fb1ebc6))
* **base-cluster:** correctly set image repository
([#1848](#1848))
([0e108dc](0e108dc))


### Miscellaneous Chores

* **base-cluster/dependencies:** update common docker tag to v1.7.0
([#1872](#1872))
([b50ea98](b50ea98))
* **base-cluster/dependencies:** update helm release reflector to
v9.1.44
([#1861](#1861))
([2cfe4e5](2cfe4e5))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Breaking Changes**
  * grafana-tempo-distributed now requires S3 configuration

* **New Features**
  * Centralized helmRepositories template management

* **Bug Fixes**
  * Increased Grafana sidecar limit
  * Corrected image repository reference

* **Chores**
  * Updated dependencies and component versions

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants