Skip to content

Allow underscores in names#31

Open
holograph wants to merge 2 commits intoazimuth-cloud:mainfrom
holograph:main
Open

Allow underscores in names#31
holograph wants to merge 2 commits intoazimuth-cloud:mainfrom
holograph:main

Conversation

@holograph
Copy link

As the name implies, this adds support for underscore in various names. I'm not entirely sure why this model is so strict to begin with (the Go SDK only strips non-printable characters and leading/tailing whitespace), but I can attest that underscores work perfectly fine in e.g. chart dependency names, which is where I ran into this :-)

Copy link

@JohnGarbutt JohnGarbutt left a comment

Choose a reason for hiding this comment

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

I think we might need to split chart names and release names, each having their own type, to make sure we are keep correctly checking the chart names.

Would that work OK for your use case?


#: Type for a name (chart or release)
Name = constr(pattern = r"^[a-z0-9-]+$")
Name = constr(pattern = r"^[a-z0-9_-]+$")

Choose a reason for hiding this comment

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

I think its because of the Chart name restrictions here:
https://helm.sh/docs/chart_best_practices/conventions/#chart-names

Maybe we need a different type for the release names, which are not as strict, it would seem?

Choose a reason for hiding this comment

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

I've actually looked into our use case a bit more carefully, apparently (per the documentation) installing a chart that has an alias dependency will, and I quote here, "put a chart in dependencies using alias as name of new dependency."

Our issue stems from getting an installed chart's metadata from a live cluster. We use mimir-distributed 5.3.0 which installs rollout_operator. Calling pyhelm3.Client(...).get_current_revision(...).chart_metadata() blows up because the returned dependency name is actually the declared alias rollout_operator, which includes an underscore.

I've pushed another commit to better express this, please have a look @JohnGarbutt

Copy link

@benbz benbz Jul 15, 2025

Choose a reason for hiding this comment

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

I've run into a similar problem with a dependency that has an alias with upper-case characters which is blocked by the same restriction

Aliases may sometimes include underscores, and in turn may be returned a dependency name when using helm.get_current_revision(...).chart_metadata().
@holograph holograph requested a review from JohnGarbutt July 10, 2025 10:16
@holograph
Copy link
Author

Ping @JohnGarbutt? Sorry for being a bother, just want to see this fixed 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments