AJ-1438: disable most entity APIs for Azure workspaces #2641
Conversation
| if (!WorkspaceType.RawlsWorkspace.equals(requestArguments.workspace.workspaceType)) { | ||
| throw new DataEntityException(s"This functionality only available to ${CloudPlatform.GCP} workspaces.") | ||
| } | ||
|
|
There was a problem hiding this comment.
This is the heart of the PR: throw an error for non-RawlsWorkspace workspaces.
| ctx | ||
| ) | ||
| .recover(bigQueryRecover) | ||
| } yield entityCopyResponse |
There was a problem hiding this comment.
While moving the important implementation inside the LocalEntityProvider, I took the opportunity to refactor and simplify this method. It is now one big for-yield instead of a mix of flatMaps and for-yields, AND it removes outbound REST calls from Rawls to Sam from being inside a database transaction.
| else { | ||
| val missingGroups = sourceWorkspaceADs -- destWorkspaceADs | ||
| val errorMsg = | ||
| s"Source workspace has an Authorization Domain containing the groups ${missingGroups.mkString(", ")}, which are missing on the destination workspace" | ||
| DBIO.failed(new RawlsExceptionWithErrorReport(ErrorReport(StatusCodes.UnprocessableEntity, errorMsg))) | ||
| throw new RawlsExceptionWithErrorReport(ErrorReport(StatusCodes.UnprocessableEntity, errorMsg)) | ||
| } |
There was a problem hiding this comment.
This authDomainCheck method has only one usage - inside copyEntities, which I rewrote above. There was no need for authDomainCheck to return its result wrapped in a ReadWriteAction.
| @@ -317,7 +317,7 @@ trait EntityApiService extends UserInfoDirectives { | |||
| entity(as[EntityCopyDefinition]) { copyDefinition => | |||
| complete { | |||
| entityServiceConstructor(ctx) | |||
| .copyEntities(copyDefinition, request.uri, linkExistingEntitiesBool) | |||
| .copyEntities(copyDefinition, linkExistingEntitiesBool) | |||
There was a problem hiding this comment.
I have no idea why the request URI was passed into copyEntities. It was never used, and I removed it from the argument list for copyEntities.
|
two service timeouts in integration tests. Jenkins retest. |
| @@ -62,6 +63,9 @@ object RawlsApiService extends LazyLogging { | |||
| Sentry.captureException(wsmApiException) | |||
| } | |||
| complete(wsmApiException.getCode -> ErrorReport(wsmApiException).copy(stackTrace = Seq())) | |||
| case dataEntityException: DataEntityException => | |||
| // propagate only the message; don't include the stack trace | |||
| complete(dataEntityException.code -> ErrorReport(dataEntityException.getMessage)) | |||
There was a problem hiding this comment.
I thought about removing stack traces from the response in the catch-all case just below this … but that seemed like it could impact use cases I didn't want to think through. So, I'm adding a case just for DataEntityException, which is only thrown by data tables. There's no need to send the stack trace to the end user for these exceptions.
|
Jenkins retest, trying to get the TSPS fix in |
|
a few 503s from jenkins. Jenkins retest. |
|
One error on "Billing Project tmp-billing-project-1f5db88701 does not exist in Rawls". Jenkins retest. |
|
well this is frustrating. More 503s. Jenkins retest. |
Ticket: https://broadworkbench.atlassian.net/browse/AJ-1438
This PR disables all writable entity APIs and most read-only entity APIs for Azure workspaces. Azure workspaces use WDS instead of Rawls for data tables; the APIs in Rawls are misleading and should not be used for Azure workspaces.
This check is implemented by using
EntityManageras a bottleneck. InsideEntityManager.resolveProvider(), we check the target workspace and throw an error if it is not a GCP (non-MC) workspace. As part of this, I needed to move thecopyEntitiesimplementation inside theEntityManager/EntityProviderstructure, instead of its previous implementation directly inEntityService.There are still a few read-only APIs that are implemented directly in
EntityServiceand won't throw an error for Azure workspaces. However, all write APIs are now protected, so there's no way to get data IN to the system for those read APIs to return. I think this satisfies the intent of the Jira ticket, without blowing up scope too much.Update 8-Dec: I tested this in a BEE with the latest UI code, and all works as expected.
PR checklist
model/, then you should publish a new officialrawls-modeland updaterawls-modelin Orchestration's dependencies.