feat: Default project Id - use GOOGLE_APPLICATION_CREDENTIALS first, …

…then GOOGLE_CLOUD_PROJECT (#1421) 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
Flank · Dec 23, 2020 · 3b4bd90 · 3b4bd90
1 parent f3f2455
commit 3b4bd90
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -102,3 +102,5 @@ fun <T> KMutableProperty<T?>.require() =
     getter.call() ?: throw FlankGeneralError(
         "Invalid value for [${setter.annotations.filterIsInstance<JsonProperty>().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
@@ -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
@@ -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
@@ -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<HttpResponseException.Builder>()
+            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) {