Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WOR-1781][WOR-1782] APIs to manage workspace settings #2983

Merged
merged 25 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
097daee
liquibase changeset to create workspace settings table
marctalbott Jul 24, 2024
1c2287a
wip, compiles but untested
marctalbott Jul 29, 2024
a4536df
swagger
marctalbott Jul 29, 2024
01e0d27
only return applied settings
marctalbott Aug 1, 2024
e541f61
rename WorkspaceSetting, handle empty settings, simplify rollback
marctalbott Aug 6, 2024
3dc24ec
return errors alongside successes, soft delete settings
marctalbott Aug 6, 2024
bbde9b5
bug fixes, require config, add validation
marctalbott Aug 7, 2024
03d7e31
start unit tests
marctalbott Aug 7, 2024
6259e9a
fix tests
marctalbott Aug 7, 2024
75301c5
more unit tests, rework some db things
marctalbott Aug 8, 2024
460df06
repository tests
marctalbott Aug 8, 2024
39342b5
parsing tests and rename lifecycle case classes
marctalbott Aug 8, 2024
029d41d
require config and update lastUpdated when updating setting record
marctalbott Aug 8, 2024
73a61e3
cleanup, move lifecycle action verification earlier
marctalbott Aug 9, 2024
5178b1e
scalafmt
marctalbott Aug 9, 2024
2c30dba
change transaction isolation level, add comments, track user who made…
marctalbott Aug 9, 2024
5dde8bb
no backticks
marctalbott Aug 9, 2024
b6e416a
don't fully overwrite, update; correct field name; handle identical s…
marctalbott Aug 12, 2024
7b538be
separate settings service
marctalbott Aug 12, 2024
7686821
separate validation test cases
marctalbott Aug 13, 2024
13c934d
swagger and fix provider routes
marctalbott Aug 13, 2024
c5232c2
fix unit tests
marctalbott Aug 13, 2024
118f779
scalafmt
marctalbott Aug 13, 2024
45ab826
break out WorkspaceSettingRepository
marctalbott Aug 14, 2024
90b7f6b
remove leftover imports
marctalbott Aug 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,5 @@
<include file="changesets/20231013_submission_monitor_script.xml" relativeToChangelogFile="true"/>
<include file="changesets/20231130_limit_clone_workspace_file_transfer.xml" relativeToChangelogFile="true"/>
<include file="changesets/20240418_workspace_monitor_args.xml" relativeToChangelogFile="true"/>
<include file="changesets/20240723_workspace_settings.xml" relativeToChangelogFile="true"/>
</databaseChangeLog>
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should also capture who initiated the change, ideally the sam/terra UID and not an email.

