New Resource/Data Source: `azurerm_private_link_service`, Data Source: `azurerm_private_link_service_endpoint_connections` and expose in `azurerm_lb` and `azurerm_subnet` #4426

WodansSon · 2019-09-25T01:48:45Z

(fixes #4701 )

azurerm/resource_arm_private_link_service.go

katbyte

Thanks for the PR @WodansSon, i've left some comments inline that need to be addressed before merge.

katbyte · 2019-10-01T22:31:05Z

azurerm/data_source_private_link_service.go

+				ValidateFunc: validate.NoEmptyStrings,
+			},
+
+			"location": azure.SchemaLocation(),


Should this be azure.SchemaLocationForDataSource ?

katbyte · 2019-10-01T22:32:19Z

azurerm/data_source_private_link_service.go

+
+			"location": azure.SchemaLocation(),
+
+			"resource_group_name": azure.SchemaResourceGroupNameDiffSuppress(),


Whats the reasoning behind suppressing case difference here?

Defensive programming, Azure tends to not be very consistent with casing... I removed it and switched it to SchemaResourceGroupNameForDataSource

katbyte · 2019-10-01T22:34:06Z

azurerm/data_source_private_link_service.go

+										Computed: true,
+									},
+									"location": azure.SchemaLocationForDataSource(),
+									"tags":     tagsForDataSourceSchema(),


We should use the new tags.SchemaForDataSource function

katbyte · 2019-10-01T22:34:33Z

website/docs/r/subnet.html.markdown

@@ -99,6 +101,7 @@ The following attributes are exported:
 * `resource_group_name` - The name of the resource group in which the subnet is created in.
 * `virtual_network_name` - The name of the virtual network in which the subnet is created in
 * `address_prefix` - The address prefix for the subnet
+* `private_link_service_network_policies` - Enable or Disable apply network policies on private link service in the subnet.


Do we need this as it is in the above docs?

This setting is only for subnet resource.

line 62 describes this?

same question

I don't know how to explain it other than what is said here, what exactly are you expecting here?

We only added one property to the schema, yet there are two new properties in the docs? was one already existing?

Yes that was the original attribute name, I missed that one.

katbyte · 2019-10-01T22:36:19Z

website/docs/r/subnet.html.markdown

@@ -59,6 +59,8 @@ The following arguments are supported:

 * `address_prefix` - (Required) The address prefix to use for the subnet.

+* `private_link_service_network_policies` - (Optional) Enable or Disable apply network policies on private link service in the subnet.


Could we word this to be more consistent with the rest of the docs:

Suggested change

* `private_link_service_network_policies` - (Optional) Enable or Disable apply network policies on private link service in the subnet.

* `private_link_service_network_policies` - (Optional) Are network policies enabled on private links in this subnet?

This is a string? What are the valid options? shouldn't this be a bool?

katbyte · 2019-10-01T23:49:34Z

azurerm/resource_arm_private_link_service_test.go

+func testAccAzureRMPrivateLinkService_basic(rInt int, location string) string {
+	standardResources := testAccAzureRMPrivateLinkServiceTemplate_standardResources(rInt, location)
+	privateLink := testAccAzureRMPrivateLinkServiceTemplate_basic(rInt)
+
+	return testAccAzureRMPrivateLinkServiceTemplate("", standardResources, privateLink)
+}
+
+func testAccAzureRMPrivateLinkService_update(rInt int, location string) string {
+	standardResources := testAccAzureRMPrivateLinkServiceTemplate_standardResources(rInt, location)
+	privateLink := testAccAzureRMPrivateLinkServiceTemplate_update(rInt)
+
+	return testAccAzureRMPrivateLinkServiceTemplate("", standardResources, privateLink)
+}
+
+func testAccAzureRMPrivateLinkService_complete(rInt int, location string) string {
+	subscriptionDataSource := testAccAzureRMPrivateLinkServiceTemplate_subscriptionDataSource()
+	standardResources := testAccAzureRMPrivateLinkServiceTemplate_standardResources(rInt, location)
+	privateLink := testAccAzureRMPrivateLinkServiceTemplate_complete(rInt)
+
+	return testAccAzureRMPrivateLinkServiceTemplate(subscriptionDataSource, standardResources, privateLink)
+}


I don't see the value in this, we could just include subscriptionDataSource bit in the standardResources and call it a day

katbyte · 2019-10-01T23:51:04Z

azurerm/resource_arm_private_link_service_test.go

