fix: scope RBAC permissions and fix hardcoded Helm resource names by dmaizel · Pull Request #522 · OctopusDeploy/helm-charts

dmaizel · 2026-03-04T23:50:10Z

No description provided.

changeset-bot · 2026-03-04T23:50:13Z

⚠️ No Changeset found

Latest commit: b2fea3c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

dmaizel · 2026-03-06T00:26:16Z

Things I want to clean up next

While working on this PR I went through the whole chart and found a few things might be worth fixing. Keeping this PR focused on RBAC/naming but wanted to write these down so they don't get lost.

Template duplication

pre-install-tentacle-pod.yaml and tentacle-deployment.yaml have ~25 copy-pasted env vars. Same deal with the secrets - there are 3 pairs of files that are basically identical except for hook annotations and the -pre suffix. I'd like to pull the shared bits into named templates

Also, script-pod-template.yaml calls set/unset directly on .Values which mutates it in place for the rest of the render. Should use deepCopy first

Modernization

values.schema.json - worth considering. Right now if someone typos a key or passes the wrong type, Helm silently deploys broken config. A schema would catch that at helm install time with a clear error. Side benefit: editors with YAML LSP support get autocomplete for values files when a schema exists. helm-values-schema-json can auto-generate a starting point from values.yaml so it's not a huge lift.
Pre-install hook is a bare Pod - no backoffLimit, no timeout, won't reschedule if the node dies. Should be a Job at minimum. Might also be worth replacing the hook with an init container so it works with helm template | kubectl apply (GitOps).
Boolean naming is all over the place - disruptionBudgetEnabled, crdDisabled, disableAutoPodCleanup, enabled. In my opinion we should keep it enabled: true/false everywhere.
No top-level rbac.create toggle - Helm best practice is to provide a single flag to disable all RBAC creation. Currently you'd have to know about each component's individual override keys.

Questions

tentacle-secret and tentacle-secret-pre have no data: field - are these populated by the app at runtime? Or leftover?
Would you be open to replacing the pre-install hook with an init container down the road?

liam-mackie

Commenting rather than approving at the moment as if it's changed, it will break the upgrade process. We'll need to update the C# code (or the upgrade script itself) to get the right auto-upgrader service account. This is likely a change that we should include in v3 to enforce compatibility with a server version that includes the changes we'd need to get that account.

I think the easiest way forward is to remove the auto-upgrade changes specifically in this PR and introduce separately in a v3-compatible change.

liam-mackie · 2026-03-17T21:13:12Z

charts/kubernetes-agent/templates/_helpers.tpl

 */}}
 {{- define "kubernetes-agent.autoUpgraderServiceAccountName" -}}
-{{- print "octopus-agent-auto-upgrader" }}
+{{- .Values.autoUpgrader.serviceAccount.name | default (printf "%s-auto-upgrader" (include "kubernetes-agent.name" .) | trunc 63 | trimSuffix "-") }}


⚠️ This change would require changes in Octopus Server to support - see here

Good catch! Reverted the auto-upgrader service account name change - it's back to the hardcoded octopus-agent-auto-upgrader. We can make it dynamic in a v3-compatible change once the server-side code is updated :)

Kept the RBAC rule override support for auto-upgrader roles/clusterroles since those don't change any default behavior - they just add the ability to override rules.

@liam-mackie

- Replace apiGroups wildcards with specific groups for core resources - Fix scriptPodDeleter ClusterRole using apiGroups * instead of core group - Fix hardcoded service account and PDB names to use release-based naming - Add override mechanisms for RBAC rules across all roles - Add validation for override rule inputs - Update tests and snapshots

Co-authored with pre-existing pattern from scriptPods overrides. selfNamespaceRoleRules and targetNamespaceRoleRules now default to [] in values.yaml like all other overrides, removing dead template code. Also align doc comments across all override values.

…ressions)

The Octopus Server C# code hardcodes "octopus-agent-auto-upgrader" as the service account name. Making it dynamic would break existing upgrades until the server-side code is updated. Deferring to a v3-compatible change. Keeps the RBAC rule override support for auto-upgrader roles/clusterroles.

liam-mackie

These changes look good to me! I'd like to get @APErebus to take a look at some point soon, since I think he had opinions on how we configured the role rules in the values.yaml - especially around how we set defaults.

dmaizel requested a review from a team as a code owner March 4, 2026 23:50

dmaizel force-pushed the fix/helm-rbac-permission-scope branch 3 times, most recently from 6a054f8 to 88afd6d Compare March 5, 2026 23:30

liam-mackie reviewed Mar 17, 2026

View reviewed changes

dmaizel added 10 commits March 22, 2026 18:55

fix: add consistent type validation and tests for all RBAC overrides

fc55a90

chore: generate helm-docs

dbd9b18

fix: extract RBAC validation helper, improve docs and test coverage

19298fd

fix(chart): truncate composed PDB and auto-upgrader SA names (add reg…

c066eff

…ressions)

test: use structural RBAC assertions and add escalate verb test

017fc66

refactor: remove redundant tests and hoist loop-invariant template calls

fe64e73

docs(chart): mark worker role as temporary

62bc595

dmaizel force-pushed the fix/helm-rbac-permission-scope branch from 88afd6d to b2fea3c Compare March 22, 2026 16:56

liam-mackie approved these changes Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: scope RBAC permissions and fix hardcoded Helm resource names#522

fix: scope RBAC permissions and fix hardcoded Helm resource names#522
dmaizel wants to merge 10 commits intomainfrom
fix/helm-rbac-permission-scope

dmaizel commented Mar 4, 2026

Uh oh!

changeset-bot bot commented Mar 4, 2026 •

edited

Loading

Uh oh!

dmaizel commented Mar 6, 2026 •

edited

Loading

Uh oh!

liam-mackie left a comment

Uh oh!

liam-mackie Mar 17, 2026

Uh oh!

dmaizel Mar 22, 2026

Uh oh!

dmaizel Mar 22, 2026

Uh oh!

liam-mackie left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dmaizel commented Mar 4, 2026

Uh oh!

changeset-bot bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

dmaizel commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things I want to clean up next

Template duplication

Modernization

Questions

Uh oh!

liam-mackie left a comment

Choose a reason for hiding this comment

Uh oh!

liam-mackie Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

dmaizel Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

dmaizel Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

liam-mackie left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Mar 4, 2026 •

edited

Loading

dmaizel commented Mar 6, 2026 •

edited

Loading