<databaseChangeLog logicalFilePath="dummy" xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.4.xsd">
<changeSet logicalFilePath="dummy" author="mtalbott" id="create_WORKSPACE_SETTINGS">
<createTable tableName="WORKSPACE_SETTINGS">
<column name="id" type="BIGINT" autoIncrement="true">
<constraints primaryKey="true"/>
</column>
<column name="WORKSPACE_ID" type="BINARY(16)">
<constraints nullable="false" referencedTableName="WORKSPACE" referencedColumnNames="id" foreignKeyName="FK_WSETTINGS_WORKSPACE_ID" deleteCascade="true"/>
</column>
<column name="TYPE" type="VARCHAR(254)">
<constraints nullable="false"/>
</column>
<column name="CONFIG" type="JSON">
<constraints nullable="false"/>
</column>
<column name="CREATED_TIME" type="DATETIME(6)" defaultValueComputed="NOW(6)">
<constraints nullable="false"/>
</column>
<column name="LAST_UPDATED" type="DATETIME(6)" defaultValueComputed="NOW(6)">
<constraints nullable="false"/>
</column>
<column name="STATUS" type="VARCHAR(254)">
<constraints nullable="false"/>
</column>
</createTable>
</changeSet>
</databaseChangeLog>
128 changes: 128 additions & 0 deletions core/src/main/resources/swagger/api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4423,6 +4423,78 @@ paths:
$ref: '#/components/schemas/ErrorReport'
500:
$ref: '#/components/responses/RawlsInternalError'
/api/workspaces/v2/{workspaceNamespace}/{workspaceName}/settings:
get:
tags:
- workspaces_v2
summary: Get workspace settings
description: Get the settings for a workspace
operationId: getWorkspaceSettings
parameters:
- $ref: '#/components/parameters/workspaceNamespacePathParam'
- $ref: '#/components/parameters/workspaceNamePathParam'
responses:
200:
description: Success
content:
'application/json':
schema:
type: array
items:
$ref: '#/components/schemas/WorkspaceSettings'
403:
description: User does not have access to requested workspace
content:
'application/json':
schema:
$ref: '#/components/schemas/ErrorReport'
404:
description: Workspace not found
content:
'application/json':
schema:
$ref: '#/components/schemas/ErrorReport'
500:
$ref: '#/components/responses/RawlsInternalError'
put:
tags:
- workspaces_v2
summary: Overwrite workspace settings
description: Overwrite the settings for a workspace
operationId: overwriteWorkspaceSettings
parameters:
- $ref: '#/components/parameters/workspaceNamespacePathParam'
- $ref: '#/components/parameters/workspaceNamePathParam'
requestBody:
description: New settings for the workspace
content:
'application/json':
schema:
type: array
items:
$ref: '#/components/schemas/WorkspaceSettings'
required: true
responses:
200:
description: Success
content:
'application/json':
schema:
$ref: '#/components/schemas/WorkspaceSettings'
403:
description: User must be an owner of the workspace to overwrite settings
content:
'application/json':
schema:
$ref: '#/components/schemas/ErrorReport'
404:
description: Workspace not found
content:
'application/json':
schema:
$ref: '#/components/schemas/ErrorReport'
500:
$ref: '#/components/responses/RawlsInternalError'
components:
schemas:
BillingAccount:
Expand Down Expand Up @@ -5818,6 +5890,62 @@ components:
- Deleting
- DeleteFailed
description: ""
WorkspaceSettings:
required:
- type
- config
type: object
properties:
'type':
type: string
description: The type of the workspace setting
enum:
- GcpBucketLifecycle
config:
description: The configuration of the workspace setting
oneOf:
- $ref: '#/components/schemas/WorkspaceSettingGcpBucketLifecycleConfig'
WorkspaceSettingGcpBucketLifecycleConfig:
required:
- rules
type: object
properties:
rules:
type: array
description: The lifecycle rules for the workspace bucket
items:
$ref: '#/components/schemas/GcpBucketLifecycleRule'
GcpBucketLifecycleRule:
required:
- action
- conditions
type: object
properties:
action:
$ref: '#/components/schemas/GcpBucketLifecycleAction'
conditions:
$ref: '#/components/schemas/GcpBucketLifecycleCondition'
GcpBucketLifecycleAction:
required:
- type
type: object
properties:
"type":
type: string
description: The type of the lifecycle action
enum:
- Delete
GcpBucketLifecycleCondition:
type: object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of these properties required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either can be omitted, but we do need at least one now that I think about it which isn't accounted for. I'll change that.

