Apply skipping certification verification to authorization as well#287
Merged
Conversation
Member
|
Can we have a test like this? nerdctl/cmd/nerdctl/push_test.go Line 121 in f6f48cd Can be a separate PR though |
AkihiroSuda
reviewed
Jul 12, 2021
| Transport: tr, | ||
| } | ||
| regOpts = append(regOpts, docker.WithClient(client)) | ||
| regOpts = append(regOpts, docker.WithClient(newInsecureClient())) |
Member
There was a problem hiding this comment.
No need to call newInsecureClient() twice
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Member
Author
I'll work on it. We'll need to host an authorization server in CI (maybe we can use https://github.com/cesanta/docker_auth). |
Member
|
@ktock Do you plan to work on tests before merging this PR or after? |
Member
Author
|
Sorry for the late. I add the test in this week but please merge it if this patch is needed urgently. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #278
This commit fixes the issue that skipping certificate verification didn't applied to the access to the authorization server.
The behaviour that propagates skip-verification config to the authorizer is compatible to the current behaviour of
ctr.