Fix OCI errors

[jwhb]

Changes

OCI errors were updated to be even with the spec:

• TAG_INVALID and MANIFEST_UNVERIFIED were removed from OCI spec: Resolve several open issues opencontainers/distribution-spec#206
• TOOMANYREQUESTS was added

Tested on CentOS 8.2.2004 and KinD with Kubernetes v1.21.1.

Why?

After bootstrapping a krustlet-wasi node I received this error:

Jul 17 18:02:33.250 ERROR kubelet::state::common::image_pull: error=error decoding response body: unknown variant TOOMANYREQUESTS, expected one of BLOB_UNKNOWN, BLOB_UPLOAD_INVALID, BLOB_UPLOAD_UNKNOWN, DIGEST_INVALID, MANIFEST_BLOB_UNKNOWN, MANIFEST_INVALID, MANIFEST_UNKNOWN, MANIFEST_UNVERIFIED, NAME_INVALID, NAME_UNKNOWN, SIZE_INVALID, UNAUTHORIZED, DENIED, UNSUPPORTED, TOO_MANY_REQUESTS at line 4 column 31

and after that more conveniently:

Jul 17 18:05:44.213 ERROR kubelet::state::common::image_pull: error=OCI API error: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit on https://registry-1.docker.io/v2/kindest/kindnetd/manifests/v20210326-1e038dc5

[radu-matei]

Hi, and thanks for your contribution!

How does this impact clients trying to connect to servers that have not been updated to conform to the latest spec changes, and might return one of the two removed variants?

I definitely agree with adding the new one, not sure about the removals.

[jwhb]

Hi @radu-matei,
thanks for your feedback. I reverted the removal of TAG_INVALID and MANIFEST_UNVERIFIED, so this PR only adds support for the TOOMANYREQUESTS error.
At this moment I can't judge about the impact of removing the two deprecated error codes.

[bacongobbler]

Can this be TooManyRequests to match the casing style of other enums? The serde macro on line 39 will change these all to SCREAMING_SNAKE_CASE. If the casing is incorrect then it will not be cased correctly.

To clarify:

• Toomanyrequests -> TOOMANYREQUESTS
• TooManyRequests -> TOO_MANY_REQUESTS

[jwhb]

I already fiddled with the SCREAMING_SNAKE_CASE and, as you said, putting TooManyRequests will map to TOO_MANY_REQUESTS. In the OCI spec it is TOOMANYREQUESTS, so I tricked the screaming snake with the value Toomanyrequests.

I hope I understood you correctly, but I couldn't make it work with TooManyRequests.

[jwhb]

So in the PR description I previously mentioned TOO_MANY_REQUESTS, which doesn't exist in the spec...

[bacongobbler]

I agree with @radu-matei's assumption. Let's leave in "deprecated" error operations to retain backwards compatibility. It is quite possible there are several OCI registries still relying and reporting those errors. It doesn't affect any existing behaviour if we keep it in; in fact it will be more assistive to the user if we continue to report these errors until we can be completely sure they've been deprecated.

Feel free to add a comment to those operations to note they've been deprecated with a link to the deprecation notice in the spec's changelog. That way we have a note to look back to in future refactors.

[bacongobbler]

TOOMANYREQUESTS

Ahh... Never mind. I see now why the casing is different. Yay for consistency!

LGTM

[radu-matei]

Could you please also add a deserialization test for the new variant?
There are some examples in the same file that should guide you on adding a new one.

Thank you so much!

[jwhb]

Could you please also add a deserialization test for the new variant?

I would, but apparently there is nothing to deserialize. We know the error code TOOMANYREQUESTS from docker.io. This is a curl output I gathered:

$ curl -fsSL -H "Authorization: Bearer $TOKEN" "https://registry-1.docker.io/v2/ansible/ansible/manifests/latest"
curl: (22) The requested URL returned error: 429 Too Many Requests

No body in the response. The OCI dist spec says:

A 4XX response code from the registry MAY return a body in any format.

Should we assume an error object like so?

    {
        "errors": [
            {
                "code": "TOOMANYREQUESTS",
                "message": "pull request limit exceeded",
                "detail": "You have reached your pull rate limit."
            },
        ]
    }

EDIT: @radu-matei: I just added the deserialization test. The JSON is just a blind guess.