Skip to content

fix(helm): Filter out empty YAML documents from helm template output#1367

Draft
Moep90 wants to merge 11 commits into
masterfrom
fix/helm-filter-empty-yaml-documents
Draft

fix(helm): Filter out empty YAML documents from helm template output#1367
Moep90 wants to merge 11 commits into
masterfrom
fix/helm-filter-empty-yaml-documents

Conversation

@Moep90
Copy link
Copy Markdown
Contributor

@Moep90 Moep90 commented Dec 21, 2025

Summary

Filter out None values from yaml.safe_load_all() output in the Helm input type to prevent empty YAML document separators (---) in compiled output.

Problem

When Helm charts have conditional templates (e.g., {{- if .Values.ollama.enabled }}), the helm template command outputs empty YAML documents between --- separators for disabled features.

Currently, yaml.safe_load_all() parses these empty documents as None values in the list, and yaml.dump_all() in write_yaml() outputs them as empty documents with --- separators.

Example output with the bug:

apiVersion: networking.k8s.io/v1
kind: Ingress
...
---
---
---
---
...

Fixes #

## Proposed Changes

*

## Docs and Tests

* [ ] Tests added
* [ ] Updated documentation

## Summary

Filter out `None` values from `yaml.safe_load_all()` output in the Helm input type to prevent empty YAML document separators (`---`) in compiled output.

## Problem

When Helm charts have conditional templates (e.g., `{{- if .Values.ollama.enabled }}`), the `helm template` command outputs empty YAML documents between `---` separators for disabled features. 

Currently, `yaml.safe_load_all()` parses these empty documents as `None` values in the list, and `yaml.dump_all()` in `write_yaml()` outputs them as empty documents with `---` separators.

**Example output with the bug:**
```yaml
apiVersion: networking.k8s.io/v1
kind: Ingress
...
---
---
---
---
...
@Moep90 Moep90 self-assigned this Dec 21, 2025
@Moep90 Moep90 requested a review from Copilot December 21, 2025 12:29
Copy link
Copy Markdown
Contributor

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 fixes an issue where empty YAML documents (represented as None by yaml.safe_load_all()) from Helm template output were being included in the compiled output, resulting in consecutive --- separators without content between them.

Key changes:

  • Filter out None values from parsed YAML documents in the Helm input compilation process

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

Comment thread kapitan/inputs/helm.py Outdated
@Moep90 Moep90 marked this pull request as draft December 21, 2025 12:31
ademariag
ademariag previously approved these changes Dec 24, 2025
@ademariag
Copy link
Copy Markdown
Contributor

@Moep90 is this ready to be merged?

@Moep90
Copy link
Copy Markdown
Contributor Author

Moep90 commented Dec 24, 2025

No, I wasn't able to verify the behavior change with tests yet.

@ademariag ademariag self-requested a review December 24, 2025 11:19
@ademariag ademariag dismissed their stale review December 24, 2025 11:20

more changes incoming

@Moep90 Moep90 requested a review from Copilot December 25, 2025 07:19
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.


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

Comment thread kapitan/inputs/helm.py Outdated
@Moep90 Moep90 requested a review from Copilot December 25, 2025 07:29
Copy link
Copy Markdown
Contributor

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

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


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

Comment thread kapitan/inputs/helm.py Outdated
Comment thread tests/test_helm_input.py Outdated
os.chdir("../../")
reset_cache()

def test_compile_helm_filters_empty_documents(self):
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The test method name doesn't follow the naming convention of the other test methods in this file, which use test_compile_<feature> format (e.g., test_compile_fetch_http_chart, test_compile_kadet_helm_chart). Consider renaming to test_compile_helm_conditional_templates for consistency.

Suggested change
def test_compile_helm_filters_empty_documents(self):
def test_compile_helm_conditional_templates(self):

Copilot uses AI. Check for mistakes.
@Moep90 Moep90 requested a review from Copilot December 25, 2025 08:03
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.


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

Comment thread kapitan/inputs/helm.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants