Skip to content

fix templating of cli_args values in helm chart#20

Open
shortstories wants to merge 3 commits into
freshworks-oss:masterfrom
shortstories:fix-cli-args
Open

fix templating of cli_args values in helm chart#20
shortstories wants to merge 3 commits into
freshworks-oss:masterfrom
shortstories:fix-cli-args

Conversation

@shortstories
Copy link
Copy Markdown

Fixes #19

Changes proposed on the PR:

  • fix templating of cli_args values

@samof76
Copy link
Copy Markdown

samof76 commented Jun 24, 2025

Thanks. This major rewamp PR is pending, will take it after that.

Copy link
Copy Markdown

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 the templating of cli_args values in the Helm chart deployment configuration. The change corrects how command-line arguments are rendered in the Kubernetes deployment YAML, replacing improper quoting with proper YAML list formatting.

  • Replaces incorrect quote function usage with proper YAML list templating for CLI arguments

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

args:
- {{ quote .Values.image.cli_args }}
args:
{{- toYaml .Values.image.cli_args | nindent 8 }}
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

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

The toYaml function will render cli_args as YAML, but the args field in Kubernetes expects a list of strings. If cli_args is already a YAML list, this will create nested YAML structure. Consider using {{- range .Values.image.cli_args }} with {{ . | quote }} for each item, or ensure cli_args contains pre-formatted string values.

Suggested change
{{- toYaml .Values.image.cli_args | nindent 8 }}
{{- range .Values.image.cli_args }}
{{- . | quote }}
{{- end }}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@shortstories can you please check if you are ok with this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@samof76
ok. applied and committed.
There was an error in the Copilot review, so I fixed that part as well.

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.

`cli_args' value in the helm chart was not applied correctly.

3 participants