diff --git a/cloud-nio/cloud-nio-impl-drs/src/main/scala/cloud/nio/impl/drs/DrsCredentials.scala b/cloud-nio/cloud-nio-impl-drs/src/main/scala/cloud/nio/impl/drs/DrsCredentials.scala index 66a89fe4014..9fae8552fa7 100644 --- a/cloud-nio/cloud-nio-impl-drs/src/main/scala/cloud/nio/impl/drs/DrsCredentials.scala +++ b/cloud-nio/cloud-nio-impl-drs/src/main/scala/cloud/nio/impl/drs/DrsCredentials.scala @@ -1,25 +1,32 @@ package cloud.nio.impl.drs import cats.syntax.validated._ +import com.azure.core.credential.TokenRequestContext +import com.azure.core.management.AzureEnvironment +import com.azure.core.management.profile.AzureProfile import com.azure.identity.DefaultAzureCredentialBuilder -import com.azure.security.keyvault.secrets.{SecretClient, SecretClientBuilder} -import com.google.auth.oauth2.{AccessToken, OAuth2Credentials} +import com.google.auth.oauth2.{AccessToken, GoogleCredentials, OAuth2Credentials} import com.typesafe.config.Config -import common.validation.ErrorOr import common.validation.ErrorOr.ErrorOr import net.ceedubs.ficus.Ficus._ import scala.concurrent.duration._ +import scala.jdk.DurationConverters._ +import scala.util.{Failure, Success, Try} /** * This trait allows us to abstract away different token attainment strategies * for different cloud environments. **/ -sealed trait DrsCredentials { +trait DrsCredentials { def getAccessToken: ErrorOr[String] } -case class GoogleDrsCredentials(credentials: OAuth2Credentials, acceptableTTL: Duration) extends DrsCredentials { +/** + * Strategy for obtaining an access token from an existing OAuth credential. This class + * is designed for use within the Cromwell engine. + */ +case class GoogleOauthDrsCredentials(credentials: OAuth2Credentials, acceptableTTL: Duration) extends DrsCredentials { //Based on method from GoogleRegistry def getAccessToken: ErrorOr[String] = { def accessTokenTTLIsAcceptable(accessToken: AccessToken): Boolean = { @@ -39,25 +46,68 @@ case class GoogleDrsCredentials(credentials: OAuth2Credentials, acceptableTTL: D } } -object GoogleDrsCredentials { - def apply(credentials: OAuth2Credentials, config: Config): GoogleDrsCredentials = - GoogleDrsCredentials(credentials, config.as[FiniteDuration]("access-token-acceptable-ttl")) +object GoogleOauthDrsCredentials { + def apply(credentials: OAuth2Credentials, config: Config): GoogleOauthDrsCredentials = + GoogleOauthDrsCredentials(credentials, config.as[FiniteDuration]("access-token-acceptable-ttl")) +} + + +/** + * Strategy for obtaining an access token from Google Application Default credentials that are assumed to already exist + * in the environment. This class is designed for use by standalone executables running in environments + * that have direct access to a Google identity (ex. CromwellDrsLocalizer). + */ +case object GoogleAppDefaultTokenStrategy extends DrsCredentials { + private final val UserInfoEmailScope = "https://www.googleapis.com/auth/userinfo.email" + private final val UserInfoProfileScope = "https://www.googleapis.com/auth/userinfo.profile" + + def getAccessToken: ErrorOr[String] = { + Try { + val scopedCredentials = GoogleCredentials.getApplicationDefault().createScoped(UserInfoEmailScope, UserInfoProfileScope) + scopedCredentials.refreshAccessToken().getTokenValue + } match { + case Success(null) => "null token value attempting to refresh access token".invalidNel + case Success(value) => value.validNel + case Failure(e) => s"Failed to refresh access token: ${e.getMessage}".invalidNel + } + } } -case class AzureDrsCredentials(identityClientId: Option[String], vaultName: String, secretName: String) extends DrsCredentials { +/** + * Strategy for obtaining an access token in an environment with available Azure identity. + * If you need to disambiguate among multiple active user-assigned managed identities, pass + * in the client id of the identity that should be used. + */ +case class AzureDrsCredentials(identityClientId: Option[String]) extends DrsCredentials { + + final val tokenAcquisitionTimeout = 30.seconds - lazy val secretClient: ErrorOr[SecretClient] = ErrorOr { - val defaultCreds = identityClientId.map(identityId => - new DefaultAzureCredentialBuilder().managedIdentityClientId(identityId) - ).getOrElse( - new DefaultAzureCredentialBuilder() - ).build() + val azureProfile = new AzureProfile(AzureEnvironment.AZURE) + val tokenScope = "https://management.azure.com/.default" - new SecretClientBuilder() - .vaultUrl(s"https://${vaultName}.vault.azure.net") - .credential(defaultCreds) - .buildClient() + def tokenRequestContext: TokenRequestContext = { + val trc = new TokenRequestContext() + trc.addScopes(tokenScope) + trc } - def getAccessToken: ErrorOr[String] = secretClient.map(_.getSecret(secretName).getValue) + def defaultCredentialBuilder: DefaultAzureCredentialBuilder = + new DefaultAzureCredentialBuilder() + .authorityHost(azureProfile.getEnvironment.getActiveDirectoryEndpoint) + + def getAccessToken: ErrorOr[String] = { + val credentials = identityClientId.foldLeft(defaultCredentialBuilder) { + (builder, clientId) => builder.managedIdentityClientId(clientId) + }.build() + + Try( + credentials + .getToken(tokenRequestContext) + .block(tokenAcquisitionTimeout.toJava) + ) match { + case Success(null) => "null token value attempting to obtain access token".invalidNel + case Success(token) => token.getToken.validNel + case Failure(error) => s"Failed to refresh access token: ${error.getMessage}".invalidNel + } + } } diff --git a/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/DrsCloudNioFileProviderSpec.scala b/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/DrsCloudNioFileProviderSpec.scala index fd7b71ef4ee..405f22b3b20 100644 --- a/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/DrsCloudNioFileProviderSpec.scala +++ b/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/DrsCloudNioFileProviderSpec.scala @@ -33,7 +33,7 @@ class DrsCloudNioFileProviderSpec extends AnyFlatSpecLike with CromwellTimeoutSp val fileSystemProvider = new MockDrsCloudNioFileSystemProvider(config = config) fileSystemProvider.drsConfig.marthaUrl should be("https://from.config") fileSystemProvider.drsCredentials match { - case GoogleDrsCredentials(_, ttl) => ttl should be(1.minute) + case GoogleOauthDrsCredentials(_, ttl) => ttl should be(1.minute) case error => fail(s"Expected GoogleDrsCredentials, found $error") } fileSystemProvider.fileProvider should be(a[DrsCloudNioFileProvider]) diff --git a/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockDrsCloudNioFileSystemProvider.scala b/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockDrsCloudNioFileSystemProvider.scala index 025d7c6d543..08683a6d820 100644 --- a/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockDrsCloudNioFileSystemProvider.scala +++ b/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockDrsCloudNioFileSystemProvider.scala @@ -17,7 +17,7 @@ class MockDrsCloudNioFileSystemProvider(config: Config = mockConfig, ), mockResolver: Option[EngineDrsPathResolver] = None, ) - extends DrsCloudNioFileSystemProvider(config, GoogleDrsCredentials(NoCredentials.getInstance, config), drsReadInterpreter) { + extends DrsCloudNioFileSystemProvider(config, GoogleOauthDrsCredentials(NoCredentials.getInstance, config), drsReadInterpreter) { override lazy val drsPathResolver: EngineDrsPathResolver = { mockResolver getOrElse diff --git a/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockEngineDrsPathResolver.scala b/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockEngineDrsPathResolver.scala index 116155753f1..8395233ce4d 100644 --- a/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockEngineDrsPathResolver.scala +++ b/cloud-nio/cloud-nio-impl-drs/src/test/scala/cloud/nio/impl/drs/MockEngineDrsPathResolver.scala @@ -14,7 +14,7 @@ class MockEngineDrsPathResolver(drsConfig: DrsConfig = MockDrsPaths.mockDrsConfi httpClientBuilderOverride: Option[HttpClientBuilder] = None, accessTokenAcceptableTTL: Duration = Duration.Inf, ) - extends EngineDrsPathResolver(drsConfig, GoogleDrsCredentials(NoCredentials.getInstance, accessTokenAcceptableTTL)) { + extends EngineDrsPathResolver(drsConfig, GoogleOauthDrsCredentials(NoCredentials.getInstance, accessTokenAcceptableTTL)) { override protected lazy val httpClientBuilder: HttpClientBuilder = httpClientBuilderOverride getOrElse MockSugar.mock[HttpClientBuilder] diff --git a/cromwell-drs-localizer/src/main/scala/drs/localizer/CommandLineParser.scala b/cromwell-drs-localizer/src/main/scala/drs/localizer/CommandLineParser.scala index b200d4b18de..3b7be2b38bd 100644 --- a/cromwell-drs-localizer/src/main/scala/drs/localizer/CommandLineParser.scala +++ b/cromwell-drs-localizer/src/main/scala/drs/localizer/CommandLineParser.scala @@ -36,12 +36,6 @@ class CommandLineParser extends scopt.OptionParser[CommandLineArguments](Usage) opt[String]('t', "access-token-strategy").text(s"Access token strategy, must be one of '$Azure' or '$Google' (default '$Google')"). action((s, c) => c.copy(accessTokenStrategy = Option(s.toLowerCase()))) - opt[String]('v', "vault-name").text("Azure vault name"). - action((s, c) => - c.copy(azureVaultName = Option(s))) - opt[String]('s', "secret-name").text("Azure secret name"). - action((s, c) => - c.copy(azureSecretName = Option(s))) opt[String]('i', "identity-client-id").text("Azure identity client id"). action((s, c) => c.copy(azureIdentityClientId = Option(s))) @@ -54,11 +48,10 @@ class CommandLineParser extends scopt.OptionParser[CommandLineArguments](Usage) c.accessTokenStrategy match { case Some(Azure) if c.googleRequesterPaysProject.nonEmpty => Left(s"Requester pays project is only valid with access token strategy '$Google'") - case Some(Azure) if List(c.azureVaultName, c.azureSecretName).exists(_.isEmpty) => - Left(s"Both vault name and secret name must be specified for access token strategy $Azure") + case Some(Google) if c.azureIdentityClientId.nonEmpty => + Left(s"Identity client id is only valid with access token strategy '$Azure'") case Some(Azure) => Right(()) - case Some(Google) if List(c.azureSecretName, c.azureVaultName, c.azureIdentityClientId).forall(_.isEmpty) => Right(()) - case Some(Google) => Left(s"One or more specified options are only valid with access token strategy '$Azure'") + case Some(Google) => Right(()) case Some(huh) => Left(s"Unrecognized access token strategy '$huh'") case None => Left("Programmer error, access token strategy should not be None") } @@ -100,8 +93,6 @@ case class CommandLineArguments(accessTokenStrategy: Option[String] = Option(Goo drsObject: Option[String] = None, containerPath: Option[String] = None, googleRequesterPaysProject: Option[String] = None, - azureVaultName: Option[String] = None, - azureSecretName: Option[String] = None, azureIdentityClientId: Option[String] = None, manifestPath: Option[String] = None, googleRequesterPaysProjectConflict: Boolean = false) diff --git a/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerDrsPathResolver.scala b/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerDrsPathResolver.scala index ac27367b932..944d418acbb 100644 --- a/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerDrsPathResolver.scala +++ b/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerDrsPathResolver.scala @@ -1,10 +1,9 @@ package drs.localizer -import cloud.nio.impl.drs.{DrsConfig, DrsPathResolver} +import cloud.nio.impl.drs.{DrsConfig, DrsCredentials, DrsPathResolver} import common.validation.ErrorOr.ErrorOr -import drs.localizer.accesstokens.AccessTokenStrategy -class DrsLocalizerDrsPathResolver(drsConfig: DrsConfig, accessTokenStrategy: AccessTokenStrategy) extends DrsPathResolver(drsConfig) { - override def getAccessToken: ErrorOr[String] = accessTokenStrategy.getAccessToken() +class DrsLocalizerDrsPathResolver(drsConfig: DrsConfig, drsCredentials: DrsCredentials) extends DrsPathResolver(drsConfig) { + override def getAccessToken: ErrorOr[String] = drsCredentials.getAccessToken } diff --git a/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerMain.scala b/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerMain.scala index 05ecf0ce48c..df184efae92 100644 --- a/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerMain.scala +++ b/cromwell-drs-localizer/src/main/scala/drs/localizer/DrsLocalizerMain.scala @@ -4,11 +4,10 @@ import cats.data.NonEmptyList import cats.effect.{ExitCode, IO, IOApp} import cats.implicits._ import cloud.nio.impl.drs.DrsPathResolver.{FatalRetryDisposition, RegularRetryDisposition} -import cloud.nio.impl.drs.{AccessUrl, DrsConfig, DrsPathResolver, MarthaField} +import cloud.nio.impl.drs._ import cloud.nio.spi.{CloudNioBackoff, CloudNioSimpleExponentialBackoff} import com.typesafe.scalalogging.StrictLogging import drs.localizer.CommandLineParser.AccessTokenStrategy.{Azure, Google} -import drs.localizer.accesstokens.{AccessTokenStrategy, AzureB2CAccessTokenStrategy, GoogleAccessTokenStrategy} import drs.localizer.downloaders.AccessUrlDownloader.Hashes import drs.localizer.downloaders._ import org.apache.commons.csv.{CSVFormat, CSVParser} @@ -29,8 +28,8 @@ object DrsLocalizerMain extends IOApp with StrictLogging { val localize: Option[IO[ExitCode]] = for { pa <- parsedArgs run <- pa.accessTokenStrategy.collect { - case Azure => runLocalizer(pa, AzureB2CAccessTokenStrategy(pa)) - case Google => runLocalizer(pa, GoogleAccessTokenStrategy) + case Azure => runLocalizer(pa, AzureDrsCredentials(pa.azureIdentityClientId)) + case Google => runLocalizer(pa, GoogleAppDefaultTokenStrategy) } } yield run @@ -55,7 +54,7 @@ object DrsLocalizerMain extends IOApp with StrictLogging { IO.pure(ExitCode.Error) } - def runLocalizer(commandLineArguments: CommandLineArguments, accessTokenStrategy: AccessTokenStrategy): IO[ExitCode] = { + def runLocalizer(commandLineArguments: CommandLineArguments, drsCredentials: DrsCredentials): IO[ExitCode] = { commandLineArguments.manifestPath match { case Some(manifestPath) => val manifestFile = new File(manifestPath) @@ -63,31 +62,31 @@ object DrsLocalizerMain extends IOApp with StrictLogging { val exitCodes: IO[List[ExitCode]] = csvParser.asScala.map(record => { val drsObject = record.get(0) val containerPath = record.get(1) - localizeFile(commandLineArguments, accessTokenStrategy, drsObject, containerPath) + localizeFile(commandLineArguments, drsCredentials, drsObject, containerPath) }).toList.sequence exitCodes.map(_.find(_ != ExitCode.Success).getOrElse(ExitCode.Success)) case None => val drsObject = commandLineArguments.drsObject.get val containerPath = commandLineArguments.containerPath.get - localizeFile(commandLineArguments, accessTokenStrategy, drsObject, containerPath) + localizeFile(commandLineArguments, drsCredentials, drsObject, containerPath) } } - private def localizeFile(commandLineArguments: CommandLineArguments, accessTokenStrategy: AccessTokenStrategy, drsObject: String, containerPath: String) = { - new DrsLocalizerMain(drsObject, containerPath, accessTokenStrategy, commandLineArguments.googleRequesterPaysProject). + private def localizeFile(commandLineArguments: CommandLineArguments, drsCredentials: DrsCredentials, drsObject: String, containerPath: String) = { + new DrsLocalizerMain(drsObject, containerPath, drsCredentials, commandLineArguments.googleRequesterPaysProject). resolveAndDownloadWithRetries(downloadRetries = 3, checksumRetries = 1, defaultDownloaderFactory, Option(defaultBackoff)).map(_.exitCode) } } class DrsLocalizerMain(drsUrl: String, downloadLoc: String, - accessTokenStrategy: AccessTokenStrategy, + drsCredentials: DrsCredentials, requesterPaysProjectIdOption: Option[String]) extends StrictLogging { def getDrsPathResolver: IO[DrsLocalizerDrsPathResolver] = { IO { val drsConfig = DrsConfig.fromEnv(sys.env) - new DrsLocalizerDrsPathResolver(drsConfig, accessTokenStrategy) + new DrsLocalizerDrsPathResolver(drsConfig, drsCredentials) } } diff --git a/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/AccessTokenStrategy.scala b/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/AccessTokenStrategy.scala deleted file mode 100644 index 756f5371c0f..00000000000 --- a/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/AccessTokenStrategy.scala +++ /dev/null @@ -1,7 +0,0 @@ -package drs.localizer.accesstokens - -import common.validation.ErrorOr.ErrorOr - -trait AccessTokenStrategy { - def getAccessToken(): ErrorOr[String] -} diff --git a/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/AzureB2CAccessTokenStrategy.scala b/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/AzureB2CAccessTokenStrategy.scala deleted file mode 100644 index 1634815d1f4..00000000000 --- a/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/AzureB2CAccessTokenStrategy.scala +++ /dev/null @@ -1,40 +0,0 @@ -package drs.localizer.accesstokens - -import cats.syntax.validated._ -import com.azure.identity.DefaultAzureCredentialBuilder -import com.azure.security.keyvault.secrets.{SecretClient, SecretClientBuilder} -import common.validation.ErrorOr -import common.validation.ErrorOr.{ErrorOr,ShortCircuitingFlatMap} -import drs.localizer.CommandLineArguments - -case class AzureB2CAccessTokenStrategy(commandLineArguments: CommandLineArguments) extends AccessTokenStrategy { - override def getAccessToken(): ErrorOr[String] = { - commandLineArguments match { - case CommandLineArguments(_, _, _, _, Some(vault), Some(secret), clientId, _, _) => - AzureKeyVaultClient(vault, clientId) flatMap { _.getSecret(secret) } - case invalid => s"Invalid command line arguments: $invalid".invalidNel - } - } -} - -// Note: The AzureKeyVaultClient code here is basically a copy/paste of the code temporarily living in the TES backend. -// All the current KeyVault interaction in Cromwell is temporary, but the code in the TES backend might be even more -// temporary than this. -class AzureKeyVaultClient(client: SecretClient) { - def getSecret(secretName: String): ErrorOr[String] = ErrorOr(client.getSecret(secretName).getValue) -} - -object AzureKeyVaultClient { - def apply(vaultName: String, identityClientId: Option[String]): ErrorOr[AzureKeyVaultClient] = ErrorOr { - val credentialBuilder = identityClientId.foldLeft(new DefaultAzureCredentialBuilder()) { - (builder, clientId) => builder.managedIdentityClientId(clientId) - } - - val client = new SecretClientBuilder() - .vaultUrl(s"https://${vaultName}.vault.azure.net") - .credential(credentialBuilder.build()) - .buildClient() - - new AzureKeyVaultClient(client) - } -} diff --git a/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/GoogleAccessTokenStrategy.scala b/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/GoogleAccessTokenStrategy.scala deleted file mode 100644 index 5af2b8af441..00000000000 --- a/cromwell-drs-localizer/src/main/scala/drs/localizer/accesstokens/GoogleAccessTokenStrategy.scala +++ /dev/null @@ -1,28 +0,0 @@ -package drs.localizer.accesstokens - -import cats.syntax.validated._ -import com.google.auth.oauth2.GoogleCredentials -import common.validation.ErrorOr.ErrorOr - -import scala.jdk.CollectionConverters._ -import scala.util.{Failure, Success, Try} - -/** - * Strategy for obtaining an access token from Google Application Default credentials that are assumed to already exist. - */ -case object GoogleAccessTokenStrategy extends AccessTokenStrategy { - private final val UserInfoEmailScope = "https://www.googleapis.com/auth/userinfo.email" - private final val UserInfoProfileScope = "https://www.googleapis.com/auth/userinfo.profile" - private final val UserInfoScopes = List(UserInfoEmailScope, UserInfoProfileScope) - - override def getAccessToken(): ErrorOr[String] = { - Try { - val scopedCredentials = GoogleCredentials.getApplicationDefault().createScoped(UserInfoScopes.asJava) - scopedCredentials.refreshAccessToken().getTokenValue - } match { - case Success(null) => "null token value attempting to refresh access token".invalidNel - case Success(value) => value.validNel - case Failure(e) => s"Failed to refresh access token: ${e.getMessage}".invalidNel - } - } -} diff --git a/cromwell-drs-localizer/src/test/scala/drs/localizer/CommandLineParserSpec.scala b/cromwell-drs-localizer/src/test/scala/drs/localizer/CommandLineParserSpec.scala index 349b2d6887b..7be30be8ac0 100644 --- a/cromwell-drs-localizer/src/test/scala/drs/localizer/CommandLineParserSpec.scala +++ b/cromwell-drs-localizer/src/test/scala/drs/localizer/CommandLineParserSpec.scala @@ -13,8 +13,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma private val drsObject = "drs://provider/object" private val containerPath = "/cromwell_root/my.bam" private val requesterPaysProject = "i_heart_egress" - private val azureVaultName = "Kwikset" - private val azureSecretName = "shhh" private val azureIdentityClientId = "itme@azure.com" private val manifestPath = "/my/manifest.txt" @@ -39,8 +37,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Google args.googleRequesterPaysProject shouldBe empty - args.azureVaultName shouldBe empty - args.azureSecretName shouldBe empty args.azureIdentityClientId shouldBe empty args.manifestPath shouldBe empty } @@ -51,8 +47,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Google args.googleRequesterPaysProject.get shouldBe requesterPaysProject - args.azureVaultName shouldBe empty - args.azureSecretName shouldBe empty args.azureIdentityClientId shouldBe empty args.manifestPath shouldBe empty } @@ -64,8 +58,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Google args.googleRequesterPaysProject.get shouldBe requesterPaysProject - args.azureVaultName shouldBe empty - args.azureSecretName shouldBe empty args.azureIdentityClientId shouldBe empty args.manifestPath shouldBe empty } @@ -77,8 +69,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Google args.googleRequesterPaysProject.get shouldBe requesterPaysProject - args.azureVaultName shouldBe empty - args.azureSecretName shouldBe empty args.azureIdentityClientId shouldBe empty args.manifestPath shouldBe empty } @@ -94,8 +84,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath shouldBe empty args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Google args.googleRequesterPaysProject shouldBe empty - args.azureVaultName shouldBe empty - args.azureSecretName shouldBe empty args.azureIdentityClientId shouldBe empty args.manifestPath.get shouldBe manifestPath } @@ -110,7 +98,7 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args shouldBe None } - it should "successfully parse an explicit Google access token stregy invocation" in { + it should "successfully parse an explicit Google access token strategy invocation" in { val args = parser.parse(Array( "--access-token-strategy", "google", drsObject, @@ -122,43 +110,13 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Google args.googleRequesterPaysProject.get shouldBe requesterPaysProject - args.azureVaultName shouldBe empty - args.azureSecretName shouldBe empty args.azureIdentityClientId shouldBe empty args.manifestPath shouldBe empty } - it should "fail to parse an Azure invocation missing vault name and secret name" in { - val args = parser.parse(Array( - "--access-token-strategy", AccessTokenStrategy.Azure, - drsObject, containerPath), CommandLineArguments()) - - args shouldBe None - } - - it should "fail to parse an Azure invocation missing vault name" in { - val args = parser.parse(Array( - "--access-token-strategy", AccessTokenStrategy.Azure, - "--secret-name", azureSecretName, - drsObject, containerPath), CommandLineArguments()) - - args shouldBe None - } - - it should "fail to parse an Azure invocation missing secret name" in { - val args = parser.parse(Array( - "--access-token-strategy", AccessTokenStrategy.Azure, - "--vault-name", azureVaultName, - drsObject, containerPath), CommandLineArguments()) - - args shouldBe None - } - it should "fail to parse an Azure invocation that specifies requester pays" in { val args = parser.parse(Array( "--access-token-strategy", AccessTokenStrategy.Azure, - "--secret-name", azureSecretName, - "--vault-name", azureVaultName, drsObject, containerPath, "--requester-pays-project", requesterPaysProject), CommandLineArguments()) @@ -169,25 +127,19 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma it should "successfully parse an Azure invocation" in { val args = parser.parse(Array( "--access-token-strategy", AccessTokenStrategy.Azure, - "--secret-name", azureSecretName, - "--vault-name", azureVaultName, drsObject, containerPath), CommandLineArguments()).get args.drsObject.get shouldBe drsObject args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Azure args.googleRequesterPaysProject shouldBe empty - args.azureVaultName.get shouldBe azureVaultName - args.azureSecretName.get shouldBe azureSecretName args.azureIdentityClientId shouldBe empty args.manifestPath shouldBe empty } - it should "successfully parse an Azure invocation with all the trimmings" in { + it should "successfully parse an Azure invocation with identity" in { val args = parser.parse(Array( "--access-token-strategy", AccessTokenStrategy.Azure, - "--vault-name", azureVaultName, - "--secret-name", azureSecretName, "--identity-client-id", azureIdentityClientId, drsObject, containerPath), CommandLineArguments()).get @@ -195,8 +147,6 @@ class CommandLineParserSpec extends AnyFlatSpec with CromwellTimeoutSpec with Ma args.containerPath.get shouldBe containerPath args.accessTokenStrategy.get shouldBe AccessTokenStrategy.Azure args.googleRequesterPaysProject shouldBe empty - args.azureVaultName.get shouldBe azureVaultName - args.azureSecretName.get shouldBe azureSecretName args.azureIdentityClientId.get shouldBe azureIdentityClientId args.manifestPath shouldBe empty } diff --git a/cromwell-drs-localizer/src/test/scala/drs/localizer/DrsLocalizerMainSpec.scala b/cromwell-drs-localizer/src/test/scala/drs/localizer/DrsLocalizerMainSpec.scala index 84b407cec41..b306c0a3a94 100644 --- a/cromwell-drs-localizer/src/test/scala/drs/localizer/DrsLocalizerMainSpec.scala +++ b/cromwell-drs-localizer/src/test/scala/drs/localizer/DrsLocalizerMainSpec.scala @@ -4,10 +4,10 @@ import cats.data.NonEmptyList import cats.effect.{ExitCode, IO} import cats.syntax.validated._ import cloud.nio.impl.drs.DrsPathResolver.FatalRetryDisposition -import cloud.nio.impl.drs.{AccessUrl, DrsConfig, MarthaField, MarthaResponse} +import cloud.nio.impl.drs.{AccessUrl, DrsConfig, DrsCredentials, MarthaField, MarthaResponse} import common.assertion.CromwellTimeoutSpec +import common.validation.ErrorOr.ErrorOr import drs.localizer.MockDrsLocalizerDrsPathResolver.{FakeAccessTokenStrategy, FakeHashes} -import drs.localizer.accesstokens.AccessTokenStrategy import drs.localizer.downloaders.AccessUrlDownloader.Hashes import drs.localizer.downloaders._ import org.scalatest.flatspec.AnyFlatSpec @@ -341,5 +341,7 @@ class MockDrsLocalizerDrsPathResolver(drsConfig: DrsConfig) extends object MockDrsLocalizerDrsPathResolver { val FakeHashes: Option[Map[String, String]] = Option(Map("md5" -> "abc123", "crc32c" -> "34fd67")) - val FakeAccessTokenStrategy: AccessTokenStrategy = () => "testing code: do not call me".invalidNel + val FakeAccessTokenStrategy: DrsCredentials = new DrsCredentials { + override def getAccessToken: ErrorOr[String] = "testing code: do not call me".invalidNel + } } diff --git a/filesystems/drs/src/main/scala/cromwell/filesystems/drs/DrsPathBuilderFactory.scala b/filesystems/drs/src/main/scala/cromwell/filesystems/drs/DrsPathBuilderFactory.scala index bc7fb598a81..ae49cf0b5e4 100644 --- a/filesystems/drs/src/main/scala/cromwell/filesystems/drs/DrsPathBuilderFactory.scala +++ b/filesystems/drs/src/main/scala/cromwell/filesystems/drs/DrsPathBuilderFactory.scala @@ -2,13 +2,12 @@ package cromwell.filesystems.drs import akka.actor.ActorSystem import cats.data.Validated.{Invalid, Valid} -import cloud.nio.impl.drs.{AzureDrsCredentials, DrsCloudNioFileSystemProvider, GoogleDrsCredentials} +import cloud.nio.impl.drs.{AzureDrsCredentials, DrsCloudNioFileSystemProvider, GoogleOauthDrsCredentials} import com.google.api.services.oauth2.Oauth2Scopes import com.typesafe.config.Config import cromwell.cloudsupport.gcp.GoogleConfiguration import cromwell.core.WorkflowOptions import cromwell.core.path.{PathBuilder, PathBuilderFactory} -import net.ceedubs.ficus.Ficus._ import scala.concurrent.{ExecutionContext, Future} @@ -24,10 +23,8 @@ class DrsPathBuilderFactory(globalConfig: Config, instanceConfig: Config, single private lazy val googleConfiguration: GoogleConfiguration = GoogleConfiguration(globalConfig) private lazy val scheme = instanceConfig.getString("auth") - // For Azure support + // For Azure support - this should be the UAMI client id private val dataAccessIdentityKey = "data_access_identity" - private lazy val azureKeyVault = instanceConfig.as[Option[String]]("azure-keyvault-name") - private lazy val azureSecretName = instanceConfig.as[Option[String]]("azure-token-secret") override def withOptions(options: WorkflowOptions)(implicit as: ActorSystem, ec: ExecutionContext): Future[PathBuilder] = { Future { @@ -37,13 +34,12 @@ class DrsPathBuilderFactory(globalConfig: Config, instanceConfig: Config, single Oauth2Scopes.USERINFO_PROFILE ) - val (googleAuthMode, drsCredentials) = (scheme, azureKeyVault, azureSecretName) match { - case ("azure", Some(vaultName), Some(secretName)) => (None, AzureDrsCredentials(options.get(dataAccessIdentityKey).toOption, vaultName, secretName)) - case ("azure", _, _) => throw new RuntimeException(s"Error while instantiating DRS path builder factory. Couldn't find azure-keyvault-name and azure-token-secret in config.") - case (googleAuthScheme, _, _) => googleConfiguration.auth(googleAuthScheme) match { + val (googleAuthMode, drsCredentials) = scheme match { + case "azure" => (None, AzureDrsCredentials(options.get(dataAccessIdentityKey).toOption)) + case googleAuthScheme => googleConfiguration.auth(googleAuthScheme) match { case Valid(auth) => ( Option(auth), - GoogleDrsCredentials(auth.credentials(options.get(_).get, marthaScopes), singletonConfig.config) + GoogleOauthDrsCredentials(auth.credentials(options.get(_).get, marthaScopes), singletonConfig.config) ) case Invalid(error) => throw new RuntimeException(s"Error while instantiating DRS path builder factory. Errors: ${error.toString}") } diff --git a/filesystems/drs/src/test/scala/cromwell/filesystems/drs/DrsPathBuilderSpec.scala b/filesystems/drs/src/test/scala/cromwell/filesystems/drs/DrsPathBuilderSpec.scala index 242b662d5ea..bc556c9bdf3 100644 --- a/filesystems/drs/src/test/scala/cromwell/filesystems/drs/DrsPathBuilderSpec.scala +++ b/filesystems/drs/src/test/scala/cromwell/filesystems/drs/DrsPathBuilderSpec.scala @@ -1,7 +1,7 @@ package cromwell.filesystems.drs import cloud.nio.impl.drs.DrsCloudNioFileProvider.DrsReadInterpreter -import cloud.nio.impl.drs.{DrsCloudNioFileSystemProvider, GoogleDrsCredentials} +import cloud.nio.impl.drs.{DrsCloudNioFileSystemProvider, GoogleOauthDrsCredentials} import com.google.cloud.NoCredentials import com.typesafe.config.{Config, ConfigFactory} import cromwell.core.TestKitSuite @@ -387,7 +387,7 @@ class DrsPathBuilderSpec extends TestKitSuite with AnyFlatSpecLike with Matchers private lazy val fakeCredentials = NoCredentials.getInstance private lazy val drsPathBuilder = DrsPathBuilder( - new DrsCloudNioFileSystemProvider(marthaConfig, GoogleDrsCredentials(fakeCredentials, 1.minutes), drsReadInterpreter), + new DrsCloudNioFileSystemProvider(marthaConfig, GoogleOauthDrsCredentials(fakeCredentials, 1.minutes), drsReadInterpreter), None, ) } diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 8906e010531..d526a05b78d 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -12,7 +12,6 @@ object Dependencies { // https://github.com/sbt/sbt/issues/4531 private val azureStorageBlobNioV = "12.0.0-beta.19" private val azureIdentitySdkV = "1.4.6" - private val azureKeyVaultSdkV = "4.3.8" private val betterFilesV = "3.9.1" /* cats-effect, fs2, http4s, and sttp (also to v3) should all be upgraded at the same time to use cats-effect 3.x. @@ -190,9 +189,6 @@ object Dependencies { "com.azure" % "azure-identity" % azureIdentitySdkV exclude("jakarta.xml.bind", "jakarta.xml.bind-api") exclude("jakarta.activation", "jakarta.activation-api"), - "com.azure" % "azure-security-keyvault-secrets" % azureKeyVaultSdkV - exclude("jakarta.xml.bind", "jakarta.xml.bind-api") - exclude("jakarta.activation", "jakarta.activation-api"), "com.azure" % "azure-core-management" % "1.7.1", "com.azure.resourcemanager" % "azure-resourcemanager" % "2.18.0" ) diff --git a/runConfigurations/Repo template_ Cromwell DRS Localizer.run.xml b/runConfigurations/Repo template_ Cromwell DRS Localizer.run.xml index 1ca941fbfa9..c718d7ff1f1 100644 --- a/runConfigurations/Repo template_ Cromwell DRS Localizer.run.xml +++ b/runConfigurations/Repo template_ Cromwell DRS Localizer.run.xml @@ -5,7 +5,7 @@