New Resource : `azurerm_arc_resource_bridge_appliance` #23108

sinbai · 2023-08-29T06:07:27Z

Swagger: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/resourceconnector/resource-manager/Microsoft.ResourceConnector/stable/2022-10-27/appliances.json

Test results:

PASS: TestAccArcResourceBridgeAppliance_basic (221.73s)
PASS: TestAccArcResourceBridgeAppliance_update (343.70s)
PASS: TestAccArcResourceBridgeAppliance_complete (243.17s)
PASS: TestAccArcResourceBridgeAppliance_requiresImport (218.86s)

Note: Need to run az provider register --namespace Microsoft.ResourceConnector to register provider before running tests.

tombuildsstuff

hi @sinbai

Thanks for this PR - I've taken a look through and left some comments inline, if those can be addressed then we should be able to take another look here.

It's worth noting that when a field has to be set via the Update method, that we can first Create the resource and then use the Update API to update this value - as such we don't need to ask users to conditionally apply their Terraform Configuration in two stages.

It's also worth noting there's conflation between the Appliance and the Resource Connector Appliance in the documentation, presumably the Appliance has to exist within AKS/VMware etc first? If so, then we're also missing test cases showing this working end-to-end - could we clarify the behaviour of this service to make it fully usable here?

Thanks!

internal/services/resourceconnector/resourceconnector_appliance_resource.go

tombuildsstuff · 2023-09-05T04:37:38Z

internal/services/resourceconnector/resourceconnector_appliance_resource.go

+			identity, err := identity.ExpandSystemAssignedFromModel(model.Identity)
+			if err != nil {
+				return fmt.Errorf("expanding SystemAssigned Identity: %+v", err)
+			}
+
+			parameters.Identity = identity


fwiw we can move this above the parameters definition and then define this inline, we don't gain anything from doing this separately:

Suggested change

identity, err := identity.ExpandSystemAssignedFromModel(model.Identity)

if err != nil {

return fmt.Errorf("expanding SystemAssigned Identity: %+v", err)

}

parameters.Identity = identity

identity, err := identity.ExpandSystemAssignedFromModel(model.Identity)

if err != nil {

return err

}

parameters.Identity = identity

tombuildsstuff · 2023-09-05T05:09:16Z

website/docs/r/resource_connector_appliance.html.markdown

+
+* `version` - (Optional) The version of the Appliance. Possible values is `latest`.
+
+-> **NOTE:** The `version` cannot be populated apart from upgrade call.


(as above) this can be removed / is something we should handle internally

After confirming with service team, the version is not a user settable property, I have removed it.

tombuildsstuff · 2023-09-05T05:09:52Z

website/docs/r/resource_connector_appliance.html.markdown

+Appliance can be imported using the `resource id`, e.g.
+
+```shell
+terraform import azurerm_resource_connector_appliance.example /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/providers/Microsoft.ResourceConnector/apliances/appliancesExample


this ID isn't correct?

/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ResourceConnector/appliances/{applianceName}

Note apliances vs appliances

appliances, fixed.

tombuildsstuff · 2023-09-05T05:10:11Z

website/docs/r/resource_connector_appliance.html.markdown

+* `create` - (Defaults to 60 minutes) Used when creating the Appliance.
+* `read` - (Defaults to 5 minutes) Used when retrieving the Appliance.
+* `update` - (Defaults to 30 minutes) Used when updating the Appliance.
+* `delete` - (Defaults to 30 minutes) Used when deleting the Appliance.


the Appliance, or Resource Connector Appliance?

Resource Connector Appliance, fixed.

tombuildsstuff · 2023-09-05T05:10:15Z

website/docs/r/resource_connector_appliance.html.markdown

+
+In addition to the Arguments listed above - the following Attributes are exported: 
+
+* `id` - The ID of the Appliance.


the Appliance, or Resource Connector Appliance?

Resource Connector Appliance, fixed.

tombuildsstuff · 2023-09-05T05:10:22Z

website/docs/r/resource_connector_appliance.html.markdown

+
+* `tags` - (Optional) A mapping of tags which should be assigned to the Appliance.
+
+* `version` - (Optional) The version of the Appliance. Possible values is `latest`.


the Appliance, or Resource Connector Appliance?

Resource Connector Appliance, fixed.

sinbai · 2023-09-12T04:14:06Z

hi @sinbai

Thanks for this PR - I've taken a look through and left some comments inline, if those can be addressed then we should be able to take another look here.

It's worth noting that when a field has to be set via the Update method, that we can first Create the resource and then use the Update API to update this value - as such we don't need to ask users to conditionally apply their Terraform Configuration in two stages.

It's also worth noting there's conflation between the Appliance and the Resource Connector Appliance in the documentation, presumably the Appliance has to exist within AKS/VMware etc first? If so, then we're also missing test cases showing this working end-to-end - could we clarify the behaviour of this service to make it fully usable here?

Thanks!

@tombuildsstuff thanks for your detailed feedback. I have fixed the inline feedback. Could you please take another look?

For above test cases showing this working end-to-end, the following is the feedback I got from the service team.

Appliance RP is a hybrid resource provider service. We have a few operations that deploy a cluster on prem for customers like Validate, Prepare, Deploy are run on prem only and can be done via Arc Appliance CLI as this requires specific parameters depending on the on prem stack being used to deploy the cluster on. 

We do create an Azure resource to help link the on prem resource to Azure, and that is done using  RP operations (that we support using various SDKs) like Create, Update, Get, List, etc, for which we are also looking to support Terraform as the RP is GAing.

Link to public doc - Azure Arc resource bridge (preview) overview - Azure Arc | Microsoft Learn

Per above feedback, I assume that we could not add end-to-end testing in TF, what do you think?

stephybun

Thanks for making those changes @sinbai.

There are a few things that still need to be fixed.

Also since this is from the Azure Arc product line this resource should be renamed to azurerm_arc_bridge_appliance/azurerm_arc_bridge_connector_appliance or something of the sort? This is because we should go with names that are familiar to the users, in this case since this is being marketed as Arc Bridge. Also existing arc resources within the provider like the Arc Kubernetes cluster or Arc Virtual Machine are also named as such.

stephybun · 2023-09-18T12:29:16Z

internal/services/resourceconnector/resourceconnector_appliance_resource.go

+	Identity          []identity.ModelSystemAssigned `tfschema:"identity"`
+	Provider          appliances.Provider            `tfschema:"infrastructure_provider"`
+	PublicKey         string                         `tfschema:"public_key"`
+	Tags              map[string]string              `tfschema:"tags"`


Suggested change

Tags map[string]string `tfschema:"tags"`

Tags map[string]interface{} `tfschema:"tags"`

stephybun · 2023-09-18T12:29:37Z

internal/services/resourceconnector/resourceconnector_appliance_resource.go

+						Provider: pointer.To(model.Provider),
+					},
+				},
+				Tags: pointer.To(model.Tags),


Suggested change

Tags: pointer.To(model.Tags),

Tags: tags.Expand(model.Tags),

stephybun · 2023-09-18T12:30:04Z

internal/services/resourceconnector/resourceconnector_appliance_resource.go

+			}
+
+			if metadata.ResourceData.HasChange("tags") {
+				parameters.Tags = pointer.To(model.Tags)


Suggested change

parameters.Tags = pointer.To(model.Tags)

parameters.Tags = tags.Expand(model.Tags)

stephybun · 2023-09-18T12:30:24Z

internal/services/resourceconnector/resourceconnector_appliance_resource.go

+			if model := resp.Model; model != nil {
+				state.Location = location.Normalize(model.Location)
+				state.Identity = identity.FlattenSystemAssignedToModel(model.Identity)
+				state.Tags = pointer.From(resp.Model.Tags)


Suggested change

state.Tags = pointer.From(resp.Model.Tags)

state.Tags = tags.Flatten(model.Tags)

stephybun · 2023-09-18T12:39:19Z

website/docs/r/resource_connector_appliance.html.markdown

+
+* `infrastructure_provider`- (Required) The infrastructure provider about the connected Resource Connector Appliance. Possible values are `HCI`,`SCVMM` and `VMWare`. Changing this forces a new resource to be created.
+
+* `public_key`- (Optional) The `public_key` is an RSA public key in PKCS1 format encoded in base64. Changing this forces a new resource to be created.


I think the previous comment on this is expecting the property to be renamed to public_key_base64 like is specified in the naming guidelines of the contributor docs.

Thanks for your explanation, fixed:)

sinbai · 2023-09-19T06:38:20Z

Thanks for making those changes @sinbai.

There are a few things that still need to be fixed.

Also since this is from the Azure Arc product line this resource should be renamed to azurerm_arc_bridge_appliance/azurerm_arc_bridge_connector_appliance or something of the sort? This is because we should go with names that are familiar to the users, in this case since this is being marketed as Arc Bridge. Also existing arc resources within the provider like the Arc Kubernetes cluster or Arc Virtual Machine are also named as such.

@stephybun thanks for you detailed feedback. I updated the resource name to azurerm_arc_resource_bridge_appliance based on What is Azure Arc resource bridge (preview), WDYT? Also, I updated the other comments, could you please take another look?

stephybun

Thanks @sinbai LGTM 🍰

<Actions> <action id="4a39167e811ac038e4a588362092472c27cfbe9e4929ae61d035f708a093a669"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azurerm" updated from "3.73.0" to "3.74.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.74.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.74.0
NOTES:

* `azurerm_synapse_sql_pool` - users that have imported `azurerm_synapse_sql_pool` resources that were created outside of Terraform using an `LRS` storage account type will need to use `ignore_changes` to avoid the resource from being destroyed and recreated.

FEATURES:

* **New Resource**: `azurerm_arc_resource_bridge_appliance` ([#23108](hashicorp/terraform-provider-azurerm#23108 **New Resource**: `azurerm_data_factory_dataset_azure_sql_table` ([#23264](hashicorp/terraform-provider-azurerm#23264 **New Resource**: `azurerm_function_app_connection` ([#23127](https://github.com/hashicorp/terraform-provider-azurerm/issues/23127))

ENHANCEMENTS:

* dependencies: updating to `v0.20230918.1115907` of `github.com/hashicorp/go-azure-sdk` ([#23337](hashicorp/terraform-provider-azurerm#23337 dependencies: downgrading to `v1.12.5` of `github.com/rickb777/date` ([#23296](hashicorp/terraform-provider-azurerm#23296 `mysql`: updating to use API Version `2022-01-01` ([#23320](hashicorp/terraform-provider-azurerm#23320 `azurerm_app_configuration` - support for the `replica` block ([#22452](hashicorp/terraform-provider-azurerm#22452 `azurerm_bot_channel_directline` - support for `user_upload_enabled`, `endpoint_parameters_enabled`, and `storage_enabled` ([#23149](hashicorp/terraform-provider-azurerm#23149 `azurerm_container_app` - support for scale rules ([#23294](hashicorp/terraform-provider-azurerm#23294 `azurerm_container_app_environment` - support for zone redundancy ([#23313](hashicorp/terraform-provider-azurerm#23313 `azurerm_container_group` - support for the `key_vault_user_identity_id` property for Customer Managed Keys ([#23332](hashicorp/terraform-provider-azurerm#23332 `azurerm_cosmosdb_account` - support for MongoDB connection strings ([#23331](hashicorp/terraform-provider-azurerm#23331 `azurerm_data_factory_dataset_delimited_text` - support for the `dynamic_file_system_enabled`, `dynamic_path_enabled`, and `dynamic_filename_enabled` properties ([#23261](hashicorp/terraform-provider-azurerm#23261 `azurerm_data_factory_dataset_parquet` - support for the `azure_blob_fs_location` block ([#23261](hashicorp/terraform-provider-azurerm#23261 `azurerm_monitor_diagnostic_setting` - validation to ensure either `category` or `category_group` are supplied in `enabled_log` and `log` blocks ([#23308](hashicorp/terraform-provider-azurerm#23308 `azurerm_network_interface` - support for the `auxiliary_mode` and `auxiliary_sku` properties ([#22979](hashicorp/terraform-provider-azurerm#22979 `azurerm_postgresql_flexible_server` - increased the maximum supported value for `storage_mb` ([#23277](hashicorp/terraform-provider-azurerm#23277 `azurerm_shared_image_version` - support for the `replicated_region_deletion_enabled` and `target_region.exclude_from_latest_enabled` properties ([#23147](hashicorp/terraform-provider-azurerm#23147 `azurerm_storage_account` - support for setting `domain_name` and `domain_guid` for `AADKERB` ([#22833](hashicorp/terraform-provider-azurerm#22833 `azurerm_storage_account_customer_managed_key` - support for cross-tenant customer-managed keys with the `federated_identity_client_id`, and `key_vault_uri` properties ([#20356](hashicorp/terraform-provider-azurerm#20356 `azurerm_web_application_firewall_policy` - support for the `rate_limit_duration`, `rate_limit_threshold`, `group_rate_limit_by`, and `request_body_inspect_limit_in_kb` properties ([#23239](https://github.com/hashicorp/terraform-provider-azurerm/issues/23239))

BUG FIXES:

* Data Source: `azurerm_container_app_environment`: fix `log_analytics_workspace_name` output to correct value ([#23298](hashicorp/terraform-provider-azurerm#23298 `azurerm_api_management_api` - set the `service_url` property when importing the resource ([#23011](hashicorp/terraform-provider-azurerm#23011 `azurerm_app_configuration` - prevent crash by nil checking the encryption configuration ([#23302](hashicorp/terraform-provider-azurerm#23302 `azurerm_app_configuration_feature` - update `percentage_filter_value` to accept correct type of float ([#23263](hashicorp/terraform-provider-azurerm#23263 `azurerm_container_app` - fix an issue with `commands` and `args` being overwritten when using multiple containers ([#23338](hashicorp/terraform-provider-azurerm#23338 `azurerm_key_vault_certificate` - fix issue where certificates couldn't be recovered anymore ([#23204](hashicorp/terraform-provider-azurerm#23204 `azurerm_key_vault_key` - the ForceNew when `expiration_date` is removed from the config file ([#23327](hashicorp/terraform-provider-azurerm#23327 `azurerm_linux_function_app` - fix a bug in setting the storage settings when using Elastic Premium plans ([#21212](hashicorp/terraform-provider-azurerm#21212 `azurerm_linux_web_app` - fix docker app stack update ([#23303](hashicorp/terraform-provider-azurerm#23303 `azurerm_linux_web_app` - fix crash in auto heal expansion ([#21328](hashicorp/terraform-provider-azurerm#21328 `azurerm_linux_web_app_slot` - fix docker app stack update ([#23303](hashicorp/terraform-provider-azurerm#23303 `azurerm_linux_web_app_slot` - fix crash in auto heal expansion ([#21328](hashicorp/terraform-provider-azurerm#21328 `azurerm_log_analytics_solution` - fix bug where the resource wasn't handling successful creation on subsequent applies ([#23312](hashicorp/terraform-provider-azurerm#23312 `azurerm_management_group_subscription_association` - fix bug to correctly mark resource as gone if not found during read ([#23335](hashicorp/terraform-provider-azurerm#23335 `azurerm_mssql_elasticpool` - remove check that prevents `license_type` from being set for certain skus ([#23262](hashicorp/terraform-provider-azurerm#23262 `azurerm_servicebus_queue` - fixing an issue where `auto_delete_on_idle` couldn't be set to `P10675199DT2H48M5.4775807S` ([#23296](hashicorp/terraform-provider-azurerm#23296 `azurerm_servicebus_topic` - fixing an issue where `auto_delete_on_idle` couldn't be set to `P10675199DT2H48M5.4775807S` ([#23296](hashicorp/terraform-provider-azurerm#23296 `azurerm_storage_account` - prevent sending unsupported blob properties in payload for `Storage` account kind ([#23288](hashicorp/terraform-provider-azurerm#23288 `azurerm_synapse_sql_pool` - expose `storage_account_type` ([#23217](hashicorp/terraform-provider-azurerm#23217 `azurerm_windows_function_app` - fix a bug in setting the storage settings when using Elastic Premium plans ([#21212](hashicorp/terraform-provider-azurerm#21212 `azurerm_windows_web_app` - fix docker app stack update ([#23303](hashicorp/terraform-provider-azurerm#23303 `azurerm_windows_web_app_slot` - fix docker app stack update ([#23303](https://github.com/hashicorp/terraform-provider-azurerm/issues/23303))

DEPRECATIONS:

* `azurerm_application_gateway` - deprecate `Standard` and `WAF` skus ([#23310](hashicorp/terraform-provider-azurerm#23310 `azurerm_bot_channel_web_chat` - deprecate `site_names` in favour of `site` block ([#23161](hashicorp/terraform-provider-azurerm#23161 `azurerm_monitor_diagnostic_setting` - deprecate `retention_policy` in favour of `azurerm_storage_management_policy` ([#23260](https://github.com/hashicorp/terraform-provider-azurerm/issues/23260))


</pre> </details> </details> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]>

github-actions · 2024-04-28T02:11:57Z

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 bot added documentation dependencies size/XL waiting-response and removed waiting-response labels Aug 29, 2023

support new resource for resource connector RP

c1d98d6

sinbai force-pushed the ResourceConnector/new_resource branch from 267a8ca to c1d98d6 Compare August 29, 2023 06:39

github-actions bot added waiting-response and removed waiting-response labels Aug 29, 2023

run make generate

4097b88

github-actions bot removed the waiting-response label Aug 29, 2023

tombuildsstuff suggested changes Sep 5, 2023

View reviewed changes

github-actions bot added the waiting-response label Sep 5, 2023

update code

8a10faa

github-actions bot added waiting-response and removed waiting-response labels Sep 11, 2023

update code

c352123

github-actions bot removed the waiting-response label Sep 11, 2023

sinbai added 2 commits September 12, 2023 10:19

remove version

4ed6e20

update code

469e451

stephybun requested changes Sep 18, 2023

View reviewed changes

github-actions bot added the waiting-response label Sep 18, 2023

update code

59eff66

github-actions bot removed the waiting-response label Sep 19, 2023

sinbai changed the title ~~New Resource : azurerm_resource_connector_appliance~~ New Resource : azurerm_arc_resource_bridge_appliance Sep 19, 2023

stephybun added service/arc new-resource labels Sep 19, 2023

stephybun approved these changes Sep 19, 2023

View reviewed changes

stephybun merged commit 1063c0f into hashicorp:main Sep 19, 2023
29 checks passed

github-actions bot added this to the v3.74.0 milestone Sep 19, 2023

stephybun added a commit that referenced this pull request Sep 20, 2023

Update for #23161 #23302 #23262 #23310 #23263 #23108

afc77d9

sinbai deleted the ResourceConnector/new_resource branch March 28, 2024 02:47

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_arc_resource_bridge_appliance` #23108

