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

add back validation (maybe?) for disable registry url validation in prune #15291

Closed
deads2k opened this issue Jul 18, 2017 · 4 comments
Closed

Comments

@deads2k
Copy link
Contributor

deads2k commented Jul 18, 2017

This was removed in disable registry url validation in prune

reenable

if _, err := url.Parse(o.RegistryUrlOverride); 
   err != nil { in pkg/cmd/admin/prune/images.go
@mfojtik
Copy link
Contributor

mfojtik commented Jul 19, 2017

@deads2k @miminar ideally I would drop that option in favor of https://trello.com/c/2cHkrqIu/954-provide-public-pull-url-for-images

@deads2k
Copy link
Contributor Author

deads2k commented Jul 19, 2017

@deads2k @miminar ideally I would drop that option in favor of https://trello.com/c/2cHkrqIu/954-provide-public-pull-url-for-images

Your call. Presumably the code you're calling will do something reasonable.

@miminar
Copy link

miminar commented Jul 19, 2017

We need to support the flag anyway. Even the best algorithm we can make cannot be right all the time.

Therefor the validation should be re-enabled. The argument isn't really a URL, it's rather a shortened pull-spec (just the registry part). I thought we could use docker's validation, but they still use url.Parse(). And their validation for insecure registries is private. I'll come up with something.

@miminar
Copy link

miminar commented Jul 26, 2017

Addressed by #14914

openshift-merge-robot added a commit that referenced this issue Aug 17, 2017
Automatic merge from submit-queue

Image pruner: Determine protocol just once

Determine the registry protocol once. Do not change to other protocol during the run. This will produce nicer output without unrelated protocol fallback errors.

A follow-up for #14114 and #14405
Resolves #15291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants