runningcode / fladle

Easily scale your Android Instrumentation Tests across Firebase Test Lab with Flank.
https://runningcode.github.io/fladle
Apache License 2.0
250 stars 48 forks source link

Improve Compatibility with Workload Identity Federation #293

Open ccjernigan opened 2 years ago

ccjernigan commented 2 years ago

When using Workload Identity Federation on GitHub Actions, there are a few compatibility issues that come up with Fladle. The token provided by the Google Auth GitHub Action doesn't include the Google Cloud Project name, which is a difference from creating a traditional long-lived service key.

This causes a problem where Fladle fails, because the Google Cloud Project name cannot be found. The workaround is to provide the project name through an environment variable GOOGLE_CLOUD_PROJECT which can get picked up by Flank. An example of this is here https://github.com/zcash/secant-android-wallet/blob/45e14e6a4bcb0d5070ea17dc06feba7050f65ea6/.github/workflows/pull-request.yml#L337

However this reveals another minor issue, which is that an error appears in the log like this:

java.lang.NullPointerException: null cannot be cast to non-null type kotlin.String
Parsing /home/runner/work/secant-android-wallet/secant-android-wallet/gha-creds-e49e0.json failed:
kotlin.Unit
    at ftl.args.ArgsHelper.getProjectIdFromJson(ArgsHelper.kt:200)
    at ftl.args.ArgsHelper.fromUserProvidedCredentials(ArgsHelper.kt:195)
    at ftl.args.ArgsHelper.getUserProjectId(ArgsHelper.kt:188)
    at ftl.args.ArgsHelper.getDefaultProjectIdOrNull(ArgsHelper.kt:185)
    at ftl.config.common.CommonFlankConfig$Companion.default(CommonFlankConfig.kt:237)
    at ftl.config.CreateKt.defaultAndroidConfig(Create.kt:16)
    at ftl.domain.RunTestAndroidKt.invoke(RunTestAndroid.kt:47)
    at ftl.presentation.cli.firebase.test.android.AndroidRunCommand.run(AndroidRunCommand.kt:58)
    at picocli.CommandLine.executeUserObject(CommandLine.java:1939)
    at picocli.CommandLine.access$1300(CommandLine.java:145)
    at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2352)
    at picocli.CommandLine$RunLast.handle(CommandLine.java:2346)
    at picocli.CommandLine$RunLast.handle(CommandLine.java:2311)
    at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
    at picocli.CommandLine.execute(CommandLine.java:20[78](https://github.com/zcash/secant-android-wallet/runs/5586029293?check_suite_focus=true#step:6:78))
    at ftl.Main$main$1.invoke(Main.kt:12)
    at ftl.Main$main$1.invoke(Main.kt:10)
    at ftl.run.exception.ExceptionHandlerKt.withGlobalExceptionHandling(ExceptionHandler.kt:28)
    at ftl.run.exception.ExceptionHandlerKt.withGlobalExceptionHandling(ExceptionHandler.kt:17)
    at ftl.Main.main(Main.kt:10)

So I'd like to suggest a few enhancements:

  1. Document the need to provide the environment variable when using Workload Identity Federation
  2. Improve the error message when the json key is missing the cloud project and there's no GOOGLE_CLOUD_PROJECT environment variable
  3. Don't print a stack trace to the log if the GOOGLE_CLOUD_PROJECT environment variable is present
runningcode commented 2 years ago

Thanks for filing this @ccjernigan and for the details. This stack trace actually comes from flank itself here: https://github.com/Flank/flank/blob/master/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt#L197

I think you might be better off filing the bug over there. The only improvement we can make in fladle is improving the error message in this case, but it would be better if flank itself improved the error message.

Let me know if my assessment is somehow incorrect.

ccjernigan commented 2 years ago

Agree—after I slept on this I realized that the Flank project was probably the better place to go for this. I filed this issue: https://github.com/Flank/flank/issues/2228

That said, this page in the Fladle documentation might merit a small mention of Workload Identity Federation https://runningcode.github.io/fladle/authentication/

runningcode commented 2 years ago

Yes, PRs welcome. I haven't used Workload Identity Federation so perhaps I'm not the best person to write those docs.