Add DNS alias support #5133
Add DNS alias support #5133
Conversation
matt-FFFFFF
commented
Dec 11, 2019
matt-FFFFFF
commented
- Update DNS SDK to 2018-05-01
- A, AAAA & CNAME record support for aliases
- New acceptance tests to cover aliases
- Update docs
* Update DNS SDK to 2018-05-01 * A and AAAA record support for aliases * Add missing } * Update modules.txt and fix imports for dns tests * Fic incorrect function name * AAAA testacc pass! + improve resource read code: * A * CNAME * AAAA * Add CNAME tests * Change CNAME tests to Zone Record Set * Update docs
There was a problem hiding this comment.
Hey @matt-FFFFFF! Thanks for opening this PR it's mostly good with just a couple of minor changes that we can make use of. I've called out a few things in one file but please mirror those changes across the rest of the files if it makes sense to do
| @@ -92,13 +100,24 @@ func resourceArmDnsARecordCreateUpdate(d *schema.ResourceData, meta interface{}) | |||
|
|
|||
| ttl := int64(d.Get("ttl").(int)) | |||
| t := d.Get("tags").(map[string]interface{}) | |||
| targetResourceId := d.Get("target_resource_id").(string) | |||
|
|
|||
| if bool(targetResourceId == "") && bool(len(d.Get("records").(*schema.Set).List()) == 0) { | |||
There was a problem hiding this comment.
This shouldn't need to be specified here since this logic is defined in the schema with:
ConflictsWith: []string{"target_resource_id"},
Also, do you need to specify at least one of these for this resource? If so, you can swap ConflictsWith with ExactlyOneOf which says one and only one of these keys must be specified. It'll look like
ExactlyOneOf: []string{"records", "target_resource_id"}
There was a problem hiding this comment.
I was confused slightly here, then I realised I had to update the plugin-sdk to 1.4.0.
|
hey @matt-FFFFFF Thanks for this PR :) There's some major refactoring coming to the codebase over the next couple of days - so that we can get this PR in prior to those changes, I hope you don't mind but I'm going to push a couple of commits to fix up the comments so that we can merge this. Thanks! |
|
hey @matt-FFFFFF Since this PR is coming from your master branch, unfortunately we're unable to push to that remote - meaning that in order to get this in I've had to take these commits into a new branch/PR to be able to fix this. As such whilst I'd like to thank you for this contribution (and ordinarily we would wait for PR feedback to be addressed) - since we need to get this merged prior to the refactor I hope you don't mind, but I'm going to close this PR in favour of #5218 Thanks! |
|
This has been released in version 1.40.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.40.0"
}
# ... other configuration ... |
|
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! |
ghost
locked and limited conversation to collaborators
