yuri-tieto commented 4 years ago

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

Description

To be able to send audit logs to Storage account protected by Firewall we have to add specific permissions for SQL Server identity to Blob storage (role Storage Blob Data Contributor). And for that to happen SQL Server identity should be created first and then it should be assigned to specified role, that as the next step we will be able to apply Audit setting, so that they will work. For this extended_auditing_policy can not be provided during SQL Server creation and should be done as a separate step. There is already issue created regarding this for MSSQL server #6906 . This one is for SQL Server.

New or Affected Resource(s)

azurerm_sql_server (update)
azurerm_sql_server_auditing (new)

Potential Terraform Configuration

resource "azurerm_sql_server_auditing" {
  storage_endpoint                        = "https://myaccount.blob.core.windows.net/"
  storage_account_access_key              = "xxx" # Optional (only required if not using SystemAssigned identity)
  storage_account_access_key_is_secondary = false # Optional (only required if not using SystemAssigned identity)
  retention_in_days                       = 30
}

References

6906

yuri-tieto commented 4 years ago

Some additional information: looks like Portal, PowerShell SDK and ARM Templates are using auditingSettings Rest API endpoint (https://docs.microsoft.com/en-us/rest/api/sql/server%20auditing%20settings/createorupdate) for creating audit, but Terraform Azure provider uses extendedAuditingSettings (https://docs.microsoft.com/en-us/rest/api/sql/server%20extended%20auditing%20settings/createorupdate) endpoint. And when I enable firewall on Storage Account, though SQL Server SystemAssigned identity is having Storage Blob Data Contributor role, audit logs are not getting written to blob storage. Is there any specific reason why terraform provider uses extendedAuditingSettings instead of auditingSettings. Will it be possible to change it, as even if this external resource for azure sql server auditing will be created, it is still not going to work for vnet/firewalled storage accounts.

yupwei68 commented 4 years ago

Hi @yuri-tieto , thanks for opening this issue. 1. azurerm_mssql_server is calling a later version service api comparing to azurerm_sql_server, which is planed to include all the functions that azurerm_sql_server owns and substitute azurerm_sql_server in the long term.

Once azurerm_mssql_server_auditing is implemented, we'll mark the internal block extended_auditing_policy as computed in both azurerm_mssql_server and azurerm_sql_server. Thus, there is no need to implement another resource azurerm_sql_server_auditing, the resources in azurerm_sql_* and azurerm_mssql_* could be used mixed.
As for extended_auditing_policy comparing to auditing_policy, we choose extended_auditing_policy because it includes more functions than auditing_policy which might be useful in the future.

4.Currently, even if you have assigned Storage Blob Data Contributor role to the sql server, you could not auditing the sql server to a storage account behind firewall because the storage_account_access_key must not be set in this case (which currently is required in Terraform). It's not related to use extended_auditing_policy or auditing_policy.

m4r74 commented 4 years ago

@yupwei68 what is the timeline for storage_account_access_key to be made as an optional field?

ghost commented 4 years ago

This has been released in version 2.27.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.27.0"
}
# ... other configuration ...

ghost commented 4 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

hashicorp / terraform-provider-azurerm