Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HIP-0019: Split manifest with Secrets storage driver #284

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

HIP-0019: Split manifest with Secrets storage driver #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa8cb43
fix: add link to HIP-0018
lelithium Feb 6, 2023
40c8a7e
feat: hip-0019 split manifest proposal
lelithium Feb 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hips/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ restricted markdown format and can be found in the
- [hip-0014: Helm Triage Maintainers](hip-0014.md)
- [hip-0015: Annotation for Images used by Helm Charts](hip-0015.md)
- [hip-0017: Helm OCI MediaType Registration](hip-0017.md)
- [hip-0018: Add Repository URL and tarball digest to a chart's release metadata](hip-0018.md)
65 changes: 65 additions & 0 deletions hips/hip-0019.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
---
hip: "0019"
title: "Secrets storage driver support for split manifests"
authors: [ "Louis Cabrol" ]
created: "2023-02-06"
type: "feature"
status: "draft"
---

## Abstract

Helm, by default in version 3, stores its Release manifest in a Secret object in the target Kubernetes namespace, after a double base64 encoding + GZIP compression.

This object type, [by Kubernetes design](https://kubernetes.io/docs/concepts/configuration/secret/#restriction-data-size) has a maximum size allowed for data restricted to 1Mib.

This imposes a limit on the maximum number of templates a Helm Release can manage, as well as limits the overall complexity a Helm Chart can reach before exceeding this limit.

## Motivation

This has been reported a number of times ([here](https://github.com/helm/helm/issues/10986), [here](https://github.com/helm/helm/issues/8281), and many others within those two links). As Charts keep growing in complexity through modular, optional components, and Releases keep growing in the number of Kubernetes objects they manage, more and more people will keep hitting this limit.

Deploying a Chart that exceeds this preset size currently yields a Kubernetes error, without first warning the user that the Release Manifest size exceeds the maximum, which isn't a good user experience.

My specific motivation for this fix is a complex, modular Helm Chart that deploys Grafana dashboards. As such, it contains (many) dashboard templates as JSON that are templated through Helm into ConfigMap objects. Those template files cannot be `helmignored`, and their cumulated size alone is close to the Secret size limit, before encoding and compression. As such, I'm starting to run into issues when deploying to complex environments that require a lot of those dashboard definitions, which could be solved by splitting the chart among different sub-charts and/or sub-releases, but this would be working around the problem, and not adressing it.

## Specification

The proposal is to create a new "type" of Secret, `helm.sh/partial.v1`, that would enable the Release Manifest to be split across multiple secrets as needed, named after the following format `sh.helm.partial.v1.<release name>.v<release version>-<partial chunk number>`. The presence of further "partial" secrets would then be indicated through an optional `continuedIn` label. Changes should only be made within the Secrets storage driver, so as to retain the current driver interface.
lelithium marked this conversation as resolved.
Show resolved Hide resolved

## Backward compatibility

Containing changes to the Secrets storage driver means there's no compatibility issue with other storage types.

For Secrets, marking the Secrets with `helm.sh/partial.v1` for partial elements means identifying releases still only relies on the presence of secrets that are named after this format `sh.helm.release.v1.<release name>.v<version>` with the `helm.sh/release.v1` type.

Older versions of Helm would not be able to interact with the new split manifests in most cases.
Copy link
Member

Choose a reason for hiding this comment

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

Older versions of Helm failing to be able to interact with the release is fine in itself I think.

But, not sure if it would be okay to enable the split/partial manifests by default. We need to try hard to account for all scenarios. And it is possible that user might install/upgrade with a new version of Helm, then attempt to interact with that release with an older version. If we have split the manifest by default, they would be broken.

OTOH, it (to me) is completely fine if a user "opts-in" to split manifests. Then it is on them to ensure their processed only use a new enough Helm version

Copy link
Author

@lelithium lelithium Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @gjenkins8, thanks for the review !

An opt-in feature gate make sense for sure.

According to https://kubernetes.io/docs/concepts/configuration/secret/#restriction-data-size this max size is always 1Mib, and we know the size of the manifest to store before applying, so the behavior could be

  • If not opted-in, releases larger than the max Secret size should error out early instead of at apply time
    • This error could mention the split manifest opt-in
  • If opted-in, they're split using the split manifest feature


## Rationale

The only storage type that contains this hardcoded limit is Secrets. ConfigMap size can be increased at the `etcd` level, and the SQL driver doesn't have such limitations by design.

This expansion to the Secrets spec allows users not to maintain a separate PSQL database to store Helm release manifests, instead allowing them to make use of the internal components Kubernetes provides, while maintaining backwards compatibility with current releases.

## How to teach this

This size limit currently isn't well documented within Helm, and only found through the aforementioned issues I linked to, so there isn't much to teach. There would not be any user-facing changes, barring the ability to deploy larger charts.

## Reference implementation

[Helm PR](https://github.com/helm/helm/pull/11791)

## Rejected ideas

None as far as I know

## Open issues

None as far as I know

## References

- [Kubernetes Secrets data size restrition](https://kubernetes.io/docs/concepts/configuration/secret/#restriction-data-size)
- Issues:
- [An issue discussing this problem](https://github.com/helm/helm/issues/10986)
- [Another issue discussing this problem](https://github.com/helm/helm/issues/8281)