feat(chart): chart-owned PVCs, HTTPRoute filters and extraManifests - #173
Open
aaronspruit wants to merge 1 commit into
Open
Conversation
Four additions. Every one is default-off or defaults to today's behaviour, so rendered output is unchanged unless a value is set. httpRoute gains matches, filters, timeouts, additionalRules and name. The rule was hardcoded to a PathPrefix / match with no filters and no way to add any, so a route behind a proxy could not be handed the real client IP (RequestHeaderModifier), responses could not carry HSTS, and hop-by-hop headers could not be stripped - which meant disabling httpRoute entirely and applying a hand-written route beside the release. matches/filters/timeouts decorate the generated rule so the user never has to know the service name; additionalRules is a raw passthrough rendered before it. name lets an adopted route keep its existing name, which matters when a Gateway API SecurityPolicy or BackendTrafficPolicy targets it by name. The shape follows the seerr chart's route block, which solves the same problem. pms.configPersistence renders the config volume as a chart-owned PersistentVolumeClaim instead of a StatefulSet volumeClaimTemplate. A volumeClaimTemplate claim is named pms-config-<statefulset>-<ordinal> and is owned by the StatefulSet, so it cannot be given a stable name and cannot carry helm.sh/resource-policy: keep. That left configExistingClaim as the only way to attach a deliberately-named claim, so the claim had to be created and kept in sync outside the chart. configExistingClaim still takes precedence. extraPersistentVolumeClaims creates additional claims the release owns, for volumes other than the config one - a backups share, a media library - that otherwise have to be applied out of band and then referenced with extraVolumes. extraManifests renders arbitrary objects with the release. The chart templates a Service, a StatefulSet, an Ingress and an HTTPRoute; anything else belonging to the deployment (Gateway API policies, NetworkPolicy, ServiceMonitor, ExternalSecret) has to be kubectl-applied beside it today, so helm uninstall leaves it behind. Both extra* values and additionalRules accept a list or a map, and are passed through tpl so they can reference values and the chart's named templates. README.md is regenerated with helm-docs 1.14.2.
aaronspruit
requested review from
MarshallAsch and
cookandy
and removed request for
a team
August 14, 2026 03:22
cilindrox
reviewed
Aug 24, 2026
cilindrox
left a comment
cilindrox
left a comment
Member
There was a problem hiding this comment.
Thanks @aaronspruit - left some comments. Would you consider opening separate PRs for the features so we can keep discussion focused?
Member
There was a problem hiding this comment.
this seems redundant, given the extraManifests feature - I think having the option to use the configExistingClaim and adding these there should cover most use cases. Maybe we can include the PVC example under the extraManifests stanza in values.yaml ?
| # name: {{ include "pms-chart.fullname" . }}-extra | ||
| # data: | ||
| # port: "{{ .Values.service.port }}" | ||
| # extraContainers: |
Member
There was a problem hiding this comment.
nit: this example should follow the extraContainers stanza above
Comment on lines
+4
to
+5
| ExternalSecret, ...) can be owned and pruned by this release instead of being | ||
| applied by hand alongside it. |
Member
There was a problem hiding this comment.
nit: wording
Suggested change
| ExternalSecret, ...) can be owned and pruned by this release instead of being | |
| applied by hand alongside it. | |
| ExternalSecret, ...) can be owned by this release. |
Comment on lines
+7
to
+9
| extraManifests may be a list or a map. Each entry is either a YAML object or a | ||
| string; both are passed through `tpl`, so entries can use the release's values | ||
| and the chart's named templates. |
Member
There was a problem hiding this comment.
I think this description is better suited for the values.yaml description of the entry
| */ -}} | ||
| {{- range $manifest := .Values.extraManifests }} | ||
| --- | ||
| {{- if kindIs "string" $manifest }} |
Member
There was a problem hiding this comment.
is this actually required or wouldn't toYaml be greedy enough that we don't care and we can rely solely on the else part of this conditional?
| # -- Name for the HTTPRoute. Defaults to the chart fullname. Set this to keep | ||
| # an existing route's name when adopting one, so anything that targets it by | ||
| # name (Gateway API SecurityPolicy, BackendTrafficPolicy, ...) keeps working | ||
| name: "" |
Member
There was a problem hiding this comment.
instead of name here, we could do a k:v map and a for_each, so k = name. Thoughts?
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
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.
Four additions. Every one is default-off or defaults to today's behaviour, so rendered output is unchanged unless a value is set.
httpRoute gains the ability to set matches, filters, timeouts, additionalRules and name to make it more customizable instead of disabling the httpRoute in the chart and applying it beside the release.
pms.configPersistence renders the config volume as a chart-owned PersistentVolumeClaim instead of a StatefulSet volumeClaimTemplate. A volumeClaimTemplate claim is named pms-config-- and is owned by the StatefulSet, so it cannot be given a stable name and cannot carry helm.sh/resource-policy: keep. That left configExistingClaim as the only way to attach a deliberately-named claim, so the claim had to be created and kept in sync outside the chart. configExistingClaim still takes precedence.
extraPersistentVolumeClaims creates additional claims the release owns, for volumes other than the config one - a backups share, a media library, etc - that otherwise have to be applied out of band and then referenced with extraVolumes.
extraManifests renders arbitrary objects with the release. The chart templates a Service, a StatefulSet, an Ingress and an HTTPRoute; anything else belonging to the deployment (Gateway API policies, NetworkPolicy, ServiceMonitor, ExternalSecret, etc) has to be applied beside the chart today, so helm uninstall leaves it behind.
Both extra* values and additionalRules accept a list or a map, and are passed through tpl so they can reference values and the chart's named templates.
README.md is regenerated with helm-docs 1.14.2.