-
Notifications
You must be signed in to change notification settings - Fork 61
Feature/azure network redesign #137
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
Merged
+1,284
−201
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
8ad760f
feat(azure): Add support for SAT
connorbrown-db 5b25679
tests(azure): Reenable tags checking, require terraform fmt in tests
connorbrown-db d168d78
fix(azure): Add various missing tags
connorbrown-db 22ad225
style(azure): various whitespace/styling updates
connorbrown-db 4a82b7e
feat(azure): Remove default storage from metastore
connorbrown-db 8e98623
feat(azure): Add catalog module
connorbrown-db 26df8d1
feat(azure): Remove for_each spoke creation
connorbrown-db e63a989
feat(azure): Add default catalog for spoke
connorbrown-db f56e524
chore(azure): Terraform fmt
connorbrown-db 1de499e
fix(azure): Make all SAT resources use the same azure provider
connorbrown-db a995c5d
style(azure): Rename local.sat_spoke to local.sat_workspace
connorbrown-db a4d61b6
docs(azure): Update README with SAT details
connorbrown-db 65dc50a
feat(azure): Switch to pessimistic pin for naming module
connorbrown-db b9f5c4f
feat(azure): Provision webauth workspace as a normal workspace, now s…
connorbrown-db 9879d8d
feat(azure): Default SAT to the hub webauth workspace
connorbrown-db f05d49f
feat(azure): Remove dedicated SAT catalog and provider
connorbrown-db 543e314
docs(azure): Improve comments and README
connorbrown-db 0abcce0
fix(azure): CMK access policy dependency moved to correct access policy
connorbrown-db 13331a7
tests(azure): Replace sat spoke test with nondefault test
connorbrown-db a1f1011
feat(azure): Add better support for resource_suffix on SAT catalog
connorbrown-db ff070ae
feat(azure): Add azure management lock to webauth workspace to preven…
connorbrown-db 0c930f4
feat(azure): Allow dynamic dependency for SAT catalog on SAT module
connorbrown-db 26dcf0f
feat(azure): Bump version of SAT module
connorbrown-db f418673
fix(azure): Make webauth workspace use hub firewall
connorbrown-db 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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,89 @@ | ||
locals { | ||
create_sat_sp = var.sat_configuration.enabled && var.sat_service_principal.client_id == "" | ||
sat_client_id = local.create_sat_sp ? azuread_service_principal.sat[0].client_id : var.sat_service_principal.client_id | ||
sat_client_secret = local.create_sat_sp ? azuread_service_principal_password.sat[0].value : var.sat_service_principal.client_secret | ||
sat_workspace = module.hub | ||
sat_catalog = var.sat_configuration.enabled ? module.hub_catalog[0] : {} | ||
} | ||
|
||
# ---------------------------------------------------------------------------------------------------------------------- | ||
# Service Principal for SAT | ||
# Note: This is separated from the SAT module to allow for a BYO-SP pattern. If the user supplies values for the | ||
# sat_service principal variable, creation will be skipped. | ||
|
||
resource "azuread_application_registration" "sat" { | ||
count = local.create_sat_sp ? 1 : 0 | ||
|
||
display_name = var.sat_service_principal.name | ||
} | ||
|
||
resource "azuread_service_principal" "sat" { | ||
count = local.create_sat_sp ? 1 : 0 | ||
|
||
client_id = azuread_application_registration.sat[0].client_id | ||
owners = [data.azurerm_client_config.current.object_id] | ||
} | ||
|
||
resource "azuread_service_principal_password" "sat" { | ||
count = local.create_sat_sp ? 1 : 0 | ||
|
||
service_principal_id = azuread_service_principal.sat[0].id | ||
} | ||
|
||
data "azurerm_subscription" "sat" { | ||
count = local.create_sat_sp ? 1 : 0 | ||
|
||
subscription_id = var.subscription_id | ||
} | ||
|
||
resource "azurerm_role_assignment" "sat_can_read_subscription" { | ||
count = local.create_sat_sp ? 1 : 0 | ||
|
||
principal_id = azuread_service_principal.sat[0].object_id | ||
scope = data.azurerm_subscription.sat[0].id | ||
role_definition_name = "Reader" | ||
} | ||
|
||
# ---------------------------------------------------------------------------------------------------------------------- | ||
# This is modularized to allow for easy count and provider arguments | ||
module "sat" { | ||
source = "./modules/sat" | ||
count = var.sat_configuration.enabled ? 1 : 0 | ||
|
||
# Update this as needed | ||
catalog_name = local.sat_catalog.catalog_name | ||
|
||
tenant_id = data.azurerm_client_config.current.tenant_id | ||
subscription_id = var.subscription_id | ||
databricks_account_id = var.databricks_account_id | ||
schema_name = var.sat_configuration.schema_name | ||
proxies = var.sat_configuration.proxies | ||
run_on_serverless = var.sat_configuration.run_on_serverless | ||
service_principal_client_id = local.sat_client_id | ||
service_principal_client_secret = local.sat_client_secret | ||
workspace_id = local.sat_workspace.workspace_id | ||
|
||
depends_on = [local.sat_catalog] | ||
|
||
# Change the provider if needed | ||
providers = { | ||
databricks = databricks.hub | ||
} | ||
} | ||
|
||
# Grant the SP created by SAT the account_admin role | ||
resource "databricks_service_principal_role" "sat_account_admin" { | ||
count = length(module.sat) | ||
|
||
role = "account_admin" | ||
service_principal_id = module.sat[0].service_principal_id | ||
} | ||
|
||
resource "databricks_permission_assignment" "sat_workspace_admin" { | ||
count = length(module.sat) | ||
|
||
permissions = ["ADMIN"] | ||
principal_id = module.sat[0].service_principal_id | ||
|
||
provider = databricks.hub | ||
} |
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.
NIT @connorbrown-db Maybe we should consider renaming the SP so that it's non-exclusive to SAT. Future customizations may leverage the same SP together with the SAT deployment
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.
For what it's worth, I kind of like it being exclusive to SAT, at least while that's all it's being used for. I think there's an argument for an "administrative" SP at some point, but there's probably counterarguments for separation of concerns as well. Agreed it's not the highest priority to solve.