-
Notifications
You must be signed in to change notification settings - Fork 250
image/registryclient: set insecure and return early in blobMirroredRepository #1096
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
Conversation
|
/lgtm |
| } | ||
| if len(alternates) == 0 { | ||
| return attemptErr | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks correct but now I want to see a unit test for it. This is the real loop we need to have unit tests on anyway.
| if err != nil { | ||
| return err | ||
| } | ||
| if len(alternates) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smarterclayton alternatively we could check for nil strategy in https://github.com/openshift/library-go/blob/master/pkg/image/registryclient/client_mirrored.go#L207 similarly to initialRepos or errorRepos
|
@dmage @ricardomaraschini @smarterclayton this should be ready for final review |
ricardomaraschini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
/assign @smarterclayton |
|
/retest |
|
/test unit |
1 similar comment
|
/test unit |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dmage, ricardomaraschini, smarterclayton, soltysh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is followup to #1084 fixing:
insecureproperly forblobMirroredRepositorygolang.org/x/net/contextwith built-incontextpackage