properties:
age:
type: integer
description: The age of the object in days
matchesPrefix:
type: array
description: Object name prefixes that this rule applies to
items:
type: string
WorkspaceSubmissionStats:
required:
- runningSubmissionsCount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.google.api.services.cloudbilling.model.ProjectBillingInfo
import com.google.api.services.cloudresourcemanager.model.Project
import com.google.api.services.directory.model.Group
import com.google.api.services.storage.model.{Bucket, BucketAccessControl, StorageObject}
import com.google.cloud.storage.BucketInfo.LifecycleRule
import org.broadinstitute.dsde.rawls.google.AccessContextManagerDAO
import org.broadinstitute.dsde.rawls.model.WorkspaceAccessLevels._
import org.broadinstitute.dsde.rawls.model._
Expand Down Expand Up @@ -63,6 +64,8 @@ trait GoogleServicesDAO extends ErrorReportable {
*/
def deleteBucket(bucketName: String): Future[Boolean]

def setBucketLifecycle(bucketName: String, lifecycle: List[LifecycleRule]): Future[Unit]

def isAdmin(userEmail: String): Future[Boolean]

def isLibraryCurator(userEmail: String): Future[Boolean]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ import com.google.api.client.googleapis.json.GoogleJsonResponseException
import com.google.api.client.http.{HttpRequest, HttpRequestInitializer, HttpResponseException, InputStreamContent}
import com.google.api.client.json.gson.GsonFactory
import com.google.api.services.cloudbilling.Cloudbilling
import com.google.api.services.cloudbilling.model.{
BillingAccount,
ListBillingAccountsResponse,
ProjectBillingInfo,
TestIamPermissionsRequest
}
import com.google.api.services.cloudbilling.model.{BillingAccount, ListBillingAccountsResponse, ProjectBillingInfo, TestIamPermissionsRequest}
import com.google.api.services.cloudresourcemanager.CloudResourceManager
import com.google.api.services.cloudresourcemanager.model._
import com.google.api.services.compute.{Compute, ComputeScopes}
Expand All @@ -41,7 +36,7 @@ import com.google.api.services.storage.{Storage, StorageScopes}
import com.google.auth.oauth2.ServiceAccountCredentials
import com.google.cloud.Identity
import com.google.cloud.storage.Storage.BucketSourceOption
import com.google.cloud.storage.{Cors, HttpMethod, StorageClass, StorageException}
import com.google.cloud.storage.{BucketInfo, Cors, HttpMethod, StorageClass, StorageException}
import io.opentelemetry.api.common.AttributeKey
import org.apache.commons.lang3.StringUtils
import org.broadinstitute.dsde.rawls.dataaccess.CloudResourceManagerV2Model.{Folder, FolderSearchResponse}
Expand All @@ -52,11 +47,7 @@ import org.broadinstitute.dsde.rawls.metrics.GoogleInstrumentedService
import org.broadinstitute.dsde.rawls.model.UserAuthJsonSupport._
import org.broadinstitute.dsde.rawls.model.WorkspaceAccessLevels._
import org.broadinstitute.dsde.rawls.model._
import org.broadinstitute.dsde.rawls.util.TracingUtils.{
setTraceSpanAttribute,
traceFutureWithParent,
traceNakedWithParent
}
import org.broadinstitute.dsde.rawls.util.TracingUtils.{setTraceSpanAttribute, traceFutureWithParent, traceNakedWithParent}
import org.broadinstitute.dsde.rawls.util.{FutureSupport, HttpClientUtilsStandard}
import org.broadinstitute.dsde.rawls.{RawlsException, RawlsExceptionWithErrorReport}
import org.broadinstitute.dsde.workbench.google.{GoogleCredentialModes, HttpGoogleIamDAO}
Expand Down Expand Up @@ -331,6 +322,13 @@ class HttpGoogleServicesDAO(val clientSecrets: GoogleClientSecrets,
}
}

override def setBucketLifecycle(bucketName: String, lifecycle: List[BucketInfo.LifecycleRule]): Future[Unit] =
googleStorageService
.setBucketLifecycle(GcsBucketName(bucketName), lifecycle)
.compile
.drain
.unsafeToFuture()

