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

Add authorization header to artifacts request #24106

Merged
merged 2 commits into from
Mar 16, 2022

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Mar 15, 2022

CircleCI's artifacts API was updated; it now errors unless you're logged in. This affects any of our workflows that download build artifacts.

To fix, I added an authorization header to the request.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 15, 2022
@@ -9,6 +9,15 @@ const {getArtifactsList, logPromise} = require('../utils');
const theme = require('../theme');

const run = async ({build, cwd, releaseChannel}) => {
const CIRCLE_TOKEN = process.env.CIRCLE_CI_API_TOKEN;
console.log({CIRCLE_TOKEN});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm

CircleCI's artifacts API was updated; it now errors unless you're
logged in. This affects any of our workflows that download
build artifacts.

To fix, I added an authorization header to the request.
@acdlite
Copy link
Collaborator Author

acdlite commented Mar 15, 2022

Hrmm I guess this doesn't work for sizebot because PRs don't have access to environment variables

@acdlite
Copy link
Collaborator Author

acdlite commented Mar 15, 2022

Maybe need to set up a proxy, at least temporarily

@sizebot
Copy link

sizebot commented Mar 16, 2022

Comparing: 0412f0c...ab32703

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 130.96 kB 130.96 kB = 41.93 kB 41.93 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 136.02 kB 136.02 kB = 43.41 kB 43.41 kB
facebook-www/ReactDOM-prod.classic.js = 435.52 kB 435.52 kB = 79.78 kB 79.78 kB
facebook-www/ReactDOM-prod.modern.js = 421.94 kB 421.94 kB = 77.77 kB 77.77 kB
facebook-www/ReactDOMForked-prod.classic.js = 435.52 kB 435.52 kB = 79.79 kB 79.79 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against ab32703

We can't use the normal download-build script in sizebot because it
depends on the CircleCI artifacts API, which was recently changed to
require authorization. And we can't pass an authorization token
without possibly leaking it to the public, since we run sizebot on
PRs from external contributors. As a temporary workaround, this job
will pull the artifacts from a public mirror that I set up. But we
should find some other solution so we don't have to maintain
the mirror.
@acdlite acdlite merged commit ba5dc6c into facebook:main Mar 16, 2022
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
* Add authorization header to artifacts request

CircleCI's artifacts API was updated; it now errors unless you're
logged in. This affects any of our workflows that download
build artifacts.

To fix, I added an authorization header to the request.

* Update sizbot to pull artifacts from public mirror

We can't use the normal download-build script in sizebot because it
depends on the CircleCI artifacts API, which was recently changed to
require authorization. And we can't pass an authorization token
without possibly leaking it to the public, since we run sizebot on
PRs from external contributors. As a temporary workaround, this job
will pull the artifacts from a public mirror that I set up. But we
should find some other solution so we don't have to maintain
the mirror.
acdlite added a commit to acdlite/react that referenced this pull request May 18, 2022
This reverts commit facebook#24106.

There was a regression in CircleCI's artifacts API recently where you
could no longer access artifacts without an authorization token. This
broke our size reporting CI job because we can't use an authorization
token on external PRs without potentially leaking it. As a temporary
workaround, I changed the size reporting job to use a public mirror of
our build artifacts.

The CircleCI API has since been fixed to no longer require
authorization, so we can revert the workaround.
acdlite added a commit to acdlite/react that referenced this pull request May 18, 2022
This reverts facebook#24106.

There was a regression in CircleCI's artifacts API recently where you
could no longer access artifacts without an authorization token. This
broke our size reporting CI job because we can't use an authorization
token on external PRs without potentially leaking it. As a temporary
workaround, I changed the size reporting job to use a public mirror of
our build artifacts.

The CircleCI API has since been fixed to no longer require
authorization, so we can revert the workaround.
acdlite added a commit that referenced this pull request May 18, 2022
This reverts #24106.

There was a regression in CircleCI's artifacts API recently where you
could no longer access artifacts without an authorization token. This
broke our size reporting CI job because we can't use an authorization
token on external PRs without potentially leaking it. As a temporary
workaround, I changed the size reporting job to use a public mirror of
our build artifacts.

The CircleCI API has since been fixed to no longer require
authorization, so we can revert the workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants