Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 403 error when using moc compiler version > 0.6.2 #41

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

ByronBecker
Copy link
Contributor

Fix

TLDR: To fix 403 forbidden compiler release downloads for versions > 0.6.2 that hit GitHub releases, update the download_compiler reqwest.get to set a User-Agent header

Motivation

I was running into issues where locally, I'm able to build my code and use moc version 0.6.20, but my GitHub CI process runs into this forbidden error when trying to download the binary (note that I started with this template, and just update the compiler version https://github.com/kritzcreek/motoko-library-template).

I don't run into this same error when using moc 0.6.2 and below, and I had tried every other version above it, but I run into the same forbidden error with 0.6.11 and 0.6.19.

I dug into this further and saw that every version up to 0.6.2 was released on https://download.dfinity.systems/motoko/{version}/{os_arch}/motoko-{version}.tar.gz,
but since then, from 0.6.3 up to 0.6.21 has been released on GitHub via the https://github.com/dfinity/motoko/releases/download/{version}/motoko-{os}-{version}.tar.gz.

Since I'm able to both locally (via vessel) and manually download (through the releases page) release binaries 0.6.3+ from the GitHub releases page, this means there's either a specific download permission setting setting that's blocking the GitHub CI role or the request vessel is making to download the specific moc compiler version was messing up.

The code where the vessel package manager is attempting to download the binary can be found here.

For reference, this is the error I was receiving in the CI logs:

Run make check-strict
Error: Failed to download Motoko binaries for version 0.6.20, with "403 Forbidden"

Details: <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>AccessDenied</Code><Message>Access Denied</Message><RequestId>HWH6Y08H658Y6KBG</RequestId><HostId>NNRcEQKUFwjZOQNNQQnVhmj6prO/0/IZBou0bUiyeZOs7+sIMqbRlHNRCU/X7XlfNfJM1cFKkcA=</HostId></Error>
find src -type f -name '*.mo' -print0 | xargs -0 /moc --package base .vessel/base/e0c95f909e17c431921f1be35800242a6ddd4a55/src -Werror --check
xargs: /moc: No such file or directory
make: *** [Makefile:9: check-strict] Error 127

Where the check-strict command is

check-strict:
	find src -type f -name '*.mo' -print0 | xargs -0 $(shell vessel bin)/moc $(shell vessel sources) -Werror --check

And the ci yaml file looked like this
https://github.com/kritzcreek/motoko-library-template/blob/main/.github/workflows/ci.yml

Since I was receiving the "Failed to download" error mentioned on [this line], I started there and found an issue where if the User-Agent header is not specified, the GitHub API will throw this error. You can see on this line that this header is set - and the API does not complain.

The Fix (and proof it works)

  • I've made the fix on my fork with the release here
  • A new package of mine using the moc compiler version 0.6.20 called motoko-color can be found here. This package is based off of the motoko-library-template package written by @kritzcreek As you can see from the check mark next to the most recent commit, each of the CI are passing (which involve downloading the motoko compiler from GitHub as the version = 0.6.20, which is > 0.6.2, and both the ci.yml and [release.yml] are hitting my this commit of my vessel fork.

@kritzcreek
Copy link
Collaborator

Nice analysis, and the fix looks simple enough and straight forward. Looks good to me, thank you!

@ByronBecker
Copy link
Contributor Author

Thanks for the review :), I believe I need a second one as a first time contributor to get this merged and then released @ninegua @nomeata

@ByronBecker
Copy link
Contributor Author

@crusso @chenyan-dfinity

Sorry to bother, need one more approval on this quick bug fix (two approvals required)!

@nomeata
Copy link
Contributor

nomeata commented Feb 11, 2022

You need approvals from actual current committees of this repo. It seems not nice that @kritzcreek got his permissions revoked, could someone at dfinity fix that? (Unless Christoph doesn't want them back, of course)

@ByronBecker
Copy link
Contributor Author

@nomeata Who should you or I tag to reissue permission. I agree it would be nice to have the original owner of this package manager have the ability to merge in new features.

@crusso
Copy link
Contributor

crusso commented Feb 11, 2022

Thanks for your perseverance @ByronBecker! Did that do the trick?

@ByronBecker
Copy link
Contributor Author

@crusso, it did - but looks like we're somehow failing CI now, which is odd to me considering where it's failing. If you don't mind - I'll take a look at it tomorrow and see if there's anything I can find (fix a hash or something like that)

@ByronBecker
Copy link
Contributor Author

@crusso Actually, it looks like the current main fails these same checks. I'm wondering if we can just merge as is then, so I don't have to fix previous issues as well in this pull request except for the bug described above

@ByronBecker
Copy link
Contributor Author

I don't have merge capabilities, so it would have to be someone currently working at Dfinity with those permissions

@crusso crusso merged commit 331cef4 into dfinity:main Feb 11, 2022
@crusso
Copy link
Contributor

crusso commented Feb 11, 2022

CI issue fixed here #42 (thanks to handholding by @kritzcreek)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants