New Resource: `azurerm_relay_hybrid_connection` #4832

DSakura207 · 2019-11-07T18:35:58Z

--- PASS: TestAccAzureRMHybridConnection_basic
--- PASS: TestAccAzureRMHybridConnection_full
--- PASS: TestAccAzureRMHybridConnection_update

Resolution for #1320.

First time pull request 😄

The resource is Azure Relay Hybrid Connection. It is basically the same as Azure App Service Hybrid Connection, while the App Service version requires special user metadata and access policy.

More changes will come, but this is the foundation. To support App Service and App Service Plan, there will be more changes in another pull request.

mbfrahry

Hey @DSakura207. These changes look great. I've requested quite a few changes but the comments are almost entirely cleanup/organization focused so we maintain consistency in the provider codebase.

mbfrahry · 2019-11-07T22:00:05Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
+)
+
+func TestAccAzureRMHybridConnection_basic(t *testing.T) {


We should add Relay to these tests names to better categorize them at a glance

Suggested change

func TestAccAzureRMHybridConnection_basic(t *testing.T) {

func TestAccAzureRMRelayHybridConnection_basic(t *testing.T) {

mbfrahry · 2019-11-07T22:00:24Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+	})
+}
+
+func TestAccAzureRMHybridConnection_full(t *testing.T) {


Suggested change

func TestAccAzureRMHybridConnection_full(t *testing.T) {

func TestAccAzureRMRelayHybridConnection_full(t *testing.T) {

mbfrahry · 2019-11-07T22:00:33Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+	})
+}
+
+func TestAccAzureRMHybridConnection_update(t *testing.T) {


Suggested change

func TestAccAzureRMHybridConnection_update(t *testing.T) {

func TestAccAzureRMRelayHybridConnection_update(t *testing.T) {

mbfrahry · 2019-11-07T22:03:27Z

azurerm/internal/services/relay/client.go

 }

 func BuildClient(o *common.ClientOptions) *Client {
 	NamespacesClient := relay.NewNamespacesClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId)
+	HybridConnectionsClient := relay.NewHybridConnectionsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId)


We should organize these like:

NamespacesClient := relay.NewNamespacesClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&NamespacesClient.Client, o.ResourceManagerAuthorizer) HybridConnectionsClient := relay.NewHybridConnectionsClientWithBaseURI(o.ResourceManagerEndpoint, o.SubscriptionId) o.ConfigureClient(&HybridConnectionsClient.Client, o.ResourceManagerAuthorizer)

mbfrahry · 2019-11-07T22:03:52Z

azurerm/provider.go

@@ -396,6 +396,7 @@ func Provider() terraform.ResourceProvider {
 		"azurerm_redis_cache":                                                            resourceArmRedisCache(),
 		"azurerm_redis_firewall_rule":                                                    resourceArmRedisFirewallRule(),
 		"azurerm_relay_namespace":                                                        resourceArmRelayNamespace(),
+		"azurerm_relay_hybrid_connection":                                                resourceArmHybridConnection(),


This should be moved up one to keep this list alphabetical

mbfrahry · 2019-11-07T22:07:18Z

azurerm/resource_arm_relay_hybrid_connection.go

+		Pending:    []string{"Pending"},
+		Target:     []string{"Deleted"},
+		Refresh:    hybridConnectionDeleteRefreshFunc(ctx, client, resourceGroup, relayNamespace, name),
+		Timeout:    60 * time.Minute,


In the Timeout section defined in the schema, you defined the Delete timeout as 30 minutes. We should sync these up so this is either 30 minutes or the delete timeout defined above is 60 minutes.

mbfrahry · 2019-11-07T22:08:15Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+
+resource "azurerm_relay_hybrid_connection" "test" {
+	name                 = "acctestrnhc-%d"
+	resource_group_name = "${azurerm_resource_group.test.name}"


Adding a space so the equals sign lines up a little better

Suggested change

resource_group_name = "${azurerm_resource_group.test.name}"

resource_group_name = "${azurerm_resource_group.test.name}"

mbfrahry · 2019-11-07T22:08:24Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+
+resource "azurerm_relay_hybrid_connection" "test" {
+	name                 = "acctestrnhc-%d"
+	resource_group_name = "${azurerm_resource_group.test.name}"


Suggested change

resource_group_name = "${azurerm_resource_group.test.name}"

resource_group_name = "${azurerm_resource_group.test.name}"

mbfrahry · 2019-11-07T22:08:44Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+}
+
+resource "azurerm_relay_hybrid_connection" "test" {
+	name                 = "acctestrnhc-%d"


We should line all these equal signs up

mbfrahry · 2019-11-07T22:08:57Z

azurerm/resource_arm_relay_hybrid_connection_test.go

+%s
+
+resource "azurerm_relay_namespace" "import" {
+	name                 = "acctestrnhc-%d"


We should line all these equal signs up

mbfrahry · 2019-11-07T22:18:03Z

Also, thank you for submitting your first PR! This is a great start

DSakura207

Hello @mbfrahry , thanks for your quick reply. I've update my branch with your suggestions. Hopefully my finger is not fat so that the commit can pass checks 🤞

mbfrahry

This all looks great! Thanks for fixing it up.

mbfrahry · 2019-11-08T02:41:22Z

@DSakura207, thanks again for working on this. It should make it into the next release

ghost · 2019-12-08T14:50:13Z

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 👉 [email protected]. Thanks!

DSakura207 added 17 commits November 5, 2019 10:55

Add HybridConnectionsClient

60a5eda

Add placeholder for resource and test

08776a1

Rename file for hybrid connection

bf6f821

Add unit test

d154568

Fix testcase and import

d6fa441

Add Relay Hybrid Connection support

360a96f

Add resource mapping

a73f489

Fix typos

c92be40

Update values for correct depedencies

8308a10

Align with Azure Portal settings

d23f6a0

Workaround for updating requires_client_authorization

1efdb73

Add test for updating requires_client_authorization

0e66879

Add document for azurerm_relay_hybrid_connection

fe6132b

Add optional requires_client_authorization in example

5246848

Support UserMetadata

1e7856b

Support UserMetadata

8ee9c9b

Add description for user_metadata

a85eb9b

ghost added size/XL documentation labels Nov 7, 2019

DSakura207 added 2 commits November 7, 2019 12:52

Fix copy-and-paste error

8884ca3

Add subcategory

bbb4c9f

mbfrahry added new-resource service/relay labels Nov 7, 2019

mbfrahry requested changes Nov 7, 2019

View reviewed changes

mbfrahry self-assigned this Nov 7, 2019

Apply code review

4d782e4

DSakura207 commented Nov 8, 2019

View reviewed changes

mbfrahry changed the title ~~New Resource: azurerm_relay_hybrid_connection~~ New Resource: azurerm_relay_hybrid_connection Nov 8, 2019

mbfrahry approved these changes Nov 8, 2019

View reviewed changes

mbfrahry merged commit 2d553f4 into hashicorp:master Nov 8, 2019

DSakura207 deleted the f/hybridconnections branch November 8, 2019 15:29

ghost locked and limited conversation to collaborators Dec 8, 2019

katbyte added this to the v1.38.0 milestone Jan 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_relay_hybrid_connection` #4832

New Resource: `azurerm_relay_hybrid_connection` #4832

DSakura207 commented Nov 7, 2019 •

edited

Loading

mbfrahry left a comment

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry Nov 7, 2019

mbfrahry commented Nov 7, 2019

DSakura207 left a comment

mbfrahry left a comment

mbfrahry commented Nov 8, 2019

ghost commented Dec 8, 2019

	func TestAccAzureRMHybridConnection_basic(t *testing.T) {
	func TestAccAzureRMRelayHybridConnection_basic(t *testing.T) {

	func TestAccAzureRMHybridConnection_full(t *testing.T) {
	func TestAccAzureRMRelayHybridConnection_full(t *testing.T) {

	func TestAccAzureRMHybridConnection_update(t *testing.T) {
	func TestAccAzureRMRelayHybridConnection_update(t *testing.T) {

	resource_group_name = "${azurerm_resource_group.test.name}"
	resource_group_name = "${azurerm_resource_group.test.name}"

New Resource: azurerm_relay_hybrid_connection #4832

New Resource: azurerm_relay_hybrid_connection #4832

Conversation

DSakura207 commented Nov 7, 2019 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbfrahry commented Nov 7, 2019

DSakura207 left a comment

Choose a reason for hiding this comment

mbfrahry left a comment

Choose a reason for hiding this comment

mbfrahry commented Nov 8, 2019

ghost commented Dec 8, 2019

New Resource: `azurerm_relay_hybrid_connection` #4832

New Resource: `azurerm_relay_hybrid_connection` #4832

DSakura207 commented Nov 7, 2019 •

edited

Loading