fix(base-cluster): add missing value to template

[SyeKlu]

Summary by CodeRabbit

• Chores
  • Changed deployment gating for the VMware chart to rely on the Velero provider being set rather than the presence of backup storage locations, preventing incorrect skips or deployments during Helm releases.

[coderabbitai]

Walkthrough

Changed vmware Helm chart gating in charts/base-cluster/values.yaml to check whether .Values.backup.provider.velero is non-nil (using ne ... nil) instead of checking for .Values.backup.backupStorageLocations presence.

Changes

Cohort / File(s)	Summary
Backup provider gating configuration charts/base-cluster/values.yaml	Replaced conditional that checked not (empty .Values.backup.backupStorageLocations) with ne (.Values.backup.provider).velero nil, so vmware chart is enabled when backup.provider.velero is non-nil.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Helm as Helm/template
  participant Values as Values (.Values)
  participant Chart as vmware chart

  note right of Values #E8F6FF: New condition\n`ne (.Values.backup.provider).velero nil`
  Helm->>Values: evaluate condition
  alt `.Values.backup.provider.velero` is non-nil
    Helm->>Chart: include vmware chart
    Chart-->>Helm: rendered
  else `.Values.backup.provider.velero` is nil
    Helm-->>Chart: skip vmware chart
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Pay attention to charts/base-cluster/values.yaml conditional logic.
• Verify other charts/values referencing backup.provider.velero are consistent with this gating change.

Possibly related PRs

• feat(base-cluster/backup)!: add k8up provider #1751 — similar change moving chart gating to provider-scoped velero checks (backup.provider.velero) instead of top-level backup.backupStorageLocations.

Suggested reviewers

• tasches
• marvinWolff

Poem

🐰
I hopped through values, quiet and spry,
Found velero nested, tucked nearby.
The vmware gate now peeks within,
A tiny change, a subtle grin. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The pull request title "fix(base-cluster): add missing value to template" describes the change as adding a missing value, but the actual modification changes an existing Helm chart condition from checking backupStorageLocations to checking the Velero provider. The title is misleading about the nature of the change—this is not about adding a previously absent value, but rather modifying an existing condition's logic. While the title does relate to the values.yaml file being changed, it doesn't accurately convey what was actually fixed or changed.	Consider revising the title to more accurately reflect the change, such as "fix(base-cluster): fix vmware helm chart condition to check velero provider" or "fix(base-cluster): change velero backup condition in base-cluster values" to clearly convey that this modifies an existing condition rather than adds a missing value.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-velero-helm-repo

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e88ddee and a63342f.

📒 Files selected for processing (1)

• charts/base-cluster/values.yaml (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-07-24T09:55:53.655Z

Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts PR: 1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., `{ cloudflare: {} }`), never a string format. The template correctly uses `{{ .Values.dns.provider | keys | first }}` to extract the provider name from the map keys.

Applied to files:

• charts/base-cluster/values.yaml

⏰ 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). (2)

• GitHub Check: check licenses
• GitHub Check: lint helm chart (base-cluster)

🔇 Additional comments (1)

charts/base-cluster/values.yaml (1)

195-195: Condition properly gates velero chart based on provider configuration.

The change correctly aligns the vmware Helm repository condition to check (.Values.backup.provider).velero consistency with the k8up pattern on Line 190. This properly gates the velero chart inclusion when the velero provider is explicitly configured, rather than relying on a separate field. The syntax and logic are sound.

One suggestion for verification: confirm that the backup.provider structure in your cluster configurations follows the map-based pattern (e.g., { velero: {...} }) as expected by this condition, similar to how dns.provider is structured. If you have integration tests or acceptance criteria that validate velero chart inclusion when configured, ensure they still pass.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

charts/base-cluster/values.yaml (1)

195-195: Consider documenting the expected backup.provider schema structure.

The condition now checks for backup.provider.backupStorageLocations, which assumes backup.provider is an object with nested properties. However, the values file only defines backup.provider: null without documenting the expected structure or providing examples.

This pattern is consistent with line 190 (k8up provider check), but users may be unclear about how to configure this nested structure. Consider adding inline comments or documentation to clarify the expected object schema.

Example documentation update:

backup:
  # Set to null by default. When configured, should be an object with provider-specific settings.
  # Example for Velero with S3 storage:
  #   provider:
  #     s3:
  #       ...
  #     backupStorageLocations:
  #       - name: default
  #         ...
  provider: null
  resources:
    ...

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfb4bce and 1c3cd38.

📒 Files selected for processing (1)

• charts/base-cluster/values.yaml (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

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.

📚 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.yaml

⏰ 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). (2)

• GitHub Check: Update release-please config file for a possibly new chart
• GitHub Check: check licenses