-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: add helm chart field #497
Conversation
dd8f1a3
to
89af502
Compare
@@ -46,6 +48,10 @@ func (Addons) VariableSchema() clusterv1.VariableSchema { | |||
Description: "Cluster configuration", | |||
Type: "object", | |||
Properties: map[string]clusterv1.JSONSchemaProps{ | |||
"helmChartRepository": { | |||
Pattern: patterns.HostAndOptionalPort, | |||
Description: "Optional OCI registry used to pull helm charts for adons", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just an OCI registry or could it also be https helm repository somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could expand this description and add example too via Example
property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the difference between helm repository and OCI registry is. That entity would need to host all of the charts though. because the repository field for each addon gets set to this value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helm repositories used to be supported only accessible via https. OCi support was later introduced meaning charts could be stored and referenced from OCI registries. The only difference is the scheme in the repository URL, using oci:// instead of https://. I think we should support both here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if all the users are going to be able to push helm charts to a single helm repository im also not sure how to validate it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's only supporting pulling from one helm repository though right for all Helm addons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, exactly the idea is that this source will have all of the helm charts the user needs
adds a helm chart field for customers who use a private OCI registry
this closes d2iq-labs#39