Set Registry Server Name #465
Conversation
The new `getRegistryAddr` will supply the web address of the registry server for the purpose of looking up the registry auth in the host machine's docker config. When a registryServer is not supplied, the `getRegistryAddr` function will attempt to parse the registry server address from the fully qualified image name. It will handle some special case logic around Dockerhub as well.
With the logic atted to image.go, the user no longer needs to explicitly set the registry server name, as it can be inferred from a fully qualified registry image name.
We updated a few versions, so a new schema is in order.
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
| return "https://index.docker.io/v1/", nil | ||
| } | ||
| // we need the full server address for the lookup | ||
| serverAddr = "https://" + addr |
There was a problem hiding this comment.
Is it ever valid for addr to start with https:// or http:// or some other url prefix? Should we error if addr starts with http(s?)://, or should we trim it?
Especially considering L466-471.
There was a problem hiding this comment.
The https:// is being added for the purpose of looking up the registry from the host docker config and we should not trim it.
As far as I know, with the special snowflake exception of docker.io there is no other url prefix that is valid here. The else block is a best effort attempt to make sure we can always look up the registry from the host config if necessary, since including it is always valid.
provider/image.go
Outdated
| // TODO: this is where we would default to Docker if we wanted to do so. | ||
| // A valid registry address includes a `.` so we could add additional helper checks here. |
There was a problem hiding this comment.
Do we want to default to docker? If we're going to check in a TODO, it would be great to have the TODO associated with an issue.
Uses the https://github.com/distribution/distribution/blob/main/reference/reference.go package to verify correct format of a fully qualified image name. Note that this does not verify the registry itslef; only that the image name has valid format. Additionally, adds error message to clarify a fully qualified image name is required in all cases, which should catch anyone trying to use Dockerhub shortnames..
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
Also improves the output for the error message, removing the print statement and wrapping the error instead.
Does the PR have any schema changes?Looking good! No breaking changes found. |
This PR adds support for not requiring the registry server name to be explicitly set.
Instead, we make a good faith attempt to infer it from the fully qualified image name, giving some basic error messages.
Additionally, some special case handling for Dockerhub is included - if we wanted to push to Dockerhub by default, this is where we could set that.
Fixes #466