Skip to content

feat(helm): make the controller ServiceAccount optional - #3022

Merged
EItanya merged 2 commits into
kagent-dev:mainfrom
TarasLykhenko:helm/controller-serviceaccount-optional
Sep 30, 2026
Merged

EItanya merged 2 commits into
kagent-dev:mainfrom
TarasLykhenko:helm/controller-serviceaccount-optional

Conversation

@TarasLykhenko

Copy link
Copy Markdown
Contributor

What

Adds two values for the controller ServiceAccount:

Value Default Effect
controller.serviceAccount.create true When false, the chart does not render the controller ServiceAccount.
controller.serviceAccount.name "" (<fullname>-controller) Name of the ServiceAccount the controller runs as and is bound to.

A new kagent.controller.serviceAccountName helper supplies the name to the ServiceAccount, the controller Deployment, and the getter, writer, leader-election and metrics-auth bindings.

Why

Some platforms create the controller ServiceAccount outside the chart, for example an IRSA or Workload Identity provisioner that owns the ServiceAccount and its role annotation. Today the chart always renders <fullname>-controller, and Helm refuses to install over a ServiceAccount it does not own:

ServiceAccount "kagent-controller" in namespace "kagent" exists and cannot be imported into the current release: invalid ownership metadata

controller.serviceAccount.annotations covers the case where the chart can own the ServiceAccount. It does not help when something else already does.

Compatibility

The defaults render exactly what the chart renders today, so existing installs don't change.

Testing

  • New helm/kagent/tests/controller-serviceaccount_test.yaml covers: the default name, create: false, a custom name on the ServiceAccount and the Deployment, and each binding's subject.
  • make helm-test: 386 tests pass.

@TarasLykhenko
TarasLykhenko requested a review from a team as a code owner September 30, 2026 09:30
@github-actions github-actions Bot added the enhancement New feature or request label Sep 30, 2026
Add controller.serviceAccount.create (default true) and
controller.serviceAccount.name (default <fullname>-controller). The
Deployment and the controller's RoleBindings and ClusterRoleBinding now
take the name from one helper, so installs where the ServiceAccount is
created outside the chart (for example by an IRSA or Workload Identity
provisioner) no longer conflict with it.

Signed-off-by: TarasLykhenko <18290033+TarasLykhenko@users.noreply.github.com>
@TarasLykhenko
TarasLykhenko force-pushed the helm/controller-serviceaccount-optional branch from 334f4a7 to 2f241f3 Compare September 30, 2026 09:32

@EItanya EItanya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 AI-generated review.

The change is focused and consistently updates the affected references. One small correctness issue with custom names is noted inline.

Validation: all 386 Helm tests passed, Helm lint passed, default manifests matched the base, and custom names worked across cluster-wide and namespaced RBAC. No live cluster testing.

{{- toYaml . | nindent 8 }}
{{- end }}
serviceAccountName: {{ include "kagent.fullname" . }}-controller
serviceAccountName: {{ include "kagent.controller.serviceAccountName" . }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we quote the ServiceAccount name wherever the helper is emitted as a YAML value, and add a numeric-name test? A valid name such as "12345" renders as a YAML number even with --set-string controller.serviceAccount.name=12345. With controller.serviceAccount.create=false, I reproduced Kubernetes Deployment decoding failing with cannot unmarshal number into Go struct field Deployment.spec.template.spec.serviceAccountName of type string, so this prevents installing against that existing ServiceAccount. The ServiceAccount metadata and RBAC subjects use the same unquoted output and need the same treatment.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with this point, after that we can absolutely 🚢

@TarasLykhenko TarasLykhenko Sep 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good catch. Quoted the name everywhere the helper is emitted, and added a numeric-name test (name: "12345") that asserts a string on each of them. With create=false and --set-string controller.serviceAccount.name=12345, the Deployment now passes a server-side dry run.

A numeric name such as 12345 rendered as a YAML number, so the API
server rejected the Deployment. Quote the name everywhere the helper is
emitted and cover it with a test.

Signed-off-by: TarasLykhenko <18290033+TarasLykhenko@users.noreply.github.com>
@EItanya
EItanya enabled auto-merge September 30, 2026 23:28
@EItanya
EItanya added this pull request to the merge queue Sep 30, 2026
Merged via the queue into kagent-dev:main with commit ba60ebd Sep 30, 2026
30 checks passed
teemow added a commit to teemow/kagent that referenced this pull request Oct 2, 2026
* upstream/main: (79 commits)
  fix(translator): accept Mistral ModelConfigs in v1alpha3 (kagent-dev#3008)
  fix: allow disabling kagent model streaming (kagent-dev#3028)
  feat(helm): make the controller ServiceAccount optional (kagent-dev#3022)
  chore: bump retired gemini 2.5 default models to 3.5 (kagent-dev#3027)
  fix(api): defer unsupported configuration fields (kagent-dev#3007)
  docs: update README for the v1 architecture (kagent-dev#2996)
  fix: keep Go ADK SQLite connections out of VM snapshots (kagent-dev#3004)
  fix(ui): report harness delete failures (kagent-dev#2964)
  fix: repair image scans and use uv-managed Python (kagent-dev#2995)
  fix(e2e): wait for controller health and parallelize independent tests (kagent-dev#2993)
  fix(adk): pass MCP embedded resource text to OpenAI-compatible models (kagent-dev#2992)
  fix(controller): simplify agent preparation and retry handling (kagent-dev#2994)
  chore(deps): bump filelock from 3.29.4 to 4.0.3 in /python (kagent-dev#2982)
  chore(deps): bump the python-minor-patch group in /python with 6 updates (kagent-dev#2977)
  chore(deps): bump the python-google-ai group in /python with 2 updates (kagent-dev#2975)
  chore(deps): update wheel requirement from >=0.47.0 to >=0.48.0 in /python (kagent-dev#2981)
  chore(deps): update anthropic[vertex] requirement from >=0.120.0 to >=1.8.0 in /python (kagent-dev#2985)
  chore(deps): bump mcp from 1.29.0 to 2.2.0 in /python (kagent-dev#2984)
  fix(adk): keep Ollama tool call argument order stable across requests (kagent-dev#2988)
  chore(deps): update google-adk[a2a] requirement from <3,>=2.6.2 to >=2.9.2,<3 in /python (kagent-dev#2983)
  ...

Signed-off-by: Timo Derstappen <teemow@gmail.com>

# Conflicts:
#	go/adk/pkg/models/anthropic_adk.go
#	go/adk/pkg/models/anthropic_adk_test.go
#	go/core/internal/translator/adkconfig/model_test.go
#	python/packages/kagent-adk/src/kagent/adk/models/_anthropic.py
teemow added a commit to teemow/kagent that referenced this pull request Oct 2, 2026
* upstream/main: (79 commits)
  fix(translator): accept Mistral ModelConfigs in v1alpha3 (kagent-dev#3008)
  fix: allow disabling kagent model streaming (kagent-dev#3028)
  feat(helm): make the controller ServiceAccount optional (kagent-dev#3022)
  chore: bump retired gemini 2.5 default models to 3.5 (kagent-dev#3027)
  fix(api): defer unsupported configuration fields (kagent-dev#3007)
  docs: update README for the v1 architecture (kagent-dev#2996)
  fix: keep Go ADK SQLite connections out of VM snapshots (kagent-dev#3004)
  fix(ui): report harness delete failures (kagent-dev#2964)
  fix: repair image scans and use uv-managed Python (kagent-dev#2995)
  fix(e2e): wait for controller health and parallelize independent tests (kagent-dev#2993)
  fix(adk): pass MCP embedded resource text to OpenAI-compatible models (kagent-dev#2992)
  fix(controller): simplify agent preparation and retry handling (kagent-dev#2994)
  chore(deps): bump filelock from 3.29.4 to 4.0.3 in /python (kagent-dev#2982)
  chore(deps): bump the python-minor-patch group in /python with 6 updates (kagent-dev#2977)
  chore(deps): bump the python-google-ai group in /python with 2 updates (kagent-dev#2975)
  chore(deps): update wheel requirement from >=0.47.0 to >=0.48.0 in /python (kagent-dev#2981)
  chore(deps): update anthropic[vertex] requirement from >=0.120.0 to >=1.8.0 in /python (kagent-dev#2985)
  chore(deps): bump mcp from 1.29.0 to 2.2.0 in /python (kagent-dev#2984)
  fix(adk): keep Ollama tool call argument order stable across requests (kagent-dev#2988)
  chore(deps): update google-adk[a2a] requirement from <3,>=2.6.2 to >=2.9.2,<3 in /python (kagent-dev#2983)
  ...

Signed-off-by: Timo Derstappen <teemow@gmail.com>
alexdarbyshire pushed a commit to alexdarbyshire/kagent that referenced this pull request Oct 5, 2026
)

## What

Adds two values for the controller ServiceAccount:

| Value | Default | Effect |
|-------|---------|--------|
| `controller.serviceAccount.create` | `true` | When `false`, the chart
does not render the controller ServiceAccount. |
| `controller.serviceAccount.name` | `""` (`<fullname>-controller`) |
Name of the ServiceAccount the controller runs as and is bound to. |

A new `kagent.controller.serviceAccountName` helper supplies the name to
the ServiceAccount, the controller Deployment, and the getter, writer,
leader-election and metrics-auth bindings.

## Why

Some platforms create the controller ServiceAccount outside the chart,
for example an IRSA or Workload Identity provisioner that owns the
ServiceAccount and its role annotation. Today the chart always renders
`<fullname>-controller`, and Helm refuses to install over a
ServiceAccount it does not own:

```
ServiceAccount "kagent-controller" in namespace "kagent" exists and cannot be imported into the current release: invalid ownership metadata
```

`controller.serviceAccount.annotations` covers the case where the chart
can own the ServiceAccount. It does not help when something else already
does.

## Compatibility

The defaults render exactly what the chart renders today, so existing
installs don't change.

## Testing

- New `helm/kagent/tests/controller-serviceaccount_test.yaml` covers:
the default name, `create: false`, a custom `name` on the ServiceAccount
and the Deployment, and each binding's subject.
- `make helm-test`: 386 tests pass.

---------

Signed-off-by: TarasLykhenko <18290033+TarasLykhenko@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants