-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
azurerm_storage_account
- For Storage
kind account, avoid setting some unsupported blob properties
#23288
azurerm_storage_account
- For Storage
kind account, avoid setting some unsupported blob properties
#23288
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1324,7 +1324,7 @@ func resourceStorageAccountCreate(d *pluginsdk.ResourceData, meta interface{}) e | |||||||||
} | ||||||||||
blobClient := meta.(*clients.Client).Storage.BlobServicesClient | ||||||||||
|
||||||||||
blobProperties, err := expandBlobProperties(val.([]interface{})) | ||||||||||
blobProperties, err := expandBlobProperties(storage.Kind(accountKind), val.([]interface{})) | ||||||||||
if err != nil { | ||||||||||
return err | ||||||||||
} | ||||||||||
|
@@ -1794,7 +1794,7 @@ func resourceStorageAccountUpdate(d *pluginsdk.ResourceData, meta interface{}) e | |||||||||
} | ||||||||||
|
||||||||||
blobClient := meta.(*clients.Client).Storage.BlobServicesClient | ||||||||||
blobProperties, err := expandBlobProperties(d.Get("blob_properties").([]interface{})) | ||||||||||
blobProperties, err := expandBlobProperties(storage.Kind(accountKind), d.Get("blob_properties").([]interface{})) | ||||||||||
if err != nil { | ||||||||||
return err | ||||||||||
} | ||||||||||
|
@@ -2584,25 +2584,33 @@ func expandStorageAccountPrivateLinkAccess(inputs []interface{}, tenantId string | |||||||||
return &privateLinkAccess | ||||||||||
} | ||||||||||
|
||||||||||
func expandBlobProperties(input []interface{}) (*storage.BlobServiceProperties, error) { | ||||||||||
func expandBlobProperties(kind storage.Kind, input []interface{}) (*storage.BlobServiceProperties, error) { | ||||||||||
props := storage.BlobServiceProperties{ | ||||||||||
BlobServicePropertiesProperties: &storage.BlobServicePropertiesProperties{ | ||||||||||
Cors: &storage.CorsRules{ | ||||||||||
CorsRules: &[]storage.CorsRule{}, | ||||||||||
}, | ||||||||||
IsVersioningEnabled: utils.Bool(false), | ||||||||||
ChangeFeed: &storage.ChangeFeed{ | ||||||||||
Enabled: utils.Bool(false), | ||||||||||
}, | ||||||||||
DeleteRetentionPolicy: &storage.DeleteRetentionPolicy{ | ||||||||||
Enabled: utils.Bool(false), | ||||||||||
}, | ||||||||||
LastAccessTimeTrackingPolicy: &storage.LastAccessTimeTrackingPolicy{ | ||||||||||
Enable: utils.Bool(false), | ||||||||||
}, | ||||||||||
}, | ||||||||||
} | ||||||||||
|
||||||||||
// `Storage` kind doesn't support: | ||||||||||
// - LastAccessTimeTrackingPolicy: Confirmed by SRP. | ||||||||||
// - ChangeFeed: See https://learn.microsoft.com/en-us/azure/storage/blobs/storage-blob-change-feed?tabs=azure-portal#enable-and-disable-the-change-feed. | ||||||||||
// - Versioning: See https://learn.microsoft.com/en-us/azure/storage/blobs/versioning-overview#how-blob-versioning-works | ||||||||||
// - Restore Policy: See https://learn.microsoft.com/en-us/azure/storage/blobs/point-in-time-restore-overview#prerequisites-for-point-in-time-restore | ||||||||||
if kind != storage.KindStorage { | ||||||||||
props.LastAccessTimeTrackingPolicy = &storage.LastAccessTimeTrackingPolicy{ | ||||||||||
Enable: utils.Bool(false), | ||||||||||
} | ||||||||||
props.ChangeFeed = &storage.ChangeFeed{ | ||||||||||
Enabled: utils.Bool(false), | ||||||||||
} | ||||||||||
props.IsVersioningEnabled = utils.Bool(false) | ||||||||||
} | ||||||||||
|
||||||||||
if len(input) == 0 || input[0] == nil { | ||||||||||
return &props, nil | ||||||||||
} | ||||||||||
|
@@ -2615,28 +2623,53 @@ func expandBlobProperties(input []interface{}) (*storage.BlobServiceProperties, | |||||||||
containerDeletePolicyRaw := v["container_delete_retention_policy"].([]interface{}) | ||||||||||
props.BlobServicePropertiesProperties.ContainerDeleteRetentionPolicy = expandBlobPropertiesDeleteRetentionPolicy(containerDeletePolicyRaw) | ||||||||||
|
||||||||||
restorePolicyRaw := v["restore_policy"].([]interface{}) | ||||||||||
props.BlobServicePropertiesProperties.RestorePolicy = expandBlobPropertiesRestorePolicy(restorePolicyRaw) | ||||||||||
|
||||||||||
corsRaw := v["cors_rule"].([]interface{}) | ||||||||||
props.BlobServicePropertiesProperties.Cors = expandBlobPropertiesCors(corsRaw) | ||||||||||
|
||||||||||
props.IsVersioningEnabled = utils.Bool(v["versioning_enabled"].(bool)) | ||||||||||
|
||||||||||
props.ChangeFeed = &storage.ChangeFeed{ | ||||||||||
Enabled: utils.Bool(v["change_feed_enabled"].(bool)), | ||||||||||
} | ||||||||||
|
||||||||||
if v := v["change_feed_retention_in_days"].(int); v != 0 { | ||||||||||
props.ChangeFeed.RetentionInDays = utils.Int32((int32)(v)) | ||||||||||
} | ||||||||||
|
||||||||||
if version, ok := v["default_service_version"].(string); ok && version != "" { | ||||||||||
props.DefaultServiceVersion = utils.String(version) | ||||||||||
} | ||||||||||
|
||||||||||
props.LastAccessTimeTrackingPolicy = &storage.LastAccessTimeTrackingPolicy{ | ||||||||||
Enable: utils.Bool(v["last_access_time_enabled"].(bool)), | ||||||||||
// `Storage` kind doesn't support: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor but given there's |
||||||||||
// - LastAccessTimeTrackingPolicy | ||||||||||
// - ChangeFeed | ||||||||||
// - Versioning | ||||||||||
// - RestorePolicy | ||||||||||
lastAccessTimeEnabled := v["last_access_time_enabled"].(bool) | ||||||||||
changeFeedEnabled := v["change_feed_enabled"].(bool) | ||||||||||
changeFeedRetentionInDays := v["change_feed_retention_in_days"].(int) | ||||||||||
restorePolicyRaw := v["restore_policy"].([]interface{}) | ||||||||||
versioningEnabled := v["versioning_enabled"].(bool) | ||||||||||
if kind != storage.KindStorage { | ||||||||||
props.BlobServicePropertiesProperties.LastAccessTimeTrackingPolicy = &storage.LastAccessTimeTrackingPolicy{ | ||||||||||
Enable: utils.Bool(lastAccessTimeEnabled), | ||||||||||
} | ||||||||||
props.BlobServicePropertiesProperties.ChangeFeed = &storage.ChangeFeed{ | ||||||||||
Enabled: utils.Bool(changeFeedEnabled), | ||||||||||
} | ||||||||||
if changeFeedRetentionInDays != 0 { | ||||||||||
props.BlobServicePropertiesProperties.ChangeFeed.RetentionInDays = utils.Int32(int32(changeFeedRetentionInDays)) | ||||||||||
} | ||||||||||
props.BlobServicePropertiesProperties.RestorePolicy = expandBlobPropertiesRestorePolicy(restorePolicyRaw) | ||||||||||
props.BlobServicePropertiesProperties.IsVersioningEnabled = &versioningEnabled | ||||||||||
} else { | ||||||||||
if lastAccessTimeEnabled { | ||||||||||
return nil, fmt.Errorf("`Storage` kind does not support `last_access_time_enabled`") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (as elsewhere) could we update these slightly:
Suggested change
|
||||||||||
} | ||||||||||
if changeFeedEnabled { | ||||||||||
return nil, fmt.Errorf("`Storage` kind does not support `change_feed_enabled`") | ||||||||||
} | ||||||||||
if changeFeedRetentionInDays != 0 { | ||||||||||
return nil, fmt.Errorf("`Storage` kind does not support `change_feed_retention_in_days`") | ||||||||||
} | ||||||||||
if len(restorePolicyRaw) != 0 { | ||||||||||
return nil, fmt.Errorf("`Storage` kind does not support `restore_policy`") | ||||||||||
} | ||||||||||
if versioningEnabled { | ||||||||||
return nil, fmt.Errorf("`Storage` kind does not support `versioning_enabled`") | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
// Sanity check for the prerequisites of restore_policy | ||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -695,6 +695,21 @@ func TestAccStorageAccount_blobPropertiesEmptyAllowedExposedHeaders(t *testing.T | |||||
}) | ||||||
} | ||||||
|
||||||
func TestAccStorageAccount_blobProperties_kindStorageNotSupportLastAccessTimeEnabled(t *testing.T) { | ||||||
data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") | ||||||
r := StorageAccountResource{} | ||||||
|
||||||
data.ResourceTest(t, r, []acceptance.TestStep{ | ||||||
{ | ||||||
Config: r.blobPropertiesStorageKindNotSupportLastAccessTimeEnabled(data), | ||||||
Check: acceptance.ComposeTestCheckFunc( | ||||||
check.That(data.ResourceName).ExistsInAzure(r), | ||||||
), | ||||||
}, | ||||||
data.ImportStep(), | ||||||
}) | ||||||
} | ||||||
|
||||||
func TestAccStorageAccount_queueProperties(t *testing.T) { | ||||||
data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") | ||||||
r := StorageAccountResource{} | ||||||
|
@@ -4409,3 +4424,52 @@ resource "azurerm_storage_account" "test" { | |||||
} | ||||||
`, data.RandomInteger, data.Locations.Primary, data.RandomString) | ||||||
} | ||||||
|
||||||
func (r StorageAccountResource) blobPropertiesStorageKindNotSupportLastAccessTimeEnabled(data acceptance.TestData) string { | ||||||
return fmt.Sprintf(` | ||||||
provider "azurerm" { | ||||||
features {} | ||||||
} | ||||||
|
||||||
resource "azurerm_resource_group" "test" { | ||||||
name = "acctestAzureRMSA-%d" | ||||||
location = "%s" | ||||||
} | ||||||
|
||||||
resource "azurerm_storage_account" "test" { | ||||||
name = "unlikely23exst2acct%s" | ||||||
resource_group_name = azurerm_resource_group.test.name | ||||||
|
||||||
location = azurerm_resource_group.test.location | ||||||
account_kind = "Storage" | ||||||
account_tier = "Standard" | ||||||
account_replication_type = "LRS" | ||||||
|
||||||
blob_properties { | ||||||
cors_rule { | ||||||
allowed_origins = ["http://www.example.com"] | ||||||
exposed_headers = ["x-tempo-*"] | ||||||
allowed_headers = ["x-tempo-*"] | ||||||
allowed_methods = ["GET", "PUT", "PATCH"] | ||||||
max_age_in_seconds = "500" | ||||||
} | ||||||
|
||||||
delete_retention_policy { | ||||||
days = 300 | ||||||
} | ||||||
|
||||||
default_service_version = "2019-07-07" | ||||||
container_delete_retention_policy { | ||||||
days = 7 | ||||||
} | ||||||
|
||||||
# Following properties are not supported for "Storage" kind | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor but given there's
Suggested change
|
||||||
last_access_time_enabled = false | ||||||
change_feed_enabled = false | ||||||
versioning_enabled = false | ||||||
# change_feed_retention_in_days | ||||||
# restore_policy | ||||||
} | ||||||
} | ||||||
`, data.RandomInteger, data.Locations.Primary, data.RandomString) | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -183,16 +183,26 @@ A `blob_properties` block supports the following: | |||||
|
||||||
* `restore_policy` - (Optional) A `restore_policy` block as defined below. This must be used together with `delete_retention_policy` set, `versioning_enabled` and `change_feed_enabled` set to `true`. | ||||||
|
||||||
-> **NOTE:** `Storage` kind Storage Account doesn't support `restore_policy`. | ||||||
|
||||||
* `versioning_enabled` - (Optional) Is versioning enabled? Default to `false`. | ||||||
|
||||||
-> **NOTE:** `Storage` kind Storage Account doesn't support `versioning_enabled`. | ||||||
|
||||||
* `change_feed_enabled` - (Optional) Is the blob service properties for change feed events enabled? Default to `false`. | ||||||
|
||||||
-> **NOTE:** `Storage` kind Storage Account doesn't support `change_feed_enabled`. | ||||||
|
||||||
* `change_feed_retention_in_days` - (Optional) The duration of change feed events retention in days. The possible values are between 1 and 146000 days (400 years). Setting this to null (or omit this in the configuration file) indicates an infinite retention of the change feed. | ||||||
|
||||||
-> **NOTE:** `Storage` kind Storage Account doesn't support `change_feed_retention_in_days`. | ||||||
|
||||||
* `default_service_version` - (Optional) The API Version which should be used by default for requests to the Data Plane API if an incoming request doesn't specify an API Version. | ||||||
|
||||||
* `last_access_time_enabled` - (Optional) Is the last access time based tracking enabled? Default to `false`. | ||||||
|
||||||
-> **NOTE:** `Storage` kind Storage Account doesn't support `last_access_time_enabled`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we update these to be:
Suggested change
Whilst I appreciate the Service Team aren't using the V1 label, I think omitting it here would cause confusion that this isn't supported in V2 too, so I think it's worth calling that out? |
||||||
|
||||||
* `container_delete_retention_policy` - (Optional) A `container_delete_retention_policy` block as defined below. | ||||||
|
||||||
--- | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor but given there's
Storage
andStorageV2
it's probably worth explicitly calling out this is only for the unversioned/v1 version here?