Azure / PSRule.Rules.Azure

Rules to validate Azure resources and infrastructure as code (IaC) using PSRule.
https://azure.github.io/PSRule.Rules.Azure/
MIT License
394 stars 86 forks source link

Storage Account failing soft-delete check #2079

Closed maxricketts closed 1 year ago

maxricketts commented 1 year ago

These two issues are flagging for soft delete and container soft delete, but I am only using the storage accounts for file services.

PSRule/Bicep/AZ Cli are on the latest versions and bicep is being deployed through a pipeline.

Soft Delete Consider enabling soft delete on storage accounts to protect blobs from accidental deletion or modification. See details online .

The following reasons were reported:

A sub-resource of type 'Microsoft.Storage/storageAccounts/blobServices' has not been specified.

Container Soft Delete Consider enabling container soft delete on storage accounts to protect blob containers from accidental deletion or modification. See details online .

The following reasons were reported:

A sub-resource of type 'Microsoft.Storage/storageAccounts/blobServices' has not been specified.

BenjaminEngeset commented 1 year ago

The rules needs some updates.

Currently both rules are expecting a specific pattern that is not right. Files are not related to the Microsoft.Storage/storageAccounts/blobServices child resource.

@BernieWhite I can contribute on this if you want.

BernieWhite commented 1 year ago

@maxricketts @BenjaminEngeset The rule does and should expect you to configure blob soft-delete if you are using a general storage account such as StorageV2. Because blobs could be configured at any point in the future via an application using this storage. Although happy to have more feedback on this.

If you are using a storage account with the kind of FileStorage then there is a bug here.

@maxricketts Are you able to confirm if you are using a general or FileStorage storage account?

If so, you can suppress the rule for that case or more broadly using a suppression group based on a tagging convention for example.

maxricketts commented 1 year ago

@BernieWhite - I agree that if in the future if we add blob to the storage account we would want it to flag. But in many use cases we are using Gen V2 with just file services so if there is no blob service defined then I would expect the rule to not flag.

Happy to discuss further if you think using suppression group is easier. If you could give me an example that would be great.

Here is the module I am defining.

module redirectStorage 'br/exgRegistry:microsoft.storage.storageaccounts:0.4.1526' = {
  scope: resourceGroup(rg.name)
  name: '${ridStorageName}${deploy}'
  params: {
    name: ridStorageName
    tags: tags
    storageAccountSku: 'Standard_GRS'
    azureFilesIdentityBasedAuthentication: {
      directoryServiceOptions: 'AADKERB'
      defaultSharePermission: 'StorageFileDataSmbShareContributor'
      activeDirectoryProperties: {
        domainGuid: domainGuid
        domainName: domainName
      }
    }
    fileServices: {
      shares: [
        {
          name: 'general'
        }
        {
          name: 'folder-redirection'
        }
      ]
    }
    networkAcls: {
      bypass: 'AzureServices'
      defaultAction: 'Deny'
      virtualNetworkRules: [
        {
          action: 'Allow'
          id: subid1
        }
        {
          action: 'Allow'
          id: subid2
        }
      ]
    }
  }
}
BernieWhite commented 1 year ago

@maxricketts At this stage I feel this is more of a use case specific scenario that will vary between customers.

There is customer use cases that fall on both sides of StorageV2 that use a lot of blob only storage accounts and customers that use StorageV2 with a lot of file share only storage accounts.

Stickly speaking for the Well-Architected Framework: if a storage account can be used for either then blob/ container and share soft-delete should be enabled. If soft-delete is enabled for blobs but no blobs are created there is no disadvantage to the configuration.

However I understand that you may want to ignore this requirement for storage accounts that only use file shares. In which cases you could consider the following:

You would do this by either:

---
# Synopsis: Ignore storage accounts used for file shares.
apiVersion: github.com/microsoft/PSRule/v1
kind: SuppressionGroup
metadata:
  name: Org.IgnoreSoftDeleteForSharesByProp
spec:
  rule:
    - Azure.Storage.SoftDelete
    - Azure.Storage.ContainerSoftDelete
  if:
    allOf:
      - type: .
        equals: Microsoft.Storage/storageAccounts
      - field: properties.azureFilesIdentityBasedAuthentication.activeDirectoryProperties
        hasValue: true

---
# Synopsis: Ignore storage accounts used for file shares.
apiVersion: github.com/microsoft/PSRule/v1
kind: SuppressionGroup
metadata:
  name: Org.IgnoreSoftDeleteForSharesByTag
spec:
  rule:
    - Azure.Storage.SoftDelete
    - Azure.Storage.ContainerSoftDelete
  if:
    allOf:
      - type: .
        equals: Microsoft.Storage/storageAccounts
      - field: tags.'resource-usage'
        equals: 'file-share'

These suppression groups, would be created in .ps-rule/ directory in a file like Suppression.Rule.yaml.


Does this help @maxricketts?

If there is a lot of the community that want to add support for a resource-usage tag method then we could look at adding this to the rule. Please upvote 👍 the original post if you would like to see this added.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] commented 1 year ago

This issue was closed because it has not had any recent activity.