+`, rInt, rInt)
+}
+
+func testAccAzureRMPrivateLinkServiceTemplate_complete(rInt int) string {


could we just use this and add the standard resources in here?

Suggested change

func testAccAzureRMPrivateLinkServiceTemplate_complete(rInt int) string {

func testAccAzureRMPrivateLinkService_complete(rInt int) string {

azurerm/resource_arm_private_link_service_test.go

katbyte · 2019-10-01T23:51:47Z

azurerm/resource_arm_private_link_service_test.go

+`, rInt, rInt)
+}
+
+func testAccAzureRMPrivateLinkServiceTemplate_update(rInt int) string {


Could we just use the standard resources template in here?

Suggested change

func testAccAzureRMPrivateLinkServiceTemplate_update(rInt int) string {

func testAccAzureRMPrivateLinkService_update(rInt int) string {

azurerm/resource_arm_private_link_service_test.go

katbyte

(duplicate)

katbyte · 2019-10-02T21:07:09Z

azurerm/resource_arm_private_link_service.go

+				return fmt.Errorf("Error checking for present of existing Private Link Service %q (Resource Group %q): %+v", name, resourceGroup, err)
+			}
+		}
+		if !utils.ResponseWasNotFound(resp.Response) {


This should be

Suggested change

if !utils.ResponseWasNotFound(resp.Response) {

if if existing.ID != nil && *existing.ID != "" { {

Co-Authored-By: kt <kt@katbyte.me>

…form-provider-azurerm into nr_private-link-service

katbyte

Thanks for the revisions @WodansSon,

Just a couple comments left that should be addressed

katbyte · 2019-11-20T22:12:40Z

azurerm/resource_arm_private_link_service.go

+			"auto_approval_subscription_ids": {
+				Type:     schema.TypeSet,
+				Optional: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},


Could we validate these are IDs?

katbyte · 2019-11-20T22:12:47Z

azurerm/resource_arm_private_link_service.go

+			"visibility_subscription_ids": {
+				Type:     schema.TypeSet,
+				Optional: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},


Could we validate these are IDs?

katbyte · 2019-11-20T22:13:04Z

azurerm/resource_arm_private_link_service.go

+			"load_balancer_frontend_ip_configuration_ids": {
+				Type:     schema.TypeSet,
+				Required: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},


Could we validate these are IDs?

katbyte · 2019-11-20T22:13:13Z

azurerm/resource_arm_private_link_service.go

+			"network_interface_ids": {
+				Type:     schema.TypeSet,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},


Could we validate these are IDs?

katbyte · 2019-11-20T22:13:58Z

azurerm/resource_arm_private_link_service.go

+
+	if props := resp.PrivateLinkServiceProperties; props != nil {
+		d.Set("alias", props.Alias)
+		if props.AutoApproval != nil {


I think the glatten function will deal with nil

katbyte · 2019-11-20T22:14:03Z

azurerm/resource_arm_private_link_service.go

+				return fmt.Errorf("Error setting `auto_approval_subscription_ids`: %+v", err)
+			}
+		}
+		if props.Visibility != nil {


I think the glatten function will deal with nil

katbyte · 2019-11-20T22:15:10Z

azurerm/resource_arm_private_link_service_test.go

+					resource.TestCheckResourceAttr(resourceName, "load_balancer_frontend_ip_configuration_ids.#", "1"),
+					resource.TestCheckResourceAttrSet(resourceName, "load_balancer_frontend_ip_configuration_ids.0"),
+				),
+			},


I think the import step is still missing?

WodansSon · 2019-11-21T02:49:36Z

katbyte

Thanks @WodansSon, LGTM now! 🚀

ghost · 2019-11-26T08:42:13Z

This has been released in version 1.37.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 = "~> 1.37.0"
}
# ... other configuration ...

sfc-gh-emonsef · 2019-12-13T18:31:09Z

Is there a plan to add properties.enableProxyProtocol feature to privatelink service.
https://docs.microsoft.com/en-us/rest/api/virtualnetwork/privatelinkservices/createorupdate
I even tried to add it manually via powershell like below:

$privateLinkService = Get-AzPrivateLinkService -Name mypvilnkservice -ResourceGroupName resourcegroupname
$privateLinkService.enableProxyProtocol = $true
$privateLinkService = Get-AzPrivateLinkService -Name  mypvilnkservice -ResourceGroupName resourcegroupname $enableProxyProtocol = $true | Set-AzPrivateLinkService

but then terraform apply would reset enableProxyProtocol back to False again.

tombuildsstuff · 2019-12-16T11:12:26Z

~~@esymonsef mind opening a new feature request to track that?~~ PR: #5178

ghost · 2019-12-22T14:50:16Z

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!

Add new resources and data sources

c320621

ghost added size/XXL documentation labels Sep 25, 2019

WodansSon added this to the v1.35.0 milestone Sep 25, 2019

WodansSon added new-data-source new-resource labels Sep 25, 2019

WodansSon requested a review from katbyte September 25, 2019 01:50

WodansSon added enhancement new-data-source new-resource and removed new-data-source new-resource labels Sep 25, 2019

WodansSon added 3 commits September 24, 2019 19:04

Manual fixup

978be18

TOC tweaks

b196c7f

Examples fix

3ad7245

WodansSon changed the title ~~New Resource/Data Source: azurerm_private_link_service and expose in azurerm_lb and azurerm_subnet~~ [WIP]New Resource/Data Source: azurerm_private_link_service and expose in azurerm_lb and azurerm_subnet Sep 25, 2019

WodansSon added the service/network-interfaces label Sep 25, 2019

WodansSon added 3 commits September 25, 2019 20:05

[WIP] Current Progress

e86b1e5

[WIP] Updates to code

99a9179

[WIP] gofmt

cfb7bc0

neil-yechenwei reviewed Sep 27, 2019

View reviewed changes

azurerm/resource_arm_private_link_service.go Outdated Show resolved Hide resolved

WodansSon added 4 commits September 27, 2019 17:50

[WIP] Final changes

b2b681e

[WIP] Updated Tests and Docs

5abd85a

[WIP] Updated test cases

19c8dfd

[WIP] fix test and lint

d4bd7e4

katbyte requested changes Oct 1, 2019

View reviewed changes

tombuildsstuff modified the milestones: v1.35.0, v1.36.0 Oct 2, 2019

katbyte reviewed Oct 2, 2019

View reviewed changes

WodansSon and others added 6 commits November 18, 2019 11:47

Update website/docs/d/private_link_service.html.markdown

8301465

Co-Authored-By: kt <kt@katbyte.me>

Merge branch 'master' into nr_private-link-service

d4100c5

gofmt provider

d55f495

Merge branch 'master' of https://github.com/terraform-providers/terra…

26d0a88

…form-provider-azurerm into nr_private-link-service

Add private link service datasource

708d10c

Updates per PR comments

e05fc52

WodansSon removed the waiting-response label Nov 20, 2019

katbyte requested changes Nov 20, 2019

View reviewed changes

katbyte added the waiting-response label Nov 20, 2019

WodansSon added 2 commits November 20, 2019 14:41

Merge branch 'master' into nr_private-link-service

50467d8

Updates per PR comments

00bc22b

WodansSon removed the waiting-response label Nov 21, 2019

WodansSon added this to the v1.37.0 milestone Nov 21, 2019

tombuildsstuff modified the milestones: v1.37.0, v1.38.0 Nov 21, 2019

Added state wait code

8bc37dc

katbyte approved these changes Nov 21, 2019

View reviewed changes

WodansSon added 2 commits November 21, 2019 15:17

Fix test collision issue

29a07fd

Update data source validation

b2dec30

WodansSon modified the milestones: v1.38.0, v1.37.0 Nov 22, 2019

WodansSon merged commit a154848 into master Nov 22, 2019

WodansSon deleted the nr_private-link-service branch November 22, 2019 00:45

WodansSon added a commit that referenced this pull request Nov 22, 2019

Private Link Service [GH-4426]

3958390

ghost locked and limited conversation to collaborators Dec 22, 2019


		"location": azure.SchemaLocation(),

		"resource_group_name": azure.SchemaResourceGroupNameDiffSuppress(),

		@@ -59,6 +59,8 @@ The following arguments are supported:

		* `address_prefix` - (Required) The address prefix to use for the subnet.

		* `private_link_service_network_policies` - (Optional) Enable or Disable apply network policies on private link service in the subnet.

	func testAccAzureRMPrivateLinkServiceTemplate_complete(rInt int) string {
	func testAccAzureRMPrivateLinkService_complete(rInt int) string {

	func testAccAzureRMPrivateLinkServiceTemplate_update(rInt int) string {
	func testAccAzureRMPrivateLinkService_update(rInt int) string {

	if !utils.ResponseWasNotFound(resp.Response) {
	if if existing.ID != nil && *existing.ID != "" { {

New Resource/Data Source: azurerm_private_link_service, Data Source: azurerm_private_link_service_endpoint_connections and expose in azurerm_lb and azurerm_subnet #4426

New Resource/Data Source: azurerm_private_link_service, Data Source: azurerm_private_link_service_endpoint_connections and expose in azurerm_lb and azurerm_subnet #4426

Conversation

WodansSon commented Sep 25, 2019 • edited Loading

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

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

katbyte left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WodansSon commented Nov 21, 2019

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Nov 26, 2019

sfc-gh-emonsef commented Dec 13, 2019 • edited Loading

tombuildsstuff commented Dec 16, 2019 • edited Loading

ghost commented Dec 22, 2019

New Resource/Data Source: `azurerm_private_link_service`, Data Source: `azurerm_private_link_service_endpoint_connections` and expose in `azurerm_lb` and `azurerm_subnet` #4426

New Resource/Data Source: `azurerm_private_link_service`, Data Source: `azurerm_private_link_service_endpoint_connections` and expose in `azurerm_lb` and `azurerm_subnet` #4426

WodansSon commented Sep 25, 2019 •

edited

Loading

katbyte left a comment •

edited

Loading

sfc-gh-emonsef commented Dec 13, 2019 •

edited

Loading

tombuildsstuff commented Dec 16, 2019 •

edited

Loading