-
Notifications
You must be signed in to change notification settings - Fork 195
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
Add .spec.insecure
to HelmRepository
for type: oci
#1288
Conversation
8fc2503
to
1e7b964
Compare
1e7b964
to
cccc1c1
Compare
.spec.insecure
to HelmRepository
.spec.insecure
to HelmRepository
for type: oci
// Insecure allows connecting to a non-TLS HTTP container registry. | ||
// This field is only taken into account if the .spec.type field is set to 'oci'. | ||
// +optional | ||
Insecure bool `json:"insecure,omitempty"` |
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 think we should make this field name more explicit to (1) make it obvious what it does and (2) allow for any future "insecure" flags to be added.
Insecure bool `json:"insecure,omitempty"` | |
InsecurePlainHTTP bool `json:"insecure,omitempty"` |
Insecure
as a prefix still makes sense to me to make it obvious this is an insecure transport.
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 field matches what we have in OCIRepository and what's in the RFC.
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.
That's unfortunate. I still believe it's a way better UX if it had a more telling name.
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 pointed that out in the RFC PR, too, by the way: fluxcd/flux2#3081 (review)
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.
The goal here at present is to get the feature in while continuing to be uniform. Dealing with the naming of the field would be a separate task, as aligning it across kinds would first require an amendment to the RFC.
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 fine with merging this as my comment on the RFC PR also got completely ignored, anyway.
cccc1c1
to
6ee5519
Compare
This PR is not ready to be merged, there are a lot of changes required for the new field to work properly. Thus, I have put this PR on hold for now. |
6ee5519
to
4b230f3
Compare
4b230f3
to
c73b5f0
Compare
Allow connecting to Helm OCI repositories over plain HTTP (non-TLS endpoint). Signed-off-by: Stefan Prodan <[email protected]>
Signed-off-by: Sanskar Jaiswal <[email protected]>
c73b5f0
to
4086c25
Compare
I've tested this PR in a real cluster with a local registry over HTTP and works great. |
Allow connecting to Helm OCI repositories over plain HTTP (non-TLS endpoint).
Example:
Closes: #957
Closes: fluxcd/helm-controller#782
Ref: helm/helm#12128