New Resource : `azurerm_storage_mover_target_endpoint` and `azurerm_storage_mover_source_endpoint` #21449

sinbai · 2023-04-18T06:33:22Z

Swagger: https://github.com/Azure/azure-rest-api-specs/blob/add28efcd3a5fd422285d992fb1ec5ee5a7a40a6/specification/storagemover/resource-manager/Microsoft.StorageMover/stable/2023-03-01/storagemover.json#L674

Test results:

mbfrahry

Hey @sinbai! Thanks for this PR. It looks fairly good except one of the attributes isn't required and the test config could be cleaned up a bit.

mbfrahry · 2023-04-18T22:22:25Z

internal/services/storagemover/storage_mover_source_endpoint_resource_test.go

+func (r StorageMoverSourceEndpointTestResource) basic(data acceptance.TestData) string {
+	template := r.template(data)
+	return fmt.Sprintf(`
+provider "azurerm" {


Could this be moved into the template?

Thanks for your feedback. Putting it outside of template because I got feedback from Tom a few days ago - "the provider block should be in each test, not the template - to allow for overriding it", see below for details. I assume that his feedback makes sense and I've been doing it ever since, WDYT?

Ohhh good to note! Thank you!

mbfrahry · 2023-04-18T22:22:34Z

internal/services/storagemover/storage_mover_source_endpoint_resource_test.go

+provider "azurerm" {
+  features {}
+}
+				%s


The indention is off here

mbfrahry · 2023-04-18T22:23:10Z

internal/services/storagemover/storage_mover_source_endpoint_resource.go

+
+		"export": {
+			Type:         pluginsdk.TypeString,
+			Required:     true,


I don't believe this is Required. The api accepts "".

Yes, you are right. The code has been updated.

sinbai · 2023-04-19T02:23:12Z

Hey @sinbai! Thanks for this PR. It looks fairly good except one of the attributes isn't required and the test config could be cleaned up a bit.

Hi @mbfrahry thanks for your detailed feedback. I have replied/updated the code. Could you please take another look?

tombuildsstuff · 2023-04-19T16:47:48Z

internal/services/storagemover/storage_mover_source_endpoint_resource.go

+			}
+
+			properties := endpoints.Endpoint{
+				Name: utils.String(model.Name),


per the ARM spec it's not valid to specify name in the payload

Suggested change

Name: utils.String(model.Name),

tombuildsstuff · 2023-04-19T16:48:08Z

internal/services/storagemover/storage_mover_source_endpoint_resource.go

+					Host:       model.Host,
+					NfsVersion: &model.NfsVersion,
+				},
+				Type: utils.String(string(endpoints.EndpointTypeNfsMount)),


the type field should never need to be set, this is implied from the ARM endpoint:

Suggested change

Type: utils.String(string(endpoints.EndpointTypeNfsMount)),

tombuildsstuff · 2023-04-19T16:48:45Z

internal/services/storagemover/storage_mover_source_endpoint_resource.go

+			}
+
+			if model := resp.Model; model != nil {
+				nfsProperties := model.Properties.(endpoints.NfsMountEndpointProperties)


we're not type checking this, if the cast fails this'll panic

tombuildsstuff · 2023-04-19T16:49:03Z

internal/services/storagemover/storage_mover_source_endpoint_resource.go

+			}
+
+			if metadata.ResourceData.HasChange("description") {
+				nfsProperties := properties.Properties.(endpoints.NfsMountEndpointProperties)


we're not type checking this, if the cast fails this'll panic

tombuildsstuff · 2023-04-19T16:49:33Z

internal/services/storagemover/storage_mover_target_endpoint_resource.go

+					BlobContainerName:        model.StorageContainerName,
+					StorageAccountResourceId: model.StorageAccountId,
+				},
+				Type: utils.String(string(endpoints.EndpointTypeAzureStorageBlobContainer)),


(as above)

Suggested change

Type: utils.String(string(endpoints.EndpointTypeAzureStorageBlobContainer)),

tombuildsstuff · 2023-04-19T16:49:37Z

internal/services/storagemover/storage_mover_target_endpoint_resource.go

+			}
+
+			properties := endpoints.Endpoint{
+				Name: utils.String(model.Name),


(as above)

Suggested change

Name: utils.String(model.Name),

tombuildsstuff · 2023-04-19T16:49:46Z

internal/services/storagemover/storage_mover_target_endpoint_resource.go

+			}
+
+			if metadata.ResourceData.HasChange("description") {
+				sbcProperties := properties.Properties.(endpoints.AzureStorageBlobContainerEndpointProperties)


tombuildsstuff · 2023-04-19T16:49:55Z

internal/services/storagemover/storage_mover_target_endpoint_resource.go

+			}
+
+			if model := resp.Model; model != nil {
+				sbcProperties := model.Properties.(endpoints.AzureStorageBlobContainerEndpointProperties)


mbfrahry

Sorry for the mis-review. I didn't see @tombuildsstuff review until after I pushed my review

sinbai · 2023-04-20T09:08:28Z

@tombuildsstuff Thanks for your feedback. I have fixed all feedback. Could you please take another look?

Test results:

mbfrahry

LGTM! Thanks for getting those changes in @sinbai

github-actions · 2023-04-21T01:35:29Z

This functionality has been released in v3.53.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2024-04-28T02:10:54Z

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.

sinbai added 3 commits April 11, 2023 18:03

fix issue 21363

07c9c35

Merge branch 'master' of github.com:sinbai/terraform-provider-azurerm

8cb43b8

Merge branch 'master' of github.com:sinbai/terraform-provider-azurerm

a05ea16

github-actions bot added dependencies documentation size/XL labels Apr 18, 2023

sinbai changed the title ~~azurerm_storage_mover_target_endpoint, azurerm_storage_mover_source_endpoint - Support storage mover endpont~~ New Resource : azurerm_storage_mover_target_endpoint and azurerm_storage_mover_source_endpoint Apr 18, 2023

mbfrahry requested changes Apr 18, 2023

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Apr 18, 2023

Merge branch 'master' of github.com:sinbai/terraform-provider-azurerm

75e5f67

github-actions bot added the waiting-response label Apr 19, 2023

tombuildsstuff suggested changes Apr 19, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

mbfrahry requested changes Apr 19, 2023

View reviewed changes

Merge branch 'master' of github.com:sinbai/terraform-provider-azurerm

7df5646

sinbai force-pushed the storagemover/new_resource_endpoint branch from 5d87579 to 35488d9 Compare April 20, 2023 08:06

github-actions bot removed the waiting-response label Apr 20, 2023

sinbai added 5 commits April 20, 2023 16:19

Merge branch 'master' of github.com:sinbai/terraform-provider-azurerm

fe268a7

support new resource storage mover endpoints

4c9b319

update code

149e98e

update code

105e065

update code

4409b27

sinbai force-pushed the storagemover/new_resource_endpoint branch from 35488d9 to 4409b27 Compare April 20, 2023 08:23

sinbai added 2 commits April 20, 2023 16:37

update modules.txt

7a12104

update code

d535958

mbfrahry approved these changes Apr 20, 2023

View reviewed changes

mbfrahry added this to the v3.53.0 milestone Apr 20, 2023

mbfrahry merged commit 748b561 into hashicorp:main Apr 20, 2023

mbfrahry added a commit that referenced this pull request Apr 20, 2023

Update CHANGELOG.md for #21449

8781044

sinbai deleted the storagemover/new_resource_endpoint branch March 28, 2024 02:50

github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource : `azurerm_storage_mover_target_endpoint` and `azurerm_storage_mover_source_endpoint` #21449

New Resource : `azurerm_storage_mover_target_endpoint` and `azurerm_storage_mover_source_endpoint` #21449

sinbai commented Apr 18, 2023 •

edited

Loading

mbfrahry left a comment

mbfrahry Apr 18, 2023

sinbai Apr 19, 2023

mbfrahry Apr 19, 2023

mbfrahry Apr 18, 2023

sinbai Apr 19, 2023

mbfrahry Apr 18, 2023

sinbai Apr 19, 2023

sinbai commented Apr 19, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

tombuildsstuff Apr 19, 2023

sinbai Apr 20, 2023

This comment was marked as outdated.

mbfrahry left a comment

sinbai commented Apr 20, 2023

mbfrahry left a comment

github-actions bot commented Apr 21, 2023

github-actions bot commented Apr 28, 2024

New Resource : azurerm_storage_mover_target_endpoint and azurerm_storage_mover_source_endpoint #21449

New Resource : azurerm_storage_mover_target_endpoint and azurerm_storage_mover_source_endpoint #21449

Conversation

sinbai commented Apr 18, 2023 • edited Loading

mbfrahry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sinbai commented Apr 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

mbfrahry left a comment

Choose a reason for hiding this comment

sinbai commented Apr 20, 2023

mbfrahry left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 21, 2023

github-actions bot commented Apr 28, 2024

New Resource : `azurerm_storage_mover_target_endpoint` and `azurerm_storage_mover_source_endpoint` #21449

New Resource : `azurerm_storage_mover_target_endpoint` and `azurerm_storage_mover_source_endpoint` #21449

sinbai commented Apr 18, 2023 •

edited

Loading