From 3b4bd90d9b80ff9d29fa766026595d47ff852258 Mon Sep 17 00:00:00 2001 From: pawelpasterz <32893017+pawelpasterz@users.noreply.github.com> Date: Wed, 23 Dec 2020 15:32:52 +0100 Subject: [PATCH] feat: Default project Id - use GOOGLE_APPLICATION_CREDENTIALS first, then GOOGLE_CLOUD_PROJECT (#1421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1407 Flank will now use `GOOGLE_APPLICATION_CREDENTIALS` env first, to parse and get project id. (previously the first one was `GOOGLE_CLOUD_PROJECT` ## Test Plan > How do we know the code works? 1. build flank 1. remember to use the fat jar to launch test runs `java -jar ./test_runner/build/libs/flank.jar` 1. create dummy `echo '{"project_id":"any funny project id here"}' >> test.json` 1. export variable `export GOOGLE_APPLICATION_CREDENTIALS="$(pwd)/test.json"` 1. export variable `export GOOGLE_CLOUD_PROJECT="another project id"` 1. run any flank test run (within the same terminal window) 1. If you are creative enough you should get 404 (interesting fact: I used `ddd` project id during local tests and it turned out there is such project, I received 403 instead 😄 ) with a message 1. 404 -> `Flank was unable to find project [id from step 4.]` 2. 403 -> `Flank encountered a 403 error when running on project [id from 2.] (from [path to file from 2.])` 1. clear `export GOOGLE_APPLICATION_CREDENTIALS=` 1. similar to 7. except project id should be from 5 1. clear `export GOOGLE_CLOUD_PROJECT=` 1. run tests, flank should proceed normally ## Checklist - [x] Documented - [x] Unit tested --- docs/index.md | 4 + .../src/main/kotlin/ftl/args/ArgsHelper.kt | 56 +++++++------- .../ftl/config/common/CommonFlankConfig.kt | 2 +- .../src/main/kotlin/ftl/gc/GcToolResults.kt | 8 +- test_runner/src/main/kotlin/ftl/util/Utils.kt | 2 + .../test/kotlin/ftl/args/ArgsHelperTest.kt | 2 +- .../kotlin/ftl/args/FetchProjectIdTest.kt | 75 +++++++++++++++++++ .../test/kotlin/ftl/gc/GcToolResultsTest.kt | 47 +++++++++++- 8 files changed, 164 insertions(+), 32 deletions(-) create mode 100644 test_runner/src/test/kotlin/ftl/args/FetchProjectIdTest.kt diff --git a/docs/index.md b/docs/index.md index 92965e0cf2..fd9ef48b15 100644 --- a/docs/index.md +++ b/docs/index.md @@ -850,6 +850,10 @@ to have a unique non-shared credential. A service account is still recommended f Follow the [test lab docs](https://firebase.google.com/docs/test-lab/android/continuous) to create a service account. - Save the credential to `$HOME/.config/gcloud/application_default_credentials.json` or set `GOOGLE_APPLICATION_CREDENTIALS` when using a custom path. - Set the project id in flank.yml or set the `GOOGLE_CLOUD_PROJECT` environment variable. +- (Since 21.01) if `projectId` is not set in a config yml file, flank uses the first available project ID among the following sources: + 1. The project ID specified in the JSON credentials file pointed by the GOOGLE_APPLICATION_CREDENTIALS environment variable [fladle](https://runningcode.github.io/fladle/configuration/#serviceaccountcredentials) + 1. The project ID specified by the GOOGLE_CLOUD_PROJECT environment variable + 1. The project ID specified in the JSON credentials file `$HOME/.config/gcloud/application_default_credentials.json` For continuous integration, base64 encode the credential as `GCLOUD_KEY`. Then write the file using a shell script. Note that gcloud CLI does not need to be installed. Flank works without any dependency on gcloud CLI. diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt index 0e2b756086..4c0604f0b5 100644 --- a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt +++ b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt @@ -30,6 +30,7 @@ import ftl.shard.createShardsByShardCount import ftl.shard.shardCountByTime import ftl.util.FlankTestMethod import ftl.util.assertNotEmpty +import ftl.util.getGACPathOrEmpty import java.io.File import java.net.URI import java.nio.file.Files @@ -44,6 +45,8 @@ object ArgsHelper { YamlObjectMapper().registerKotlinModule() } + private var projectIdSource: String? = null + fun assertFileExists(file: String, name: String) { if (!file.exist()) throw FlankGeneralError("'$file' $name doesn't exist") } @@ -137,7 +140,7 @@ object ArgsHelper { // Make best effort to list/create the bucket. // Due to permission issues, the user may not be able to list or create buckets. fun createGcsBucket(projectId: String, bucket: String): String { - if (bucket.isBlank()) return GcToolResults.getDefaultBucket(projectId) + if (bucket.isBlank()) return GcToolResults.getDefaultBucket(projectId, projectIdSource) ?: throw FlankGeneralError("Failed to make bucket for $projectId") if (useMock) return bucket @@ -178,29 +181,29 @@ object ArgsHelper { return bucket } - private fun serviceAccountProjectId(): String? { - try { - if (!defaultCredentialPath.toFile().exists()) return null - - return JsonObjectParser(JSON_FACTORY).parseAndClose( - Files.newInputStream(defaultCredentialPath), - Charsets.UTF_8, - GenericJson::class.java - )["project_id"] as String - } catch (e: Exception) { - logLn("Parsing $defaultCredentialPath failed:") - logLn(e.printStackTrace()) - } - - return null - } - - fun getDefaultProjectId(): String? { - if (useMock) return "mockProjectId" - - // Allow users control over project by checking using Google's logic first before falling back to JSON. - return ServiceOptions.getDefaultProjectId() ?: serviceAccountProjectId() - } + fun getDefaultProjectIdOrNull(): String? = if (useMock) "mockProjectId" + // Allow users control over project by checking using Google's logic first before falling back to JSON. + else fromUserProvidedCredentials() + ?: ServiceOptions.getDefaultProjectId()?.let { if (it.isBlank()) null else it } + ?: fromDefaultCredentials() + + private fun fromDefaultCredentials() = getProjectIdFromJson(defaultCredentialPath) + + private fun fromUserProvidedCredentials() = + getProjectIdFromJson(Paths.get(getGACPathOrEmpty())) + + private fun getProjectIdFromJson(jsonPath: Path): String? = if (!jsonPath.toFile().exists()) null + else runCatching { + projectIdSource = jsonPath.toAbsolutePath().toString() + JsonObjectParser(JSON_FACTORY).parseAndClose( + Files.newInputStream(jsonPath), + Charsets.UTF_8, + GenericJson::class.java + )["project_id"] as String + }.onFailure { + logLn("Parsing $jsonPath failed:") + logLn(it.printStackTrace()) + }.getOrNull() // https://stackoverflow.com/a/2821201/2450315 private val envRegex = Pattern.compile("\\$([a-zA-Z_]+[a-zA-Z0-9_]*)") @@ -232,7 +235,10 @@ object ArgsHelper { forcedShardCount: Int? = null ): CalculateShardsResult { if (filteredTests.isEmpty()) { - return CalculateShardsResult(emptyList(), emptyList()) // Avoid unnecessary computing if we already know there aren't tests to run. + return CalculateShardsResult( + emptyList(), + emptyList() + ) // Avoid unnecessary computing if we already know there aren't tests to run. } val (ignoredTests, testsToExecute) = filteredTests.partition { it.ignored } val shards = if (args.disableSharding) { diff --git a/test_runner/src/main/kotlin/ftl/config/common/CommonFlankConfig.kt b/test_runner/src/main/kotlin/ftl/config/common/CommonFlankConfig.kt index 743dc114a4..359d462ee8 100644 --- a/test_runner/src/main/kotlin/ftl/config/common/CommonFlankConfig.kt +++ b/test_runner/src/main/kotlin/ftl/config/common/CommonFlankConfig.kt @@ -186,7 +186,7 @@ data class CommonFlankConfig @JsonIgnore constructor( const val defaultLocalResultsDir = "results" fun default() = CommonFlankConfig().apply { - project = ArgsHelper.getDefaultProjectId() ?: "" + project = ArgsHelper.getDefaultProjectIdOrNull() maxTestShards = 1 shardTime = -1 repeatTests = 1 diff --git a/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt b/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt index 1f89503f09..dd0c3cd39a 100644 --- a/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt +++ b/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt @@ -138,12 +138,12 @@ object GcToolResults { ).executeWithRetry() } - fun getDefaultBucket(projectId: String): String? = try { + fun getDefaultBucket(projectId: String, source: String? = null): String? = try { service.Projects().initializeSettings(projectId).executeWithRetry().defaultBucket } catch (ftlProjectError: FTLProjectError) { // flank needs to rewrap the exception with additional info about project when (ftlProjectError) { - is PermissionDenied -> throw FlankGeneralError(permissionDeniedErrorMessage(projectId, ftlProjectError.message)) + is PermissionDenied -> throw FlankGeneralError(permissionDeniedErrorMessage(projectId, source, ftlProjectError.message)) is ProjectNotFound -> throw FlankGeneralError(projectNotFoundErrorMessage(projectId, ftlProjectError.message)) is FailureToken -> UserAuth.throwAuthenticationError() } @@ -204,8 +204,8 @@ object GcToolResults { .executeWithRetry() } -private val permissionDeniedErrorMessage = { projectId: String, message: String? -> - """Flank encountered a 403 error when running on project $projectId. Please verify this credential is authorized for the project and has the required permissions. +private val permissionDeniedErrorMessage = { projectId: String, projectIdSource: String?, message: String? -> + """Flank encountered a 403 error when running on project $projectId${projectIdSource?.let {" (from $it)"} ?: ""}. Please verify this credential is authorized for the project and has the required permissions. Consider authentication with a Service Account https://github.com/Flank/flank#authenticate-with-a-service-account or with a Google account https://github.com/Flank/flank#authenticate-with-a-google-account diff --git a/test_runner/src/main/kotlin/ftl/util/Utils.kt b/test_runner/src/main/kotlin/ftl/util/Utils.kt index bf1da77c3e..9806b3e7ba 100644 --- a/test_runner/src/main/kotlin/ftl/util/Utils.kt +++ b/test_runner/src/main/kotlin/ftl/util/Utils.kt @@ -102,3 +102,5 @@ fun KMutableProperty.require() = getter.call() ?: throw FlankGeneralError( "Invalid value for [${setter.annotations.filterIsInstance().first().value}]: no argument value found" ) + +fun getGACPathOrEmpty(): String = System.getenv("GOOGLE_APPLICATION_CREDENTIALS").orEmpty() diff --git a/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt b/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt index 7dec0e3c91..13c1d7f532 100644 --- a/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt @@ -138,7 +138,7 @@ class ArgsHelperTest { @Test fun `getDefaultProjectId succeeds`() { - assertThat(ArgsHelper.getDefaultProjectId()) + assertThat(ArgsHelper.getDefaultProjectIdOrNull()) .isEqualTo("mockProjectId") } diff --git a/test_runner/src/test/kotlin/ftl/args/FetchProjectIdTest.kt b/test_runner/src/test/kotlin/ftl/args/FetchProjectIdTest.kt new file mode 100644 index 0000000000..10e9aa7008 --- /dev/null +++ b/test_runner/src/test/kotlin/ftl/args/FetchProjectIdTest.kt @@ -0,0 +1,75 @@ +package ftl.args + +import com.google.cloud.ServiceOptions +import com.google.common.truth.Truth.assertThat +import ftl.config.FtlConstants +import ftl.config.defaultCredentialPath +import ftl.util.getGACPathOrEmpty +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.junit.After +import org.junit.AfterClass +import org.junit.Before +import org.junit.BeforeClass +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +class FetchProjectIdTest { + + // mock server is not torn down between test classes + // this is workaround until better solution is implemented + companion object { + @JvmStatic + @BeforeClass + fun noUseMock() { + FtlConstants.useMock = false + } + + @JvmStatic + @AfterClass + fun useMock() { + FtlConstants.useMock = true + } + } + + @get:Rule + val folder = TemporaryFolder() + + private lateinit var gac: File + + private lateinit var def: File + + @Before + fun setup() { + gac = folder.newFile("gap.json").also { it.writeText("""{"project_id": "id_from_gac"}""") } + def = folder.newFile("def.json").also { it.writeText("""{"project_id": "id_from_def"}""") } + } + + @After + fun teardown() = unmockkAll() + + @Test + fun `should fetch project id from GCLOUD_APLICATION_CREDENTIALS`() { + mockkStatic("ftl.util.Utils") { + every { getGACPathOrEmpty() } returns gac.absolutePath.toString() + assertThat(ArgsHelper.getDefaultProjectIdOrNull()).isEqualTo("id_from_gac") + } + } + + @Test + fun `should fetch project id from default credentials`() { + mockkStatic( + "ftl.util.Utils", + "ftl.config.CredentialsKt", + ServiceOptions::class.qualifiedName ?: "" + ) { + every { defaultCredentialPath } returns def.toPath() + every { getGACPathOrEmpty() } returns "" + every { ServiceOptions.getDefaultProjectId() } returns null + assertThat(ArgsHelper.getDefaultProjectIdOrNull()).isEqualTo("id_from_def") + } + } +} diff --git a/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt b/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt index 38279a49c9..a9a0616110 100644 --- a/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt +++ b/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt @@ -56,7 +56,7 @@ class GcToolResultsTest { } @Test - fun `getDefaultBucket on 403 error should throw exception with specific message`() { + fun `getDefaultBucket on 403 error should throw exception with specific message - no source`() { val expected = """ Flank encountered a 403 error when running on project $projectId. Please verify this credential is authorized for the project and has the required permissions. Consider authentication with a Service Account https://github.com/Flank/flank#authenticate-with-a-service-account @@ -100,6 +100,51 @@ class GcToolResultsTest { } } + @Test + fun `getDefaultBucket on 403 error should throw exception with specific message - with source`() { + val expected = """ + Flank encountered a 403 error when running on project $projectId (from /Any/path/to/json.json). Please verify this credential is authorized for the project and has the required permissions. + Consider authentication with a Service Account https://github.com/Flank/flank#authenticate-with-a-service-account + or with a Google account https://github.com/Flank/flank#authenticate-with-a-google-account + + Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 403 Forbidden + { + "code" : 403, + "errors" : [ { + "domain" : "global", + "message" : "The caller does not have permission", + "reason" : "forbidden" + } ], + "message" : "The caller does not have permission", + "status" : "PERMISSION_DENIED" + } + """.trimIndent() + mockkObject(GcToolResults) { + every { GcToolResults.service.applicationName } returns projectId + + val exceptionBuilder = mockk() + every { exceptionBuilder.message } returns """ + 403 Forbidden + { + "code" : 403, + "errors" : [ { + "domain" : "global", + "message" : "The caller does not have permission", + "reason" : "forbidden" + } ], + "message" : "The caller does not have permission", + "status" : "PERMISSION_DENIED" + } + """.trimIndent() + val mockJSonException = GoogleJsonResponseException(exceptionBuilder, null) + every { GcToolResults.service.Projects().initializeSettings(projectId) } throws PermissionDenied( + mockJSonException + ) + val exception = getThrowable { GcToolResults.getDefaultBucket(projectId, "/Any/path/to/json.json") } + assertEquals(expected, exception.message) + } + } + @Test(expected = FlankGeneralError::class) fun `getDefaultBucket on PermissionDenied error should throw FlankGeneralError`() { mockkObject(GcToolResults) {