Azure / bicep-registry-modules

Bicep registry modules
MIT License
467 stars 316 forks source link

[AVM Module Issue]: Storage Account - Soft delete retention days inconsistency #2707

Open riosengineer opened 2 months ago

riosengineer commented 2 months ago

Check for previous/existing GitHub issues

Issue Type?

Bug

Module Name

avm/res/storage/storage-account

(Optional) Module Version

No response

Description

Hi there!

This isn't a bug, more of a consistency thing I noticed. Should the soft delete retention value be 7 for blob service? As it is currently set to 6 days, however, the file service is set to 7 days. So, you end up with inconsistent default deployments.

Blob https://github.com/Azure/bicep-registry-modules/blob/main/avm/res/storage/storage-account/blob-service/main.bicep

@minValue(1)
@description('Optional. How long this blob can be restored. It should be less than DeleteRetentionPolicy days.')
param restorePolicyDays int = 6

File https://github.com/Azure/bicep-registry-modules/blob/main/avm/res/storage/storage-account/file-service/main.bicep

@description('Optional. The service properties for soft delete.')
param shareDeleteRetentionPolicy object = {
  enabled: true
  days: 7
}
image

I am wondering if there was a reason, or simply a typo. If so, I am happy to contribute back and match the values but I wanted to check in with the owner first to see if I am missing something here. I did search issues and couldn't see anything specific to this issue, unless it's in comments somewhere, apologies if so.

Thanks

Dan

(Optional) Correlation Id

No response

microsoft-github-policy-service[bot] commented 2 months ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

avm-team-linter[bot] commented 2 months ago

@riosengineer, thanks for submitting this issue for the avm/res/storage/storage-account module!

[!IMPORTANT] A member of the @Azure/avm-res-storage-storageaccount-module-owners-bicep or @Azure/avm-res-storage-storageaccount-module-contributors-bicep team will review it soon!

avm-team-linter[bot] commented 2 months ago

[!WARNING] This issue couldn't be assigend due to an internal error. @ktremain, please make sure this issue is assigned to you and please provide an initial response as soon as possible, in accordance with the AVM Support statement.

ktremain commented 2 months ago

@riosengineer Thanks for raising this issue, let me look into it and consider the best course of action for this.

microsoft-github-policy-service[bot] commented 2 months ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue 🚩" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
ktremain commented 1 month ago

I have discussed this with the Terraform Storage Module owner, and we are going to ensure that all storage retentions default to 7 days - I will get a PR raised to adjust the default in this module shortly.

Thanks for spotting and reporting this inconsistency and helping to improve the AVM modules! :)