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

ref(admin): Upload sourcemaps to Sentry for the admin tool #4441

Merged
merged 7 commits into from
Jul 31, 2023

Conversation

evanh
Copy link
Member

@evanh evanh commented Jun 29, 2023

Add the webpack integration for Sentry into webpack, and setup the integration
key to be passed down into the build step.

NOTE: The secret has already been created for this.

Add the webpack integration for Sentry into webpack, and setup the integration
key to be passed down into the build step.
@evanh evanh requested a review from a team as a code owner June 29, 2023 15:44
@@ -42,6 +43,7 @@ jobs:
--build-arg BUILDKIT_INLINE_CACHE=1 \
--build-arg SHOULD_BUILD_ADMIN_UI="$SHOULD_BUILD_ADMIN_UI" \
--build-arg SHOULD_BUILD_RUST=false \
--build-arg ADMIN_SOURCEMAP_KEY="$ADMIN_SOURCEMAP_KEY" \
Copy link
Member

Choose a reason for hiding this comment

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

as written this value will be in the image -- if it's actually secret this will leak the secret

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't think of that. How would you recommend doing this?

Copy link
Member

Choose a reason for hiding this comment

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

I think getsentry does this through the deploy pipeline -- I'm not entirely sure would have to check that

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that too, but our images are built during CI, and I need this value during the build steps I think.

Copy link
Member

Choose a reason for hiding this comment

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

is the value an actual secret? if not then we can just inline it

does it need to be in the source?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it should be a secret? It's an internal integration API key for our Sentry projects. It would allow anyone with the key at minimum to upload files to our internal projects (that's what I plan to do with it).

Copy link
Member

Choose a reason for hiding this comment

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

Can we use docker build --secret id=mysecret,env=MYSECRET .?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that will help here -- the value is exposed in ENV and appears to get baked in by webpack

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I can have a separate step that builds webpack outside of the docker image just to get the sourcemaps uploaded. I don't need this value after the image is built.

@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (252227f) 90.28% compared to head (653b0af) 90.28%.

❗ Current head 653b0af differs from pull request most recent head ae26e93. Consider uploading reports for the commit ae26e93 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4441   +/-   ##
=======================================
  Coverage   90.28%   90.28%           
=======================================
  Files         817      817           
  Lines       40386    40386           
  Branches      285      285           
=======================================
  Hits        36462    36462           
  Misses       3883     3883           
  Partials       41       41           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

name: "build sourcemaps"
runs-on: ubuntu-latest
env:
SENTRY_AUTH_TOKEN: ${{ secrets.SNUBA_SENTRY_SOURCEMAP_KEY }}
Copy link
Member Author

Choose a reason for hiding this comment

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

@asottile-sentry I don't think this question was answered: how do I go about getting this secret set up?

Copy link
Member

Choose a reason for hiding this comment

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

the team which owns the repo (I believe sns) should have admin to add / alter repository-specific secrets

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect, thank you for all your help!

@evanh evanh merged commit 0a3a57d into master Jul 31, 2023
24 checks passed
@evanh evanh deleted the evanh/fix/add-frontend-sourcemaps branch July 31, 2023 19:55
davidtsuk pushed a commit that referenced this pull request Aug 10, 2023
Add the webpack integration for Sentry into webpack, and setup the integration
key to be passed down into the build step.
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.

4 participants