Missing private link DNS zones #481
Merged
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
|
Thank you for raising this @bentaylorwork I will test and review the code changes, but just wanted to check whether you would be willing to add any other missing services as documented in the Azure services DNS zone configuration document? This will be a breaking change due to the associated input variable schema update so before we release this we will need to get them all updated. There are also some interesting new additions to these, specifically the new static web apps URI which requires a
Although we could support this, it will require some additional configuration settings adding to take a list of required As we have limited policy coverage in this space, an alternative approach may be to exclude this one and ask users to provide the full URI using the config.private_dns_zones input variable. We might also be better including this as a broader set of changes as we move to Terraform Thoughts please @sitarant @jtracey93 @matt-FFFFFF ? |
krowlandson
suggested changes
Oct 7, 2022
There was a problem hiding this comment.
This looks great, and thank you for your contribution @bentaylorwork 🚀
Please take a look at the feedback and let me know if you would be happy to add to this or would need someone else to add the other services as mentioned in my PR comment
| @@ -42,7 +42,7 @@ Default: `{}` | |||
| <br> | |||
|
|
|||
| <!-- markdownlint-disable-next-line MD013 --> | |||
| [**configure_connectivity_resources**][configure_connectivity_resources] `object({ settings = object({ hub_networks = list( object({ enabled = bool config = object({ address_space = list(string) location = string link_to_ddos_protection_plan = bool dns_servers = list(string) bgp_community = string subnets = list( object({ name = string address_prefixes = list(string) network_security_group_id = string route_table_id = string }) ) virtual_network_gateway = object({ enabled = bool config = object({ address_prefix = string gateway_sku_expressroute = string gateway_sku_vpn = string advanced_vpn_settings = object({ enable_bgp = bool active_active = bool private_ip_address_allocation = string default_local_network_gateway_id = string vpn_client_configuration = list( object({ address_space = list(string) aad_tenant = string aad_audience = string aad_issuer = string root_certificate = list( object({ name = string public_cert_data = string }) ) revoked_certificate = list( object({ name = string public_cert_data = string }) ) radius_server_address = string radius_server_secret = string vpn_client_protocols = list(string) vpn_auth_types = list(string) }) ) bgp_settings = list( object({ asn = number peer_weight = number peering_addresses = list( object({ ip_configuration_name = string apipa_addresses = list(string) }) ) }) ) custom_route = list( object({ address_prefixes = list(string) }) ) }) }) }) azure_firewall = object({ enabled = bool config = object({ address_prefix = string enable_dns_proxy = bool dns_servers = list(string) sku_tier = string base_policy_id = string private_ip_ranges = list(string) threat_intelligence_mode = string threat_intelligence_allowlist = list(string) availability_zones = object({ zone_1 = bool zone_2 = bool zone_3 = bool }) }) }) spoke_virtual_network_resource_ids = list(string) enable_outbound_virtual_network_peering = bool }) }) ) vwan_hub_networks = list( object({ enabled = bool config = object({ address_prefix = string location = string sku = string routes = list( object({ address_prefixes = list(string) next_hop_ip_address = string }) ) expressroute_gateway = object({ enabled = bool config = object({ scale_unit = number }) }) vpn_gateway = object({ enabled = bool config = object({ bgp_settings = list( object({ asn = number peer_weight = number instance_0_bgp_peering_address = list( object({ custom_ips = list(string) }) ) instance_1_bgp_peering_address = list( object({ custom_ips = list(string) }) ) }) ) routing_preference = string scale_unit = number }) }) azure_firewall = object({ enabled = bool config = object({ enable_dns_proxy = bool dns_servers = list(string) sku_tier = string base_policy_id = string private_ip_ranges = list(string) threat_intelligence_mode = string threat_intelligence_allowlist = list(string) availability_zones = object({ zone_1 = bool zone_2 = bool zone_3 = bool }) }) }) spoke_virtual_network_resource_ids = list(string) enable_virtual_hub_connections = bool }) }) ) ddos_protection_plan = object({ enabled = bool config = object({ location = string }) }) dns = object({ enabled = bool config = object({ location = string enable_private_link_by_service = object({ azure_automation_webhook = bool azure_automation_dscandhybridworker = bool azure_sql_database_sqlserver = bool azure_synapse_analytics_sqlserver = bool azure_synapse_analytics_sql = bool storage_account_blob = bool storage_account_table = bool storage_account_queue = bool storage_account_file = bool storage_account_web = bool azure_data_lake_file_system_gen2 = bool azure_cosmos_db_sql = bool azure_cosmos_db_mongodb = bool azure_cosmos_db_cassandra = bool azure_cosmos_db_gremlin = bool azure_cosmos_db_table = bool azure_database_for_postgresql_server = bool azure_database_for_mysql_server = bool azure_database_for_mariadb_server = bool azure_key_vault = bool azure_kubernetes_service_management = bool azure_search_service = bool azure_container_registry = bool azure_app_configuration_stores = bool azure_backup = bool azure_site_recovery = bool azure_event_hubs_namespace = bool azure_service_bus_namespace = bool azure_iot_hub = bool azure_relay_namespace = bool azure_event_grid_topic = bool azure_event_grid_domain = bool azure_web_apps_sites = bool azure_machine_learning_workspace = bool signalr = bool azure_monitor = bool cognitive_services_account = bool azure_file_sync = bool azure_data_factory = bool azure_data_factory_portal = bool azure_cache_for_redis = bool }) private_link_locations = list(string) public_dns_zones = list(string) private_dns_zones = list(string) enable_private_dns_zone_virtual_network_link_on_hubs = bool enable_private_dns_zone_virtual_network_link_on_spokes = bool }) }) }) location = any tags = any advanced = any })` | |||
| [**configure_connectivity_resources**][configure_connectivity_resources] `object({ settings = object({ hub_networks = list( object({ enabled = bool config = object({ address_space = list(string) location = string link_to_ddos_protection_plan = bool dns_servers = list(string) bgp_community = string subnets = list( object({ name = string address_prefixes = list(string) network_security_group_id = string route_table_id = string }) ) virtual_network_gateway = object({ enabled = bool config = object({ address_prefix = string gateway_sku_expressroute = string gateway_sku_vpn = string advanced_vpn_settings = object({ enable_bgp = bool active_active = bool private_ip_address_allocation = string default_local_network_gateway_id = string vpn_client_configuration = list( object({ address_space = list(string) aad_tenant = string aad_audience = string aad_issuer = string root_certificate = list( object({ name = string public_cert_data = string }) ) revoked_certificate = list( object({ name = string public_cert_data = string }) ) radius_server_address = string radius_server_secret = string vpn_client_protocols = list(string) vpn_auth_types = list(string) }) ) bgp_settings = list( object({ asn = number peer_weight = number peering_addresses = list( object({ ip_configuration_name = string apipa_addresses = list(string) }) ) }) ) custom_route = list( object({ address_prefixes = list(string) }) ) }) }) }) azure_firewall = object({ enabled = bool config = object({ address_prefix = string enable_dns_proxy = bool dns_servers = list(string) sku_tier = string base_policy_id = string private_ip_ranges = list(string) threat_intelligence_mode = string threat_intelligence_allowlist = list(string) availability_zones = object({ zone_1 = bool zone_2 = bool zone_3 = bool }) }) }) spoke_virtual_network_resource_ids = list(string) enable_outbound_virtual_network_peering = bool }) }) ) vwan_hub_networks = list( object({ enabled = bool config = object({ address_prefix = string location = string sku = string routes = list( object({ address_prefixes = list(string) next_hop_ip_address = string }) ) expressroute_gateway = object({ enabled = bool config = object({ scale_unit = number }) }) vpn_gateway = object({ enabled = bool config = object({ bgp_settings = list( object({ asn = number peer_weight = number instance_0_bgp_peering_address = list( object({ custom_ips = list(string) }) ) instance_1_bgp_peering_address = list( object({ custom_ips = list(string) }) ) }) ) routing_preference = string scale_unit = number }) }) azure_firewall = object({ enabled = bool config = object({ enable_dns_proxy = bool dns_servers = list(string) sku_tier = string base_policy_id = string private_ip_ranges = list(string) threat_intelligence_mode = string threat_intelligence_allowlist = list(string) availability_zones = object({ zone_1 = bool zone_2 = bool zone_3 = bool }) }) }) spoke_virtual_network_resource_ids = list(string) enable_virtual_hub_connections = bool }) }) ) ddos_protection_plan = object({ enabled = bool config = object({ location = string }) }) dns = object({ enabled = bool config = object({ location = string enable_private_link_by_service = object({ azure_automation_webhook = bool azure_automation_dscandhybridworker = bool azure_sql_database_sqlserver = bool azure_synapse = bool azure_synapse_dev = bool azure_synapse_analytics_sqlserver = bool azure_synapse_analytics_sql = bool storage_account_blob = bool storage_account_table = bool storage_account_queue = bool storage_account_file = bool storage_account_web = bool azure_data_lake_file_system_gen2 = bool azure_cosmos_db_sql = bool azure_cosmos_db_mongodb = bool azure_cosmos_db_cassandra = bool azure_cosmos_db_gremlin = bool azure_cosmos_db_table = bool azure_database_for_postgresql_server = bool azure_database_for_mysql_server = bool azure_database_for_mariadb_server = bool azure_key_vault = bool azure_kubernetes_service_management = bool azure_search_service = bool azure_container_registry = bool azure_app_configuration_stores = bool azure_backup = bool azure_site_recovery = bool azure_event_hubs_namespace = bool azure_service_bus_namespace = bool azure_iot_hub = bool azure_relay_namespace = bool azure_event_grid_topic = bool azure_event_grid_domain = bool azure_web_apps_sites = bool azure_machine_learning_workspace = bool signalr = bool azure_monitor = bool cognitive_services_account = bool azure_file_sync = bool azure_data_factory = bool azure_data_factory_portal = bool azure_cache_for_redis = bool azure_purview = bool azure_purview_studio = bool }) private_link_locations = list(string) public_dns_zones = list(string) private_dns_zones = list(string) enable_private_dns_zone_virtual_network_link_on_hubs = bool enable_private_dns_zone_virtual_network_link_on_spokes = bool }) }) }) location = any tags = any advanced = any })` | |||
There was a problem hiding this comment.
I appreciate this is a tricky one to format in markdown, but please can you check the spacing to avoid large gaps like the below:
There was a problem hiding this comment.
Should be resolved in latest commit.
| azure_data_factory = true | ||
| azure_data_factory_portal = true | ||
| azure_cache_for_redis = true | ||
| azure_automation_webhook = true |
There was a problem hiding this comment.
It looks like you've introduced a "double-indentation" on these lines (4 spaces, rather than 2).
Please can you double check and update accordingly?
Whilst we don't have linting on code snippets in markdown (yet), we would like to ensure the code reflects what would pass terraform fmt checks.
There was a problem hiding this comment.
Should be resolved in latest commit.
modules/connectivity/locals.tf
Outdated
| @@ -1288,6 +1288,8 @@ locals { | |||
| azure_sql_database_sqlserver = ["privatelink.database.windows.net"] | |||
| azure_synapse_analytics_sqlserver = ["privatelink.database.windows.net"] | |||
| azure_synapse_analytics_sql = ["privatelink.sql.azuresynapse.net"] | |||
| azure_synapse = ["privatelink.azuresynapse.net"] | |||
There was a problem hiding this comment.
Would this be better referred to as azure_synapse_studio?
@mboswell and @marvinbuss please can you input on this one, but also a broader view on the current state of our data product coverage? i.e. are the azure_synapse_analytics_sqlserver and azure_synapse_analytics_sql entries still relevant as the former no longer seems to be mentioned in the docs? 😄
There was a problem hiding this comment.
Hi @krowlandson,
Sorry, I just saw that you tagged me in this comment. privatelink.database.windows.net is not relevant for Synapse. This is only relevant for Azure SQL Servers.
Synapse only requires:
- For SQL endpoints:
privatelink.sql.azuresynapse.net - For dev endpoints:
privatelink.dev.azuresynapse.net - For Data Explorer endpoints:
privatelink.kusto.azuresynapse.net - For Synapse Studio:
privatelink.azuresynapse.net
Feel free to rename one of the variables to azure_synapse_studio accordingly.
There was a problem hiding this comment.
Hi @krowlandson,
Sorry, I just saw that you tagged me in this comment. privatelink.database.windows.net is not relevant for Synapse. This is only relevant for Azure SQL Servers.
Synapse only requires:
- For SQL endpoints:
privatelink.sql.azuresynapse.net - For dev endpoints:
privatelink.dev.azuresynapse.net - For Data Explorer endpoints:
privatelink.kusto.azuresynapse.net - For Synapse Studio:
privatelink.azuresynapse.net
Feel free to rename one of the variables to azure_synapse_studio accordingly.
There was a problem hiding this comment.
Cool, thank you for the confirmation @marvinbuss 👍🏻
I thought it was odd that the docs were updated, as the module reflects what the docs said at the time of authoring this capability, so we definitely need to fix this then 😄
|
I've done a quick review of the DNS zone diff and see the following which we should include in a release:
|
|
I can take a look at adding some of the missing Private DNS Zones early next week. Do you want me to add them to this PR or open a new one? |
That's great, thank you. I think it would be good to include in a single PR so we know what they are all covered off when merging to main and cutting our next release. Something else that will need updating is the test framework settings here: 🪨 |
|
I have dealt with all the private link dns zones apart from the ones in bold below. I will try and get to them in the next few days.
|
bentaylorwork
changed the title
This is awesome stuff... thank you @bentaylorwork !! I will try to make some time tomorrow to provide feedback on current progress as a couple of minor things I would like to change for consistency, but also to clarify some of the thinking around where services share an underlying namespace 👍🏻 |
added 7 commits
October 19, 2022 09:57
|
/azp run update |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@bentaylorwork ... I've made some updates so we can keep this progressing, however I need to validate why this is happening on our
I believe it's due to your PR coming from the Working locally, I observed the same permissions error when trying to push to just |
|
/azp run update |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@bentaylorwork... just to provide a quick summary of the updates I've made to this PR:
Super excited for this contribution, and thank you again for your help! I'll hold off completing my review and merging to give you a day or two to see if there's anything else you would like to add / change / suggest 👍🏻 |
|
/azp run unit |
|
Azure Pipelines successfully started running 1 pipeline(s). |
added 3 commits
October 26, 2022 14:46
…-enterprise-scale into bentaylorwork/main
|
/azp run update |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run unit |
|
Azure Pipelines successfully started running 1 pipeline(s). |
krowlandson
approved these changes
Oct 26, 2022
There was a problem hiding this comment.
Although we have a few further DNS related updates we want to implement, I think this is a great start and gets us past a significant milestone in this work.
Thank you for your contribution @bentaylorwork 💯
LGTM 🚀
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.
Overview/Summary
Adds Private Endpoint DNS zones for Purview and Synapse. The DNS zones are present in the ARM version of the ESLZ with a reference to them here:
https://github.com/Azure/Enterprise-Scale/blob/3b35eb22471e23436e5a0bb4cddc224b2edec686/eslzArm/eslzArm.json#L862
This PR fixes/adds/changes/removes
Breaking Changes
Testing Evidence
Terraform plans and apply work locally
As part of this Pull Request I have
mainbranch/docs/wiki/whats-new.md)