oci: Add flag --plain-http to enable working with HTTP registries #12128
Conversation
There was a problem hiding this comment.
@aryan9600 Thank you so much for this much needed enhancement. Started to test various different scenarios. Unfortunately, the following fails:
helm upgrade -i --plain-http -n plain-http --create-namespace plain-http oci://<registry>/helm/plain-http
Error: Get "https://<registry>/v2/helm/plain-http/tags/list": tls: failed to verify certificate: x509: certificate signed by unknown authoritySpecifying the version of the chart does work
Add a new flag `--plain-http` to the following commands: * `helm install` * `helm pull` * `helm push` * `helm template` * `helm upgrade` * `helm show` This flag instructs the registry client to use plain HTTP connections, thus enabling upload/download of charts from OCI registries served at an HTTP endpoint. Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
|
@sabre1041 this should be fixed, the call to fetch tags also respects this flag now. |
|
@aryan9600 indeed the tags listing is working, however unit test |
e938e36 to
b0a593a
Compare
Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
|
ping @scottrigby |
|
I would like to suggest naming this flag something else to make the security implications clearer, e.g. This has precedence: |
Hi @lindhe Just to be clear, |
Just added comment to #11167 that "--insecure-skip-tls-verify" doesn't work either for "helm dep update": And of course: |
There was a problem hiding this comment.
@aryan9600 Thanks so much, this PR looks and works great 👏
Without this PR, plain HTTP registries - except for localhost - fail with a message:
http: server gave HTTP response to HTTPS client
With this PR, passing --plain-http flag allows this to succeed.
This PR is good to go 👍
I suggest as a follow-up we update the Helm website/docs to clarify the purpose of --plain-http and --insecure-skip-tls-verify since these seem to get confused.
| @@ -39,15 +39,17 @@ func TestOCIGetter(t *testing.T) { | |||
| ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") | |||
| timeout := time.Second * 5 | |||
| transport := &http.Transport{} | |||
| insecureSkipTLSverify := false | |||
| insecureSkipVerifyTLS := false | |||
There was a problem hiding this comment.
Note for later. This was initially confusing because this var was initially misnamed in #7254. We can make a follow-up PR to address this consistently everywhere else it occurs, but your change is fine here. Thanks!
What this PR does / why we need it:
Add a new flag
--plain-httpto the following commands:helm installhelm pullhelm pushhelm templatehelm upgradehelm showThis flag instructs the registry client to use plain HTTP connections,
thus enabling upload/download of charts from OCI registries served at
an HTTP endpoint.
closes #11682
Special notes for your reviewer:
If applicable: