oci-distribution: fix push to JFrog (and ghcr.io) #670
Conversation
|
First, could you please run |
I just rebased on top of |
|
No, rebasing is enough, as we fixed that already in main, good call. |
The oci-distribution crate is currently pushing layers using the "monolithic upload" API offered by distribution v2[1]. According to the official specification, the layer has to be uploaded using this API: ``` PUT /v2/<name>/blobs/uploads/<uuid>?digest=<digest> ``` Prior to this commit, the code was building the destination url using simple string interpolation. That resulted in making a request like: ``` PUT /v2/<name>/blobs/uploads/<uuid>&digest=<digest> ``` Interestingly enough, this didn't seem to be a problem to the distribution servers. Apparently the Go implementation is more relaxed when it comes to parsing an incoming request. The GitHub Package registry (ghcr.io), on the other hand rejects these requests with a 404 response. This commits fixes the problem by using the `Reqwest::Url` structure to create the target url. [1] https://github.com/distribution/distribution/blob/v2.7.1/docs/spec/api.md#monolithic-upload Signed-off-by: Flavio Castelli <fcastelli@suse.com>
|
@radu-matei I see that you commented on a version of this PR before I force pushed and your comment got lost in the GitHub database void 🙈 Your comment:
I cannot see where you commented exactly,but I assume it was about the |
|
I also fixed some clippy warnings in the way. |
|
@kflansburg I know you have a PR that improves the OCI crate (#682), could you please take a look? |
|
This looks great. My only feedback may be to ask for unit tests so that these changes are covered by tests. Otherwise I'm okay with merging this. Thanks for working on this! |
Introduce the `RequestBuilderWrapper` internal type that allows to call to request building functions that can consume information stored in the client. The main goal of the wrapper type is that functions that are implemented for it that alter the request builder wrapper are composable. When all the composing that requires some context from the client has been done, `into_request_builder()` is called, handing a regular `RequestBuilder` type that can be used to further add more information to the request, and eventually, trigger it. Signed-off-by: Rafael Fernández López <rfernandezlopez@suse.com>
When issuing requests to the OCI registry, only set the `Accept` header on endpoints that will use it in order to produce a response taking this parameter into account. The `oci-distribution` crate is currently having issues to push to the JFrog Artifactory registry, because this registry is strict in what headers are provided. This behavior is not specified in the OCI distribution specification, so the OCI registry implementors are open to behave in different ways. In any case, sending the `Accept` header on all requests with a fixed set of MIME types can be improved, and this change applies this header only on the requests that are expecting the server to reply with one of the given MIME types. Signed-off-by: Rafael Fernández López <rfernandezlopez@suse.com>
Signed-off-by: Rafael Fernández López <rfernandezlopez@suse.com>
Thank you! I have updated the PR with relevant tests, for |
Supersedes: #617
There are several relevant commits:
6769e72: this is a fix for servers that are strict about the first query argument on the URI. JFrog is strict as GitHub registry in this sense, and they reject a request of the form
uri&query=. They expecturi?query1=<value>&query2=<value>.b64f3e3: building block for the next commit. This exposes a
RequestBuilderWrappertype that can be used to initialize from a client, with a certain operation on it, and that allows to execute modifier functions that might need client information. When done, this type can be converted to a regularRequestBuilder, where further operations can be performed, and eventually, issue the request. This is a way for easily extendable operations on theRequestBuilderWrapperbecause those operations are composable.e8fb47a: this is fixing Pushing to an artifactory OCI registry results in 406 Not Acceptable kubewarden/kwctl#59. If the server is strict, if an
Acceptheader is sent, and it knows beforehand that this operation won't generate an object at all, or of any of the given MIME types in theAcceptheader, it can return a 406 Not Acceptable response, as JFrog is doing in this case.All this being said: this PR is not fully fixing pushing to JFrog because it is not compliant with https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-manifests. When pushing the manifest JFrog is not returning a
Locationheader, but this is not on theoci-distributionplate. I'm going to report that to the JFrog team.