New Resource : `azurerm_arc_resource_bridge_appliance` #23108

sinbai commented Aug 29, 2023 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Sep 5, 2023

sinbai Sep 12, 2023

tombuildsstuff Sep 5, 2023

sinbai Sep 12, 2023

tombuildsstuff Sep 5, 2023

sinbai Sep 12, 2023

tombuildsstuff Sep 5, 2023

sinbai Sep 12, 2023

tombuildsstuff Sep 5, 2023

sinbai Sep 12, 2023

tombuildsstuff Sep 5, 2023

sinbai Sep 12, 2023

sinbai commented Sep 12, 2023

stephybun left a comment

stephybun Sep 18, 2023

sinbai Sep 19, 2023

stephybun Sep 18, 2023

sinbai Sep 19, 2023

stephybun Sep 18, 2023

sinbai Sep 19, 2023

stephybun Sep 18, 2023

sinbai Sep 19, 2023

stephybun Sep 18, 2023

sinbai Sep 19, 2023

sinbai commented Sep 19, 2023

stephybun left a comment

github-actions bot commented Apr 28, 2024


		* `version` - (Optional) The version of the Appliance. Possible values is `latest`.

		-> NOTE: The `version` cannot be populated apart from upgrade call.


		In addition to the Arguments listed above - the following Attributes are exported:

		* `id` - The ID of the Appliance.


		* `tags` - (Optional) A mapping of tags which should be assigned to the Appliance.

		* `version` - (Optional) The version of the Appliance. Possible values is `latest`.

	Tags map[string]string `tfschema:"tags"`
	Tags map[string]interface{} `tfschema:"tags"`

	parameters.Tags = pointer.To(model.Tags)
	parameters.Tags = tags.Expand(model.Tags)

	state.Tags = pointer.From(resp.Model.Tags)
	state.Tags = tags.Flatten(model.Tags)


		* `infrastructure_provider`- (Required) The infrastructure provider about the connected Resource Connector Appliance. Possible values are `HCI`,`SCVMM` and `VMWare`. Changing this forces a new resource to be created.

		* `public_key`- (Optional) The `public_key` is an RSA public key in PKCS1 format encoded in base64. Changing this forces a new resource to be created.

New Resource : azurerm_arc_resource_bridge_appliance #23108

New Resource : azurerm_arc_resource_bridge_appliance #23108

Conversation

sinbai commented Aug 29, 2023 • edited Loading

tombuildsstuff 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

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 Sep 12, 2023

stephybun 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sinbai commented Sep 19, 2023

stephybun left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 28, 2024

New Resource : `azurerm_arc_resource_bridge_appliance` #23108

New Resource : `azurerm_arc_resource_bridge_appliance` #23108

sinbai commented Aug 29, 2023 •

edited

Loading