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

Feature request: add support for resource import (zones/records) #31

Closed
dannyk81 opened this issue Sep 3, 2019 · 6 comments · Fixed by #32
Closed

Feature request: add support for resource import (zones/records) #31

dannyk81 opened this issue Sep 3, 2019 · 6 comments · Fixed by #32

Comments

@dannyk81
Copy link
Contributor

dannyk81 commented Sep 3, 2019

I think it would be great to add the ability to import resources (zones and records).

I will try to provide a working PR, since in most cases this just needs a pass-through state importer definition in the schema:

		Importer: &schema.ResourceImporter{
			State: schema.ImportStatePassthrough,
		},
@dannyk81
Copy link
Contributor Author

dannyk81 commented Sep 4, 2019

Have a prototype of zone import already working

configuration:

resource "powerdns_zone" "test_com" {
  name = "test.com."
  kind = "Master"

  nameservers = formatlist("%s.", keys(local.pdns_servers))
}
$ terraform import powerdns_zone.test_com '{"name": "test.com."}'
Acquiring state lock. This may take a few moments...
powerdns_zone.test_com: Importing from ID "{\"name\": \"test.com.\"}"...
powerdns_zone.test_com: Import prepared!
  Prepared powerdns_zone for import
powerdns_zone.test_com: Refreshing state... [id=test.com.]

Import successful!

The resources that were imported are shown above. These resources are now in
your Terraform state and will henceforth be managed by Terraform.

Releasing state lock. This may take a few moments...
ck. This may take a few moments...
powerdns_zone.test_com: Importing from ID "{\"name\": \"test.com.\"}"...
powerdns_zone.test_com: Import prepared!
  Prepared powerdns_zone for import
powerdns_zone.test_com: Refreshing state... [id=test.com.]

Import successful!

state show:

terraform state show powerdns_zone.test_com
# powerdns_zone.test_com:
resource "powerdns_zone" "test_com" {
    id          = "test.com."
    kind        = "Master"
    name        = "test.com."
    nameservers = [
        "pdns-01.test.com.",
        "pdns-02.test.com.",
    ]
}

and a plan after import:

powerdns_zone.test_com: Refreshing state... [id=test.com.]

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

I'll keep working on the same for record resource and will also add tests.

@splashx
Copy link
Contributor

splashx commented Sep 4, 2019

@dannyk81 Using schema.ImportStatePassthrough wouldn’t work?

@dannyk81
Copy link
Contributor Author

dannyk81 commented Sep 4, 2019

@splashx indeed that was my initial attempt, but using schema.ImportStatePassthrough comes with a challenge: "This function requires the Read function to be able to refresh the entire resource with d.Id() ONLY." https://www.terraform.io/docs/extend/resources/import.html#importer-state-function

  1. with the zone resource, I believe I have a working solution - however it requires changes to the Read and Exists methods, I will push this soon for you to take a look at. Edit: additionally, the Read didn't fetch the zone nameservers which are required attribute for the resource, so I'm adding that as well.

  2. with the record resource, this is tricky - since we need both the Zone name and the Record ID to be able to retrieve the RR Set.

@dannyk81
Copy link
Contributor Author

dannyk81 commented Sep 4, 2019

@splashx just pushed latest changes.

  1. zone resource uses schema.ImportStatePassthrough
  2. tests for importing added to both

I think this is ready for a review, let me know what you think 😄

@dannyk81
Copy link
Contributor Author

dannyk81 commented Sep 4, 2019

@splashx have a question about https://github.com/terraform-providers/terraform-provider-powerdns/blob/3d8c32c5ffe7c6e7932886a86f94d7e51f94eef9/powerdns/resource_powerdns_record.go#L101

since this is deleting any record with an empty rrSet, shouldn't this line be d.SetId("") after a successful delete?

@splashx
Copy link
Contributor

splashx commented Sep 5, 2019

since this is deleting any record with an empty rrSet, shouldn't this line be d.SetId("") after a successful delete?

You're right here, but the fix most likely will involve touching other places so I've opened another issue to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants