-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Remove sentry.properties secrets in favour of public values and an env-injected token #23189
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
4611962
3d76c98
04449ee
71c467d
2e0f98d
a95a4be
b01e9f3
6ce1a28
4033f5c
cadd35d
ad10bcd
f457a58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import io.sentry.android.gradle.extensions.InstrumentationFeature | ||
| import io.sentry.android.gradle.tasks.SentryCliExecTask | ||
|
|
||
| plugins { | ||
| alias(libs.plugins.android.application) | ||
|
|
@@ -64,7 +65,14 @@ static String pathForSmartFlankGcsPath(String app) { | |
| return "gs://fladle/wordpress-android/${app}/smart-flank/JUnitReport.xml" | ||
| } | ||
|
|
||
| // A secret-injection step that resolves to nothing exports an empty SENTRY_AUTH_TOKEN rather | ||
| // than leaving it unset, so treat blank as missing — otherwise the guard below passes and | ||
| // sentry-cli fails mid-upload with an opaque auth error. | ||
| def sentryAuthToken = providers.environmentVariable("SENTRY_AUTH_TOKEN").orNull?.trim() ?: null | ||
|
|
||
| sentry { | ||
| authToken = sentryAuthToken | ||
|
|
||
| tracingInstrumentation { | ||
| enabled = true | ||
| features = [InstrumentationFeature.DATABASE] | ||
|
|
@@ -83,6 +91,17 @@ sentry { | |
| telemetry = false | ||
| } | ||
|
|
||
| tasks.withType(SentryCliExecTask).configureEach { | ||
| doFirst { | ||
| if (sentryAuthToken == null) { | ||
| throw new GradleException( | ||
|
Comment on lines
+94
to
+97
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have to do this here and not on line 71? Is there any advantage to rely on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah okay, I think I know why: you don't want to validate when just configuring the build (so it's possible to run build without secrets) but only fail on missing token when someone calls a Sentry task, makes sense |
||
| "SENTRY_AUTH_TOKEN is not set (or is blank). " + | ||
| "Set it to upload debug files to Sentry." | ||
| ) | ||
| } | ||
| } | ||
|
Copilot marked this conversation as resolved.
|
||
| } | ||
|
|
||
| allOpen { | ||
| // allows mocking for classes w/o directly opening them for release builds | ||
| annotation 'org.wordpress.android.testing.OpenClassAnnotation' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| defaults.project=jetpack-android | ||
| defaults.org=a8c |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| defaults.project=wordpress-android | ||
| defaults.org=a8c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice no
sentry.propertiesentry was removed. I haven't followed up with the Git history but it seems likely to me that the file was there in the pre-Jetpack days and was not deleted at the time of the fork.