bytecodealliance / wasm-pkg-tools

Apache License 2.0
48 stars 11 forks source link

Improve Error Messages for Incorrect Reference Format when Pushing to Azure Container Registry #64

Closed duffney closed 2 months ago

duffney commented 3 months ago

When pushing a component to Azure Container Registry, I encountered an authentication error. However, the root cause was actually an issue with the reference format being passed into the flag.

 wkg oci push wasiclippy.azurecr.io hello_wasi_http.wasm -u $username -p $password
Error: Unable to push image
Caused by:
    Server error: url https://index.docker.io/v2/library/wasiclippy.azurecr.io/blobs/uploads/, code: 401, message: {"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":[{"Type":"repository","Class":"","Name":"library/wasiclippy.azurecr.io","Action":"pull"},{"Type":"repository","Class":"","Name":"library/wasiclippy.azurecr.io","Action":"push"}]}]}

Improvement Suggestion: It would be helpful if the error message indicated if the reference format is incorrect and specify the required format.

yoshuawuyts commented 3 months ago

To add some additional context here: @duffney and I were on stream earlier today, and we actually ran into this twice: once when pushing to ghcr, and once when pushing to azure artifacts. The error on ghcr ended up being more descriptive - which is how we found out that we were actually writing the reference string incorrectly for both.

If wkg would validate reference strings locally before pushing, that would make it so even if the server provides a less-than-helpful error, the user still knows exactly what they got wrong. Which overall should make for a more pleasant user-experience.

thomastaylor312 commented 2 months ago

Hmmmm...this is an interesting problem. So technically the CLI is using FromStr to parse your string into a reference, so if it were an invalid reference (according to the regex) then it should fail and tell you it is invalid. This might actually be a parsing issue with oci-distribution itself. According to the regex, everything before the tag is the "name" which gets split into a domain and the repository. So technically what happens is that wasiclippy.azurecr.io matches the regex, but is missing a repository. As far as I can tell from the spec and what done in distribution, this should be validated when parsing a manifest. I am going to close this issue and open a bug in the crate

duffney commented 2 months ago

Appreciate the response @thomastaylor312, thank you!