docker: Mirantis Secure Registry doesn't support POST#5288
docker: Mirantis Secure Registry doesn't support POST#5288kzys wants to merge 1 commit intocontainerd:masterfrom
Conversation
kzys
force-pushed
the
msr-500
branch
2 times, most recently
from
8a7b672 to
1cd2de5
Compare
kzys
changed the title
Mirantis Secure Registry (MSR) doesn't support POST and returns 500. While retrying on 500 could be problematic, it is hard to detect MSR since the Server header on the HTTP response is just "nginx/1.16.1". Since the retry here is only once with GET, the negative impact is negligible. https://www.mirantis.com/software/docker/image-registry/ Signed-off-by: Kazuyoshi Kato <[email protected]>
|
Build succeeded.
|
|
@AkihiroSuda I will submit the cherry-pick PR once this PR has been merged. |
dmcgowan
left a comment
dmcgowan
left a comment
There was a problem hiding this comment.
500 is not a status code we want to fallback on. This is definitely one that needs to be fixed by the registry operator.
|
I do agree we should not retry on 500. However MSR is a product that people can install and update on their own cadence. Even Mirantis fixes the issue, it may take weeks/months to actually have the fix in all MSR installations. One thing we could do is checking MSR based on a heuristic logic regarding the JSON response like below and only have a fallback when it is like MSR. It wouldn't address the risk of misbehaving though. |
|
Another path we could take is having a configuration option to use |
|
@dmcgowan This is the DTR codebase, from what I understand, but maybe it is being fronted by something else as I don't think we had any issues reported with original DTR/Docker EE and containerd? |
|
Sorry, seems "MSR doesn't support POST" is oversimplification of the situation. Here is what Docker does. POST to /auth/token actually works here. And here is what containerd does. |
|
Docker's POST above was actually getting an refresh token, whereas containerd was getting an access token. So the correct summary of the issue is "Mirantis Secure Registry doesn't support POST for getting an access token". |
|
@dmcgowan How about adding another AuthorizerOpt that let authHandler uses |
|
I tried to reach out to the Mirantis team through our joined slack channel to have a look at this |
The Authorizer is already interfaced to allow for using a custom authorizer. If there needs to be more options to override the authorizer, we should make that change. This implementation of the authorizer shouldn't have any changes which alter the protocol though. I'm going to close this one, but this can be tracked further in an issue until we find an acceptable solution or the bug is fixed by the registry operators. |
|
Thanks @corhere 👍 |
8 participants
Mirantis Secure Registry (MSR) doesn't support POST and returns 500.
While retrying on 500 could be problematic, it is hard to detect MSR
since the Server header on the HTTP response is just "nginx/1.16.1".
Since the retry here is only once with GET, the negative impact is
negligible.
https://www.mirantis.com/software/docker/image-registry/