chore: Remove requirements for upload tokens #153

nicholas-codecov · 2024-08-08T13:55:09Z

Description

This PR removes the requirement for upload tokens in the plugin-core which will affect all of the plugins downstream. It also introduces some new functionality to detect the VCS provider being used, and will append that to the request body, while request the pre-signed URL.

The logic for parsing the Git Service from the CLI was sourced from: https://github.com/codecov/codecov-cli/blob/main/codecov_cli/helpers/git.py

Details for the new request body field, and requirement for the git_service body param is from this PR: codecov/codecov-api#741

Closes codecov/engineering-team#2208

Notable Changes

Add in ability to get pre-signed URL without a token, given a valid bundle name
- Conditionally add token to headers if present
Add in new function to grab the git service from the CLI and parse the URL
Add in new configuration option to override the git service param
Create/Update tests

codecov-notifications · 2024-08-08T13:57:16Z

Codecov Report

Attention: Patch coverage is 96.04520% with 7 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files	Patch %	Lines
packages/nuxt-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/remix-vite-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/rollup-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/solidstart-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/sveltekit-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/vite-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/webpack-plugin/src/index.ts	0.00%	1 Missing ⚠️

Components	Coverage Δ
Plugin core	`96.84% <100.00%> (+0.13%)`	⬆️
Rollup plugin	`10.81% <0.00%> (ø)`
Vite plugin	`11.02% <0.00%> (ø)`
Webpack plugin	`50.11% <0.00%> (ø)`

📢 Thoughts on this report? Let us know!

codecov · 2024-08-08T13:59:02Z

Codecov Report

Attention: Patch coverage is 96.04520% with 7 lines in your changes missing coverage. Please review.

Project coverage is 75.15%. Comparing base (f290775) to head (eddd2ab).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
packages/nuxt-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/remix-vite-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/rollup-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/solidstart-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/sveltekit-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/vite-plugin/src/index.ts	0.00%	1 Missing ⚠️
packages/webpack-plugin/src/index.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

