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

Support for setting access policies on storage containers #3722

Closed
chpspiir opened this issue Jun 24, 2019 · 34 comments · Fixed by #25804
Closed

Support for setting access policies on storage containers #3722

chpspiir opened this issue Jun 24, 2019 · 34 comments · Fixed by #25804

Comments

@chpspiir
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

Description

Managing retention policies (immutability and legal holds) on storage containers is not yet supported by azurerm_storage_container. It seems that only ACL policies are supported.

It would be great to manage these policies at the infrastructure-level rather than at code level.

I looked at the API docs for the blob service, and I am having trouble figuring out what operation to use:
https://docs.microsoft.com/en-us/rest/api/storageservices/blob-service-rest-api.

Is this because it isn't exposed in the REST API? Or is it a naming confusion that I am not aware of?

If someone could point me in the right direction, I would gladly take a stab at implementing it.

New or Affected Resource(s)

  • azurerm_storage_container

Potential Terraform Configuration

resource "azurerm_storage_container" "test" {
  name                  = "vhds"
  resource_group_name   = "${azurerm_resource_group.test.name}"
  storage_account_name  = "${azurerm_storage_account.test.name}"
  container_access_type = "private"

  retention_policy {
    type = "legal-hold/immutable-retention" # It might be up for debate whether legal hold and immutable retention should get their own blocks. Mostly for UX.
    retention_time = "7d"
  }
}

References

https://docs.microsoft.com/en-us/rest/api/storageservices/blob-service-rest-api.

  • #0000
@tombuildsstuff tombuildsstuff added this to the v1.32.0 milestone Jul 2, 2019
@tombuildsstuff tombuildsstuff removed this from the v1.32.0 milestone Jul 2, 2019
@tombuildsstuff
Copy link
Contributor

hey @chpspiir

Thanks for opening this issue.

This functionality lives within the Data Plane API's for Azure Storage, rather than within the Resource Manager API's - which as such needs a new version of the Azure Storage SDK than the version we're currently using. Unfortunately we're unable to use the replacement SDK - since we've been blocked on this for a while (and this is now blocking a large number of issues) we've ended up writing our own SDK, which I'm just finishing up work on at the moment.

Once that's finished (and integrated) - it should be possible to implement this, since I can confirm the new SDK includes support for both of these fields.

Thanks!

@chpspiir
Copy link
Author

Sounds great! I haven't worked with terraform providers before, but I am up for giving it a stab when the SDK changes are ready. Let me know if I can help :)

@landro
Copy link

landro commented Aug 21, 2019

hey @tombuildsstuff , do you have a best-guess ETA for the integration of the new storage sdk you're working on?

@mamoit
Copy link

mamoit commented Jan 16, 2020

Once that's finished (and integrated) - it should be possible to implement this, since I can confirm the new SDK includes support for both of these fields.

@tombuildsstuff How is the progress on the new SDK? Does it still block this issue?

@tombuildsstuff
Copy link
Contributor

@landro / @mamoit the new Storage SDK's been merged and integrated - however it looks like this needs to be added to the Container's API since this has been added since the SDK was written for that to be added

@jblafage
Copy link

jblafage commented Apr 6, 2020

@tombuildsstuff : do you think if the following kind of REST api around Azure Storage could take part of this Storage SDK??

The idea is to be able to create a stored access policy for a given container and then generate a sas key based on this access policy. If it could be managed over Terraform it could facilitate implementations. The main advantage using stored access policies is that we can revoke all generated SAS keys based on a given stored access policy.

@ttjackott
Copy link

Our terraform configuration would massively benefit from this from a security perspective. At the moment we're struggling how to find a workaround to this so our SAS keys can be created under access policies.

@ariciputi
Copy link

Hi @tombuildsstuff,
any news on this? It would be nice to have it added either to the Blob Container or to the Storage Account. Not having this it's a showstopper for the Terraform's adoption in the regulated environment where I work.

@alexwlchan
Copy link

It’s no substitute for being able to define legal holds and the like in Terraform, but now that Terraform 0.13 highlights changes to outputs, we've added an output for "is a legal hold defined on our Blob Storage container":

$ terraform plan -out=terraform.plan


Changes to Outputs:
  ~ azure_has_legal_hold = true -> false

------------------------------------------------------------------------

This plan was saved to: terraform.plan

To perform exactly these actions, run the following command to apply:
    terraform apply "terraform.plan"

Releasing state lock. This may take a few moments...

It means we're more likely to notice if the immutability policy or legal hold goes walkabout.

@Furze

This comment has been minimized.

@tombuildsstuff
Copy link
Contributor

Blocked on #10765

@tombuildsstuff tombuildsstuff added this to the Blocked milestone Mar 1, 2021
@vinoth12988
Copy link

@tombuildsstuff @alexwlchan - Any Progress on this feature. Are there any workaround to enable immutable Access policy at container level..

@kensykora
Copy link

I was able to work around this limitation using the Azure CLI and the null_resource provider. Definitely not elegant, but it works. You could also use an arm template if you wanted to.

