From 79c2bff6644b0f3ee43565ce0deb7c2acdd81ce6 Mon Sep 17 00:00:00 2001 From: Saloni Shah Date: Thu, 7 Mar 2024 14:31:07 -0500 Subject: [PATCH] WM-2461] Add support for running private workflows on Azure (#7373) --- core/src/main/resources/reference.conf | 1 + .../MaterializeWorkflowDescriptorActor.scala | 17 +++++-- ...terializeWorkflowDescriptorActorSpec.scala | 10 +++- .../auth/GithubAuthVendingSupport.scala | 21 ++++++++- .../auth/GithubAuthVendingSupportSpec.scala | 47 +++++++++++++++++-- 5 files changed, 85 insertions(+), 11 deletions(-) diff --git a/core/src/main/resources/reference.conf b/core/src/main/resources/reference.conf index c43dbce60ee..672d82c8c80 100644 --- a/core/src/main/resources/reference.conf +++ b/core/src/main/resources/reference.conf @@ -577,6 +577,7 @@ services { class = "cromwell.services.auth.impl.GithubAuthVendingActor" config { enabled = false + auth.azure = false # Notes: # - don't include the 'Bearer' before the token # - this config value should be removed when support for fetching tokens from ECM has been added to Cromwell diff --git a/engine/src/main/scala/cromwell/engine/workflow/lifecycle/materialization/MaterializeWorkflowDescriptorActor.scala b/engine/src/main/scala/cromwell/engine/workflow/lifecycle/materialization/MaterializeWorkflowDescriptorActor.scala index 6b515114149..8e57c77f2de 100644 --- a/engine/src/main/scala/cromwell/engine/workflow/lifecycle/materialization/MaterializeWorkflowDescriptorActor.scala +++ b/engine/src/main/scala/cromwell/engine/workflow/lifecycle/materialization/MaterializeWorkflowDescriptorActor.scala @@ -2,6 +2,7 @@ package cromwell.engine.workflow.lifecycle.materialization import akka.actor.{ActorRef, FSM, LoggingFSM, Props, Status} import akka.pattern.pipe +import akka.util.Timeout import cats.data.EitherT._ import cats.data.NonEmptyList import cats.data.Validated.{Invalid, Valid} @@ -36,6 +37,7 @@ import cromwell.filesystems.gcs.batch.GcsBatchCommandBuilder import cromwell.languages.util.ImportResolver._ import cromwell.languages.util.LanguageFactoryUtil import cromwell.languages.{LanguageFactory, ValidatedWomNamespace} +import cromwell.services.auth.GithubAuthVendingSupport import cromwell.services.metadata.MetadataService._ import cromwell.services.metadata.{MetadataEvent, MetadataKey, MetadataValue} import eu.timepit.refined.refineV @@ -50,6 +52,7 @@ import wom.runtime.WomOutputRuntimeExtractor import wom.values.{WomString, WomValue} import scala.concurrent.Future +import scala.concurrent.duration.DurationInt import scala.language.postfixOps import scala.util.{Failure, Success, Try} @@ -183,7 +186,7 @@ object MaterializeWorkflowDescriptorActor { } // TODO WOM: need to decide where to draw the line between language specific initialization and WOM -class MaterializeWorkflowDescriptorActor(serviceRegistryActor: ActorRef, +class MaterializeWorkflowDescriptorActor(override val serviceRegistryActor: ActorRef, workflowId: WorkflowId, cromwellBackends: => CromwellBackends, importLocalFilesystem: Boolean, @@ -191,7 +194,8 @@ class MaterializeWorkflowDescriptorActor(serviceRegistryActor: ActorRef, hogGroup: HogGroup ) extends LoggingFSM[MaterializeWorkflowDescriptorActorState, Unit] with StrictLogging - with WorkflowLogging { + with WorkflowLogging + with GithubAuthVendingSupport { import MaterializeWorkflowDescriptorActor._ val tag = self.path.name @@ -204,6 +208,8 @@ class MaterializeWorkflowDescriptorActor(serviceRegistryActor: ActorRef, protected val pathBuilderFactories: List[PathBuilderFactory] = EngineFilesystems.configuredPathBuilderFactories + final private val githubAuthVendingTimeout = Timeout(60.seconds) + startWith(ReadyToMaterializeState, ()) when(ReadyToMaterializeState) { @@ -346,7 +352,12 @@ class MaterializeWorkflowDescriptorActor(serviceRegistryActor: ActorRef, for { _ <- publishLabelsToMetadata(id, labels.asMap, serviceRegistryActor) zippedImportResolver <- zippedResolverCheck - importResolvers = zippedImportResolver.toList ++ localFilesystemResolvers :+ HttpResolver(None, Map.empty) + importAuthProviderOpt <- importAuthProvider(conf)(githubAuthVendingTimeout).toIOChecked + importResolvers = zippedImportResolver.toList ++ localFilesystemResolvers :+ HttpResolver( + None, + Map.empty, + importAuthProviderOpt.toList + ) sourceAndResolvers <- fromEither[IO]( LanguageFactoryUtil.findWorkflowSource(sourceFiles.workflowSource, sourceFiles.workflowUrl, importResolvers) ) diff --git a/server/src/test/scala/cromwell/engine/workflow/lifecycle/MaterializeWorkflowDescriptorActorSpec.scala b/server/src/test/scala/cromwell/engine/workflow/lifecycle/MaterializeWorkflowDescriptorActorSpec.scala index 216afa6ee4f..cf845af5d30 100644 --- a/server/src/test/scala/cromwell/engine/workflow/lifecycle/MaterializeWorkflowDescriptorActorSpec.scala +++ b/server/src/test/scala/cromwell/engine/workflow/lifecycle/MaterializeWorkflowDescriptorActorSpec.scala @@ -1,6 +1,6 @@ package cromwell.engine.workflow.lifecycle -import akka.actor.Props +import akka.actor.{ActorRef, Props} import akka.testkit.TestDuration import cats.data.Validated.{Invalid, Valid} import com.typesafe.config.ConfigFactory @@ -14,6 +14,7 @@ import cromwell.engine.workflow.lifecycle.materialization.MaterializeWorkflowDes MaterializeWorkflowDescriptorFailureResponse, MaterializeWorkflowDescriptorSuccessResponse } +import cromwell.services.auth.GithubAuthVendingSupport import cromwell.util.SampleWdl.HelloWorld import cromwell.{CromwellTestKitSpec, CromwellTestKitWordSpec} import org.scalatest.BeforeAndAfter @@ -23,7 +24,10 @@ import wom.values.{WomInteger, WomString} import scala.concurrent.duration._ -class MaterializeWorkflowDescriptorActorSpec extends CromwellTestKitWordSpec with BeforeAndAfter { +class MaterializeWorkflowDescriptorActorSpec + extends CromwellTestKitWordSpec + with BeforeAndAfter + with GithubAuthVendingSupport { private val ioActor = system.actorOf(SimpleIoActor.props) private val workflowId = WorkflowId.randomId() @@ -89,6 +93,8 @@ class MaterializeWorkflowDescriptorActorSpec extends CromwellTestKitWordSpec wit private val fooHogGroup = HogGroup("foo") + override def serviceRegistryActor: ActorRef = NoBehaviorActor + "MaterializeWorkflowDescriptorActor" should { "accept valid WDL, inputs and options files" in { val materializeWfActor = system.actorOf( diff --git a/services/src/main/scala/cromwell/services/auth/GithubAuthVendingSupport.scala b/services/src/main/scala/cromwell/services/auth/GithubAuthVendingSupport.scala index a8d406578ae..63fc9404e07 100644 --- a/services/src/main/scala/cromwell/services/auth/GithubAuthVendingSupport.scala +++ b/services/src/main/scala/cromwell/services/auth/GithubAuthVendingSupport.scala @@ -3,7 +3,12 @@ package cromwell.services.auth import akka.actor.ActorRef import akka.pattern.{AskSupport, AskTimeoutException} import akka.util.Timeout +import cats.data.Validated.{Invalid, Valid} +import cats.implicits.catsSyntaxValidatedId +import com.typesafe.config.Config import com.typesafe.scalalogging.StrictLogging +import common.validation.ErrorOr.ErrorOr +import cromwell.cloudsupport.azure.AzureCredentials import cromwell.languages.util.ImportResolver.{GithubImportAuthProvider, ImportAuthProvider} import cromwell.services.auth.GithubAuthVending.{ GithubAuthRequest, @@ -13,16 +18,16 @@ import cromwell.services.auth.GithubAuthVending.{ NoGithubAuthResponse } +import net.ceedubs.ficus.Ficus._ import scala.concurrent.{ExecutionContext, Future} trait GithubAuthVendingSupport extends AskSupport with StrictLogging { def serviceRegistryActor: ActorRef - implicit val timeout: Timeout implicit val ec: ExecutionContext - def importAuthProvider(token: String): ImportAuthProvider = new GithubImportAuthProvider { + def importAuthProvider(token: String)(implicit timeout: Timeout): ImportAuthProvider = new GithubImportAuthProvider { override def authHeader(): Future[Map[String, String]] = serviceRegistryActor .ask(GithubAuthRequest(token)) @@ -42,4 +47,16 @@ trait GithubAuthVendingSupport extends AskSupport with StrictLogging { Future.failed(new Exception("Failed to resolve github auth token", error)) } } + + def importAuthProvider(config: Config)(implicit timeout: Timeout): ErrorOr[Option[ImportAuthProvider]] = { + val isAuthAzure = config.as[Boolean]("services.GithubAuthVending.config.auth.azure") + + if (isAuthAzure) { + val azureToken = AzureCredentials.getAccessToken() + azureToken match { + case Valid(token) => Option(importAuthProvider(token)).validNel + case Invalid(err) => s"Failed to fetch Azure token. Error: ${err.toString}".invalidNel + } + } else None.validNel + } } diff --git a/services/src/test/scala/cromwell/services/auth/GithubAuthVendingSupportSpec.scala b/services/src/test/scala/cromwell/services/auth/GithubAuthVendingSupportSpec.scala index f9bc3e127f4..9a20851ebdd 100644 --- a/services/src/test/scala/cromwell/services/auth/GithubAuthVendingSupportSpec.scala +++ b/services/src/test/scala/cromwell/services/auth/GithubAuthVendingSupportSpec.scala @@ -3,7 +3,10 @@ package cromwell.services.auth import akka.actor.ActorRef import akka.testkit.TestProbe import akka.util.Timeout +import cats.data.Validated.{Invalid, Valid} +import com.typesafe.config.ConfigFactory import cromwell.core.TestKitSuite +import cromwell.languages.util.ImportResolver.GithubImportAuthProvider import cromwell.services.ServiceRegistryActor.ServiceRegistryFailure import cromwell.services.auth.GithubAuthVending.GithubAuthRequest import cromwell.services.auth.GithubAuthVendingSupportSpec.TestGithubAuthVendingSupport @@ -16,6 +19,21 @@ import scala.concurrent.duration._ class GithubAuthVendingSupportSpec extends TestKitSuite with AnyFlatSpecLike with Matchers with Eventually { + private def azureGithubAuthVendingConfig(enabled: Boolean = true) = ConfigFactory + .parseString( + s""" + |services { + | GithubAuthVending { + | config { + | auth.azure = ${enabled} + | } + | } + |} + |""".stripMargin + ) + + implicit val timeout = Timeout(10.seconds) + behavior of "GithubAuthVendingSupport" it should "send a message to the service registry and handle success response" in { @@ -60,8 +78,8 @@ class GithubAuthVendingSupportSpec extends TestKitSuite with AnyFlatSpecLike wit it should "handle timeouts" in { val serviceRegistryActor = TestProbe() - val testSupport = new TestGithubAuthVendingSupport(serviceRegistryActor.ref, 1.millisecond) - val provider = testSupport.importAuthProvider("user-token") + val testSupport = new TestGithubAuthVendingSupport(serviceRegistryActor.ref) + val provider = testSupport.importAuthProvider("user-token")(Timeout(1.millisecond)) val authHeader: Future[Map[String, String]] = provider.authHeader() eventually { @@ -88,11 +106,32 @@ class GithubAuthVendingSupportSpec extends TestKitSuite with AnyFlatSpecLike wit } } + it should "return Github import auth provider when Azure auth is enabled" in { + val serviceRegistryActor = TestProbe() + val testSupport = new TestGithubAuthVendingSupport(serviceRegistryActor.ref) + + testSupport.importAuthProvider(azureGithubAuthVendingConfig()) match { + case Valid(providerOpt) => + providerOpt.isEmpty shouldBe false + providerOpt.get.isInstanceOf[GithubImportAuthProvider] shouldBe true + providerOpt.get.validHosts shouldBe List("github.com", "githubusercontent.com", "raw.githubusercontent.com") + case Invalid(e) => fail(s"Unexpected failure: $e") + } + } + + it should "return no import auth provider when Azure auth is disabled" in { + val serviceRegistryActor = TestProbe() + val testSupport = new TestGithubAuthVendingSupport(serviceRegistryActor.ref) + + testSupport.importAuthProvider(azureGithubAuthVendingConfig(false)) match { + case Valid(providerOpt) => providerOpt.isEmpty shouldBe true + case Invalid(e) => fail(s"Unexpected failure: $e") + } + } } object GithubAuthVendingSupportSpec { - class TestGithubAuthVendingSupport(val serviceRegistryActor: ActorRef, val timeout: Timeout = 10.seconds) - extends GithubAuthVendingSupport { + class TestGithubAuthVendingSupport(val serviceRegistryActor: ActorRef) extends GithubAuthVendingSupport { implicit override val ec: ExecutionContext = ExecutionContext.global }