override def isAdmin(userEmail: String): Future[Boolean] =
hasGoogleRole(adminGroupName, userEmail)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ trait DataAccess
with WorkspaceFeatureFlagComponent
with WorkspaceManagerResourceMonitorRecordComponent
with FastPassGrantComponent
with MultiregionalBucketMigrationHistory {
with MultiregionalBucketMigrationHistory
with WorkspaceSettingComponent {

this: DriverComponent =>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package org.broadinstitute.dsde.rawls.dataaccess.slick

import org.broadinstitute.dsde.rawls.model.WorkspaceJsonSupport.GcpBucketLifecycleConfigFormat
import org.broadinstitute.dsde.rawls.model.WorkspaceSettingConfig.GcpBucketLifecycleConfig
import org.broadinstitute.dsde.rawls.model.WorkspaceSettingTypes.WorkspaceSettingType
import org.broadinstitute.dsde.rawls.model._

import java.sql.Timestamp
import java.util.{Date, UUID}

case class WorkspaceSettingRecord(`type`: String,
workspaceId: UUID,
config: String,
status: String,
createdTime: Timestamp,
lastUpdated: Timestamp
)

object WorkspaceSettingRecord {
object SettingStatus extends SlickEnum {
type SettingStatus = Value
val Pending: Value = Value("Pending")
val Applied: Value = Value("Applied")
val Deleted: Value = Value("Deleted")
}

def toWorkspaceSettingRecord(workspaceId: UUID, workspaceSettings: WorkspaceSetting): WorkspaceSettingRecord = {
import spray.json._
import DefaultJsonProtocol._
import WorkspaceJsonSupport._

val currentTime = new Timestamp(new Date().getTime)
val configString = workspaceSettings.config.toJson.compactPrint
WorkspaceSettingRecord(workspaceSettings.`type`.toString,
workspaceId,
configString,
WorkspaceSettingRecord.SettingStatus.Pending.toString,
currentTime,
currentTime
)
}

def toWorkspaceSettings(workspaceSettingRecord: WorkspaceSettingRecord): WorkspaceSetting = {
import spray.json._

val settingType = WorkspaceSettingTypes.withName(workspaceSettingRecord.`type`)
val settingConfig = settingType match {
case WorkspaceSettingTypes.GcpBucketLifecycle =>
workspaceSettingRecord.config.parseJson.convertTo[GcpBucketLifecycleConfig]
}

WorkspaceSetting(settingType, settingConfig)
}
}

trait WorkspaceSettingComponent {
this: DriverComponent with WorkspaceComponent =>

import driver.api._
class WorkspaceSettingTable(tag: Tag) extends Table[WorkspaceSettingRecord](tag, "WORKSPACE_SETTINGS") {
def `type` = column[String]("type", O.Length(254))
def workspaceId = column[UUID]("workspace_id")
def config = column[String]("config")
def status = column[String]("status", O.Length(254))
def createdTime = column[Timestamp]("created_time")
def lastUpdated = column[Timestamp]("last_updated")

def * = (`type`, workspaceId, config, status, createdTime, lastUpdated) <> (WorkspaceSettingRecord.tupled,
WorkspaceSettingRecord.unapply
)
}

object workspaceSettingQuery extends TableQuery(new WorkspaceSettingTable(_)) {
def saveAll(workspaceId: UUID,
workspaceSettings: List[WorkspaceSetting]
): ReadWriteAction[List[WorkspaceSetting]] = {
val records = workspaceSettings.map(WorkspaceSettingRecord.toWorkspaceSettingRecord(workspaceId, _))
(workspaceSettingQuery ++= records).map(_ => workspaceSettings)
}

def updateSettingStatus(workspaceId: UUID,
workspaceSettingType: WorkspaceSettingType,
currentStatus: WorkspaceSettingRecord.SettingStatus.SettingStatus,
newStatus: WorkspaceSettingRecord.SettingStatus.SettingStatus
): ReadWriteAction[Int] =
workspaceSettingQuery
.filter(record =>
record.workspaceId === workspaceId && record.`type` === workspaceSettingType.toString && record.status === currentStatus.toString
)
.map(rec => (rec.status, rec.lastUpdated))
.update((newStatus.toString, new Timestamp(new Date().getTime)))

def deleteSettingTypeForWorkspaceByStatus(workspaceId: UUID,
settingType: WorkspaceSettingType,
status: WorkspaceSettingRecord.SettingStatus.SettingStatus
): ReadWriteAction[Int] =
filter(record =>
record.workspaceId === workspaceId && record.`type` === settingType.toString && record.status === status.toString
).delete

def listSettingsForWorkspaceByStatus(workspaceId: UUID,
status: WorkspaceSettingRecord.SettingStatus.SettingStatus
): ReadAction[List[WorkspaceSetting]] =
filter(rec => rec.workspaceId === workspaceId && rec.status === status.toString).result
.map(_.map(WorkspaceSettingRecord.toWorkspaceSettings).toList)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ trait WorkspaceApiServiceV2 extends UserInfoDirectives {
}
}
}
} ~
pathPrefix("settings") {
pathEndOrSingleSlash {
get {
complete {
workspaceServiceConstructor(ctx)
.getWorkspaceSettings(workspaceName)
.map(StatusCodes.OK -> _)
}
} ~
put {
entity(as[List[WorkspaceSetting]]) { settings =>
complete {
workspaceServiceConstructor(ctx)
.setWorkspaceSettings(workspaceName, settings)
.map(StatusCodes.OK -> _)
}
}
}
}
}
} ~
pathPrefix("bucketMigration") {
Expand Down
Loading
Loading