resource "null_resource" "immutability_policy" {
  triggers = {
    resource_group_name  = azurerm_storage_account.ops_security.resource_group_name
    storage_account_name = azurerm_storage_account.ops_security.name
    container_name       = azurerm_storage_container.audit_logs.name
    subscription_id      = data.azurerm_subscription.ops.subscription_id
  }

  provisioner "local-exec" {
    when    = create
    command = <<BASH
      az storage container immutability-policy create \
        --resource-group ${self.triggers.resource_group_name} \
        --account-name ${self.triggers.storage_account_name} \
        --container-name ${self.triggers.container_name} \
        --subscription ${self.triggers.subscription_id} \
        --period 365 # days
    BASH
  }
}

@petrkoutnycz
Copy link

So is this thread about container access or retention policy? I am quite confused.

@nicoarb
Copy link

nicoarb commented Mar 30, 2022

Are there any news? We are missing the ability to create container access policies and generate sas tokens heavily.

@shadow100000
Copy link

Ouch

@alexismosquera
Copy link

Hi!.
Any news about this issue?.

@psddp
Copy link

psddp commented Apr 29, 2022

The @kensykora solution is great but the command will run only once.
For some cases (like this policy), you want to make sure the command runs on updates also, to make sure it is recreated if missing (manually deleted).

A workaround can be to add a new trigger that contains a timestamp:

resource "null_resource" "immutability_policy" {
  triggers = {
    always_run           = "${timestamp()}"
    resource_group_name  = azurerm_storage_account.ops_security.resource_group_name
    storage_account_name = azurerm_storage_account.ops_security.name
    container_name       = azurerm_storage_container.audit_logs.name
    subscription_id      = data.azurerm_subscription.ops.subscription_id
  }

  provisioner "local-exec" {
    command = <<BASH
      az storage container immutability-policy create \
        --resource-group ${self.triggers.resource_group_name} \
        --account-name ${self.triggers.storage_account_name} \
        --container-name ${self.triggers.container_name} \
        --subscription ${self.triggers.subscription_id} \
        --period 365 # days
    BASH
  }
}

@kensykora
Copy link

@psddp a better workaround these days might be to use the azapi provider instead which (i don't think) was available when i wrote that workaround. These days for addressing azurerm provider gaps, azapi it's my go-to.

@clowa
Copy link
Contributor

clowa commented May 1, 2022

Here is an example of the solution mentioned by @kensykora. Thanks a lot! I wasn't aware of the azapi provider before.

terraform {
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = ">=3.4.0"
    }
    azapi = {
      source  = "azure/azapi"
      version = ">=0.1.1"
    }
    random = {
      source  = "random"
      version = ">=3.1.3"
    }
  }
}

provider "azurerm" {
  features {}
  skip_provider_registration = true
}

provider "azapi" {
  skip_provider_registration = true
}

variable "app_name" {
  type = string
}

variable "tags" {
  type = map(string)
}

locals {
  tags = merge({
    "managed-by" = "terraform"
  }, var.tags)
}

resource "random_id" "this" {
  byte_length = 4
}

resource "azurerm_resource_group" "this" {
  name     = upper("RG-${var.app_name}")
  location = "Germany West Central"
}

resource "azurerm_storage_account" "this" {
  name                     = lower("SA0${var.app_name}0${random_id.this.hex}")
  resource_group_name      = azurerm_resource_group.this.name
  location                 = azurerm_resource_group.this.location
  account_tier             = "Standard"
  account_replication_type = "LRS"

  tags = local.tags
}

resource "azurerm_storage_container" "this" {
  name                  = lower("BLB-${var.app_name}")
  storage_account_name  = azurerm_storage_account.this.name
  container_access_type = "private"
}

# For some reason the immutable policy must be treated as an existing resource
resource "azapi_update_resource" "this" {
  type      = "Microsoft.Storage/storageAccounts/blobServices/containers/immutabilityPolicies@2021-08-01"
  name      = "default"
  parent_id = azurerm_storage_container.this.resource_manager_id

  body = jsonencode({
    properties = {
      allowProtectedAppendWrites            = false
      # allowProtectedAppendWritesAll         = null
      immutabilityPeriodSinceCreationInDays = 4
    }
  })
  response_export_values = ["etag"]
}

# This output maybe isn't properly decoded and should be improved
output "etag" {
  value = jsondecode(azapi_update_resource.this.output).etag
}

The reference of the Azure Resource Manager template can be found here.

@ms-henglu
Copy link
Contributor

Hi @clowa ,

Thanks for the example! About the etag, in its GET response, it has the quotes. I have created an issue for that: Azure/azure-rest-api-specs#19134

@kimsey0
Copy link
Contributor

kimsey0 commented Oct 11, 2022

We would very much like to be able to create storage accounts with version-level immutability support enabled, since this can only be set on account creation, not enabled afterwards.

When creating an account with the Azure CLI, version-level immutability is enabled with the --enable-alw parameter for the az storage account create command.

@favoretti
Copy link
Collaborator

