fix(install): do not add --set for an empty string #807
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FalcoSuessgott
requested review from
EItanya,
ilackarms and
yuval-k
as code owners
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the kagent install command would generate an invalid Helm upgrade command when the KAGENT_HELM_EXTRA_ARGS environment variable is empty or unset. The bug caused a malformed command with a trailing --set flag without any arguments.
- Added validation to skip empty string values before appending
--setflags to the Helm command - Removed an unnecessary blank line for code cleanup
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Tom Morelly <tom.morelly@clearroute.io>
FalcoSuessgott
force-pushed
the
kagent-install-fix
branch
from
6f92b2c to
9b470dc
Compare
EItanya
approved these changes
Aug 25, 2025
inFocus7
pushed a commit
to inFocus7/kagent
that referenced
this pull request
Aug 25, 2025
`kagent install` would construct an invalid `helm upgrade` command due
to a missing `--set` parameter.
When invoking `kagent install` (with no specified config params or other
`kagent` env vars) & running a `ps aux` in parallel I can see that
`kagent` runs the following command
```go
helm upgrade --install kagent oci://ghcr.io/kagent-dev/kagent/helm/kagent --version 0.6.3 --namespace kagent --create-namespace --wait --history-max 2 --timeout 5m --set providers.default=openAI --set providers.openAI.apiKey=<REDACTED> --set `
```
This command is invalid and errors with `Error: flag needs an argument:
--set`.
The reason that we pass `[]{""}` to `installChart()` is because we
obtain the values from
[here](https://github.com/kagent-dev/kagent/blob/main/go/cli/internal/cli/install.go#L78):
```go
helmExtraArgs := GetEnvVarWithDefault(KAGENT_HELM_EXTRA_ARGS, "")
```
where if `KAGENT_HELM_EXTRA_ARGS` is not present we return `""`.
This PR simply checks if the value is `!= ""` before appending to `helm
upgrade`
Signed-off-by: Tom Morelly <tom.morelly@clearroute.io>
yanivmn
pushed a commit
to yanivmn/kagent
that referenced
this pull request
Aug 28, 2025
`kagent install` would construct an invalid `helm upgrade` command due
to a missing `--set` parameter.
When invoking `kagent install` (with no specified config params or other
`kagent` env vars) & running a `ps aux` in parallel I can see that
`kagent` runs the following command
```go
helm upgrade --install kagent oci://ghcr.io/kagent-dev/kagent/helm/kagent --version 0.6.3 --namespace kagent --create-namespace --wait --history-max 2 --timeout 5m --set providers.default=openAI --set providers.openAI.apiKey=<REDACTED> --set `
```
This command is invalid and errors with `Error: flag needs an argument:
--set`.
The reason that we pass `[]{""}` to `installChart()` is because we
obtain the values from
[here](https://github.com/kagent-dev/kagent/blob/main/go/cli/internal/cli/install.go#L78):
```go
helmExtraArgs := GetEnvVarWithDefault(KAGENT_HELM_EXTRA_ARGS, "")
```
where if `KAGENT_HELM_EXTRA_ARGS` is not present we return `""`.
This PR simply checks if the value is `!= ""` before appending to `helm
upgrade`
Signed-off-by: Tom Morelly <tom.morelly@clearroute.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
kagent installwould construct an invalidhelm upgradecommand due to a missing--setparameter.When invoking
kagent install(with no specified config params or otherkagentenv vars) & running aps auxin parallel I can see thatkagentruns the following commandThis command is invalid and errors with
Error: flag needs an argument: --set.The reason that we pass
[]{""}toinstallChart()is because we obtain the values from here:where if
KAGENT_HELM_EXTRA_ARGSis not present we return"".This PR simply checks if the value is
!= ""before appending tohelm upgrade