Skip to content

Commit

Permalink
Enhance permission denied exception logs (#875)
Browse files Browse the repository at this point in the history
  • Loading branch information
pawelpasterz authored Jul 6, 2020
1 parent aded032 commit 54930da
Show file tree
Hide file tree
Showing 6 changed files with 226 additions and 17 deletions.
57 changes: 57 additions & 0 deletions docs/permissions_denied_behavior.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down
29 changes: 26 additions & 3 deletions test_runner/src/main/kotlin/ftl/gc/GcToolResults.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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()
}
20 changes: 12 additions & 8 deletions test_runner/src/main/kotlin/ftl/http/ExecuteWithRetry.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,8 +16,7 @@ import kotlin.math.roundToInt
fun <T> AbstractGoogleJsonClientRequest<T>.executeWithRetry(): T = withRetry { this.execute() }

private inline fun <T> withRetry(crossinline block: () -> T): T = runBlocking {
var lastErr: IOException? = null

var lastError: IOException? = null
repeat(4) {
try {
return@runBlocking block()
Expand All @@ -24,16 +25,19 @@ private inline fun <T> 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)
11 changes: 11 additions & 0 deletions test_runner/src/main/kotlin/ftl/util/FlankException.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ftl.util

import ftl.json.SavedMatrix
import java.io.IOException

/**
* Base class for all custom flank's exceptions
Expand Down Expand Up @@ -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)
122 changes: 118 additions & 4 deletions test_runner/src/test/kotlin/ftl/gc/GcToolResultsTest.kt
Original file line number Diff line number Diff line change
@@ -1,43 +1,157 @@
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<AndroidArgs>()
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)
}

@Test
fun `createToolResultsHistory succeeds`() {
val args = mockk<AndroidArgs>()
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<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) }
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<HttpResponseException.Builder>()
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)
}
}
}

0 comments on commit 54930da

Please sign in to comment.