Components	Coverage Δ
Plugin core	`96.84% <100.00%> (+0.13%)`	⬆️
Rollup plugin	`10.81% <0.00%> (ø)`
Vite plugin	`11.02% <0.00%> (ø)`
Webpack plugin	`50.11% <0.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codecov-staging · 2024-08-08T14:02:04Z

Bundle Report

Changes will increase total bundle size by 234.48kB ⬆️

Bundle name	Size	Change
@codecov/vite-plugin-cjs	2.8kB	2.8kB ⬆️
@codecov/vite-plugin-esm	1.24kB	1.24kB ⬆️
@codecov/nuxt-plugin-cjs	1.41kB	1.41kB ⬆️
@codecov/nuxt-plugin-esm	830 bytes	830 bytes ⬆️
@codecov/remix-vite-plugin-esm	957 bytes	957 bytes ⬆️
@codecov/example-vite-app-esm	150.61kB	150.61kB ⬆️
@codecov/nextjs-webpack-plugin-esm	1.88kB	1.88kB ⬆️
@codecov/rollup-plugin-esm	1.3kB	1.3kB ⬆️
@codecov/sveltekit-plugin-esm	891 bytes	891 bytes ⬆️
@codecov/bundler-plugin-core-esm	10.26kB	10.26kB ⬆️
@codecov/webpack-plugin-esm	3.36kB	3.36kB ⬆️
@codecov/remix-vite-plugin-cjs	1.32kB	1.32kB ⬆️
@codecov/rollup-plugin-cjs	2.82kB	2.82kB ⬆️
@codecov/bundler-plugin-core-cjs	45.44kB	45.44kB ⬆️
@codecov/nextjs-webpack-plugin-cjs	2.11kB	2.11kB ⬆️
@codecov/solidstart-plugin-cjs	1.33kB	1.33kB ⬆️
@codecov/solidstart-plugin-esm	949 bytes	949 bytes ⬆️
@codecov/webpack-plugin-cjs	4.35kB	4.35kB ⬆️
@codecov/sveltekit-plugin-cjs	1.33kB	1.33kB ⬆️
@codecov/example-next-app-edge-server-array-push	(removed)	354 bytes ⬇️
@codecov/example-next-15-app-edge-server-array-push	(removed)	356 bytes ⬇️

codecov · 2024-08-08T14:02:12Z

Bundle Report

Changes will increase total bundle size by 154.85kB ⬆️

Bundle name	Size	Change
@codecov/vite-plugin-esm	1.24kB	18 bytes ⬇️
@codecov/remix-vite-plugin-esm	957 bytes	18 bytes ⬇️
@codecov/bundler-plugin-core-cjs	45.44kB	1.82kB ⬆️
@codecov/bundler-plugin-core-esm	10.26kB	1.69kB ⬆️
@codecov/rollup-plugin-esm	2.33kB	1.01kB ⬆️
@codecov/nuxt-plugin-esm	830 bytes	25 bytes ⬇️
@codecov/sveltekit-plugin-esm	891 bytes	206 bytes ⬇️
@codecov/webpack-plugin-esm	3.36kB	42 bytes ⬆️
@codecov/solidstart-plugin-esm	949 bytes	18 bytes ⬇️
@codecov/example-sveltekit-app-server-esm	974.58kB	1 bytes ⬆️
@codecov/example-tokenless-app-esm	150.57kB	150.57kB ⬆️

AbhiPrasad · 2024-08-12T13:06:11Z

.changeset/kind-camels-give.md

+"@codecov/webpack-plugin": patch
+---
+
+Remove requirement for uploadToken being present adding support for tokenless uploads.


h: We need to update the README of the packages as well

Hrrm trying to think of a nice way to word "this", do you think having two examples or just adjusting the current codeblock to have the same options but just commenting around the differences would be better?

I think we should make tokenless the default snippet, and then add an extra section below about what to do if you need to use the token, but keep that at the bottom.

AbhiPrasad · 2024-08-12T13:09:55Z

packages/bundler-plugin-core/src/types.ts

+   *
+   * Example `gitService: 'github'`
+   */
+  gitService?: ValidGitService;


If you add an uploadToken, you don't need a gitService right? We should update the types + docstrings for this accordingly.

Are you talking about like a discriminated union type of sorts?

Union would be nice, but I'm fine with only adjusting the docstring as well

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

packages/bundler-plugin-core/src/utils/findGitService.ts

AbhiPrasad · 2024-08-12T13:30:14Z

packages/bundler-plugin-core/src/utils/getPreSignedURL.ts

+    }
+  } else if (uploadToken) {
+    headers.set("Authorization", `token ${uploadToken}`);
+  } else {


these nested conditionals feel like a code smell to me, but I can't figure out a better way to address this so I wouldn't worry about it for now. Something to think about if we come back to this file

suejung-sentry

This is awesome!

packages/bundler-plugin-core/src/types.ts

suejung-sentry · 2024-08-28T21:08:01Z

examples/tokenless/vite.config.ts

+    codecovVitePlugin({
+      enableBundleAnalysis: true,
+      bundleName: "@codecov/example-tokenless-app",
+      uploadToken: process.env.TOKENLESS_UPLOAD_TOKEN,


It is a little confusing to me that this tokenless example includes the token here - would it make sense to omit it?

So tokenless only works on forked branches forkOwner:branchName, so when you're running it "locally" i.e. I make a PR and open it here, there's no forkOwner in the branch name so tokenless functionality doesn't work, but the token secret will be present. However when a user forks a repo and makes an upstream pull request the secrets aren't shared for obvious reason so that's when tokenless kicks in. It's a confusing thing that I still barely know anything about 😂

Oooh okay I see! Got it - thanks for breaking that down!!

(un-resolving so the thread is not hidden, good context for people)

packages/nextjs-webpack-plugin/README.md

nicholas-codecov requested review from AbhiPrasad and removed request for AbhiPrasad August 8, 2024 14:50

AbhiPrasad reviewed Aug 12, 2024

View reviewed changes

AbhiPrasad approved these changes Aug 26, 2024

View reviewed changes

nicholas-codecov added 20 commits August 26, 2024 13:38

remove NoUploadTokenError

716a83c

update getPreSignedURL to remove requirement for uploadToken

c371e9c

update Output tests

5f087a3

add in changeset

eac3ca3

allow users to override git service for tokenless uploads

1903a8e

update types with gitService

ac5526c

add back no upload token error and new undefined git service error

9b1f678

add in new function used to find the origin git service

1edaf90

update getPreSignedURL to support tokenless uploads

4ecd466

actually use the passed git service

d63d4f4

apply feedback

20ddadc

add tokenless examples to readme's

06ff6bd

adjust docstring value

0e971a2

update doc string examples

dac7bca

update readme examples

6f0a7d3

more docstring updates

e7e7888

update readme copy

4221ae8

add in tokenless example

8185ae8

forgot to add to ci

388df7a

add nextjs to changeset

724ef8f

update nextjs readme

189178c

nicholas-codecov force-pushed the gh-eng-2208-remove-requirements-for-upload-tokens branch from c1db25d to 189178c Compare August 26, 2024 16:42

nicholas-codecov added 3 commits August 27, 2024 07:52

update github actions to override the branch value if from a forked PR

e1a60d8

remove tokenless example, as it only works with forks

e1ff1aa

update readme with new examples

7ca44df

nicholas-codecov requested a review from AbhiPrasad August 27, 2024 12:15

nicholas-codecov added 2 commits August 27, 2024 09:18

add in tokenless example

bf510dd

add git service to other examples

ca6b771

suejung-sentry approved these changes Aug 28, 2024

View reviewed changes

small typo fix

eddd2ab

nicholas-codecov merged commit ec6b13a into main Sep 3, 2024
52 checks passed

nicholas-codecov deleted the gh-eng-2208-remove-requirements-for-upload-tokens branch September 3, 2024 12:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Remove requirements for upload tokens #153

chore: Remove requirements for upload tokens #153

nicholas-codecov commented Aug 8, 2024 •

edited

Loading

codecov-notifications bot commented Aug 8, 2024 •

edited

Loading

codecov bot commented Aug 8, 2024 •

edited

Loading

codecov-staging bot commented Aug 8, 2024 •

edited

Loading

codecov bot commented Aug 8, 2024 •

edited

Loading

AbhiPrasad Aug 12, 2024

nicholas-codecov Aug 12, 2024

AbhiPrasad Aug 19, 2024

AbhiPrasad Aug 12, 2024

nicholas-codecov Aug 12, 2024

AbhiPrasad Aug 19, 2024

AbhiPrasad Aug 12, 2024

suejung-sentry left a comment

suejung-sentry Aug 28, 2024

nicholas-codecov Aug 29, 2024 •

edited

Loading

suejung-sentry Aug 29, 2024

nicholas-codecov Sep 3, 2024

chore: Remove requirements for upload tokens #153

chore: Remove requirements for upload tokens #153

Conversation

nicholas-codecov commented Aug 8, 2024 • edited Loading

Description

Notable Changes

codecov-notifications bot commented Aug 8, 2024 • edited Loading

Codecov Report

codecov bot commented Aug 8, 2024 • edited Loading

Codecov Report

codecov-staging bot commented Aug 8, 2024 • edited Loading

Bundle Report

codecov bot commented Aug 8, 2024 • edited Loading

Bundle Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suejung-sentry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicholas-codecov Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicholas-codecov commented Aug 8, 2024 •

edited

Loading

codecov-notifications bot commented Aug 8, 2024 •

edited

Loading

codecov bot commented Aug 8, 2024 •

edited

Loading

codecov-staging bot commented Aug 8, 2024 •

edited

Loading

codecov bot commented Aug 8, 2024 •

edited

Loading

nicholas-codecov Aug 29, 2024 •

edited

Loading