Skip to content

Added allow capital letter in dependency name#9

Open
turdusmerula wants to merge 1 commit intoazimuth-cloud:mainfrom
turdusmerula:main
Open

Added allow capital letter in dependency name#9
turdusmerula wants to merge 1 commit intoazimuth-cloud:mainfrom
turdusmerula:main

Conversation

@turdusmerula
Copy link

@turdusmerula turdusmerula commented Apr 21, 2024

I am deploying the Authentik helm chart with pyhelm3 and I receive an error I think is not okay.

Inside the values.yaml file there is the following block:

serviceAccount:
  # -- Create service account. Needed for managed outposts
  create: true
  # -- additional service account annotations
  annotations: {}
  serviceAccountSecret:
    # As we use the authentik-remote-cluster chart as subchart, and that chart
    # creates a service account secret by default which we don't need here,
    # disable its creation
    enabled: false
  fullnameOverride: authentik

The dependency name serviceAccount does not validate with the rule r"^[a-z0-9-]+$" from models.py:42 because it contains a capital letter.
However the chart deploys perfectly fine with helm3 with no issue on the capital letter for the dependency name, I think the correct rule should be r"^[a-zA-Z0-9-]+$"which corrects the error I'm facing

@turdusmerula turdusmerula changed the title Added allow capital letter in resource name Added allow capital letter in dependency name Apr 21, 2024
@voidlily
Copy link

voidlily commented Jul 16, 2024

I'm also seeing this with the policy-reporter helm chart and the kyvernoPlugin dependency. This PR would fix that chart as well.


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

Choose a reason for hiding this comment

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

I think the problem we have is this type gets re-used in a few too many places. Its based on this documentation:
https://helm.sh/docs/chart_best_practices/conventions/#chart-names

I think we need to rework this one, potentially.

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.

3 participants

Comments