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

New features in azurerm_storage_management_policy #11163

Merged
merged 33 commits into from
Apr 27, 2021

Conversation

yupwei68
Copy link
Contributor

@yupwei68 yupwei68 commented Mar 31, 2021

Fix: #9617
#10399
resource:
=== RUN TestAccStorageManagementPolicy_basic
=== PAUSE TestAccStorageManagementPolicy_basic
=== CONT TestAccStorageManagementPolicy_basic
--- PASS: TestAccStorageManagementPolicy_basic (171.66s)
=== RUN TestAccStorageManagementPolicy_singleAction
=== PAUSE TestAccStorageManagementPolicy_singleAction
=== CONT TestAccStorageManagementPolicy_singleAction
--- PASS: TestAccStorageManagementPolicy_singleAction (172.81s)
=== RUN TestAccStorageManagementPolicy_singleActionUpdate
=== PAUSE TestAccStorageManagementPolicy_singleActionUpdate
=== CONT TestAccStorageManagementPolicy_singleActionUpdate
--- PASS: TestAccStorageManagementPolicy_singleActionUpdate (235.65s)
=== RUN TestAccStorageManagementPolicy_multipleRule
=== PAUSE TestAccStorageManagementPolicy_multipleRule
=== CONT TestAccStorageManagementPolicy_multipleRule
--- PASS: TestAccStorageManagementPolicy_multipleRule (172.52s)
=== RUN TestAccStorageManagementPolicy_updateMultipleRule
=== PAUSE TestAccStorageManagementPolicy_updateMultipleRule
=== CONT TestAccStorageManagementPolicy_updateMultipleRule
--- PASS: TestAccStorageManagementPolicy_updateMultipleRule (237.07s)
=== RUN TestAccStorageManagementPolicy_blobTypes
=== PAUSE TestAccStorageManagementPolicy_blobTypes
=== CONT TestAccStorageManagementPolicy_blobTypes
--- PASS: TestAccStorageManagementPolicy_blobTypes (171.88s)
=== RUN TestAccStorageManagementPolicy_blobIndexMatch
=== PAUSE TestAccStorageManagementPolicy_blobIndexMatch
=== CONT TestAccStorageManagementPolicy_blobIndexMatch
--- PASS: TestAccStorageManagementPolicy_blobIndexMatch (347.53s)
=== RUN TestAccStorageManagementPolicy_complete
=== PAUSE TestAccStorageManagementPolicy_complete
=== CONT TestAccStorageManagementPolicy_complete
--- PASS: TestAccStorageManagementPolicy_complete (171.03s)
=== RUN TestAccStorageManagementPolicy_update
=== PAUSE TestAccStorageManagementPolicy_update
=== CONT TestAccStorageManagementPolicy_update
--- PASS: TestAccStorageManagementPolicy_update (364.83s)
PASS

data source:
=== RUN TestAccDataSourceStorageManagementPolicy_basic
=== PAUSE TestAccDataSourceStorageManagementPolicy_basic
=== CONT TestAccDataSourceStorageManagementPolicy_basic
--- PASS: TestAccDataSourceStorageManagementPolicy_basic (161.02s)
=== RUN TestAccDataSourceStorageManagementPolicy_blobTypes
=== PAUSE TestAccDataSourceStorageManagementPolicy_blobTypes
=== CONT TestAccDataSourceStorageManagementPolicy_blobTypes
--- PASS: TestAccDataSourceStorageManagementPolicy_blobTypes (158.81s)
PASS

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @yupwei68

Thanks for this PR.

Taking a look through there appears to be a large number of breaking changes in this PR which'll need to be reverted for us to merge this prior to 3.0, is there a reason we've changed these from integers to floats?

Thanks!

@yupwei68
Copy link
Contributor Author

yupwei68 commented Apr 7, 2021

Hi Tom, thanks for your comments. I've reverted the breaking changes. Changes have been pushed. Tests have passed. Please continue reviewing.

@ghost ghost added size/XL and removed size/XXL labels Apr 21, 2021
@yupwei68
Copy link
Contributor Author

Thanks kt for your comments. All changes have been pushed. All tests have passed.
Just one minor concern, I personally think the new names are better, but they're different in structure with the existing fields, like:
image

@ghost ghost removed the waiting-response label Apr 21, 2021
@yupwei68
Copy link
Contributor Author

I've reverted all the breaking changes.

@yupwei68 yupwei68 requested a review from katbyte April 21, 2021 07:30
@yupwei68 yupwei68 closed this Apr 21, 2021
@yupwei68 yupwei68 reopened this Apr 21, 2021
@Simonverge
Copy link

@yupwei68, @tombuildsstuff , @katbyte can we have an ETA / status on this ?
we have a customer waiting for the features

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yupwei68 - aside from one comment left inline about a failing test i think this looks good

@@ -53,6 +53,29 @@ func dataSourceStorageManagementPolicy() *schema.Resource {
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},

"match_blob_index_tag": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this another "not yet in global property?"

------- Stdout: -------
=== RUN   TestAccStorageManagementPolicy_blobIndexMatch
=== PAUSE TestAccStorageManagementPolicy_blobIndexMatch
=== CONT  TestAccStorageManagementPolicy_blobIndexMatch
    testing.go:620: Step 3/6 error: Error running apply: exit status 1
        
        Error: creating Azure Storage Management Policy "/subscriptions/*******/resourceGroups/acctestRG-storage-210426173350412751/providers/Microsoft.Storage/storageAccounts/unlikely23exst2acctu9ru0": storage.ManagementPoliciesClient#CreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="FeatureNotSupportedForAccount" Message="Tag based filtering is not supported for the account."
        
          on terraform_plugin_test.tf line 30, in resource "azurerm_storage_management_policy" "test":
          30: resource "azurerm_storage_management_policy" "test" {
        
--- FAIL: TestAccStorageManagementPolicy_blobIndexMatch (212.08s)
FAIL

------- Stderr: -------
2021/04/26 17:33:49 [DEBUG] not using binary driver name, it's no longer needed
2021/04/26 17:33:50 [DEBUG] not using binary driver name, it's no longer needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yupwei68
Copy link
Contributor Author

Hi kt, thanks for comments. Please enable the feature in the subscription and retry the tests. Thanks!

@ghost ghost removed the waiting-response label Apr 27, 2021
@katbyte katbyte added this to the v2.57.0 milestone Apr 27, 2021
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that worked @yupwei68! thanks, LGTM now 👍

@katbyte katbyte merged commit e98b0fb into hashicorp:master Apr 27, 2021
katbyte added a commit that referenced this pull request Apr 27, 2021
@yupwei68 yupwei68 deleted the wyp-storage-mp2 branch April 28, 2021 02:14
@ghost
Copy link

ghost commented Apr 30, 2021

This has been released in version 2.57.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.57.0"
}
# ... other configuration ...

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, 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 May 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for "versions" in action block of azurerm_storage_management_policy rule
4 participants