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.

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

Conversation

lelithium
Copy link

HIP for a split manifest support for larger-than-1MiB manifests.

Also fixes a link to HIP-0018 in README

@ralfv
Copy link

ralfv commented Feb 13, 2023

+1 for such a feature

See also #256


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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants