New resource - azurerm_data_factory_integration_runtime_self_hosted #6535

pearcec · 2020-04-19T19:24:41Z

This resource type supports but a standalone Self-hosted and linking via rbac_authorization. The
key_authorization has no documentation other than the REST interface. The Powershell Cmdlet does
not make use of it. So it has been left out.

A full test is setup and necessary for the linked environment to work.

An example and documetnation is included to make the linked setup easier to understand. People
may also reference the test case.

Fixing issue #6503.

sdebruyn · 2020-05-12T18:00:47Z

I really would like to have this resource. Is there anything I can do to help? I see that it has a documentation label? Does that mean that it needs docs before it can go through? Maybe a CONTRIBUTING.md file could be useful to let us know what we can do to make it easier for you guys :)

katbyte

thanks for this pr @pearcec! overall looks good but i've left some comments inline that should be addressed

katbyte · 2020-05-07T15:20:19Z

...m/internal/services/datafactory/resource_arm_data_factory_integration_runtime_self_hosted.go

+						},
+					},
+				},
+				Set: resourceArmDataFactoryIntegrationRuntimeSelfHostedRbacAuthorizationHash,


I'm not sure we need a custom hash function here? 🤔

katbyte · 2020-05-14T20:27:02Z

...services/datafactory/tests/resource_arm_data_factory_integration_runtime_self_hosted_test.go

+		name := rs.Primary.Attributes["name"]
+		factoryName := rs.Primary.Attributes["data_factory_name"]
+		resourceGroup := rs.Primary.Attributes["resource_group_name"]


could we use a resource specific parse ID function here on the resources's id?

katbyte · 2020-05-14T20:27:20Z

...services/datafactory/tests/resource_arm_data_factory_integration_runtime_self_hosted_test.go

+		name := rs.Primary.Attributes["name"]
+		factoryName := rs.Primary.Attributes["data_factory_name"]
+		resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]


could we use a resource specific parseID function here?

katbyte · 2020-05-14T20:27:47Z

...services/datafactory/tests/resource_arm_data_factory_integration_runtime_self_hosted_test.go

+}
+
+resource "azurerm_resource_group" "test" {
+  name     = "acctestRG-%d"


could we use

Suggested change

name = "acctestRG-%d"

name = "acctestRG-df-%d"

katbyte · 2020-05-14T20:29:05Z

...services/datafactory/tests/resource_arm_data_factory_integration_runtime_self_hosted_test.go

+}
+
+resource "azurerm_resource_group" "host" {
+  name     = "acctesthostRG-%d"


and

Suggested change

name = "acctesthostRG-%d"

name = "acctestRG-df-%d-host"

katbyte · 2020-05-14T20:30:41Z

...m/internal/services/datafactory/resource_arm_data_factory_integration_runtime_self_hosted.go

+	id, err := azure.ParseAzureResourceID(d.Id())
+	if err != nil {
+		return err
+	}
+	resourceGroup := id.ResourceGroup
+	factoryName := id.Path["factories"]
+	name := id.Path["integrationruntimes"]


could we write a parse ID function specific to this resource and use it here like in #5692

katbyte · 2020-05-14T20:31:34Z

...m/internal/services/datafactory/resource_arm_data_factory_integration_runtime_self_hosted.go

+	id, err := azure.ParseAzureResourceID(d.Id())
+	if err != nil {
+		return err
+	}
+	resourceGroup := id.ResourceGroup
+	factoryName := id.Path["factories"]
+	name := id.Path["integrationruntimes"]


and a resource specific parse ID function here

None of these are IDs they are names. So there isn't really any path to parse. Both name and data_factory_name are validated via the schema.

Hi @pearcec
I think Kt was referring to the pattern of a custom parser for the whole resource ID (e.g. /subscriptions/12345678-1234-1234-1234-12345678abc/resourceGroups/exampleResourceGroup/providers/Microsoft.DataFactory/factories/exampleFactoryName/integrationruntimes/exampleIntegrationRuntime which could be parsed into a custom type, and then accessed as id.FactoryName and id.Name - (example here)

ok makes sense, I will take a look.

Ok I added it. I rebased it as well.

katbyte · 2020-05-14T20:32:02Z

...m/internal/services/datafactory/resource_arm_data_factory_integration_runtime_self_hosted.go

+		SelfHostedIntegrationRuntimeTypeProperties: expandAzureRmDataFactoryIntegrationRuntimeSelfHostedTypeProperties(d),
+	}
+
+	basicIntegrationRuntime, _ := selfHostedIntegrationRuntime.AsBasicIntegrationRuntime()


we should be checking the result of the cast for nil

katbyte · 2020-05-14T20:32:23Z

...m/internal/services/datafactory/resource_arm_data_factory_integration_runtime_self_hosted.go

+				),
+			},
+
+			"description": {


we should place optional properties after required

katbyte · 2020-05-14T20:32:36Z

...m/internal/services/datafactory/resource_arm_data_factory_integration_runtime_self_hosted.go

+			"auth_key_1": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
+
+			"auth_key_2": {


computed properties should go after optional

pearcec · 2020-05-15T16:00:03Z

@katbyte Hi - I made some adjustments and committed the changes. There are also some inline comments to the ParseID request.

Stanpol · 2020-06-11T14:22:32Z

Can we expect that this PR will be a part of v2.15.0 milestone?

pearcec · 2020-06-11T16:59:30Z

I updated the code to address @katbyte changes.

This resource type supports but a standalone Self-hosted and linking via rbac_authorization. The key_authorization has no documentation other than the REST interface. The Powershell Cmdlet does not make use of it. So it has been left out. A full test is setup and necessary for the linked environment to work. An example and documetnation is included to make the linked setup easier to understand. People may also reference the test case. Fixing issue hashicorp#6503.

- Adjust schema for accuracy and ordering - Clarify response from expandAzureRmDataFactoryIntegrationRuntimeSelfHostedTypeProperties - Adjust test names - Rename files to match current refactor

katbyte

Hi @pearcec - thanks for the updates. Overall this looks good now aside from using the custom parse ID (which we can then add to resource import to check the ID there)

...erm/internal/services/datafactory/tests/data_factory_integration_runtime_self_hosted_test.go

azurerm/internal/services/datafactory/data_factory_integration_runtime_self_hosted.go

ghost · 2020-06-19T02:34:29Z

This has been released in version 2.15.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.15.0"
}
# ... other configuration ...

ghost · 2020-07-17T14:50:43Z

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!

ghost added size/XL documentation labels Apr 19, 2020

katbyte added the service/data-factory label May 7, 2020

katbyte requested changes May 14, 2020

View reviewed changes

katbyte added the waiting-response label May 14, 2020

pearcec force-pushed the support-for-azurerm_data_factory_integration_runtime_self_hosted-#6503 branch from e744456 to 972720f Compare May 15, 2020 15:11

ghost removed the waiting-response label May 15, 2020

Christian Pearce added 4 commits June 12, 2020 09:20

terrafmt fmt fix

ecb6e45

Removing deadcode

07df537

Reviewer change request fixes

733cea7

- Adjust schema for accuracy and ordering - Clarify response from expandAzureRmDataFactoryIntegrationRuntimeSelfHostedTypeProperties - Adjust test names - Rename files to match current refactor

pearcec force-pushed the support-for-azurerm_data_factory_integration_runtime_self_hosted-#6503 branch from 972720f to 733cea7 Compare June 12, 2020 13:21

katbyte requested changes Jun 12, 2020

View reviewed changes

Add Data FActory Integration Runtime resource specific parse function

3b82935

ghost added size/XXL and removed size/XL labels Jun 12, 2020

Additional use of DataFactoryIntegrationRuntimeParseID

a4fdedd

pearcec requested a review from katbyte June 12, 2020 18:05

katbyte changed the title ~~New resource DF Self-host Integration Runtime~~ New resource - azurerm_data_factory_integration_runtime_self_hosted Jun 16, 2020

katbyte merged commit bc06656 into hashicorp:master Jun 16, 2020

katbyte added this to the v2.15.0 milestone Jun 16, 2020

katbyte added a commit that referenced this pull request Jun 16, 2020

update CHANGELOG.md to include #6535

2326ad0

hashicorp locked and limited conversation to collaborators Jul 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New resource - azurerm_data_factory_integration_runtime_self_hosted #6535

New resource - azurerm_data_factory_integration_runtime_self_hosted #6535

pearcec commented Apr 19, 2020

sdebruyn commented May 12, 2020

katbyte left a comment

katbyte May 7, 2020

katbyte May 14, 2020

katbyte May 14, 2020

katbyte May 14, 2020

katbyte May 14, 2020

katbyte May 14, 2020

katbyte May 14, 2020

pearcec May 14, 2020

jackofallops Jun 12, 2020

pearcec Jun 12, 2020

pearcec Jun 12, 2020 •

edited

katbyte May 14, 2020

katbyte May 14, 2020

katbyte May 14, 2020

pearcec commented May 15, 2020

Stanpol commented Jun 11, 2020

pearcec commented Jun 11, 2020

katbyte left a comment

ghost commented Jun 19, 2020

ghost commented Jul 17, 2020

New resource - azurerm_data_factory_integration_runtime_self_hosted #6535

New resource - azurerm_data_factory_integration_runtime_self_hosted #6535

Conversation

pearcec commented Apr 19, 2020

sdebruyn commented May 12, 2020

katbyte 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

pearcec Jun 12, 2020 • edited

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

pearcec commented May 15, 2020

Stanpol commented Jun 11, 2020

pearcec commented Jun 11, 2020

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Jun 19, 2020

ghost commented Jul 17, 2020

pearcec Jun 12, 2020 •

edited