Skip to content
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

Missing DINE policy for private DNS zone of file subresource #520

Closed
tpatrizio opened this issue Nov 22, 2022 · 6 comments
Closed

Missing DINE policy for private DNS zone of file subresource #520

tpatrizio opened this issue Nov 22, 2022 · 6 comments
Assignees

Comments

@tpatrizio
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Versions

terraform: 1.1.9

azure provider: >=3.0.2

module: 2.0.1

Description

Describe the bug

The Deploy-Private-DNS-Zones policy set includes only the DINE-Private-DNS-Azure-File-Sync policy (ID: 06695360-db88-47f6-b976-7500d4297475) but it is missing the built-in DINE-Private-DNS-Azure-File policy (ID: 6df98d03-368a-4438-8730-a93c4d7693d6). IN addition, the parameter associated to the File-Sync polic is named azureFilePrivateDnsZoneId but it refers to the file share private domain (privatelink.afs.azure.net)

Creating a file share (file subresource) doesn't trigger the DINE policy to register the A record in the Private DNS Zone privatelink.file.core.windows.net

  1. create a file share in a storage account
  2. configure the private endpoint for the file share
  3. the A record is not created in the privatelink.file.core.windows.net private DNS zone
  4. the activity log down't report any activity for the DINE policy

Screenshots

Additional context

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Nov 22, 2022
@krowlandson krowlandson self-assigned this Nov 23, 2022
@ghost ghost removed the Needs: Triage 🔍 Needs triaging by the team label Nov 23, 2022
@krowlandson
Copy link
Contributor

A very timely request @tpatrizio... we are making a number of updates to DNS, including policy updates and we are also including corresponding updates for DNS within the next release of this module which are already merged to main with a few more to come shortly.

@tpatrizio
Copy link
Author

Hi @krowlandson thanks for taking time to read my post. I took a look at the main branch but I sill cannot find the built-in DINE policy DINE-Private-DNS-Azure-File policy in the policy assignement file (policy_assignment_es_deploy_private_dns_zones.tmpl.json), even if the private DNS zone for the file subresource is correctly created and referenced in the local.tf file, along with the other storage acount subresources.

    storage_account_blob     = ["privatelink.blob.core.windows.net"]
    storage_account_file     = ["privatelink.file.core.windows.net"]
    storage_account_queue    = ["privatelink.queue.core.windows.net"]
    storage_account_table    = ["privatelink.table.core.windows.net"]
    storage_account_web      = ["privatelink.web.core.windows.net"]

Am I sill missing something ?

Thank for your help

@krowlandson
Copy link
Contributor

We're just pulling it across to this repo and testing, but you can see the addition in the PR I mentioned for the Azure/Enterprise-Scale repo:

image

Is this the one you are looking for? If so, this will be included in the upcoming release.

@tpatrizio
Copy link
Author

Thanks a lot for the clarification, I missed the point of the PR to be merged. Just a last question, should I take special care when upgrading from version 2.0.1 to the upcoming 2.4.x release of the module, or is it a smooth process?

@ghost ghost added Needs: Attention 👋 Needs attention from the maintainers and removed Needs: Author Feedback labels Nov 24, 2022
@krowlandson
Copy link
Contributor

krowlandson commented Nov 24, 2022

Should be reasonably smooth... if you check each of the release notes for the versions between, we highlight any code changes which are needed.

For example, to support v2.1.0 you would need to consider the following:


- ⚠️ To address #387 whilst putting the customer in control of whether these are deployed, we have added two new inputs to the `configure_management_resources` input variable. Customers using this input must update their code to reflect these changes. For more information, please refer to the following: 1. [settings.log_analytics.config.enable_solution_for_sql_vulnerability_assessment](https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/wiki/%5BVariables%5D-configure_management_resources#settingslog_analyticsenable_solution_for_sql_vulnerability_assessment) 5. [settings.log_analytics.config.enable_solution_for_sql_advanced_threat_detection](https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/wiki/%5BVariables%5D-configure_management_resources#settingslog_analyticsenable_solution_for_sql_advanced_threat_detection)

As an additional FYI, the upcoming release is going to support optional() types in our complex object input variables. This will make upgrades much easier in the future as newly added features should not require code updates unless you want to change the default behaviour.

@ghost ghost removed the Needs: Attention 👋 Needs attention from the maintainers label Nov 24, 2022
@krowlandson
Copy link
Contributor

This is now merged into main for inclusion within the next release so I will close this issue but please feel free to comment or re-open if you have further questions.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants