[TOAZ-355] [TOAZ-356] Use Managed Identity auth when running Azure Control Plane, support for Service Catalog deployed Azure Managed Apps #1451
Conversation
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
dvoet
force-pushed
the
grantaar-create-bee-workflow-test
branch
from
74029cf to
001523d
Compare
…t' into grantaar-create-bee-workflow-test # Conflicts: # src/main/resources/sam.conf # src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala # src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AzureServicesConfig.scala # src/test/scala/org/broadinstitute/dsde/workbench/sam/api/TestSamRoutes.scala # src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala
…t' into grantaar-create-bee-workflow-test
|
aaronegrant
changed the title
dvoet
reviewed
Jun 20, 2024
| @@ -228,31 +228,41 @@ class AzureService( | |||
| /** Resolves a managed resource group in Azure and returns the terra.billingProfileId tag value. This is used for access control checks during route handling. | |||
| */ | |||
| def getBillingProfileId(request: GetOrCreatePetManagedIdentityRequest, samRequestContext: SamRequestContext): IO[Option[BillingProfileId]] = | |||
| // get the billing profile id from the database | |||
| // if not there, for backwards compatibility, get the billing profile id from a tag on the Azure resource | |||
There was a problem hiding this comment.
this backwards compatibility is no longer needed
dvoet
reviewed
Jun 20, 2024
| @@ -139,26 +119,6 @@ class AzureRoutesSpec extends AnyFlatSpec with Matchers with ScalatestRouteTest | |||
| } | |||
| } | |||
|
|
|||
| it should "successfully create a pet managed identity for a user using MRG Azure tag for backwards compatibility" in { | |||
There was a problem hiding this comment.
backwards compatibility removed
dvoet
approved these changes
Jun 20, 2024
Shakespeared
approved these changes
Jun 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refactor SAM ARM authorization code to support using a Managed Identity app id instead of Service Principal clientid/clientsecret/tenantid if optional config 'AZURE_MANAGED_APP_WORKLOAD_CLIENT_ID' is set
Service Catalog deployed app support, enabled with 'AZURE_SERVICE_CATALOG_ENABLED' config
Sam maintains its own list of Terra managed app plans https://github.com/broadinstitute/sam/blob/develop/src/main/resources/sam.conf#L192 and validates a billing profile managed app against this list during billing profile creation.
Service catalog managed apps do not have a plan defined so we will need change the validation in Sam to accept this.