-
Notifications
You must be signed in to change notification settings - Fork 359
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
BT-711 Refresh SAS token for filesystem on expiry #6831
Merged
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
90be7d3
BT-711 Refresh SAS token for filesystem on expiry
kraefrei 77dcf19
Rough cut of token refresh using exceptions
kraefrei 00011bc
Ignore tests, and minor cleanup
kraefrei 1a9e556
Remove stray line
kraefrei 45b9b45
Draft of manager class for handling expiring file systems
kraefrei 8f00cc1
Style fixes
kraefrei 832e546
Refactor of blobfilesystemManager and tests covering its functionality
kraefrei c907917
Refined tests to validate close filesystem as separate unit
kraefrei 009ac6a
Merge branch 'develop' into BT-711
kraefrei cd02eb2
Ignore connected tests
kraefrei e113bb1
Clean up of some things
kraefrei 313b9fc
Refactor BlobFileSystemManager to separate file, and some other cleanup
kraefrei 6ac6c32
Some additional scala-ifying
kraefrei e687360
Small cleanup
kraefrei ca965da
Merge branch 'develop' of https://github.com/broadinstitute/cromwell …
kraefrei 930449c
Correcting imports
kraefrei 193e246
trigger tests
kraefrei 20c5db7
trigger tests
kraefrei File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
140 changes: 140 additions & 0 deletions
140
filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
package cromwell.filesystems.blob | ||
|
||
import com.azure.core.credential.AzureSasCredential | ||
import com.azure.core.management.AzureEnvironment | ||
import com.azure.core.management.profile.AzureProfile | ||
import com.azure.identity.DefaultAzureCredentialBuilder | ||
import com.azure.resourcemanager.AzureResourceManager | ||
import com.azure.storage.blob.nio.AzureFileSystem | ||
import com.azure.storage.blob.sas.{BlobContainerSasPermission, BlobServiceSasSignatureValues} | ||
import com.azure.storage.blob.{BlobContainerClient, BlobContainerClientBuilder} | ||
import com.azure.storage.common.StorageSharedKeyCredential | ||
|
||
import java.net.URI | ||
import java.nio.file.{FileSystem, FileSystemNotFoundException, FileSystems} | ||
import java.time.temporal.ChronoUnit | ||
import java.time.{Duration, Instant, OffsetDateTime} | ||
import scala.jdk.CollectionConverters._ | ||
import scala.util.{Failure, Success, Try} | ||
import com.azure.resourcemanager.storage.models.StorageAccountKey | ||
|
||
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) | ||
def closeFileSystem(uri: URI): Option[Unit] = getFileSystem(uri).toOption.map(_.close) | ||
} | ||
|
||
object BlobFileSystemManager { | ||
def parseTokenExpiry(token: AzureSasCredential): Option[Instant] = for { | ||
expiryString <- token.getSignature.split("&").find(_.startsWith("se")).map(_.replaceFirst("se=","")).map(_.replace("%3A", ":")) | ||
instant = Instant.parse(expiryString) | ||
} 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)) | ||
} | ||
def hasTokenExpired(tokenExpiry: Instant, buffer: Duration): Boolean = Instant.now.plus(buffer).isAfter(tokenExpiry) | ||
def uri(endpoint: EndpointURL) = new URI("azb://?endpoint=" + endpoint) | ||
} | ||
case class BlobFileSystemManager( | ||
container: BlobContainerName, | ||
endpoint: EndpointURL, | ||
expiryBufferMinutes: Long, | ||
blobTokenGenerator: BlobTokenGenerator, | ||
fileSystemAPI: FileSystemAPI = FileSystemAPI(), | ||
private val initialExpiration: Option[Instant] = None) { | ||
private var expiry: Option[Instant] = initialExpiration | ||
val buffer: Duration = Duration.of(expiryBufferMinutes, ChronoUnit.MINUTES) | ||
|
||
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] = { | ||
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 => blobTokenGenerator.generateAccessToken.flatMap(generateFilesystem(uri, container, _)) | ||
} | ||
// If the token has expired, OR there is no token record, try to close the FS and regenerate | ||
case true => | ||
fileSystemAPI.closeFileSystem(uri) | ||
blobTokenGenerator.generateAccessToken.flatMap(generateFilesystem(uri, container, _)) | ||
} | ||
} | ||
} | ||
|
||
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))) | ||
} | ||
|
||
} | ||
|
||
sealed trait BlobTokenGenerator {def generateAccessToken: Try[AzureSasCredential]} | ||
object BlobTokenGenerator { | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, subscription: Option[String]): BlobTokenGenerator = { | ||
createBlobTokenGenerator(container, endpoint, None, None, subscription) | ||
} | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, workspaceId: Option[WorkspaceId], workspaceManagerURL: Option[WorkspaceManagerURL], subscription: Option[String]): 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): BlobTokenGenerator = createBlobTokenGenerator(container, endpoint, None) | ||
def createBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, workspaceId: Option[WorkspaceId], workspaceManagerURL: Option[WorkspaceManagerURL]): BlobTokenGenerator = | ||
createBlobTokenGenerator(container, endpoint, workspaceId, workspaceManagerURL, None) | ||
|
||
} | ||
|
||
case class WSMBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, workspaceId: WorkspaceId, workspaceManagerURL: WorkspaceManagerURL) extends BlobTokenGenerator { | ||
def generateAccessToken: Try[AzureSasCredential] = Failure(new NotImplementedError) | ||
} | ||
|
||
case class NativeBlobTokenGenerator(container: BlobContainerName, endpoint: EndpointURL, subscription: Option[String] = None) extends BlobTokenGenerator { | ||
|
||
private val azureProfile = new AzureProfile(AzureEnvironment.AZURE) | ||
private def azureCredentialBuilder = new DefaultAzureCredentialBuilder() | ||
.authorityHost(azureProfile.getEnvironment.getActiveDirectoryEndpoint) | ||
.build | ||
private def authenticateWithSubscription(sub: String) = AzureResourceManager.authenticate(azureCredentialBuilder, azureProfile).withSubscription(sub) | ||
private def authenticateWithDefaultSubscription = AzureResourceManager.authenticate(azureCredentialBuilder, azureProfile).withDefaultSubscription() | ||
private def azure = subscription.map(authenticateWithSubscription(_)).getOrElse(authenticateWithDefaultSubscription) | ||
|
||
private def findAzureStorageAccount(name: StorageAccountName) = azure.storageAccounts.list.asScala.find(_.name.equals(name.value)) | ||
.map(Success(_)).getOrElse(Failure(new Exception("Azure Storage Account not found"))) | ||
private def buildBlobContainerClient(credential: StorageSharedKeyCredential, endpoint: EndpointURL, container: BlobContainerName): BlobContainerClient = { | ||
new BlobContainerClientBuilder() | ||
.credential(credential) | ||
.endpoint(endpoint.value) | ||
.containerName(container.value) | ||
.buildClient() | ||
} | ||
private val bcsp = new BlobContainerSasPermission() | ||
.setReadPermission(true) | ||
.setCreatePermission(true) | ||
.setListPermission(true) | ||
|
||
|
||
def generateAccessToken: Try[AzureSasCredential] = for { | ||
uri <- BlobPathBuilder.parseURI(endpoint.value) | ||
configuredAccount <- BlobPathBuilder.parseStorageAccount(uri) | ||
azureAccount <- findAzureStorageAccount(configuredAccount) | ||
keys = azureAccount.getKeys.asScala | ||
key <- keys.headOption.fold[Try[StorageAccountKey]](Failure(new Exception("Storage account has no keys")))(Success(_)) | ||
first = key.value | ||
sskc = new StorageSharedKeyCredential(configuredAccount.value, first) | ||
bcc = buildBlobContainerClient(sskc, endpoint, container) | ||
bsssv = new BlobServiceSasSignatureValues(OffsetDateTime.now.plusDays(1), bcsp) | ||
asc = new AzureSasCredential(bcc.generateSas(bsssv)) | ||
} yield asc | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We've discussed begrudgingly accepting that this method will throw. We should ensure it throws something useful, though. I think we should throw different informative error messages depending on whether we failed to get the filesystem or failed to create the NIO path.