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

Helm3 CRDs #10

Merged
merged 4 commits into from
Jan 23, 2020
Merged

Helm3 CRDs #10

merged 4 commits into from
Jan 23, 2020

Conversation

yspotts
Copy link
Contributor

@yspotts yspotts commented Jan 22, 2020

This provides support for Helm 3 CRDs. In Helm 3, CRDs are defined in the new crds folder: https://helm.sh/docs/topics/chart_best_practices/custom_resource_definitions/.

This solves an issue of using Kong as a dependency (subchart) of an umbrella chart and using the CRD (for example KongPlugin). This is currently not possible (or at the very least very challenging) in Helm 2 charts; see here: (helm/helm#2994).

I have followed the strategy of this project for example helm/charts#18721 to ensure backward compatibility for helm2 by copying over the resource definitions to the templates directory as well. (See this as well: helm/charts#19008)

Note that the new crds directory does not evaluate templates, and so values must be hard coded. Again, I followed the path outlined in the links above.

Copy link
Member

@hbagdi hbagdi left a comment

Choose a reason for hiding this comment

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

Can we please update the docs in the README to document that installCRDs will not work with Helm 3?
This is similar to how https://github.com/helm/charts/pull/18721/files updates the documentation.

metadata:
name: kongplugins.configuration.konghq.com
labels:
app.kubernetes.io/name: name
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this label from all the resources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uggh, I meant to have the value be kong. Should I just remove the entire label or replace name with kong?

Copy link
Member

Choose a reason for hiding this comment

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

Let's just remove it because it doesn't provide any value as far as I can see.

@yspotts
Copy link
Contributor Author

yspotts commented Jan 22, 2020

@hbagdi good call - will do

@hbagdi hbagdi merged commit 0a73684 into Kong:master Jan 23, 2020
@hbagdi
Copy link
Member

hbagdi commented Jan 23, 2020

Thank you for your contribution!

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.

2 participants