diff --git a/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureDirectoryStream.java b/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureDirectoryStream.java index 917f712ddfc..817121e958e 100644 --- a/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureDirectoryStream.java +++ b/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureDirectoryStream.java @@ -3,12 +3,6 @@ package com.azure.storage.blob.nio; -import com.azure.core.util.logging.ClientLogger; -import com.azure.storage.blob.BlobContainerClient; -import com.azure.storage.blob.models.BlobItem; -import com.azure.storage.blob.models.BlobListDetails; -import com.azure.storage.blob.models.ListBlobsOptions; - import java.io.IOException; import java.nio.file.DirectoryIteratorException; import java.nio.file.DirectoryStream; @@ -18,6 +12,12 @@ import java.util.NoSuchElementException; import java.util.Set; +import com.azure.core.util.logging.ClientLogger; +import com.azure.storage.blob.BlobContainerClient; +import com.azure.storage.blob.models.BlobItem; +import com.azure.storage.blob.models.BlobListDetails; +import com.azure.storage.blob.models.ListBlobsOptions; + /** * A type for iterating over the contents of a directory. * @@ -88,7 +88,7 @@ private static class AzureDirectoryIterator implements Iterator { if (path.isRoot()) { String containerName = path.toString().substring(0, path.toString().length() - 1); AzureFileSystem afs = ((AzureFileSystem) path.getFileSystem()); - containerClient = ((AzureFileStore) afs.getFileStore(containerName)).getContainerClient(); + containerClient = ((AzureFileStore) afs.getFileStore()).getContainerClient(); } else { AzureResource azureResource = new AzureResource(path); listOptions.setPrefix(azureResource.getBlobClient().getBlobName() + AzureFileSystem.PATH_SEPARATOR); diff --git a/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureFileSystem.java b/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureFileSystem.java index 6f981b1b45e..381ed0289d7 100644 --- a/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureFileSystem.java +++ b/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzureFileSystem.java @@ -3,19 +3,6 @@ package com.azure.storage.blob.nio; -import com.azure.core.credential.AzureSasCredential; -import com.azure.core.http.HttpClient; -import com.azure.core.http.policy.HttpLogDetailLevel; -import com.azure.core.http.policy.HttpPipelinePolicy; -import com.azure.core.util.CoreUtils; -import com.azure.core.util.logging.ClientLogger; -import com.azure.storage.blob.BlobServiceClient; -import com.azure.storage.blob.BlobServiceClientBuilder; -import com.azure.storage.blob.implementation.util.BlobUserAgentModificationPolicy; -import com.azure.storage.common.StorageSharedKeyCredential; -import com.azure.storage.common.policy.RequestRetryOptions; -import com.azure.storage.common.policy.RetryPolicyType; - import java.io.IOException; import java.nio.file.FileStore; import java.nio.file.FileSystem; @@ -27,14 +14,31 @@ import java.nio.file.attribute.FileAttributeView; import java.nio.file.attribute.UserPrincipalLookupService; import java.nio.file.spi.FileSystemProvider; +import java.time.Duration; +import java.time.Instant; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.regex.PatternSyntaxException; -import java.util.stream.Collectors; + +import com.azure.core.credential.AzureSasCredential; +import com.azure.core.http.HttpClient; +import com.azure.core.http.policy.HttpLogDetailLevel; +import com.azure.core.http.policy.HttpPipelinePolicy; +import com.azure.core.util.CoreUtils; +import com.azure.core.util.logging.ClientLogger; +import com.azure.storage.blob.BlobServiceClient; +import com.azure.storage.blob.BlobServiceClientBuilder; +import com.azure.storage.blob.implementation.util.BlobUserAgentModificationPolicy; +import com.azure.storage.common.StorageSharedKeyCredential; +import com.azure.storage.common.policy.RequestRetryOptions; +import com.azure.storage.common.policy.RetryPolicyType; /** * Implement's Java's {@link FileSystem} interface for Azure Blob Storage. @@ -67,6 +71,11 @@ public final class AzureFileSystem extends FileSystem { */ public static final String AZURE_STORAGE_SAS_TOKEN_CREDENTIAL = "AzureStorageSasTokenCredential"; + /** + * Expected type: String + */ + public static final String AZURE_STORAGE_PUBLIC_ACCESS_CREDENTIAL = "AzureStoragePublicAccessCredential"; + /** * Expected type: com.azure.core.http.policy.HttpLogLevelDetail */ @@ -159,9 +168,9 @@ public final class AzureFileSystem extends FileSystem { private final Long putBlobThreshold; private final Integer maxConcurrencyPerRequest; private final Integer downloadResumeRetries; - private final Map fileStores; private FileStore defaultFileStore; private boolean closed; + private Instant expiry; AzureFileSystem(AzureFileSystemProvider parentFileSystemProvider, String endpoint, Map config) throws IOException { @@ -181,7 +190,7 @@ public final class AzureFileSystem extends FileSystem { this.downloadResumeRetries = (Integer) config.get(AZURE_STORAGE_DOWNLOAD_RESUME_RETRIES); // Initialize and ensure access to FileStores. - this.fileStores = this.initializeFileStores(config); + this.defaultFileStore = this.initializeFileStore(config); } catch (RuntimeException e) { throw LoggingUtility.logError(LOGGER, new IllegalArgumentException("There was an error parsing the " + "configurations map. Please ensure all fields are set to a legal value of the correct type.", e)); @@ -221,7 +230,7 @@ public FileSystemProvider provider() { @Override public void close() throws IOException { this.closed = true; - this.parentFileSystemProvider.closeFileSystem(this.getFileSystemUrl()); + this.parentFileSystemProvider.closeFileSystem(this.getFileSystemUrl() + "/" + defaultFileStore.name()); } /** @@ -282,9 +291,7 @@ public Iterable getRootDirectories() { If the file system was set to use all containers in the account, the account will be re-queried and the list may grow or shrink if containers were added or deleted. */ - return fileStores.keySet().stream() - .map(name -> this.getPath(name + AzurePath.ROOT_DIR_SUFFIX)) - .collect(Collectors.toList()); + return Arrays.asList(this.getPath(defaultFileStore.name() + AzurePath.ROOT_DIR_SUFFIX)); } /** @@ -304,7 +311,7 @@ public Iterable getFileStores() { If the file system was set to use all containers in the account, the account will be re-queried and the list may grow or shrink if containers were added or deleted. */ - return this.fileStores.values(); + return Arrays.asList(defaultFileStore); } /** @@ -397,6 +404,12 @@ private BlobServiceClient buildBlobServiceClient(String endpoint, Map builder.credential((StorageSharedKeyCredential) config.get(AZURE_STORAGE_SHARED_KEY_CREDENTIAL)); } else if (config.containsKey(AZURE_STORAGE_SAS_TOKEN_CREDENTIAL)) { builder.credential((AzureSasCredential) config.get(AZURE_STORAGE_SAS_TOKEN_CREDENTIAL)); + this.setExpiryFromSAS((AzureSasCredential) config.get(AZURE_STORAGE_SAS_TOKEN_CREDENTIAL)); + } else if (config.containsKey(AZURE_STORAGE_PUBLIC_ACCESS_CREDENTIAL)) { + // The Blob Service Client Builder requires at least one kind of authentication to make requests + // For public files however, this is unnecessary. This key-value pair is to denote the case + // explicitly when we supply a placeholder SAS credential to bypass this requirement. + builder.credential((AzureSasCredential) config.get(AZURE_STORAGE_PUBLIC_ACCESS_CREDENTIAL)); } else { throw LoggingUtility.logError(LOGGER, new IllegalArgumentException(String.format("No credentials were " + "provided. Please specify one of the following when constructing an AzureFileSystem: %s, %s.", @@ -430,23 +443,17 @@ private BlobServiceClient buildBlobServiceClient(String endpoint, Map return builder.buildClient(); } - private Map initializeFileStores(Map config) throws IOException { - String fileStoreNames = (String) config.get(AZURE_STORAGE_FILE_STORES); - if (CoreUtils.isNullOrEmpty(fileStoreNames)) { + private FileStore initializeFileStore(Map config) throws IOException { + String fileStoreName = (String) config.get(AZURE_STORAGE_FILE_STORES); + if (CoreUtils.isNullOrEmpty(fileStoreName)) { throw LoggingUtility.logError(LOGGER, new IllegalArgumentException("The list of FileStores cannot be " + "null.")); } Boolean skipConnectionCheck = (Boolean) config.get(AZURE_STORAGE_SKIP_INITIAL_CONTAINER_CHECK); Map fileStores = new HashMap<>(); - for (String fileStoreName : fileStoreNames.split(",")) { - FileStore fs = new AzureFileStore(this, fileStoreName, skipConnectionCheck); - if (this.defaultFileStore == null) { - this.defaultFileStore = fs; - } - fileStores.put(fileStoreName, fs); - } - return fileStores; + this.defaultFileStore = new AzureFileStore(this, fileStoreName, skipConnectionCheck); + return this.defaultFileStore; } @Override @@ -470,12 +477,11 @@ Path getDefaultDirectory() { return this.getPath(this.defaultFileStore.name() + AzurePath.ROOT_DIR_SUFFIX); } - FileStore getFileStore(String name) throws IOException { - FileStore store = this.fileStores.get(name); - if (store == null) { - throw LoggingUtility.logError(LOGGER, new IOException("Invalid file store: " + name)); + FileStore getFileStore() throws IOException { + if (this.defaultFileStore == null) { + throw LoggingUtility.logError(LOGGER, new IOException("FileStore not initialized")); } - return store; + return defaultFileStore; } Long getBlockSize() { @@ -489,4 +495,24 @@ Long getPutBlobThreshold() { Integer getMaxConcurrencyPerRequest() { return this.maxConcurrencyPerRequest; } + + public Optional getExpiry() { + return Optional.ofNullable(expiry); + } + + private void setExpiryFromSAS(AzureSasCredential token) { + List strings = Arrays.asList(token.getSignature().split("&")); + Optional expiryString = strings.stream() + .filter(s -> s.startsWith("se")) + .findFirst() + .map(s -> s.replaceFirst("se=","")) + .map(s -> s.replace("%3A", ":")); + this.expiry = expiryString.map(es -> Instant.parse(es)).orElse(null); + } + + public boolean isExpired(Duration buffer) { + return Optional.ofNullable(this.expiry) + .map(e -> Instant.now().plus(buffer).isAfter(e)) + .orElse(true); + } } diff --git a/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzurePath.java b/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzurePath.java index 9742af1f696..917895ba39e 100644 --- a/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzurePath.java +++ b/azure-blob-nio/src/main/java/com/azure/storage/blob/nio/AzurePath.java @@ -735,7 +735,7 @@ public BlobClient toBlobClient() throws IOException { String fileStoreName = this.rootToFileStore(root.toString()); BlobContainerClient containerClient = - ((AzureFileStore) this.parentFileSystem.getFileStore(fileStoreName)).getContainerClient(); + ((AzureFileStore) this.parentFileSystem.getFileStore()).getContainerClient(); String blobName = this.withoutRoot(); if (blobName.isEmpty()) { diff --git a/build.sbt b/build.sbt index 814e966b364..2c9a8068992 100644 --- a/build.sbt +++ b/build.sbt @@ -103,10 +103,11 @@ lazy val azureBlobNio = (project in file("azure-blob-nio")) lazy val azureBlobFileSystem = (project in file("filesystems/blob")) .withLibrarySettings("cromwell-azure-blobFileSystem", blobFileSystemDependencies) .dependsOn(core) - .dependsOn(core % "test->test") - .dependsOn(common % "test->test") .dependsOn(cloudSupport) .dependsOn(azureBlobNio) + .dependsOn(core % "test->test") + .dependsOn(common % "test->test") + .dependsOn(azureBlobNio % "test->test") lazy val awsS3FileSystem = (project in file("filesystems/s3")) .withLibrarySettings("cromwell-aws-s3filesystem", s3FileSystemDependencies) diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemConfig.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemConfig.scala index f68bf7f5176..c5467c78ffe 100644 --- a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemConfig.scala +++ b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemConfig.scala @@ -11,13 +11,9 @@ import java.util.UUID // WSM config is needed for accessing WSM-managed blob containers created in Terra workspaces. // If the identity executing Cromwell has native access to the blob container, this can be ignored. final case class WorkspaceManagerConfig(url: WorkspaceManagerURL, - workspaceId: WorkspaceId, - containerResourceId: ContainerResourceId, overrideWsmAuthToken: Option[String]) // dev-only -final case class BlobFileSystemConfig(endpointURL: EndpointURL, - blobContainerName: BlobContainerName, - subscriptionId: Option[SubscriptionId], +final case class BlobFileSystemConfig(subscriptionId: Option[SubscriptionId], expiryBufferMinutes: Long, workspaceManagerConfig: Option[WorkspaceManagerConfig]) @@ -26,8 +22,6 @@ object BlobFileSystemConfig { final val defaultExpiryBufferMinutes = 10L def apply(config: Config): BlobFileSystemConfig = { - val endpointURL = parseString(config, "endpoint").map(EndpointURL) - val blobContainer = parseString(config, "container").map(BlobContainerName) val subscriptionId = parseUUIDOpt(config, "subscription").map(_.map(SubscriptionId)) val expiryBufferMinutes = parseLongOpt(config, "expiry-buffer-minutes") @@ -37,17 +31,15 @@ object BlobFileSystemConfig { if (config.hasPath("workspace-manager")) { val wsmConf = config.getConfig("workspace-manager") val wsmURL = parseString(wsmConf, "url").map(WorkspaceManagerURL) - val workspaceId = parseUUID(wsmConf, "workspace-id").map(WorkspaceId) - val containerResourceId = parseUUID(wsmConf, "container-resource-id").map(ContainerResourceId) val overrideWsmAuthToken = parseStringOpt(wsmConf, "b2cToken") - (wsmURL, workspaceId, containerResourceId, overrideWsmAuthToken) + (wsmURL, overrideWsmAuthToken) .mapN(WorkspaceManagerConfig) .map(Option(_)) } else None.validNel - (endpointURL, blobContainer, subscriptionId, expiryBufferMinutes, wsmConfig) + (subscriptionId, expiryBufferMinutes, wsmConfig) .mapN(BlobFileSystemConfig.apply) .unsafe("Couldn't parse blob filesystem config") } @@ -58,9 +50,6 @@ object BlobFileSystemConfig { private def parseStringOpt(config: Config, path: String) = validate[Option[String]] { config.as[Option[String]](path) } - private def parseUUID(config: Config, path: String) = - validate[UUID] { UUID.fromString(config.as[String](path)) } - private def parseUUIDOpt(config: Config, path: String) = validate[Option[UUID]] { config.as[Option[String]](path).map(UUID.fromString) } diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala index ac8f01d2cc7..6b6088c7689 100644 --- a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala +++ b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala @@ -1,5 +1,6 @@ package cromwell.filesystems.blob +import bio.terra.workspace.client.ApiException import com.azure.core.credential.AzureSasCredential import com.azure.storage.blob.nio.{AzureFileSystem, AzureFileSystemProvider} import com.azure.storage.blob.sas.{BlobContainerSasPermission, BlobServiceSasSignatureValues} @@ -9,18 +10,19 @@ import common.validation.Validation._ import cromwell.cloudsupport.azure.{AzureCredentials, AzureUtils} import java.net.URI +import java.nio.file._ import java.nio.file.spi.FileSystemProvider -import java.nio.file.{FileSystem, FileSystemNotFoundException} import java.time.temporal.ChronoUnit import java.time.{Duration, Instant, OffsetDateTime} +import java.util.UUID import scala.jdk.CollectionConverters._ import scala.util.{Failure, Success, Try} // We encapsulate this functionality here so that we can easily mock it out, to allow for testing without // actually connecting to Blob storage. -case class FileSystemAPI(private val provider: FileSystemProvider = new AzureFileSystemProvider()) { - def getFileSystem(uri: URI): Try[FileSystem] = Try(provider.getFileSystem(uri)) - def newFileSystem(uri: URI, config: Map[String, Object]): FileSystem = provider.newFileSystem(uri, config.asJava) +case class AzureFileSystemAPI(private val provider: FileSystemProvider = new AzureFileSystemProvider()) { + def getFileSystem(uri: URI): Try[AzureFileSystem] = Try(provider.getFileSystem(uri).asInstanceOf[AzureFileSystem]) + def newFileSystem(uri: URI, config: Map[String, Object]): Try[AzureFileSystem] = Try(provider.newFileSystem(uri, config.asJava).asInstanceOf[AzureFileSystem]) def closeFileSystem(uri: URI): Option[Unit] = getFileSystem(uri).toOption.map(_.close) } /** @@ -36,25 +38,25 @@ object BlobFileSystemManager { } yield instant def buildConfigMap(credential: AzureSasCredential, container: BlobContainerName): Map[String, Object] = { - Map((AzureFileSystem.AZURE_STORAGE_SAS_TOKEN_CREDENTIAL, credential), - (AzureFileSystem.AZURE_STORAGE_FILE_STORES, container.value), - (AzureFileSystem.AZURE_STORAGE_SKIP_INITIAL_CONTAINER_CHECK, java.lang.Boolean.TRUE)) + // Special handling is done here to provide a special key value pair if the placeholder token is provided + // This is due to the BlobClient requiring an auth token even for public blob paths. + val sasTuple = if (credential == PLACEHOLDER_TOKEN) (AzureFileSystem.AZURE_STORAGE_PUBLIC_ACCESS_CREDENTIAL, PLACEHOLDER_TOKEN) + else (AzureFileSystem.AZURE_STORAGE_SAS_TOKEN_CREDENTIAL, credential) + + Map(sasTuple, (AzureFileSystem.AZURE_STORAGE_FILE_STORES, container.value), + (AzureFileSystem.AZURE_STORAGE_SKIP_INITIAL_CONTAINER_CHECK, java.lang.Boolean.TRUE)) } - def hasTokenExpired(tokenExpiry: Instant, buffer: Duration): Boolean = Instant.now.plus(buffer).isAfter(tokenExpiry) - def uri(endpoint: EndpointURL) = new URI("azb://?endpoint=" + endpoint) + def combinedEnpointContainerUri(endpoint: EndpointURL, container: BlobContainerName) = new URI("azb://?endpoint=" + endpoint + "/" + container.value) + + val PLACEHOLDER_TOKEN = new AzureSasCredential("this-is-a-public-sas") } -class BlobFileSystemManager(val container: BlobContainerName, - val endpoint: EndpointURL, - val expiryBufferMinutes: Long, +class BlobFileSystemManager(val expiryBufferMinutes: Long, val blobTokenGenerator: BlobSasTokenGenerator, - val fileSystemAPI: FileSystemAPI = FileSystemAPI(), - private val initialExpiration: Option[Instant] = None) extends LazyLogging { + val fileSystemAPI: AzureFileSystemAPI = AzureFileSystemAPI()) extends LazyLogging { def this(config: BlobFileSystemConfig) = { this( - config.blobContainerName, - config.endpointURL, config.expiryBufferMinutes, BlobSasTokenGenerator.createBlobTokenGeneratorFromConfig(config) ) @@ -63,39 +65,46 @@ class BlobFileSystemManager(val container: BlobContainerName, def this(rawConfig: Config) = this(BlobFileSystemConfig(rawConfig)) val buffer: Duration = Duration.of(expiryBufferMinutes, ChronoUnit.MINUTES) - private var expiry: Option[Instant] = initialExpiration - def getExpiry: Option[Instant] = expiry - def uri: URI = BlobFileSystemManager.uri(endpoint) - def isTokenExpired: Boolean = expiry.exists(BlobFileSystemManager.hasTokenExpired(_, buffer)) - def shouldReopenFilesystem: Boolean = isTokenExpired || expiry.isEmpty - def retrieveFilesystem(): Try[FileSystem] = { + def retrieveFilesystem(endpoint: EndpointURL, container: BlobContainerName): Try[FileSystem] = { + val uri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint, container) synchronized { - shouldReopenFilesystem match { - case false => fileSystemAPI.getFileSystem(uri).recoverWith { - // If no filesystem already exists, this will create a new connection, with the provided configs - case _: FileSystemNotFoundException => - logger.info(s"Creating new blob filesystem for URI $uri") - blobTokenGenerator.generateBlobSasToken.flatMap(generateFilesystem(uri, container, _)) + fileSystemAPI.getFileSystem(uri).filter(!_.isExpired(buffer)).recoverWith { + // If no filesystem already exists, this will create a new connection, with the provided configs + case _: FileSystemNotFoundException => { + logger.info(s"Creating new blob filesystem for URI $uri") + generateFilesystem(uri, container, endpoint) } - // If the token has expired, OR there is no token record, try to close the FS and regenerate - case true => + case _ : NoSuchElementException => { + // When the filesystem expires, the above filter results in a + // NoSuchElementException. If expired, close the filesystem + // and reopen the filesystem with the fresh token logger.info(s"Closing & regenerating token for existing blob filesystem at URI $uri") fileSystemAPI.closeFileSystem(uri) - blobTokenGenerator.generateBlobSasToken.flatMap(generateFilesystem(uri, container, _)) + generateFilesystem(uri, container, endpoint) + } } } } - private def generateFilesystem(uri: URI, container: BlobContainerName, token: AzureSasCredential): Try[FileSystem] = { - expiry = BlobFileSystemManager.parseTokenExpiry(token) - if (expiry.isEmpty) return Failure(new Exception("Could not reopen filesystem, no expiration found")) - Try(fileSystemAPI.newFileSystem(uri, BlobFileSystemManager.buildConfigMap(token, container))) + /** + * Create a new filesystem pointing to a particular container and storage account, + * generating a SAS token from WSM as needed + * + * @param uri a URI formatted to include the scheme, storage account endpoint and container + * @param container the container to open as a filesystem + * @param endpoint the endpoint containing the storage account for the container to open + * @return a try with either the successfully created filesystem, or a failure containing the exception + */ + private def generateFilesystem(uri: URI, container: BlobContainerName, endpoint: EndpointURL): Try[AzureFileSystem] = { + blobTokenGenerator.generateBlobSasToken(endpoint, container) + .flatMap((token: AzureSasCredential) => { + fileSystemAPI.newFileSystem(uri, BlobFileSystemManager.buildConfigMap(token, container)) + }) } - } -sealed trait BlobSasTokenGenerator { def generateBlobSasToken: Try[AzureSasCredential] } +sealed trait BlobSasTokenGenerator { def generateBlobSasToken(endpoint: EndpointURL, container: BlobContainerName): Try[AzureSasCredential] } object BlobSasTokenGenerator { /** @@ -122,35 +131,23 @@ object BlobSasTokenGenerator { // WSM-mediated mediated SAS token generator // parameterizing client instead of URL to make injecting mock client possible - BlobSasTokenGenerator.createBlobTokenGenerator( - config.blobContainerName, - config.endpointURL, - wsmConfig.workspaceId, - wsmConfig.containerResourceId, - wsmClient, - wsmConfig.overrideWsmAuthToken - ) + BlobSasTokenGenerator.createBlobTokenGenerator(wsmClient, wsmConfig.overrideWsmAuthToken) }.getOrElse( // Native SAS token generator - BlobSasTokenGenerator.createBlobTokenGenerator(config.blobContainerName, config.endpointURL, config.subscriptionId) + BlobSasTokenGenerator.createBlobTokenGenerator(config.subscriptionId) ) /** * Native SAS token generator, uses the DefaultAzureCredentialBuilder in the local environment * to produce a SAS token. * - * @param container The BlobContainerName of the blob container to be accessed by the generated SAS token - * @param endpoint The EndpointURL containing the storage account of the blob container to be accessed by - * this SAS token * @param subscription Optional subscription parameter to use for local authorization. * If one is not provided the default subscription is used * @return A NativeBlobTokenGenerator, able to produce a valid SAS token for accessing the provided blob * container and endpoint locally */ - def createBlobTokenGenerator(container: BlobContainerName, - endpoint: EndpointURL, - subscription: Option[SubscriptionId]): BlobSasTokenGenerator = { - NativeBlobSasTokenGenerator(container, endpoint, subscription) + def createBlobTokenGenerator(subscription: Option[SubscriptionId]): BlobSasTokenGenerator = { + NativeBlobSasTokenGenerator(subscription) } /** @@ -158,11 +155,6 @@ object BlobSasTokenGenerator { * to request a SAS token from the WSM to access the given blob container. If an overrideWsmAuthToken * is provided this is used instead. * - * @param container The BlobContainerName of the blob container to be accessed by the generated SAS token - * @param endpoint The EndpointURL containing the storage account of the blob container to be accessed by - * this SAS token - * @param workspaceId The WorkspaceId of the account to authenticate against - * @param containerResourceId The ContainterResourceId of the blob container as WSM knows it * @param workspaceManagerClient The client for making requests against WSM * @param overrideWsmAuthToken An optional WsmAuthToken used for authenticating against the WSM for a valid * SAS token to access the given container and endpoint. This is a dev only option that is only intended @@ -170,54 +162,56 @@ object BlobSasTokenGenerator { * @return A WSMBlobTokenGenerator, able to produce a valid SAS token for accessing the provided blob * container and endpoint that is managed by WSM */ - def createBlobTokenGenerator(container: BlobContainerName, - endpoint: EndpointURL, - workspaceId: WorkspaceId, - containerResourceId: ContainerResourceId, - workspaceManagerClient: WorkspaceManagerApiClientProvider, + def createBlobTokenGenerator(workspaceManagerClient: WorkspaceManagerApiClientProvider, overrideWsmAuthToken: Option[String]): BlobSasTokenGenerator = { - WSMBlobSasTokenGenerator(container, endpoint, workspaceId, containerResourceId, workspaceManagerClient, overrideWsmAuthToken) + WSMBlobSasTokenGenerator(workspaceManagerClient, overrideWsmAuthToken) } } -case class WSMBlobSasTokenGenerator(container: BlobContainerName, - endpoint: EndpointURL, - workspaceId: WorkspaceId, - containerResourceId: ContainerResourceId, - wsmClientProvider: WorkspaceManagerApiClientProvider, +case class WSMBlobSasTokenGenerator(wsmClientProvider: WorkspaceManagerApiClientProvider, overrideWsmAuthToken: Option[String]) extends BlobSasTokenGenerator { /** * Generate a BlobSasToken by using the available authorization information * If an overrideWsmAuthToken is provided, use this in the wsmClient request * Else try to use the environment azure identity to request the SAS token + * @param endpoint The EndpointURL of the blob container to be accessed by the generated SAS token + * @param container The BlobContainerName of the blob container to be accessed by the generated SAS token * * @return an AzureSasCredential for accessing a blob container */ - def generateBlobSasToken: Try[AzureSasCredential] = { + def generateBlobSasToken(endpoint: EndpointURL, container: BlobContainerName): Try[AzureSasCredential] = { val wsmAuthToken: Try[String] = overrideWsmAuthToken match { case Some(t) => Success(t) case None => AzureCredentials.getAccessToken(None).toTry } + container.workspaceId match { + // If this is a Terra workspace, request a token from WSM + case Success(workspaceId) => { + (for { + wsmAuth <- wsmAuthToken + wsmAzureResourceClient = wsmClientProvider.getControlledAzureResourceApi(wsmAuth) + resourceId <- getContainerResourceId(workspaceId, container, wsmAuth) + sasToken <- wsmAzureResourceClient.createAzureStorageContainerSasToken(workspaceId, resourceId) + } yield sasToken).recoverWith { + // If the storage account was still not found in WSM, this may be a public filesystem + case exception: ApiException if exception.getCode == 404 => Try(BlobFileSystemManager.PLACEHOLDER_TOKEN) + } + } + // Otherwise assume that the container is public and use a placeholder + // SAS token to bypass the BlobClient authentication requirement + case Failure(_) => Try(BlobFileSystemManager.PLACEHOLDER_TOKEN) + } + } - for { - wsmAuth <- wsmAuthToken - wsmClient = wsmClientProvider.getControlledAzureResourceApi(wsmAuth) - sasToken <- Try( // Java library throws - wsmClient.createAzureStorageContainerSasToken( - workspaceId.value, - containerResourceId.value, - null, - null, - null, - null - ).getToken) - } yield new AzureSasCredential(sasToken) + def getContainerResourceId(workspaceId: UUID, container: BlobContainerName, wsmAuth : String): Try[UUID] = { + val wsmResourceClient = wsmClientProvider.getResourceApi(wsmAuth) + wsmResourceClient.findContainerResourceId(workspaceId, container) } } -case class NativeBlobSasTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, subscription: Option[SubscriptionId] = None) extends BlobSasTokenGenerator { +case class NativeBlobSasTokenGenerator(subscription: Option[SubscriptionId] = None) extends BlobSasTokenGenerator { private val bcsp = new BlobContainerSasPermission() .setReadPermission(true) .setCreatePermission(true) @@ -227,10 +221,12 @@ case class NativeBlobSasTokenGenerator(container: BlobContainerName, endpoint: E /** * Generate a BlobSasToken by using the local environment azure identity * This will use a default subscription if one is not provided. + * @param endpoint The EndpointURL of the blob container to be accessed by the generated SAS token + * @param container The BlobContainerName of the blob container to be accessed by the generated SAS token * * @return an AzureSasCredential for accessing a blob container */ - def generateBlobSasToken: Try[AzureSasCredential] = for { + def generateBlobSasToken(endpoint: EndpointURL, container: BlobContainerName): Try[AzureSasCredential] = for { bcc <- AzureUtils.buildContainerClientFromLocalEnvironment(container.toString, endpoint.toString, subscription.map(_.toString)) bsssv = new BlobServiceSasSignatureValues(OffsetDateTime.now.plusDays(1), bcsp) asc = new AzureSasCredential(bcc.generateSas(bsssv)) diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilder.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilder.scala index 35c518c0a43..3aa26eb3c11 100644 --- a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilder.scala +++ b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilder.scala @@ -12,12 +12,13 @@ import scala.language.postfixOps import scala.util.{Failure, Success, Try} object BlobPathBuilder { - + private val blobHostnameSuffix = ".blob.core.windows.net" sealed trait BlobPathValidation - case class ValidBlobPath(path: String) extends BlobPathValidation + case class ValidBlobPath(path: String, container: BlobContainerName, endpoint: EndpointURL) extends BlobPathValidation case class UnparsableBlobPath(errorMessage: Throwable) extends BlobPathValidation - def invalidBlobPathMessage(container: BlobContainerName, endpoint: EndpointURL) = s"Malformed Blob URL for this builder. Expecting a URL for a container $container and endpoint $endpoint" + def invalidBlobHostMessage(endpoint: EndpointURL) = s"Malformed Blob URL for this builder: The endpoint $endpoint doesn't contain the expected host string '{SA}.blob.core.windows.net/'" + def invalidBlobContainerMessage(endpoint: EndpointURL) = s"Malformed Blob URL for this builder: Could not parse container" def parseURI(string: String): Try[URI] = Try(URI.create(UrlEscapers.urlFragmentEscaper().escape(string))) def parseStorageAccount(uri: URI): Try[StorageAccountName] = uri.getHost.split("\\.").find(_.nonEmpty).map(StorageAccountName(_)) .map(Success(_)).getOrElse(Failure(new Exception("Could not parse storage account"))) @@ -40,28 +41,31 @@ object BlobPathBuilder { * * If the configured container and storage account do not match, the string is considered unparsable */ - def validateBlobPath(string: String, container: BlobContainerName, endpoint: EndpointURL): BlobPathValidation = { + def validateBlobPath(string: String): BlobPathValidation = { val blobValidation = for { testUri <- parseURI(string) - endpointUri <- parseURI(endpoint.value) + testEndpoint = EndpointURL(testUri.getScheme + "://" + testUri.getHost()) testStorageAccount <- parseStorageAccount(testUri) - endpointStorageAccount <- parseStorageAccount(endpointUri) - hasContainer = testUri.getPath.split("/").find(_.nonEmpty).contains(container.value) - hasEndpoint = testStorageAccount.equals(endpointStorageAccount) - blobPathValidation = (hasContainer && hasEndpoint) match { - case true => ValidBlobPath(testUri.getPath.replaceFirst("/" + container, "")) - case false => UnparsableBlobPath(new MalformedURLException(invalidBlobPathMessage(container, endpoint))) + testContainer = testUri.getPath.split("/").find(_.nonEmpty) + isBlobHost = testUri.getHost().contains(blobHostnameSuffix) && testUri.getScheme().contains("https") + blobPathValidation = (isBlobHost, testContainer) match { + case (true, Some(container)) => ValidBlobPath( + testUri.getPath.replaceFirst("/" + container, ""), + BlobContainerName(container), + testEndpoint) + case (false, _) => UnparsableBlobPath(new MalformedURLException(invalidBlobHostMessage(testEndpoint))) + case (true, None) => UnparsableBlobPath(new MalformedURLException(invalidBlobContainerMessage(testEndpoint))) } } yield blobPathValidation blobValidation recover { case t => UnparsableBlobPath(t) } get } } -class BlobPathBuilder(container: BlobContainerName, endpoint: EndpointURL)(private val fsm: BlobFileSystemManager) extends PathBuilder { +class BlobPathBuilder()(private val fsm: BlobFileSystemManager) extends PathBuilder { def build(string: String): Try[BlobPath] = { - validateBlobPath(string, container, endpoint) match { - case ValidBlobPath(path) => Try(BlobPath(path, endpoint, container)(fsm)) + validateBlobPath(string) match { + case ValidBlobPath(path, container, endpoint) => Try(BlobPath(path, endpoint, container)(fsm)) case UnparsableBlobPath(errorMessage: Throwable) => Failure(errorMessage) } } @@ -121,7 +125,7 @@ case class BlobPath private[blob](pathString: String, endpoint: EndpointURL, con override def pathWithoutScheme: String = parseURI(endpoint.value).map(u => List(u.getHost, container, pathString.stripPrefix("/")).mkString("/")).get private def findNioPath(path: String): NioPath = (for { - fileSystem <- fsm.retrieveFilesystem() + fileSystem <- fsm.retrieveFilesystem(endpoint, container) // The Azure NIO library uses `{container}:` to represent the root of the path nioPath = fileSystem.getPath(s"${container.value}:", path) // This is purposefully an unprotected get because the NIO API needing an unwrapped path object. diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala index c263841dc8a..47245552dc2 100644 --- a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala +++ b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala @@ -8,11 +8,26 @@ import cromwell.core.path.PathBuilderFactory.PriorityBlob import java.util.UUID import scala.concurrent.{ExecutionContext, Future} +import scala.util.Try final case class SubscriptionId(value: UUID) {override def toString: String = value.toString} -final case class BlobContainerName(value: String) {override def toString: String = value} +final case class BlobContainerName(value: String) { + override def toString: String = value + lazy val workspaceId: Try[UUID] = { + Try(UUID.fromString(value.replaceFirst("sc-",""))) + } +} final case class StorageAccountName(value: String) {override def toString: String = value} -final case class EndpointURL(value: String) {override def toString: String = value} +final case class EndpointURL(value: String) { + override def toString: String = value + lazy val storageAccountName : Try[StorageAccountName] = { + val sa = for { + host <- value.split("//").findLast(_.nonEmpty) + storageAccountName <- host.split("\\.").find(_.nonEmpty) + } yield StorageAccountName(storageAccountName) + sa.toRight(new Exception(s"Storage account name could not be parsed from $value")).toTry + } +} final case class WorkspaceId(value: UUID) {override def toString: String = value.toString} final case class ContainerResourceId(value: UUID) {override def toString: String = value.toString} final case class WorkspaceManagerURL(value: String) {override def toString: String = value} @@ -21,7 +36,7 @@ final case class BlobPathBuilderFactory(globalConfig: Config, instanceConfig: Co override def withOptions(options: WorkflowOptions)(implicit as: ActorSystem, ec: ExecutionContext): Future[BlobPathBuilder] = { Future { - new BlobPathBuilder(fsm.container, fsm.endpoint)(fsm) + new BlobPathBuilder()(fsm) } } diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/WorkspaceManagerApiClientProvider.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/WorkspaceManagerApiClientProvider.scala index a9f52d92a91..276738c98b6 100644 --- a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/WorkspaceManagerApiClientProvider.scala +++ b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/WorkspaceManagerApiClientProvider.scala @@ -1,7 +1,13 @@ package cromwell.filesystems.blob -import bio.terra.workspace.api.ControlledAzureResourceApi +import bio.terra.workspace.api._ import bio.terra.workspace.client.ApiClient +import bio.terra.workspace.model.{ResourceType, StewardshipType} +import com.azure.core.credential.AzureSasCredential + +import java.util.UUID +import scala.jdk.CollectionConverters._ +import scala.util.Try /** * Represents a way to get a client for interacting with workspace manager controlled resources. @@ -12,7 +18,8 @@ import bio.terra.workspace.client.ApiClient * For testing, create an anonymous subclass as in `org.broadinstitute.dsde.rawls.dataaccess.workspacemanager.HttpWorkspaceManagerDAOSpec` */ trait WorkspaceManagerApiClientProvider { - def getControlledAzureResourceApi(token: String): ControlledAzureResourceApi + def getControlledAzureResourceApi(token: String): WsmControlledAzureResourceApi + def getResourceApi(token: String): WsmResourceApi } class HttpWorkspaceManagerClientProvider(baseWorkspaceManagerUrl: WorkspaceManagerURL) extends WorkspaceManagerApiClientProvider { @@ -22,9 +29,40 @@ class HttpWorkspaceManagerClientProvider(baseWorkspaceManagerUrl: WorkspaceManag client } - def getControlledAzureResourceApi(token: String): ControlledAzureResourceApi = { + def getResourceApi(token: String): WsmResourceApi = { + val apiClient = getApiClient + apiClient.setAccessToken(token) + WsmResourceApi(new ResourceApi(apiClient)) + } + + def getControlledAzureResourceApi(token: String): WsmControlledAzureResourceApi = { val apiClient = getApiClient apiClient.setAccessToken(token) - new ControlledAzureResourceApi(apiClient) + WsmControlledAzureResourceApi(new ControlledAzureResourceApi(apiClient)) + } +} + +case class WsmResourceApi(resourcesApi : ResourceApi) { + def findContainerResourceId(workspaceId : UUID, container: BlobContainerName): Try[UUID] = { + for { + workspaceResources <- Try(resourcesApi.enumerateResources(workspaceId, 0, 10, ResourceType.AZURE_STORAGE_CONTAINER, StewardshipType.CONTROLLED).getResources()) + workspaceStorageContainerOption = workspaceResources.asScala.find(r => r.getMetadata().getName() == container.value) + workspaceStorageContainer <- workspaceStorageContainerOption.toRight(new Exception("No storage container found for this workspace")).toTry + resourceId = workspaceStorageContainer.getMetadata().getResourceId() + } yield resourceId + } +} +case class WsmControlledAzureResourceApi(controlledAzureResourceApi : ControlledAzureResourceApi) { + def createAzureStorageContainerSasToken(workspaceId: UUID, resourceId: UUID): Try[AzureSasCredential] = { + for { + sas <- Try(controlledAzureResourceApi.createAzureStorageContainerSasToken( + workspaceId, + resourceId, + null, + null, + null, + null + ).getToken) + } yield new AzureSasCredential(sas) } } diff --git a/filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000000..1f0955d450f --- /dev/null +++ b/filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline diff --git a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/AzureFileSystemSpec.scala b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/AzureFileSystemSpec.scala new file mode 100644 index 00000000000..e0463bab740 --- /dev/null +++ b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/AzureFileSystemSpec.scala @@ -0,0 +1,25 @@ +package cromwell.filesystems.blob + +import com.azure.storage.blob.nio.{AzureFileSystem, AzureFileSystemProvider} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.Instant +import scala.compat.java8.OptionConverters._ +import scala.jdk.CollectionConverters._ + +class AzureFileSystemSpec extends AnyFlatSpec with Matchers { + val now = Instant.now() + val container = BlobContainerName("testConainer") + val exampleSas = BlobPathBuilderFactorySpec.buildExampleSasToken(now) + val exampleConfig = BlobFileSystemManager.buildConfigMap(exampleSas, container) + val exampleStorageEndpoint = BlobPathBuilderSpec.buildEndpoint("testStorageAccount") + val exampleCombinedEndpoint = BlobFileSystemManager.combinedEnpointContainerUri(exampleStorageEndpoint, container) + + it should "parse an expiration from a sas token" in { + val provider = new AzureFileSystemProvider() + val filesystem : AzureFileSystem = provider.newFileSystem(exampleCombinedEndpoint, exampleConfig.asJava).asInstanceOf[AzureFileSystem] + filesystem.getExpiry.asScala shouldBe Some(now) + filesystem.getFileStores.asScala.map(_.name()).exists(_ == container.value) shouldBe true + } +} diff --git a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobFileSystemConfigSpec.scala b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobFileSystemConfigSpec.scala index 607ad5606f7..68804113763 100644 --- a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobFileSystemConfigSpec.scala +++ b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobFileSystemConfigSpec.scala @@ -5,14 +5,8 @@ import common.exception.AggregatedMessageException import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import java.util.UUID - class BlobFileSystemConfigSpec extends AnyFlatSpec with Matchers { - private val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - private val container = BlobContainerName("storageContainer") - private val workspaceId = WorkspaceId(UUID.fromString("B0BAFE77-0000-0000-0000-000000000000")) - private val containerResourceId = ContainerResourceId(UUID.fromString("F00B4911-0000-0000-0000-000000000000")) private val workspaceManagerURL = WorkspaceManagerURL("https://wsm.example.com") private val b2cToken = "b0gus-t0ken" @@ -20,12 +14,8 @@ class BlobFileSystemConfigSpec extends AnyFlatSpec with Matchers { val config = BlobFileSystemConfig( ConfigFactory.parseString( s""" - |container = "$container" - |endpoint = "$endpoint" """.stripMargin) ) - config.blobContainerName should equal(container) - config.endpointURL should equal(endpoint) config.expiryBufferMinutes should equal(BlobFileSystemConfig.defaultExpiryBufferMinutes) } @@ -33,25 +23,17 @@ class BlobFileSystemConfigSpec extends AnyFlatSpec with Matchers { val config = BlobFileSystemConfig( ConfigFactory.parseString( s""" - |container = "$container" - |endpoint = "$endpoint" |expiry-buffer-minutes = "20" |workspace-manager { | url = "$workspaceManagerURL" - | workspace-id = "$workspaceId" - | container-resource-id = "$containerResourceId" | b2cToken = "$b2cToken" |} | """.stripMargin) ) - config.blobContainerName should equal(container) - config.endpointURL should equal(endpoint) config.expiryBufferMinutes should equal(20L) config.workspaceManagerConfig.isDefined shouldBe true config.workspaceManagerConfig.get.url shouldBe workspaceManagerURL - config.workspaceManagerConfig.get.workspaceId shouldBe workspaceId - config.workspaceManagerConfig.get.containerResourceId shouldBe containerResourceId config.workspaceManagerConfig.get.overrideWsmAuthToken.contains(b2cToken) shouldBe true } @@ -59,17 +41,14 @@ class BlobFileSystemConfigSpec extends AnyFlatSpec with Matchers { val rawConfig = ConfigFactory.parseString( s""" - |container = "$container" - |endpoint = "$endpoint" |expiry-buffer-minutes = "10" |workspace-manager { - | url = "$workspaceManagerURL" - | container-resource-id = "$containerResourceId" + | b2cToken = "$b2cToken" |} | """.stripMargin) val error = intercept[AggregatedMessageException](BlobFileSystemConfig(rawConfig)) - error.getMessage should include("No configuration setting found for key 'workspace-id'") + error.getMessage should include("No configuration setting found for key 'url'") } } diff --git a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderFactorySpec.scala b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderFactorySpec.scala index 881cd3669a1..c4ee102c58b 100644 --- a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderFactorySpec.scala +++ b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderFactorySpec.scala @@ -1,16 +1,18 @@ package cromwell.filesystems.blob import com.azure.core.credential.AzureSasCredential +import com.azure.storage.blob.nio.AzureFileSystem import common.mock.MockSugar import org.mockito.Mockito._ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import java.nio.file.{FileSystem, FileSystemNotFoundException} +import java.nio.file.FileSystemNotFoundException import java.time.format.DateTimeFormatter import java.time.temporal.ChronoUnit import java.time.{Duration, Instant, ZoneId} -import scala.util.{Failure, Try} +import java.util.UUID +import scala.util.{Failure, Success, Try} object BlobPathBuilderFactorySpec { @@ -37,23 +39,12 @@ class BlobPathBuilderFactorySpec extends AnyFlatSpec with Matchers with MockSuga expiry should contain(expiryTime) } - it should "verify an unexpired token will be processed as unexpired" in { - val expiryTime = generateTokenExpiration(11L) - val expired = BlobFileSystemManager.hasTokenExpired(expiryTime, Duration.ofMinutes(10L)) - expired shouldBe false - } - - it should "test an expired token will be processed as expired" in { - val expiryTime = generateTokenExpiration(9L) - val expired = BlobFileSystemManager.hasTokenExpired(expiryTime, Duration.ofMinutes(10L)) - expired shouldBe true - } - it should "test that a filesystem gets closed correctly" in { val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - val azureUri = BlobFileSystemManager.uri(endpoint) - val fileSystems = mock[FileSystemAPI] - val fileSystem = mock[FileSystem] + val container = BlobContainerName("test") + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint, container) + val fileSystems = mock[AzureFileSystemAPI] + val fileSystem = mock[AzureFileSystem] when(fileSystems.getFileSystem(azureUri)).thenReturn(Try(fileSystem)) when(fileSystems.closeFileSystem(azureUri)).thenCallRealMethod() @@ -61,106 +52,156 @@ class BlobPathBuilderFactorySpec extends AnyFlatSpec with Matchers with MockSuga verify(fileSystem, times(1)).close() } - it should "test retrieveFileSystem with expired filesystem" in { + it should "test retrieveFileSystem with expired Terra filesystem" in { val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - val expiredToken = generateTokenExpiration(9L) + //val expiredToken = generateTokenExpiration(9L) val refreshedToken = generateTokenExpiration(69L) val sasToken = BlobPathBuilderFactorySpec.buildExampleSasToken(refreshedToken) - val container = BlobContainerName("storageContainer") + val container = BlobContainerName("sc-" + UUID.randomUUID().toString()) val configMap = BlobFileSystemManager.buildConfigMap(sasToken, container) - val azureUri = BlobFileSystemManager.uri(endpoint) - - val fileSystems = mock[FileSystemAPI] + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint, container) + + //Mocking this final class requires the plugin Mock Maker Inline plugin, configured here + //at filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker + val azureFileSystem = mock[AzureFileSystem] + when(azureFileSystem.isExpired(Duration.ofMinutes(10L))).thenReturn(true) + val fileSystems = mock[AzureFileSystemAPI] + when(fileSystems.getFileSystem(azureUri)).thenReturn(Success(azureFileSystem)) val blobTokenGenerator = mock[BlobSasTokenGenerator] - when(blobTokenGenerator.generateBlobSasToken).thenReturn(Try(sasToken)) + when(blobTokenGenerator.generateBlobSasToken(endpoint, container)).thenReturn(Try(sasToken)) - val fsm = new BlobFileSystemManager(container, endpoint, 10L, blobTokenGenerator, fileSystems, Some(expiredToken)) - fsm.getExpiry should contain(expiredToken) - fsm.isTokenExpired shouldBe true - fsm.retrieveFilesystem() + val fsm = new BlobFileSystemManager(10L, blobTokenGenerator, fileSystems) + fsm.retrieveFilesystem(endpoint, container) - fsm.getExpiry should contain(refreshedToken) - fsm.isTokenExpired shouldBe false - verify(fileSystems, never()).getFileSystem(azureUri) + verify(fileSystems, times(1)).getFileSystem(azureUri) verify(fileSystems, times(1)).newFileSystem(azureUri, configMap) verify(fileSystems, times(1)).closeFileSystem(azureUri) } - it should "test retrieveFileSystem with an unexpired fileSystem" in { + it should "test retrieveFileSystem with an unexpired Terra fileSystem" in { val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - val initialToken = generateTokenExpiration(11L) + //val initialToken = generateTokenExpiration(11L) val refreshedToken = generateTokenExpiration(71L) val sasToken = BlobPathBuilderFactorySpec.buildExampleSasToken(refreshedToken) - val container = BlobContainerName("storageContainer") + val container = BlobContainerName("sc-" + UUID.randomUUID().toString()) val configMap = BlobFileSystemManager.buildConfigMap(sasToken, container) - val azureUri = BlobFileSystemManager.uri(endpoint) - // Need a fake filesystem to supply the getFileSystem simulated try - val dummyFileSystem = mock[FileSystem] + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint,container) - val fileSystems = mock[FileSystemAPI] - when(fileSystems.getFileSystem(azureUri)).thenReturn(Try(dummyFileSystem)) + //Mocking this final class requires the plugin Mock Maker Inline plugin, configured here + //at filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker + val azureFileSystem = mock[AzureFileSystem] + when(azureFileSystem.isExpired(Duration.ofMinutes(10L))).thenReturn(false) + val fileSystems = mock[AzureFileSystemAPI] + when(fileSystems.getFileSystem(azureUri)).thenReturn(Try(azureFileSystem)) val blobTokenGenerator = mock[BlobSasTokenGenerator] - when(blobTokenGenerator.generateBlobSasToken).thenReturn(Try(sasToken)) + when(blobTokenGenerator.generateBlobSasToken(endpoint, container)).thenReturn(Try(sasToken)) - val fsm = new BlobFileSystemManager(container, endpoint, 10L, blobTokenGenerator, fileSystems, Some(initialToken)) - fsm.getExpiry should contain(initialToken) - fsm.isTokenExpired shouldBe false - fsm.retrieveFilesystem() + val fsm = new BlobFileSystemManager(10L, blobTokenGenerator, fileSystems) + fsm.retrieveFilesystem(endpoint, container) - fsm.getExpiry should contain(initialToken) - fsm.isTokenExpired shouldBe false verify(fileSystems, times(1)).getFileSystem(azureUri) verify(fileSystems, never()).newFileSystem(azureUri, configMap) verify(fileSystems, never()).closeFileSystem(azureUri) } - it should "test retrieveFileSystem with an uninitialized filesystem" in { + it should "test retrieveFileSystem with an uninitialized Terra filesystem" in { val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") val refreshedToken = generateTokenExpiration(71L) val sasToken = BlobPathBuilderFactorySpec.buildExampleSasToken(refreshedToken) - val container = BlobContainerName("storageContainer") + val container = BlobContainerName("sc-" + UUID.randomUUID().toString()) val configMap = BlobFileSystemManager.buildConfigMap(sasToken, container) - val azureUri = BlobFileSystemManager.uri(endpoint) + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint, container) - val fileSystems = mock[FileSystemAPI] + //Mocking this final class requires the plugin Mock Maker Inline plugin, configured here + //at filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker + val azureFileSystem = mock[AzureFileSystem] + when(azureFileSystem.isExpired(Duration.ofMinutes(10L))).thenReturn(false) + val fileSystems = mock[AzureFileSystemAPI] when(fileSystems.getFileSystem(azureUri)).thenReturn(Failure(new FileSystemNotFoundException)) + when(fileSystems.newFileSystem(azureUri, configMap)).thenReturn(Try(azureFileSystem)) val blobTokenGenerator = mock[BlobSasTokenGenerator] - when(blobTokenGenerator.generateBlobSasToken).thenReturn(Try(sasToken)) + when(blobTokenGenerator.generateBlobSasToken(endpoint, container)).thenReturn(Try(sasToken)) - val fsm = new BlobFileSystemManager(container, endpoint, 10L, blobTokenGenerator, fileSystems, Some(refreshedToken)) - fsm.getExpiry.isDefined shouldBe true - fsm.isTokenExpired shouldBe false - fsm.retrieveFilesystem() + val fsm = new BlobFileSystemManager(0L, blobTokenGenerator, fileSystems) + fsm.retrieveFilesystem(endpoint, container) - fsm.getExpiry should contain(refreshedToken) - fsm.isTokenExpired shouldBe false verify(fileSystems, times(1)).getFileSystem(azureUri) verify(fileSystems, times(1)).newFileSystem(azureUri, configMap) verify(fileSystems, never()).closeFileSystem(azureUri) } - it should "test retrieveFileSystem with an unknown filesystem" in { + it should "test retrieveFileSystem with expired non-Terra filesystem" in { val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - val refreshedToken = generateTokenExpiration(71L) - val sasToken = BlobPathBuilderFactorySpec.buildExampleSasToken(refreshedToken) - val container = BlobContainerName("storageContainer") + val sasToken = BlobFileSystemManager.PLACEHOLDER_TOKEN + val container = BlobContainerName("sc-" + UUID.randomUUID().toString()) val configMap = BlobFileSystemManager.buildConfigMap(sasToken, container) - val azureUri = BlobFileSystemManager.uri(endpoint) - - val fileSystems = mock[FileSystemAPI] + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint, container) + + //Mocking this final class requires the plugin Mock Maker Inline plugin, configured here + //at filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker + val azureFileSystem = mock[AzureFileSystem] + when(azureFileSystem.isExpired(Duration.ofMinutes(10L))).thenReturn(true) + val fileSystems = mock[AzureFileSystemAPI] + when(fileSystems.getFileSystem(azureUri)).thenReturn(Success(azureFileSystem)) val blobTokenGenerator = mock[BlobSasTokenGenerator] - when(blobTokenGenerator.generateBlobSasToken).thenReturn(Try(sasToken)) + when(blobTokenGenerator.generateBlobSasToken(endpoint, container)).thenReturn(Try(sasToken)) - val fsm = new BlobFileSystemManager(container, endpoint, 10L, blobTokenGenerator, fileSystems) - fsm.getExpiry.isDefined shouldBe false - fsm.isTokenExpired shouldBe false - fsm.retrieveFilesystem() + val fsm = new BlobFileSystemManager(10L, blobTokenGenerator, fileSystems) + fsm.retrieveFilesystem(endpoint, container) - fsm.getExpiry should contain(refreshedToken) - fsm.isTokenExpired shouldBe false - verify(fileSystems, never()).getFileSystem(azureUri) + verify(fileSystems, times(1)).getFileSystem(azureUri) verify(fileSystems, times(1)).newFileSystem(azureUri, configMap) verify(fileSystems, times(1)).closeFileSystem(azureUri) } + + it should "test retrieveFileSystem with an unexpired non-Terra fileSystem" in { + val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") + val sasToken = BlobFileSystemManager.PLACEHOLDER_TOKEN + val container = BlobContainerName("sc-" + UUID.randomUUID().toString()) + val configMap = BlobFileSystemManager.buildConfigMap(sasToken, container) + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint,container) + + //Mocking this final class requires the plugin Mock Maker Inline plugin, configured here + //at filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker + val azureFileSystem = mock[AzureFileSystem] + when(azureFileSystem.isExpired(Duration.ofMinutes(10L))).thenReturn(false) + val fileSystems = mock[AzureFileSystemAPI] + when(fileSystems.getFileSystem(azureUri)).thenReturn(Try(azureFileSystem)) + + val blobTokenGenerator = mock[BlobSasTokenGenerator] + when(blobTokenGenerator.generateBlobSasToken(endpoint, container)).thenReturn(Try(sasToken)) + + val fsm = new BlobFileSystemManager(10L, blobTokenGenerator, fileSystems) + fsm.retrieveFilesystem(endpoint, container) + + verify(fileSystems, times(1)).getFileSystem(azureUri) + verify(fileSystems, never()).newFileSystem(azureUri, configMap) + verify(fileSystems, never()).closeFileSystem(azureUri) + } + + it should "test retrieveFileSystem with an uninitialized non-Terra filesystem" in { + val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") + val sasToken = BlobFileSystemManager.PLACEHOLDER_TOKEN + val container = BlobContainerName("sc-" + UUID.randomUUID().toString()) + val configMap = BlobFileSystemManager.buildConfigMap(sasToken, container) + val azureUri = BlobFileSystemManager.combinedEnpointContainerUri(endpoint, container) + + //Mocking this final class requires the plugin Mock Maker Inline plugin, configured here + //at filesystems/blob/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker + val azureFileSystem = mock[AzureFileSystem] + when(azureFileSystem.isExpired(Duration.ofMinutes(10L))).thenReturn(false) + val fileSystems = mock[AzureFileSystemAPI] + when(fileSystems.getFileSystem(azureUri)).thenReturn(Failure(new FileSystemNotFoundException)) + when(fileSystems.newFileSystem(azureUri, configMap)).thenReturn(Try(azureFileSystem)) + val blobTokenGenerator = mock[BlobSasTokenGenerator] + when(blobTokenGenerator.generateBlobSasToken(endpoint, container)).thenReturn(Try(sasToken)) + + val fsm = new BlobFileSystemManager(0L, blobTokenGenerator, fileSystems) + fsm.retrieveFilesystem(endpoint, container) + + verify(fileSystems, times(1)).getFileSystem(azureUri) + verify(fileSystems, times(1)).newFileSystem(azureUri, configMap) + verify(fileSystems, never()).closeFileSystem(azureUri) + } } diff --git a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderSpec.scala b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderSpec.scala index eef6db8e942..a8ca7d58d6f 100644 --- a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderSpec.scala +++ b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderSpec.scala @@ -18,41 +18,23 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { val container = BlobContainerName("container") val evalPath = "/path/to/file" val testString = endpoint.value + "/" + container + evalPath - BlobPathBuilder.validateBlobPath(testString, container, endpoint) match { - case BlobPathBuilder.ValidBlobPath(path) => path should equal(evalPath) + BlobPathBuilder.validateBlobPath(testString) match { + case BlobPathBuilder.ValidBlobPath(path, parsedContainer, parsedEndpoint) => { + path should equal(evalPath) + parsedContainer should equal(container) + parsedEndpoint should equal(endpoint) + } case BlobPathBuilder.UnparsableBlobPath(errorMessage) => fail(errorMessage) } } - it should "bad storage account fails causes URI to fail parse into a path" in { - val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - val container = BlobContainerName("container") - val evalPath = "/path/to/file" - val testString = BlobPathBuilderSpec.buildEndpoint("badStorageAccount").value + container.value + evalPath - BlobPathBuilder.validateBlobPath(testString, container, endpoint) match { - case BlobPathBuilder.ValidBlobPath(path) => fail(s"Valid path: $path found when verifying mismatched storage account") - case BlobPathBuilder.UnparsableBlobPath(errorMessage) => errorMessage.getMessage should equal(BlobPathBuilder.invalidBlobPathMessage(container, endpoint)) - } - } - - it should "bad container fails causes URI to fail parse into a path" in { - val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") - val container = BlobContainerName("container") - val evalPath = "/path/to/file" - val testString = endpoint.value + "badContainer" + evalPath - BlobPathBuilder.validateBlobPath(testString, container, endpoint) match { - case BlobPathBuilder.ValidBlobPath(path) => fail(s"Valid path: $path found when verifying mismatched container") - case BlobPathBuilder.UnparsableBlobPath(errorMessage) => errorMessage.getMessage should equal(BlobPathBuilder.invalidBlobPathMessage(container, endpoint)) - } - } - it should "provide a readable error when getting an illegal nioPath" in { val endpoint = BlobPathBuilderSpec.buildEndpoint("storageAccount") val container = BlobContainerName("container") val evalPath = "/path/to/file" val exception = new Exception("Failed to do the thing") val fsm = mock[BlobFileSystemManager] - when(fsm.retrieveFilesystem()).thenReturn(Failure(exception)) + when(fsm.retrieveFilesystem(endpoint, container)).thenReturn(Failure(exception)) val path = BlobPath(evalPath, endpoint, container)(fsm) val testException = Try(path.nioPath).failed.toOption testException should contain(exception) @@ -95,15 +77,14 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { private val endpoint: EndpointURL = BlobPathBuilderSpec.buildEndpoint("centaurtesting") private val container: BlobContainerName = BlobContainerName("test-blob") - def makeBlobPathBuilder(blobEndpoint: EndpointURL, - container: BlobContainerName): BlobPathBuilder = { - val blobTokenGenerator = NativeBlobSasTokenGenerator(container, blobEndpoint, Some(subscriptionId)) - val fsm = new BlobFileSystemManager(container, blobEndpoint, 10, blobTokenGenerator) - new BlobPathBuilder(container, blobEndpoint)(fsm) + def makeBlobPathBuilder(): BlobPathBuilder = { + val blobTokenGenerator = NativeBlobSasTokenGenerator(Some(subscriptionId)) + val fsm = new BlobFileSystemManager(10, blobTokenGenerator) + new BlobPathBuilder()(fsm) } it should "read md5 from small files <5g" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val evalPath = "/testRead.txt" val testString = endpoint.value + "/" + container + evalPath val blobPath1: BlobPath = (builder build testString).get @@ -111,7 +92,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "read md5 from large files >5g" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val evalPath = "/Rocky-9.2-aarch64-dvd.iso" val testString = endpoint.value + "/" + container + evalPath val blobPath1: BlobPath = (builder build testString).get @@ -119,7 +100,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "choose the root/metadata md5 over the native md5 for files that have both" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val evalPath = "/redundant_md5_test.txt" val testString = endpoint.value + "/" + container + evalPath val blobPath1: BlobPath = (builder build testString).get @@ -127,7 +108,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "gracefully return `None` when neither hash is found" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val evalPath = "/no_md5_test.txt" val testString = endpoint.value + "/" + container + evalPath val blobPath1: BlobPath = (builder build testString).get @@ -135,7 +116,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "resolve an absolute path string correctly to a path" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val rootString = s"${endpoint.value}/${container.value}/cromwell-execution" val blobRoot: BlobPath = builder build rootString getOrElse fail() blobRoot.toAbsolutePath.pathAsString should equal ("https://centaurtesting.blob.core.windows.net/test-blob/cromwell-execution") @@ -144,7 +125,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "build a blob path from a test string and read a file" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val endpointHost = BlobPathBuilder.parseURI(endpoint.value).map(_.getHost).getOrElse(fail("Could not parse URI")) val evalPath = "/test/inputFile.txt" val testString = endpoint.value + "/" + container + evalPath @@ -160,7 +141,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "build duplicate blob paths in the same filesystem" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val evalPath = "/test/inputFile.txt" val testString = endpoint.value + "/" + container + evalPath val blobPath1: BlobPath = builder build testString getOrElse fail() @@ -173,7 +154,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "resolve a path without duplicating container name" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val rootString = s"${endpoint.value}/${container.value}/cromwell-execution" val blobRoot: BlobPath = builder build rootString getOrElse fail() blobRoot.toAbsolutePath.pathAsString should equal ("https://centaurtesting.blob.core.windows.net/test-blob/cromwell-execution") @@ -182,7 +163,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "correctly remove a prefix from the blob path" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val rootString = s"${endpoint.value}/${container.value}/cromwell-execution/" val execDirString = s"${endpoint.value}/${container.value}/cromwell-execution/abc123/myworkflow/task1/def4356/execution/" val fileString = s"${endpoint.value}/${container.value}/cromwell-execution/abc123/myworkflow/task1/def4356/execution/stdout" @@ -195,7 +176,7 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers with MockSugar { } it should "not change a path if it doesn't start with a prefix" in { - val builder = makeBlobPathBuilder(endpoint, container) + val builder = makeBlobPathBuilder() val otherRootString = s"${endpoint.value}/${container.value}/foobar/" val fileString = s"${endpoint.value}/${container.value}/cromwell-execution/abc123/myworkflow/task1/def4356/execution/stdout" val otherBlobRoot: BlobPath = builder build otherRootString getOrElse fail()