Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_private_endpoint - expose private_dns_zone_group, private_dns_zone_configs, and custom_dns_configs #7246

Merged
merged 18 commits into from Jun 18, 2020

Conversation

WodansSon
Copy link
Collaborator

(fixes #6571)

@ghost ghost added the size/XL label Jun 8, 2020
@WodansSon WodansSon changed the title [WIP] azurerm_private_endpoint - expose custom_dns_configs [WIP] azurerm_private_endpoint - expose custom_dns_configs add New Resource: azurerm_private_dns_zone_group Jun 8, 2020
@WodansSon WodansSon changed the title [WIP] azurerm_private_endpoint - expose custom_dns_configs add New Resource: azurerm_private_dns_zone_group [WIP] azurerm_private_endpoint - expose private_dns_zone_group, private_dns_zone_configs, and custom_dns_configs and Jun 11, 2020
@WodansSon WodansSon added this to the v2.14.0 milestone Jun 11, 2020
@ghost ghost added size/XXL and removed size/XL labels Jun 11, 2020
@WodansSon
Copy link
Collaborator Author

image

@WodansSon WodansSon changed the title [WIP] azurerm_private_endpoint - expose private_dns_zone_group, private_dns_zone_configs, and custom_dns_configs and azurerm_private_endpoint - expose private_dns_zone_group, private_dns_zone_configs, and custom_dns_configs and Jun 11, 2020
@WodansSon WodansSon changed the title azurerm_private_endpoint - expose private_dns_zone_group, private_dns_zone_configs, and custom_dns_configs and azurerm_private_endpoint - expose private_dns_zone_group, private_dns_zone_configs, and custom_dns_configs Jun 11, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v2.14.0, v2.15.0 Jun 11, 2020
@tombuildsstuff tombuildsstuff self-requested a review June 11, 2020 08:52
@tombuildsstuff tombuildsstuff removed this from the v2.14.0 milestone Jun 11, 2020
@tombuildsstuff tombuildsstuff added this to the v2.15.0 milestone Jun 11, 2020
Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partial review as discussed offline

Name: id.Path["privateDnsZoneGroups"],
ResourceGroup: id.ResourceGroup,
ID: input,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use the PopSegment and ValidateNoEmptySegments functions here to ensure this is the ID of a private endpoint and not something else

Copy link
Collaborator Author

@WodansSon WodansSon Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly Fixed. I can't use ValidateNoEmptySegments because I am parsing the information for multiple resources from one resource ID(e.g. /subscriptions/XXXX/resourceGroups/jcline-privateDns-rg/providers/Microsoft.Network/privateEndpoints/contoso-cosmosdb/privateDnsZoneGroups/privatelink.postgres.database.azure.com2/privateDnsZoneConfigs/finance-contoso-com.

return privateDnsZoneGroup, nil
}

func PrivateDnsZoneResourceIDs(input []interface{}) ([]NameResourceGroup, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same here)

return results, nil
}

func PrivateEndpointResourceID(input string) (NameResourceGroup, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same here)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Name: id.Path["privateDnsZones"],
ResourceGroup: id.ResourceGroup,
ID: v,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same here)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

}

privateDnsZone := NameResourceGroup{
Name: id.Path["privateDnsZones"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we pull this out into a separate ParseDNSZoneResourceID method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Required: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: azure.ValidateResourceID,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which then means we can validate this is a Private DNS Zone ID here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

}
if resp.ID == nil || *resp.ID == "" {
return fmt.Errorf("API returns a nil/empty id on Private Endpoint %q (Resource Group %q): %+v", name, resourceGroup, err)
}
d.SetId(*resp.ID)

// now create the dns zone group
// first I have to see if the dns zone group exists, if it does I need to delete it an re-create it because you can only have one per private endpoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens to traffic when this is deleted, is this dropped? if so - should we be locating the existing one and updating instead?

Copy link
Collaborator Author

@WodansSon WodansSon Jun 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only delete if the name has changed, else I will need to force new the attribute which would delete the entire private endpoint. Once the private DNS zone group is deleted the traffic is automatically re-routed to the existing private endpoint FQDN which is displayed via the custom DNS configs attribute.

WodansSon and others added 5 commits June 15, 2020 21:00
Co-authored-by: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-authored-by: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-authored-by: Tom Harvey <tombuildsstuff@users.noreply.github.com>
@WodansSon
Copy link
Collaborator Author

=== RUN TestAccAzureRMPrivateEndpoint_basic
=== PAUSE TestAccAzureRMPrivateEndpoint_basic
=== RUN TestAccAzureRMPrivateEndpoint_updateTag
=== PAUSE TestAccAzureRMPrivateEndpoint_updateTag
=== RUN TestAccAzureRMPrivateEndpoint_requestMessage
=== PAUSE TestAccAzureRMPrivateEndpoint_requestMessage
=== RUN TestAccAzureRMPrivateEndpoint_privateDnsZoneGroup
=== PAUSE TestAccAzureRMPrivateEndpoint_privateDnsZoneGroup
=== RUN TestAccAzureRMPrivateEndpoint_privateDnsZoneRename
=== PAUSE TestAccAzureRMPrivateEndpoint_privateDnsZoneRename
=== RUN TestAccAzureRMPrivateEndpoint_privateDnsZoneUpdate
=== PAUSE TestAccAzureRMPrivateEndpoint_privateDnsZoneUpdate
=== RUN TestAccAzureRMPrivateEndpoint_privateDnsZoneRemove
=== PAUSE TestAccAzureRMPrivateEndpoint_privateDnsZoneRemove
=== CONT TestAccAzureRMPrivateEndpoint_basic
=== CONT TestAccAzureRMPrivateEndpoint_privateDnsZoneRename
=== CONT TestAccAzureRMPrivateEndpoint_privateDnsZoneRemove
=== CONT TestAccAzureRMPrivateEndpoint_privateDnsZoneUpdate
=== CONT TestAccAzureRMPrivateEndpoint_requestMessage
=== CONT TestAccAzureRMPrivateEndpoint_privateDnsZoneGroup
=== CONT TestAccAzureRMPrivateEndpoint_updateTag
--- PASS: TestAccAzureRMPrivateEndpoint_basic (179.87s)
--- PASS: TestAccAzureRMPrivateEndpoint_updateTag (237.52s)
--- PASS: TestAccAzureRMPrivateEndpoint_requestMessage (255.87s)
--- PASS: TestAccAzureRMPrivateEndpoint_privateDnsZoneGroup (313.20s)
--- PASS: TestAccAzureRMPrivateEndpoint_privateDnsZoneRename (362.99s)
--- PASS: TestAccAzureRMPrivateEndpoint_privateDnsZoneRemove (390.92s)
--- PASS: TestAccAzureRMPrivateEndpoint_privateDnsZoneUpdate (485.42s)
PASS
ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/tests 485.510s

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aside from one comment LGTM 👍

Comment on lines 550 to 551
result["id"] = *input.ID
result["name"] = *input.Name
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should nil check these?

@WodansSon
Copy link
Collaborator Author

image

@WodansSon WodansSon merged commit ce3eb26 into master Jun 18, 2020
@WodansSon WodansSon deleted the e_privateEndpoint_privateDnsZoneConfigs branch June 18, 2020 20:58
WodansSon added a commit that referenced this pull request Jun 18, 2020
@ghost
Copy link

ghost commented Jun 19, 2020

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
Copy link

ghost commented Jul 19, 2020

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!

@hashicorp hashicorp locked and limited conversation to collaborators Jul 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for list of private_ip_address attributes on azurerm_private_endpoint resource
3 participants