Support app service application logs blob storage #3520
Merged
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.
I've just started having a look at #1082. I didn't want to get much further without seeking feedback because this is already raising several questions.
This PR just addresses writing app service application logs to blob storage.
Service SAS Tokens
It seems that the
applicationLogs.azureBlobStorage.sasUrl
Resource Manager property requires a Container Service SAS rather than an Account SAS, which is a shame because the latter could be provided byazurerm_storage_account_sas
.I couldn't find any documentation proving this was the case, but attempts to use an Account SAS return
to the app service.
There is an outstanding issue to support this Feature Request: Add support for Azure Container SAS tokens which I haven't implemented. It's the user's responsibility to generate the Container Service SAS token. Is this approach acceptable for now?
DIAGNOSTICS* Application Setting Syncing
Updating the
applicationLogs.azureBlobStorage
Resource Manager properties also writes two new application settingsDIAGNOSTICS_AZUREBLOBCONTAINERSASURL
andDIAGNOSTICS_AZUREBLOBRETENTIONINDAYS
. When the properties are changed the application settings are updated; when the application settings are changed the properties are updated. I therefore remove the application settings at read time: this prevents them from being maintained in the state and means they don't need to be maintained in a .tf configuration in both places. I think something similar is going on already in theazure_arm_function_app
resource. Is this a good idea?Defaults
There's some discussion on the original issue about what values to set when the logs blocks are removed. I haven't really addressed this. It seems that when the Azure SDK
UpdateDiagnosticLogsConfig
method is hit with empty values, the values in Resource Manager seem to return to their defaults, so it seems OK.Putting it all together, the following seems to be working okay for our needs:
Thanks in advance for looking at this -- any feedback gratefully received!