-
Notifications
You must be signed in to change notification settings - Fork 360
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
WX-728 Add configurable WSM client to Cromwell #6948
Changes from all commits
d128eb2
b854949
3f4b998
01a0dd1
453aa64
b8a0f64
10931f5
a135d48
0841e10
94777ed
6a6dbf9
3be7c3d
ed95227
f38bb2c
666c7e5
828cb18
b3fad38
24d9c10
f7dc8ef
6b2abb8
4c0373f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"fileChecksum.inputFile": "https://<storage-account>.blob.core.windows.net/cromwell/user-inputs/inputFile.txt" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
task md5 { | ||
File inputFile | ||
command { | ||
echo "`date`: Running checksum on ${inputFile}..." | ||
md5sum ${inputFile} > md5sum.txt | ||
echo "`date`: Checksum is complete." | ||
} | ||
output { | ||
File result = "md5sum.txt" | ||
} | ||
runtime { | ||
docker: 'ubuntu:18.04' | ||
preemptible: true | ||
} | ||
} | ||
|
||
workflow fileChecksum { | ||
File inputFile | ||
call md5 { input: inputFile=inputFile} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,8 @@ import scala.util.{Failure, Success, Try} | |
import com.azure.resourcemanager.storage.models.StorageAccountKey | ||
import com.typesafe.scalalogging.LazyLogging | ||
|
||
import java.util.UUID | ||
|
||
case class FileSystemAPI() { | ||
def getFileSystem(uri: URI): Try[FileSystem] = Try(FileSystems.getFileSystem(uri)) | ||
def newFileSystem(uri: URI, config: Map[String, Object]): FileSystem = FileSystems.newFileSystem(uri, config.asJava) | ||
|
@@ -82,31 +84,32 @@ case class BlobFileSystemManager( | |
sealed trait BlobTokenGenerator {def generateAccessToken: Try[AzureSasCredential]} | ||
object BlobTokenGenerator { | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, subscription: Option[SubscriptionId]): BlobTokenGenerator = { | ||
createBlobTokenGenerator(container, endpoint, None, None, subscription) | ||
NativeBlobTokenGenerator(container, endpoint, subscription) | ||
} | ||
def createBlobTokenGenerator(container: BlobContainerName, | ||
endpoint: EndpointURL, | ||
workspaceId: Option[WorkspaceId], | ||
workspaceManagerURL: Option[WorkspaceManagerURL], | ||
subscription: Option[SubscriptionId] | ||
): BlobTokenGenerator = { | ||
(container: BlobContainerName, endpoint: EndpointURL, workspaceId, workspaceManagerURL) match { | ||
case (container, endpoint, None, None) => | ||
NativeBlobTokenGenerator(container, endpoint, subscription) | ||
case (container, endpoint, Some(workspaceId), Some(workspaceManagerURL)) => | ||
WSMBlobTokenGenerator(container, endpoint, workspaceId, workspaceManagerURL) | ||
case _ => | ||
throw new Exception("Arguments provided do not match any available BlobTokenGenerator implementation.") | ||
} | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, workspaceId: WorkspaceId, workspaceManagerClient: WorkspaceManagerApiClientProvider): BlobTokenGenerator = { | ||
WSMBlobTokenGenerator(container, endpoint, workspaceId, workspaceManagerClient) | ||
} | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL): BlobTokenGenerator = createBlobTokenGenerator(container, endpoint, None) | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, workspaceId: Option[WorkspaceId], workspaceManagerURL: Option[WorkspaceManagerURL]): BlobTokenGenerator = | ||
createBlobTokenGenerator(container, endpoint, workspaceId, workspaceManagerURL, None) | ||
Comment on lines
86
to
-104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pared down this helper class to exactly two functions, one for each of |
||
|
||
} | ||
|
||
case class WSMBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, workspaceId: WorkspaceId, workspaceManagerURL: WorkspaceManagerURL) extends BlobTokenGenerator { | ||
def generateAccessToken: Try[AzureSasCredential] = Failure(new NotImplementedError) | ||
case class WSMBlobTokenGenerator( | ||
container: BlobContainerName, | ||
endpoint: EndpointURL, | ||
workspaceId: WorkspaceId, | ||
wsmClient: WorkspaceManagerApiClientProvider) extends BlobTokenGenerator { | ||
|
||
def generateAccessToken: Try[AzureSasCredential] = Try { | ||
val token = wsmClient.getControlledAzureResourceApi.createAzureStorageContainerSasToken( | ||
UUID.fromString(workspaceId.value), | ||
UUID.fromString("00001111-2222-3333-aaaa-bbbbccccdddd"), | ||
null, | ||
null, | ||
null, | ||
null | ||
).getToken // TODO `null` items may be required, investigate in WX-696 | ||
|
||
new AzureSasCredential(token) // TODO Does `signature` actually mean token? save for WX-696 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice breadcrumbs! |
||
} | ||
} | ||
|
||
case class NativeBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, subscription: Option[SubscriptionId] = None) extends BlobTokenGenerator { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package cromwell.filesystems.blob | ||
|
||
import bio.terra.workspace.api.ControlledAzureResourceApi | ||
import bio.terra.workspace.client.ApiClient | ||
|
||
/** | ||
* Represents a way to get various workspace manager clients | ||
* | ||
* Pared down from `org.broadinstitute.dsde.rawls.dataaccess.workspacemanager.WorkspaceManagerApiClientProvider` | ||
* | ||
* For testing, create an anonymous subclass as in `org.broadinstitute.dsde.rawls.dataaccess.workspacemanager.HttpWorkspaceManagerDAOSpec` | ||
*/ | ||
trait WorkspaceManagerApiClientProvider { | ||
def getApiClient: ApiClient | ||
|
||
def getControlledAzureResourceApi: ControlledAzureResourceApi | ||
|
||
} | ||
|
||
class HttpWorkspaceManagerClientProvider(baseWorkspaceManagerUrl: WorkspaceManagerURL, token: String) extends WorkspaceManagerApiClientProvider { | ||
def getApiClient: ApiClient = { | ||
val client: ApiClient = new ApiClient() | ||
client.setBasePath(baseWorkspaceManagerUrl.value) | ||
client.setAccessToken(token) | ||
|
||
client | ||
} | ||
|
||
def getControlledAzureResourceApi: ControlledAzureResourceApi = | ||
new ControlledAzureResourceApi(getApiClient) | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These Centaur files do not have an associated
.test
file yet so they are not actually running, but I did think it was time to promote them from Slack/local disk to source control.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love having references to specific Azure resources checked in. Even if we did have good integration testing set up, I think we'd get them from Vault or whatever rather than putting them in source control. I agree that the team needs better access to this stuff, though. Maybe throw it all into a wiki page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a bunch of places we do this with GCP, specifically the buckets in
broad-dsde-cromwell-dev
.cromwell/centaur/src/main/resources/standardTestCases/input_localization/localize_file_larger_than_disk_space.wdl
Line 20 in d16138a
We don't have to go down the same route for Azure, but I'd prefer to defer that discussion to when we set up integration testing. For now, I genericized it since I think we truly do have no idea what account we'll be in then.