So I've generated latest stable SDK that would potentially allow us to implement immutability policies (I need those myself ;)). I'll discuss with @tombuildsstuff to agree on an approach and will try take a stab at it.

@favoretti favoretti self-assigned this Oct 13, 2022
@kimsey0
Copy link
Contributor

kimsey0 commented Oct 13, 2022

@favoretti: Sounds great. Will you also add support for account-level immutability policies (not just container-level) or should I create a separate issue for that? It seems to be quite simple to set up from the Azure CLI implementation and it's supported in the SDK version we use here. I'd guess it'd be a change to pass in another parameter here:

parameters := storage.AccountCreateParameters{
ExtendedLocation: expandEdgeZone(d.Get("edge_zone").(string)),
Location: &location,
Sku: &storage.Sku{
Name: storage.SkuName(storageType),
},
Tags: tags.Expand(t),
Kind: storage.Kind(accountKind),
AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{
PublicNetworkAccess: publicNetworkAccess,
EnableHTTPSTrafficOnly: &enableHTTPSTrafficOnly,
NetworkRuleSet: expandStorageAccountNetworkRules(d, tenantId),
IsHnsEnabled: &isHnsEnabled,
EnableNfsV3: &nfsV3Enabled,
AllowSharedKeyAccess: &allowSharedKeyAccess,
DefaultToOAuthAuthentication: &defaultToOAuthAuthentication,
AllowCrossTenantReplication: &crossTenantReplication,
},
}

@favoretti
Copy link
Collaborator

@kimsey0 As discussed with @tombuildsstuff offline - I'll first add Storage Account-level immutability policy flag and then we'll look into introducing blob-level ones, since that requires a bit more shenanigans.

favoretti added a commit to favoretti/terraform-provider-azurerm that referenced this issue Oct 14, 2022
@strictness
Copy link

@tombuildsstuff : do you think if the following kind of REST api around Azure Storage could take part of this Storage SDK??

The idea is to be able to create a stored access policy for a given container and then generate a sas key based on this access policy. If it could be managed over Terraform it could facilitate implementations. The main advantage using stored access policies is that we can revoke all generated SAS keys based on a given stored access policy.

That's exactly what we are trying to accomplish. Is there any progress on this implementation?

@markharveynz
Copy link

We want to implement "VERSION-LEVEL IMMUTABILITY SUPPORT" on a container without a policy. This is required to allow Veeam to use the container and make the storage immutable itself and it cannot have an access policy, rather it needs the tick box done when the container is created.

Document outlining this is: https://learn.microsoft.com/en-us/azure/storage/blobs/immutable-policy-configure-version-scope?tabs=azure-portal#enable-version-level-immutability-for-a-new-container

@favoretti
Copy link
Collaborator

@markharveynz by "we want to implement" - do you mean you will be submitting a PR or you are requesting a new enhancement?

If it's the latter - I'd suggest making a new issue. Will be easier to track progress on it.

@markharveynz
Copy link

We want to implement the option of "VERSION-LEVEL IMMUTABILITY SUPPORT" on a container as you would when you create a container in the portal, by using Terraform. We do not want to have a policy assigned to the container which is what will happen if you turn on "VERSION-LEVEL IMMUTABILITY SUPPORT" after you create a container as you have to apply a policy.

@SteveBurkettNZ
Copy link

Interest for this feature is probably being generated by the release of Veeam Backup & Replication v12 back in February which now supports immutable backups on Azure.

@DamaniN
Copy link

DamaniN commented Apr 24, 2023

@markharveynz & @SteveBurkettNZ, I've opened #21512 to have "VERSION-LEVEL IMMUTABILITY SUPPORT" added to azurerm_storage_container. Please vote on it.

@KaishM
Copy link

KaishM commented Apr 27, 2023

Has retention policies (immutability and legal holds) on storage containers been supported now by azurerm_storage_container ? I do not see any PR.

@philmph
Copy link

philmph commented May 15, 2023

For people stumbling across this who want to deploy a container with Version-level immutability support enabled without assigning a policy (-> setting set at container creation instead of updating it after creation):

On your azurerm_storage_account resource add the following block

resource "azurerm_storage_account" "this" {

  # ...

  # ID of the resulting "Microsoft.Storage/storageAccounts/blobServices" object is
  # "${azurerm_storage_account.this[0].id}/blobServices/default". "default" is the default name.
  blob_properties {
    versioning_enabled = true
  }
}

For the container(s)

variable "sa_containers" {
  default     = ["default"]
  description = "List of containers to be created in the Azure Storage Account."
  type        = list(string)
}

resource "azapi_resource" "containers" {
  for_each = toset(var.sa_containers)

  type = "Microsoft.Storage/storageAccounts/blobServices/containers@2022-09-01"
  name      = each.key

  # We append '/blobServices/default' to the storage_account.id see desc. above
  parent_id = "${azurerm_storage_account.this.id}/blobServices/default"

  body = jsonencode({
    properties = {
      immutableStorageWithVersioning = {
        enabled = true
      }
    }
  })
}

References used:

blobServices
containers

Copy link

github-actions bot commented Jun 2, 2024

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.