tidy

supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala

@@ -15,7 +15,7 @@ import common.validation.ErrorOr.ErrorOr
 import common.validation.Validation._
 import cromwell.backend.BackendJobLifecycleActor
 import cromwell.backend.async.{AbortedExecutionHandle, ExecutionHandle, FailedNonRetryableExecutionHandle, PendingExecutionHandle}
-import cromwell.backend.impl.tes.TesAsyncBackendJobExecutionActor.{determineWSMSasEndpointFromInputs, generateLocalizedSasScriptPreamble, maybeConvertToBlob}
+import cromwell.backend.impl.tes.TesAsyncBackendJobExecutionActor.{determineWSMSasEndpointFromInputs, generateLocalizedSasScriptPreamble}
 import cromwell.backend.impl.tes.TesResponseJsonFormatter._
 import cromwell.backend.standard.{StandardAsyncExecutionActor, StandardAsyncExecutionActorParams, StandardAsyncJob}
 import cromwell.core.path.{DefaultPathBuilder, Path}
@@ -62,7 +62,6 @@ object TesAsyncBackendJobExecutionActor {
   private def generateLocalizedSasScriptPreamble(getSasWsmEndpoint: String) : String = {
     // BEARER_TOKEN: https://learn.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/how-to-use-vm-token#get-a-token-using-http
     // NB: Scala string interpolation and bash variable substitution use similar syntax. $$ is an escaped $, much like \\ is an escaped \.
-    // NB: For easier debugging/logging, we echo the first 4 characters of the acquired sas token. If something goes wrong with the curl, these will be "null"
     s"""
        |### BEGIN ACQUIRE LOCAL SAS TOKEN ###
        |# Install dependencies
@@ -78,19 +77,18 @@ object TesAsyncBackendJobExecutionActor {
        |BEARER_TOKEN="$${BEARER_TOKEN%\\"}"
        |
        |# Use the precomputed endpoint from cromwell + WSM to acquire a sas token
-       |GET_SAS_ENDPOINT=$getSasWsmEndpoint
        |sas_response_json=$$(curl -s \\
        |                    --retry 3 \\
        |                    --retry-delay 2 \\
-       |                    -X POST "$$GET_SAS_ENDPOINT" \\
+       |                    -X POST "$getSasWsmEndpoint" \\
        |                    -H "Content-Type: application/json" \\
        |                    -H "accept: */*" \\
        |                    -H "Authorization: Bearer $${BEARER_TOKEN}")
        |
        |# Store token as environment variable
        |export AZURE_STORAGE_SAS_TOKEN=$$(echo "$${sas_response_json}" | jq -r '.token')
        |
-       |# Echo the first 3 characters for logging/debugging purposes. We expect them to be sv= if everything went well.
+       |# Echo the first characters for logging/debugging purposes. "null" indicates something went wrong.
        |echo Acquired sas token: "$${AZURE_STORAGE_SAS_TOKEN:0:3}****"
        |### END ACQUIRE LOCAL SAS TOKEN ###
        |""".stripMargin
@@ -99,6 +97,7 @@ object TesAsyncBackendJobExecutionActor {
   private def maybeConvertToBlob(pathToTest: Try[Path]): Try[BlobPath] = {
     pathToTest.collect { case blob: BlobPath => blob }
   }
+
   /**
    * Under certain situations (and only on Terra), we want the VM running a TES task to have the ability to acquire a
    * fresh sas token for itself. In order to be able to do this, we provide it with a precomputed endpoint it can use.
@@ -107,27 +106,27 @@ object TesAsyncBackendJobExecutionActor {
    * @param taskInputs The inputs to this particular TesTask. If any are blob files, the first  will be used to
    *                   determine the storage container to retrieve the sas token for.
    * @param pathGetter A function to convert string filepath into a cromwell Path object.
-   * @param blobConverter A function to convert a Path into a Blob path, if possible. Use the default: this is a parameter
-   *                      only because the usual means of identifying if a Path is a BlobPath doesn't work with mocked types.
+   * @param blobConverter A function to convert a Path into a Blob path, if possible. Provided for testing purposes.
+   * @return A URL endpoint that, when called with proper authentication, will return a sas token.
+   *         Returns 'None' if one should not be used for this task.
    */
   def determineWSMSasEndpointFromInputs(taskInputs: List[Input],
                                         pathGetter: String => Try[Path],
                                         blobConverter: Try[Path] => Try[BlobPath] = maybeConvertToBlob): Option[String] = {
     val shouldLocalizeSas = true //TODO: Make this a Workflow Option or come from the WDL
     if (!shouldLocalizeSas) return None
 
-    val blobFiles = taskInputs.collect{ //Collect all inputs with URLs defined as (in)valid blob paths
+    // Collect all of the inputs that are valid blob paths
+    val blobFiles = taskInputs.collect{
       case input if input.url.isDefined => BlobPathBuilder.validateBlobPath(input.url.get)
-    }.collect{ //Collect only the valid blob paths
+    }.collect{
       case valid: BlobPathBuilder.ValidBlobPath => valid
     }
-
     if(blobFiles.isEmpty) return None
-    // We use the first blob file in the list as a template for determining the localized sas params
-    val blobPath: Try[BlobPath] = blobConverter(pathGetter(blobFiles.head.toUrl))
 
+    // We use the first blob file in the list as a template for determining the localized sas params
     val sasTokenEndpoint = for {
-      blob <- blobPath
+      blob <- blobConverter(pathGetter(blobFiles.head.toUrl))
       wsmEndpoint <- blob.wsmEndpoint
       workspaceId <- blob.parseTerraWorkspaceIdFromPath
       containerResourceId <- blob.containerWSMResourceId
@@ -141,7 +140,6 @@ object TesAsyncBackendJobExecutionActor {
   }
 }
 
-case
 class TesAsyncBackendJobExecutionActor(override val standardParams: StandardAsyncExecutionActorParams)
   extends BackendJobLifecycleActor with StandardAsyncExecutionActor with TesJobCachingActorHelper {
   implicit val actorSystem = context.system
@@ -176,9 +174,11 @@ class TesAsyncBackendJobExecutionActor(override val standardParams: StandardAsyn
       _.collectAsSeq { case w: WomFile => w }
     }
     val taskInputs: List[Input] = TesTask.buildTaskInputs(callInputFiles, workflowName, mapCommandLineWomFile)
-    super.scriptPreamble ++ determineWSMSasEndpointFromInputs(taskInputs, getPath).map{
+    val tesTaskPreamble = determineWSMSasEndpointFromInputs(taskInputs, getPath).map {
       endpoint => generateLocalizedSasScriptPreamble(endpoint)
     }.getOrElse("")
+
+    super.scriptPreamble ++ tesTaskPreamble
   }
 
   override def mapCommandLineWomFile(womFile: WomFile): WomFile = {

supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActorSpec.scala

@@ -68,10 +68,9 @@ class TesAsyncBackendJobExecutionActorSpec extends AnyFlatSpec with Matchers wit
   mockBlobPath.wsmEndpoint returns Try(mockWsmEndpoint)
   mockBlobPath.parseTerraWorkspaceIdFromPath returns Try(UUID.fromString(mockWorkspaceId))
   mockBlobPath.containerWSMResourceId returns Try(UUID.fromString(mockContainerResourceId))
-  mockBlobPath.md5 returns "blobmd5"
+  mockBlobPath.md5 returns "BLOB_MD5"
 
   val mockPath: Path = mock[Path]
-  mockPath.md5 returns "nonBlobmd5"
   def mockPathGetter(pathString: String): Try[Path] = {
     val foundBlobPath: Success[BlobPath] = Success(mockBlobPath)
     val foundNonBlobPath: Success[Path] = Success(mockPath)
@@ -80,9 +79,8 @@ class TesAsyncBackendJobExecutionActorSpec extends AnyFlatSpec with Matchers wit
   }
 
   def mockBlobConverter(pathToConvert: Try[Path]): Try[BlobPath] = {
-    val mockMd5 = mockBlobPath.md5
-    val pathMd5 = pathToConvert.get.md5
-    if (mockMd5.equals(pathMd5)) pathToConvert.asInstanceOf[Try[BlobPath]] else Failure(new Exception("failed"))
+    //using a stubbed md5 rather than matching on type because type matching of mocked types at runtime causes problems
+    if (pathToConvert.get.md5.equals("BLOB_MD5")) pathToConvert.asInstanceOf[Try[BlobPath]] else Failure(new Exception("failed"))
   }
 
   it should "not return sas endpoint when no blob paths are provided" in {
@@ -94,13 +92,11 @@ class TesAsyncBackendJobExecutionActorSpec extends AnyFlatSpec with Matchers wit
 
   it should "return a sas endpoint based on inputs when blob paths are provided" in {
     val expected = s"$mockWsmEndpoint/api/workspaces/v1/$mockWorkspaceId/resources/controlled/azure/storageContainer/$mockContainerResourceId/getSasToken"
+    val blobInput: List[Input] = List(blobInput_0)
     val blobInputs: List[Input] = List(blobInput_0, blobInput_1)
     val mixedInputs: List[Input] = List(notBlobInput_1, blobInput_0, blobInput_1)
+    TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(blobInput, mockPathGetter, mockBlobConverter).get shouldEqual expected
     TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(blobInputs, mockPathGetter, mockBlobConverter).get shouldEqual expected
     TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(mixedInputs, mockPathGetter, mockBlobConverter).get shouldEqual expected
   }
-
-  it should "generate proper script preamble" in {
-
-  }
-}
+}