-
Notifications
You must be signed in to change notification settings - Fork 12
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
[TOAZ-355] [TOAZ-356] Use Managed Identity auth when running Azure Control Plane, support for Service Catalog deployed Azure Managed Apps #1404
Conversation
…nd Service Principal auth
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala
Show resolved
Hide resolved
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.
Left a couple comments above, but overall the approach makes sense to me.
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AzureServicesConfig.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
Outdated
Show resolved
Hide resolved
Refactor of validateAuthorizedAppUser completed
src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
Outdated
Show resolved
Hide resolved
…ic for deciding when to check for ServiceCatalogManagedApps vs MarketplaceManagedApps
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.
Not super familiar with this azure functionality but changes seem reasonable to me, requested minor naming changes for clarity
Quality Gate passedIssues Measures |
Refactor SAM ARM authorization code to support using a Managed Identity app id instead of Service Principal clientid/clientsecret
Service Catalog deployed app support:
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.