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

Push to OCI insecure registry #10408

Closed
wants to merge 2 commits into from
Closed

Conversation

pytimer
Copy link

@pytimer pytimer commented Nov 29, 2021

What this PR does / why we need it:

This PR implement push chart to OCI registry, or pull chart from OCI registry.

Fixes: #6324

Usage:

# push
$ ./bin/helm push consul-3.9.5.tgz oci://docker.registry:30001/library --insecure-skip-tls-verify 

# pull
$ ./bin/helm pull oci://docker.registry:30001/library/consul --version 3.9.5 --insecure-skip-tls-verify

# install
$ ./bin/helm install consul oci://docker.registry:30001/library/consul --version 3.9.5 --insecure-skip-tls-verify

# upgrade
$ ./bin/helm upgrade --install consul oci://docker.registry:30001/library/consul --version 3.9.6 --insecure-skip-tls-verify

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

@helm-bot helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 29, 2021
@pytimer pytimer changed the title WIP: Add push to OCI registry WIP: Push to OCI registry Nov 30, 2021
@pytimer pytimer changed the title WIP: Push to OCI registry Push to OCI registry Nov 30, 2021
@scottrigby scottrigby added the bug Categorizes issue or PR as related to a bug. label Jan 13, 2022
@scottrigby scottrigby added this to the 3.8.1 milestone Jan 13, 2022
@pytimer
Copy link
Author

pytimer commented Jan 14, 2022

Need to update this pr code, because oci feature move out experimental. related pr: #10546

@pytimer
Copy link
Author

pytimer commented Jan 18, 2022

Update push/pull chart to oci insecure registry from the latest code, because oci feature move out experimental.

@pytimer pytimer changed the title Push to OCI registry Push to OCI insecure registry Jan 18, 2022
@@ -54,5 +54,9 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
},
}

