-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 resource azurerm_storage_sync_cloud_endpoint
#8540
Conversation
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.
hi @yupwei68
Thanks for this PR - I've taken a look through and left some comments inline - if we can fix those up then we should be able to take another look through here.
Thanks!
azurerm/internal/services/storage/resource_arm_storage_sync_cloud_endpoint.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/storage/resource_arm_storage_sync_cloud_endpoint.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/storage/resource_arm_storage_sync_cloud_endpoint.go
Outdated
Show resolved
Hide resolved
azurerm/internal/services/storage/tests/resource_arm_storage_sync_cloud_endpoint_test.go
Outdated
Show resolved
Hide resolved
storage_account_name = azurerm_storage_account.example.name | ||
lifecycle { | ||
ignore_changes = [ | ||
acl, metadata |
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.
we shouldn't be shipping this in examples - what extra configuration is necessary here?
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.
There'll be a storage sync signature assigned to the metadata, which we could not get from the storage sync service. Could we just ignore the metadata?
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.
And as for acl
, it's assigned to the following when a cloud endpoint is created, However, in share definition, start
and expiry
is necessary and no empty string requested:
acl {
id = "GhostedRecall"
access_policy {
permissions = "r"
}
}
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.
I agree with tom, if we are ignoring changes we need to fix the resources so its not required.
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.
Sure. Create a PR for the fix #8811
Hi @tombuildsstuff Sorry for the mistakes made. Please continue reviewing. |
azurerm/internal/services/storage/tests/resource_arm_storage_sync_cloud_endpoint_test.go
Outdated
Show resolved
Hide resolved
Dependent on #8811. |
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.
Thanks @yupwei68 - this looks good now, however it seems like some tests are failing:
=== CONT TestAccAzureRMStorageSyncCloudEndpoint_requiresImport
TestAccAzureRMStorageSyncCloudEndpoint_requiresImport: testing.go:684: Step 0 error: errors during apply:
Error: waiting for Storage Sync Cloud Endpoint "acctest-CEP-201026174017620407" to be created: Code="MgmtStorageAccountAuthorizationFailed" Message="Unable to read specified storage account. Please check the permissions and try again after some time."
on /opt/teamcity-agent/temp/buildTmp/tf-test115906221/main.tf line 38:
(source code not available)
TestAccAzureRMStorageSyncCloudEndpoint_basic: testing.go:684: Step 0 error: errors during apply:
Error: waiting for Storage Sync Cloud Endpoint "acctest-CEP-201026174017617273" to be created: Code="MgmtStorageAccountAuthorizationFailed" Message="Unable to read specified storage account. Please check the permissions and try again after some time."
on /opt/teamcity-agent/temp/buildTmp/tf-test146444038/main.tf line 38:
(source code not available)
--- FAIL: TestAccAzureRMStorageSyncCloudEndpoint_requiresImport (751.91s)
azurerm/internal/services/storage/resource_arm_storage_sync_cloud_endpoint.go
Outdated
Show resolved
Hide resolved
@katbyte Thanks for the comment. Sorry that I can not reproduce this error. Would you mind retrying? === RUN TestAccAzureRMStorageSyncCloudEndpoint_basic |
Hi @katbyte I've tried several times, I could not reproduce this error. Would you mind testing it again? |
I'm still getting this error 🤷♀️
you can see this in TC |
Thanks @yupwei68 - closer now! only one failure test, and its a different error:
|
I've merged the upstream master |
azurerm/internal/services/storage/resource_arm_storage_sync_cloud_endpoint.go
Show resolved
Hide resolved
Thanks for comments @katbyte . Please continue reviewing. === RUN TestAccAzureRMStorageSyncCloudEndpoint_basic |
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.
Thanks @yupwei68 - LGTM 👍
@yupwei68 - once we fix the merge conflict this should be good to merge |
Thanks for your comments @katbyte . Tests have passed. Please continue reviewing. |
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.
THanks
This has been released in version 2.40.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.40.0"
}
# ... other configuration ... |
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
Fix: #2315.
Extend: #7843
Totally there will be three resources:
azurerm_storage_sync
azurerm_storage_sync_group
azurerm_cloud_endpoint
=== RUN TestAccAzureRMCloudEndpoint_basic
=== PAUSE TestAccAzureRMCloudEndpoint_basic
=== CONT TestAccAzureRMCloudEndpoint_basic
--- PASS: TestAccAzureRMCloudEndpoint_basic (390.42s)
=== RUN TestAccAzureRMCloudEndpoint_requiresImport
=== PAUSE TestAccAzureRMCloudEndpoint_requiresImport
=== CONT TestAccAzureRMCloudEndpoint_requiresImport
--- PASS: TestAccAzureRMCloudEndpoint_requiresImport (400.43s)