google / go-containerregistry

Go library and CLIs for working with container registries
Apache License 2.0
3.04k stars 522 forks source link

using https scheme in image URI fails when parsing #779

Open tompscanlan opened 3 years ago

tompscanlan commented 3 years ago

Hi, I seem to have it a problem parsing https as a scheme. Original issue was filed here: https://github.com/k14s/kbld/issues/48

The issue boils down to

Error parsing reference: "https://harbor.internal.com/mygroup/test-carvel:kbld-rand-1601525839289730000-62652921773" is not a valid repository/tag: invalid reference format

If I remove https://, the repo url is parsed correctly.

possibly related to #766 ?

jonjohnsonjr commented 3 years ago

This is working as intended, but I'm open to adding support for this as a new feature. Image references are not exactly URLs (as described in the godoc for https://godoc.org/github.com/google/go-containerregistry/pkg/name (which I should probably rewrite to be in the README)).

I think I'd be okay with mapping http:// to https://godoc.org/github.com/google/go-containerregistry/pkg/name#Insecure and otherwise just stripping a https:// prefix.

tompscanlan commented 3 years ago

@jonjohnsonjr even an error specifically calling out that the scheme should not be part of the image name would have helped me. While I loosely understood that image names are not urls, I didn't realize that was part of the spec for naming.

Thanks for sharing that godoc. You document "we do our best to infer if we use http or https from the given hostname." If a scheme is supplied, wouldn't using it to determine http or https be a best guess?

Your proposal sounds close to that, so I'm on board if you think that's the right direction.

hasheddan commented 3 years ago

@jonjohnsonjr happy to help out here as needed :+1:

jonjohnsonjr commented 3 years ago

I'd merge a PR that just makes this error message better for now @hasheddan 👍

We may want to actually interpret http:// and https:// in the future, but I'd like to see how that affects the code and think about situations where it would be undesirable before committing to it. We could file a separate issue to track that and solicit feedback.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

joe-kimmel-vmw commented 2 years ago

Just chiming in that passing https:// into the library is still a minor stumbling block and from my perspective the ideal resolution would be for this library to correctly interpret and strip the protocol. Thanks for the library and all your maintenance efforts!