f := cmd.Flags()
f.BoolVar(&client.InsecureSkipTLSverify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart upload")
Copy link
Contributor

@jdolitsky jdolitsky Feb 8, 2022

Choose a reason for hiding this comment

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

Should this just be --insecure ?

Nvm, looks like this is same flag we use for helm repo commands

Copy link
Author

Choose a reason for hiding this comment

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

I see the helm pull use insecure-skip-tls-verify, so i add this flag in helm push, should i change it?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, lets keep like this

Comment on lines +80 to +84
if p.InsecureSkipTLSverify || p.PlainHTTP {
if err := p.cfg.RegistryClient.WithResolver(p.InsecureSkipTLSverify, p.PlainHTTP); err != nil {
return out.String(), err
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@pytimer - can we also make sure this is used in helm install / helm upgrade?

See #10659 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

I missing it, i will try it tomorrow.

Copy link
Author

Choose a reason for hiding this comment

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

I see the helm install/upgrade code, now --insecure-skip-tls-verify not used in install/upgrade command when scheme is oci://.
I can do it in helm install/upgrade to support insecure OCI registry, but i'm not sure that in this pr or open another pr to do it.

Copy link
Contributor

@jdolitsky jdolitsky Feb 10, 2022

Choose a reason for hiding this comment

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

if you have the bandwidth to add to install/upgrade, please do!

@arukiidou
Copy link

arukiidou commented Feb 11, 2022

it worked with self-signed ssl!

Push

> helm push traefik-10.14.1.tgz oci://[gitlab]:5005/arukiidou/grype --insecure-skip-tls-verify
Pushed: [gitlab]:5005/arukiidou/grype/traefik:10.14.1
Digest: sha256:2b9166854adf09d2aa0d24dd166431539e369caa94bb57750bd87f195917a248

> helm push traefik-10.14.1.tgz oci://[gitlab]:5005/arukiidou/grype
Error: failed to do request: Head "https://[gitlab]:5005/v2/arukiidou/grype/traefik/blobs/sha256:32fd640f11acced4fdb4d13866394e2a94a0085de340b59b807f9a95cbfb1f9b": x509: certificate signed by unknown authority

image

pull

> helm pull oci://[gitlab]:5005/arukiidou/grype/traefik   --insecure-skip-tls-verify
Pulled: [gitlab]:5005/arukiidou/grype/traefik:10.14.1
Digest: sha256:2b9166854adf09d2aa0d24dd166431539e369caa94bb57750bd87f195917a248

> helm pull oci://[gitlab]:5005/arukiidou/grype/traefik
Error: Get "https://[gitlab]:5005/v2/arukiidou/grype/traefik/tags/list": x509: certificate signed by unknown authority

@geowalrus4gh
Copy link

Waiting for this fix to implement OCI based image and helm repository using Harbor.

@pytimer
Copy link
Author

pytimer commented Feb 17, 2022

I will commit helm install/upgrade code when use insecure OCI registry or self-certificate registry on the next week, there are some problems with the code need to be update in my local develop branch.

@pytimer
Copy link
Author

pytimer commented Feb 25, 2022

Now this pr supports helm install/upgrade/pull/push with self-signed certificate OCI registry.

@jdolitsky please review the code if you have time, thanks.

@jdolitsky
Copy link
Contributor

@pytimer - can you please fix the DCO error?

@jdolitsky
Copy link
Contributor

I believe this is in conflict with #10638 ...

@antgamdia - once @pytimer fixes DCO, would you be able to push on top of this PR/fork? I think ClientOptResolver if preferable over the WithResolver proposed here.

@pytimer - let us know if this sounds OK to you

@pytimer
Copy link
Author

pytimer commented Feb 25, 2022

@pytimer - can you please fix the DCO error?

I will fix it today.

@pytimer
Copy link
Author

pytimer commented Feb 25, 2022

@antgamdia - once @pytimer fixes DCO, would you be able to push on top of this PR/fork? I think ClientOptResolver if preferable over the WithResolver proposed here.

OK. If need me to do anything, please tell me, thanks.

Signed-off-by: pytimer <lixin20101023@gmail.com>
Signed-off-by: pytimer <lixin20101023@gmail.com>
@antgamdia
Copy link
Contributor

.antgamdia - once .pytimer fixes DCO, would you be able to push on top of this PR/fork? I think ClientOptResolver if preferable over the WithResolver proposed here.

Sure! Will do during next week

@dreamerkr
Copy link

3.10.3 version, helm push not find -insecure-skip-tls-verify

@sabre1041
Copy link
Contributor

@dreamerkr this PR was not merged yet so it did not make it into the 3.10 release

@dreamerkr
Copy link

@dreamerkr this PR was not merged yet so it did not make it into the 3.10 release

now,How to solve this problem? the best solution use https? but I only want try it

@sabre1041
Copy link
Contributor

@dreamerkr currently, yes only an HTTPS expository is supported. As mentioned, there are ongoing efforts to bring the plain HTTP based integrations

@ashrayjain
Copy link

Can we get a release with this PR merged please?

@mattfarina mattfarina modified the milestones: 3.11.0, 3.12.0, 3.11.1 Jan 18, 2023
@dixudx
Copy link

dixudx commented Feb 2, 2023

Could we get this feature merged? It has been here for a long time. We need this feature. Thanks.

@yxxhero yxxhero modified the milestones: 3.11.1, 3.12.0 Feb 2, 2023
@beaniebag
Copy link

Tested v3.11.3
helm registry works with --insecure
helm push/pull works with --insecure-skip-tls-verify

@willzhang
Copy link

Tested v3.11.3 helm registry works with --insecure helm push/pull works with --insecure-skip-tls-verify

still not work in v3.11.3

root@ubuntu:~# helm version
version.BuildInfo{Version:"v3.11.3", GitCommit:"323249351482b3bbfc9f5004f65d400aa70f9ae7", GitTreeState:"clean", GoVersion:"go1.20.3"}

login

root@ubuntu:~# helm registry login --insecure sealos.hub:5000 -u admin -p passw0rd 
WARNING: Using --password via the CLI is insecure. Use --password-stdin.
INFO[0000] Error logging in to endpoint, trying next endpoint  error="Get \"https://sealos.hub:5000/v2/\": http: server gave HTTP response to HTTPS client"
Login Succeeded
root@ubuntu:~# 
root@ubuntu:~# 
root@ubuntu:~# helm push nginx-13.2.34.tgz oci://sealos.hub:5000/library --insecure-skip-tls-verify
Error: failed to do request: Head "https://sealos.hub:5000/v2/library/nginx/blobs/sha256:d5f440f331d6ce10a5af6c0ce82633dc49217b8f04531d5a27143ff51afe07a2": http: server gave HTTP response to HTTPS client
root@ubuntu:~# 

@beaniebag
Copy link

Tested v3.11.3 helm registry works with --insecure helm push/pull works with --insecure-skip-tls-verify

still not work in v3.11.3

root@ubuntu:~# helm version
version.BuildInfo{Version:"v3.11.3", GitCommit:"323249351482b3bbfc9f5004f65d400aa70f9ae7", GitTreeState:"clean", GoVersion:"go1.20.3"}

login

root@ubuntu:~# helm registry login --insecure sealos.hub:5000 -u admin -p passw0rd 
WARNING: Using --password via the CLI is insecure. Use --password-stdin.
INFO[0000] Error logging in to endpoint, trying next endpoint  error="Get \"https://sealos.hub:5000/v2/\": http: server gave HTTP response to HTTPS client"
Login Succeeded
root@ubuntu:~# 
root@ubuntu:~# 
root@ubuntu:~# helm push nginx-13.2.34.tgz oci://sealos.hub:5000/library --insecure-skip-tls-verify
Error: failed to do request: Head "https://sealos.hub:5000/v2/library/nginx/blobs/sha256:d5f440f331d6ce10a5af6c0ce82633dc49217b8f04531d5a27143ff51afe07a2": http: server gave HTTP response to HTTPS client
root@ubuntu:~# 

Ah, yep you're correct. I confused this with a server using a self-signed certificate

@joejulian joejulian modified the milestones: 3.12.0, 3.13.0 May 5, 2023
@joejulian joejulian added the oci Related to Helm OCI feature label May 9, 2023
@cyxinda
Copy link

cyxinda commented Jun 7, 2023

root@ubuntu:/home/cyxinda/workspace/test# helm version
version.BuildInfo{Version:"v3.12.0", GitCommit:"c9f554d75773799f72ceef38c51210f1842a1dea", GitTreeState:"clean", GoVersion:"go1.20.3"}
root@ubuntu:/home/cyxinda/workspace# helm push test-0.1.0.tgz oci://harbor.192.168.5.6.nip.io/my --ca-file /etc/containerd/certs.d/harbor.192.168.5.6.nip.io/ca.crt --cert-file  /etc/containerd/certs.d/harbor.192.168.5.6.nip.io/harbor.192.168.5.6.nip.io.cert --key-file  /etc/containerd/certs.d/harbor.192.168.5.6.nip.io/harbor.192.168.5.6.nip.io.key
Pushed: harbor.192.168.5.6.nip.io/my/test:0.1.0
Digest: sha256:15244ea1d01d8d7f4617252687c8c143ef543d61c9e13657ef30139258b8944d

root@ubuntu:/home/cyxinda/workspace/test# helm pull oci://harbor.192.168.5.6.nip.io/my/test --version 0.1.0 --ca-file /etc/containerd/certs.d/harbor.192.168.5.6.nip.io/ca.crt --cert-file  /etc/containerd/certs.d/harbor.192.168.5.6.nip.io/harbor.192.168.5.6.nip.io.cert --key-file  /etc/containerd/certs.d/harbor.192.168.5.6.nip.io/harbor.192.168.5.6.nip.io.key
Pulled: harbor.192.168.5.6.nip.io/my/test:0.1.0
Digest: sha256:15244ea1d01d8d7f4617252687c8c143ef543d61c9e13657ef30139258b8944d
root@ubuntu:/home/cyxinda/workspace/test# ls
test-0.1.0.tgz

@joejulian joejulian added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Has One Approval This PR has one approval. It still needs a second approval to be merged. and removed awaiting review labels Aug 1, 2023
@joejulian
Copy link
Contributor

@pytimer Can you please rebase?

@joejulian joejulian removed this from the 3.13.0 milestone Aug 11, 2023
@BesartSulejmani
Copy link

When will this be merged? We actually need this badly.

@sabre1041
Copy link
Contributor

@BesartSulejmani Capabilities should now be available

#12128

We should be good to close this PR as I believe that everything has been covered? @mattfarina @joejulian @gjenkins8

@joejulian
Copy link
Contributor

We should be good to close this PR as I believe that everything has been covered

I agree. Closing

@joejulian joejulian closed this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug. Has One Approval This PR has one approval. It still needs a second approval to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. oci Related to Helm OCI feature size/M Denotes a PR that changes 30-99 lines, ignoring generated files. Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How can I push chart to an insecure OCI registry with helm v3