diff --git a/docs/permissions_denied_behavior.md b/docs/permissions_denied_behavior.md new file mode 100644 index 0000000000..2c9f69ab48 --- /dev/null +++ b/docs/permissions_denied_behavior.md @@ -0,0 +1,57 @@ +# Flank permissions denied behavior + +Reported on: [Clean flank not authorized error messages #874](https://github.com/Flank/flank/issues/874) + +Changed on: [Enhance permission denied exception logs #875](https://github.com/Flank/flank/pull/875) + +## 1. User don't have permission to project (403) + +When user don't have permission to project Flank should returns message like: + +```json + +Flank encountered a 403 error when running on project $project_name. Please verify this credential is authorized for the project. +Consider authentication a with 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" +} + +``` + +You can reproduce the error by setting PROJECT_ID to a project that the firebase account doesn't have permission to access. + +## 2. Project not found (404) + +When project not found on firebase Flank should return message like: + +```json + +Flank was unable to find project $project_name. Please verify the project id. + +Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 404 Not Found +{ + "code" : 404, + "errors" : [ { + "domain" : "global", + "message" : "Project not found: $project_name", + "reason" : "notFound" + } ], + "message" : "Project not found: $project_name", + "status" : "NOT_FOUND" +} + +``` + +You can reproduce the error by setting PROJECT_ID to a project that doesn't exist. + +## 3. On this two cases Flank throws FlankCommonException and exit with code: 1 diff --git a/release_notes.md b/release_notes.md index ab8fb5dfe5..4363c3c982 100644 --- a/release_notes.md +++ b/release_notes.md @@ -7,7 +7,7 @@ - [#866](https://github.com/Flank/flank/pull/866) Fix printing all matrix links. ([piotradamczyk5](https://github.com/piotradamczyk5)) - [#862](https://github.com/Flank/flank/pull/862) Added printing outcome details. ([piotradamczyk5](https://github.com/piotradamczyk5), [jan-gogo](https://github.com/jan-gogo)) - [#876](https://github.com/Flank/flank/pull/876) Added --directories-to-pull validation and avoid making request with empty toolStepResult. ([piotradamczyk5](https://github.com/piotradamczyk5)) -- +- [#875](https://github.com/Flank/flank/pull/875) Enhance permission denied exception logs. ([adamfilipow92](https://github.com/adamfilipow92), [pawelpasterz](https://github.com/pawelpasterz)) - ## v20.06.2 @@ -21,7 +21,7 @@ - [#831](https://github.com/Flank/flank/pull/831) Refactor config entities and arguments. ([jan-gogo](https://github.com/jan-gogo)) - [#817](https://github.com/Flank/flank/pull/817) Add AndroidTestContext as base data for dump shards & test execution. ([jan-gogo](https://github.com/jan-gogo)) -- [#801](https://github.com/Flank/flank/pull/801) Omit missing app apk if additional-app-test-apks specified. ([jan-gogo](https://github.com/jan-gogo)) +- [#801](https://github.com/Flank/flank/pull/801) Omit missing app apk if additional-app-test-apks specified. ([jan-gogo](https://github.com/jan-gogo)) - [#784](https://github.com/Flank/flank/pull/784) Add output-style option. ([jan-gogo](https://github.com/jan-gogo)) - [#779](https://github.com/Flank/flank/pull/779) Print retries & display additional info. ([jan-gogo](https://github.com/jan-gogo)) - [#793](https://github.com/Flank/flank/issues/793) Better error message on file not found. ([adamfilipow92](https://github.com/adamfilipow92)) diff --git a/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt b/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt index 1a1d4f7153..7e62f0f8cf 100644 --- a/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt +++ b/test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt @@ -15,6 +15,10 @@ import ftl.config.FtlConstants.applicationName import ftl.config.FtlConstants.httpCredential import ftl.config.FtlConstants.httpTransport import ftl.http.executeWithRetry +import ftl.util.FlankCommonException +import ftl.util.FTLProjectError +import ftl.util.PermissionDenied +import ftl.util.ProjectNotFound object GcToolResults { @@ -114,8 +118,27 @@ object GcToolResults { ).executeWithRetry() } - fun getDefaultBucket(projectId: String): String? { - val response = service.Projects().initializeSettings(projectId).executeWithRetry() - return response.defaultBucket + fun getDefaultBucket(projectId: String): 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 FlankCommonException(permissionDeniedErrorMessage(projectId, ftlProjectError.message)) + is ProjectNotFound -> throw FlankCommonException(projectNotFoundErrorMessage(projectId, ftlProjectError.message)) + } } } + +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. +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 + +$message""".trimIndent() +} + +private val projectNotFoundErrorMessage = { projectId: String, message: String? -> + """Flank was unable to find project $projectId. Please verify the project id. + +$message""".trimIndent() +} diff --git a/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt b/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt index 9bf865cfbe..64f699b609 100644 --- a/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt +++ b/test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt @@ -3,6 +3,8 @@ package ftl.http import com.google.api.client.googleapis.services.json.AbstractGoogleJsonClientRequest import com.google.api.client.http.HttpResponseException import ftl.config.FtlConstants +import ftl.util.PermissionDenied +import ftl.util.ProjectNotFound import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import java.io.IOException @@ -14,8 +16,7 @@ import kotlin.math.roundToInt fun AbstractGoogleJsonClientRequest.executeWithRetry(): T = withRetry { this.execute() } private inline fun withRetry(crossinline block: () -> T): T = runBlocking { - var lastErr: IOException? = null - + var lastError: IOException? = null repeat(4) { try { return@runBlocking block() @@ -24,16 +25,19 @@ private inline fun withRetry(crossinline block: () -> T): T = runBlocking { // https://github.com/Flank/flank/issues/701 FtlConstants.bugsnag?.notify(FlankGoogleApiError(err)) - lastErr = err - // HttpStatusCodes from google api client does not have 429 code - if (err is HttpResponseException && err.statusCode == 429) { - return@repeat + lastError = err + if (err is HttpResponseException) { + // we want to handle some FTL errors with special care + when (err.statusCode) { + 429 -> return@repeat + 403 -> throw PermissionDenied(err) + 404 -> throw ProjectNotFound(err) + } } delay(exp(it - 1.0).roundToInt().toLong()) } } - - throw IOException("Request failed", lastErr) + throw IOException("Request failed", lastError) } private class FlankGoogleApiError(exception: Throwable) : Error(exception) diff --git a/test_runner/src/main/kotlin/ftl/util/FlankException.kt b/test_runner/src/main/kotlin/ftl/util/FlankException.kt index 2b7386c251..554ad7b83b 100644 --- a/test_runner/src/main/kotlin/ftl/util/FlankException.kt +++ b/test_runner/src/main/kotlin/ftl/util/FlankException.kt @@ -1,6 +1,7 @@ package ftl.util import ftl.json.SavedMatrix +import java.io.IOException /** * Base class for all custom flank's exceptions @@ -60,3 +61,13 @@ class FlankFatalError(message: String) : FlankException(message) * @param message [String] message to be printed to [System.err] */ class FlankCommonException(message: String) : FlankException(message) + +/** + * Base class for project related exceptions. + * Should be caught and rewrap to FlankCommonException with project id info attached. + * + * @param exc [IOException] + */ +sealed class FTLProjectError(exc: IOException) : FlankException("Caused by: $exc") +class PermissionDenied(exc: IOException) : FTLProjectError(exc) +class ProjectNotFound(exc: IOException) : FTLProjectError(exc) diff --git a/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt b/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt index 2fcf1549b8..b1726e6f37 100644 --- a/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt +++ b/test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt @@ -1,29 +1,41 @@ package ftl.gc +import com.google.api.client.googleapis.json.GoogleJsonResponseException +import com.google.api.client.http.HttpResponseException import com.google.api.services.testing.model.ToolResultsHistory import com.google.common.truth.Truth.assertThat import ftl.args.AndroidArgs +import ftl.config.FtlConstants import ftl.test.util.FlankTestRunner +import ftl.test.util.TestHelper.getThrowable +import ftl.util.FlankCommonException +import ftl.util.PermissionDenied +import ftl.util.ProjectNotFound import io.mockk.every import io.mockk.mockk +import io.mockk.mockkObject import io.mockk.unmockkAll import org.junit.After +import org.junit.Assert.assertEquals import org.junit.Test import org.junit.runner.RunWith +import java.io.IOException @RunWith(FlankTestRunner::class) class GcToolResultsTest { + private val projectId = "123" + @After fun tearDown() = unmockkAll() @Test fun `createToolResultsHistory null succeeds`() { val args = mockk() - every { args.project } returns "123" + every { args.project } returns projectId every { args.resultsHistoryName } returns null - val expected = ToolResultsHistory().setProjectId("123") + val expected = ToolResultsHistory().setProjectId(projectId) assertThat(GcToolResults.createToolResultsHistory(args)).isEqualTo(expected) } @@ -31,13 +43,115 @@ class GcToolResultsTest { @Test fun `createToolResultsHistory succeeds`() { val args = mockk() - every { args.project } returns "123" + every { args.project } returns projectId every { args.resultsHistoryName } returns "custom history" val expected = ToolResultsHistory() - .setProjectId("123") + .setProjectId(projectId) .setHistoryId("mockId") assertThat(GcToolResults.createToolResultsHistory(args)).isEqualTo(expected) } + + @Test + fun `getDefaultBucket on 403 error should throw exception with specific message`() { + 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 + 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) } + assertEquals(expected, exception.message) + } + } + + @Test(expected = FlankCommonException::class) + fun `getDefaultBucket on PermissionDenied error should throw FlankCommonException`() { + mockkObject(GcToolResults) { + every { GcToolResults.service.applicationName } returns projectId + every { GcToolResults.service.Projects().initializeSettings(projectId) } throws PermissionDenied(IOException()) + GcToolResults.getDefaultBucket(projectId) + } + } + + @Test(expected = FlankCommonException::class) + fun `getDefaultBucket on ProjectNotFound error should throw FlankCommonException`() { + mockkObject(GcToolResults) { + every { GcToolResults.service.applicationName } returns projectId + every { GcToolResults.service.Projects().initializeSettings(projectId) } throws ProjectNotFound(IOException()) + GcToolResults.getDefaultBucket(projectId) + } + } + + @Test + fun `getDefaultBucket on 404 error should throw exception with specific message`() { + val expected = """ + Flank was unable to find project $projectId. Please verify the project id. + + Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 404 Not Found + { + "code" : 404, + "errors" : [ { + "domain" : "global", + "message" : "Project not found: $projectId", + "reason" : "notFound" + } ], + "message" : "Project not found: $projectId", + "status" : "NOT_FOUND" + } + """.trimIndent() + mockkObject(GcToolResults) { + every { GcToolResults.service.applicationName } returns FtlConstants.applicationName + + val exceptionBuilder = mockk() + every { exceptionBuilder.message } returns """ + 404 Not Found + { + "code" : 404, + "errors" : [ { + "domain" : "global", + "message" : "Project not found: $projectId", + "reason" : "notFound" + } ], + "message" : "Project not found: $projectId", + "status" : "NOT_FOUND" + } + """.trimIndent() + val mockJSonException = GoogleJsonResponseException(exceptionBuilder, null) + every { GcToolResults.service.Projects().initializeSettings(projectId) } throws ProjectNotFound(mockJSonException) + val exception = getThrowable { GcToolResults.getDefaultBucket(projectId) } + assertEquals(expected, exception.message) + } + } }