[Terraform] DNS beta private managed_zones

[drebes]

---

[all]

Adds visibility, forwardingConfig and privateVisibilityConfig beta fields.

[terraform]

[terraform-beta]

Support for private google_dns_managed_zone, including outbound forwarding. Fixes hashicorp/terraform-provider-google#870

[ansible]

[inspec]

[rileykarson]

Hey @drebes! Do you mind adding Fixes https://github.com/terraform-providers/terraform-provider-google/issues/870 to the first comment of this PR so that merging it closes the Terraform feature request?

[drebes]

Blocked on #1051 to be merged. Currently not compiling with

$ make fmt test
==> Fixing source code with gofmt...
gofmt -w ./google-beta
==> Checking that code complies with gofmt requirements...
go test -i $(go list ./... |grep -v 'vendor') || exit 1
# github.com/terraform-providers/terraform-provider-google-beta/google-beta
google-beta/resource_dns_managed_zone.go:69:18: undefined: dnsManagedZoneNetworksSchema
make: *** [test] Error 1

[rileykarson]

Merged #1051! If you base your changes off master this will work, sorry about the extra round trip. You shouldn't need the custom set func anymore either.

It turns out your test exposes a bug in Terraform's diff/state logic (hashicorp/terraform#19658). We're hoping some Terraform 0.12 changes will fix it for us, so we'd like to keep the failing test. If it's still broken upstream by 2.0.0 time, one of us will fix the behaviour in the provider.

[danawillow]

Wouldn't the custom set func still be necessary to take care of cases where the network has the version number embedded in it?

[rileykarson]

Oh! Yup you're right.

[drebes]

I've added support to outbound forwarding now that it's in public beta. I still need to review if the new acceptance test error is related to the initial test error failing due to hashicorp/terraform#19658.

[modular-magician]

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 1343d2c) have been included in your existing downstream PRs.

[modular-magician]

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google-beta#268
depends: hashicorp/terraform-provider-google#2735

[rileykarson]

@drebes: Let me know if my reply in hashicorp/terraform#19658 is satisfying! I'm pretty confident it's an upstream issue and that it should work as we intend.

We expect fairly large under the hood changes to the Terraform provider SDK around TypeSet , and I expect that will fix the problem. Once @danawillow's review is finished, I'm perfectly happy merging this with what appears to be correct MM/downstream code with a broken downstream test in this corner case. I have hashicorp/terraform-provider-google#2671 filed against myself to verify the behaviour before we release and investigate to fix the MM-side problem if there is one, or work around the bug by changing to TypeList if not.

Let me know if either of you disagree with doing that! I think that getting the code into master will make it easier to fix the issue or share it upstream if the issue persists after the SDK upgrade.

[drebes]

I'll push the update for dnsManagedZonePrivateVisibilityConfigNetworksSchema() and wait for a decision to be made on hashicorp/terraform#19658 wether that's WAI or not. I've tried to replace the update_verb to a PUT but the update code was missing some expected fields then:

        * google_dns_managed_zone.private: Error updating ManagedZone "private-zone-fz1q7fdlvz": googleapi: Error 400: The 'entity.managedZone.name' parameter is required but was missing.
        More details:
        Reason: required, Message: The 'entity.managedZone.name' parameter is required but was missing.
        Reason: required, Message: The 'entity.managedZone.dnsName' parameter is required but was missing.
        Reason: required, Message: The 'entity.managedZone.description' parameter is required but was missing.
        Reason: required, Message: The 'entity.managedZone.id' parameter is required but was missing.
        Reason: required, Message: The 'entity.managedZone.nameServers[0]' parameter is required but was missing.
        Reason: required, Message: The 'entity.managedZone.creationTime' parameter is required but was missing.
        Reason: required, Message: The 'entity.managedZone.visibility' parameter is required but was missing.

These go beyond the updatable fields and hence are not in the generated code.

[modular-magician]

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 20d1fd0) have been included in your existing downstream PRs.

[rileykarson]

Hey @drebes! If you don't mind rebasing this, all of our existing issues in hashicorp/terraform-provider-google#2753 and there was an update to hashicorp/terraform#19658.

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. They will authorize it to run through our CI pipeline, which will generate downstream PRs.

Thanks for your contribution! A human will be with you soon.

[drebes]

I've rebased but the private zone update test is still failing, seems to still be related to hashicorp/terraform#19658.

[rileykarson]

Yep! Some related work caused the HashiCorp folks to think it might have been fixed, but it turns out that the issue is still present; hopefully it'll be resolved soon.

[drebes]

I see hashicorp/terraform#19658 has been closed, let me know when I can rebase and retry the tests.

[cemo]

Any news on this? This is so important :(

[james-oppel-exa]

Any news on this? This is so important :(

You can fork the repository, merge in the changes @drebes made, build the custom provider, and place it next to your main.tf. That's what I did. There's side effects, but I isolated this to a single module that handles private DNS and it's working fine for me.

[rileykarson]

Hey all! Sorry for the delay here. Right now, we're delaying merging this until the underlying issue (hashicorp/terraform#19658) is fixed. That's been closed, which is great news! But the bad news is that it has to be released for us to be able to pick it up, and release this resource.

That fix is part of Terraform 0.12's SDK, so we'll only be able to pick it up when it's time for us to merge that; that means around the time of 0.12-beta1. Our 2.0.0 release is looking like it will be a distinct release, and we don't have an estimate for when HashiCorp will have the SDK in a state to integrate and release with the new SDK.

In the meantime, you're absolutely correct that you could apply the changes locally / fork the provider and apply them etc, and then build from there. There's a known bug that @drebes helped uncover during specific kinds of updates, hashicorp/terraform#19658, and if that's not a problem for you that approach will work.

Sorry again for the delay here, and we'll integrate this as soon as it's possible for us to.

[bukzor]

@rileykarson There's no chance of getting this into a provider compatible with terraform 0.11? It would be better if we can work on integrating the major changes of 0.12 separately from any provider-google changes.

Is there a stable release branch that private-zone support can be backported to?

[rileykarson]

It's worth noting, 2.0.0 has been split out from the 0.12 SDK so 2.0.0 will be released with the 0.11 (and I think 0.10) SDK. This change will likely be merged a in a separate release from the 0.12 SDK, the next release after the SDK has been merged. I'll talk with the provider team about potentially merging this with the update case partially broken, given that it's under... slightly special circumstances, but I'm not sure our decision will be any different than to wait it out.

[bukzor]

Thanks for at least considering it, @rileykarson. I'm glad to know that it will be released and fixed within terraform 0.11. What exactly is the failure scenario? I was planning to go ahead and use this patch, under void warranty. We have exactly one record in our zone, so perhaps we're in the special case that can't encounter the bug?

[rileykarson]

Going from this block to the other causes Terraform to lose track of the unchanged value, and then it begins alternating which value it believes is present in config. If you use a single value or your updates replace all the values entirely, you'll be fine:

  private_visibility_config {
    networks {
      network_url = "${google_compute_network.network-1.self_link}"
    }
    networks {
      network_url = "${google_compute_network.network-3.self_link}"
    }
  }

  private_visibility_config {
    networks {
      network_url = "${google_compute_network.network-1.self_link}"
    }
    networks {
      network_url = "${google_compute_network.network-2.self_link}"
    }
  }

[drebes]

There is a possible short-term workaround if you're managing a small number of zones: creating the zone with gcloud, then importing them as a normal (public) zone and using that in Terraform to create new records (technically you could even skip the import and refer to the zone by name directly on your records sets). Record sets don't change if you're using public or private zones, they work exactly the same way.

Once we support release the beta provider with private zone support, you should be able to update your code and Terraform will start considering it a public zone.

Here's an example (proceed at your own risk):

[drebes@infra]$ gcloud config set project drebes-playground-infra-1871
Updated property [core/project].
[drebes@infra]$ gcloud compute networks list
NAME  SUBNET_MODE  BGP_ROUTING_MODE  IPV4_RANGE  GATEWAY_IPV4
prod  CUSTOM       GLOBAL
[drebes@infra]$ gcloud beta dns managed-zones create example-private --dns-name="private.example.com" --description="A private zone" --visibility=private --networks=prod
Created [https://www.googleapis.com/dns/v1beta2/projects/drebes-playground-infra-1871/managedZones/example-private].

[drebes@infra]$ terraform import google_dns_managed_zone.example-private example-private
Error: resource address "google_dns_managed_zone.example-private" does not exist in the configuration.

Before importing this resource, please create its configuration in the root module. For example:

resource "google_dns_managed_zone" "example-private" {
  # (resource arguments)
}
[drebes@infra]$ cat > private-dns.tf
resource "google_dns_managed_zone" "example-private" {
  name        = "example-private"
  dns_name    = "private.example.com."
  description = "A private zone"
}
^D
[drebes@infra]$ terraform import google_dns_managed_zone.example-private example-private
google_dns_managed_zone.example-private: Importing from ID "example-private"...
google_dns_managed_zone.example-private: Import complete!
  Imported google_dns_managed_zone (ID: example-private)
google_dns_managed_zone.example-private: Refreshing state... (ID: example-private)

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.

[drebes@infra]$ cat >> private-dns.tf

resource "google_dns_record_set" "frontend" {
  name = "frontend.${google_dns_managed_zone.example-private.dns_name}"
  type = "A"
  ttl  = 300

  managed_zone = "${google_dns_managed_zone.example-private.name}"

  rrdatas = ["172.16.1.10"]
}
^D
[drebes@infra]$ terraform plan
…
------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  + google_dns_record_set.frontend
      id:           <computed>
      managed_zone: "example-private"
      name:         "frontend.private.example.com."
      project:      <computed>
      rrdatas.#:    "1"
      rrdatas.0:    "172.16.1.10"
      ttl:          "300"
      type:         "A"


Plan: 1 to add, 0 to change, 0 to destroy.

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

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

[drebes@infra]$ terraform apply -auto-approve
…
google_dns_record_set.frontend: Creating...
  managed_zone: "" => "example-private"
  name:         "" => "frontend.private.example.com."
  project:      "" => "<computed>"
  rrdatas.#:    "" => "1"
  rrdatas.0:    "" => "172.16.1.10"
  ttl:          "" => "300"
  type:         "" => "A"
google_dns_record_set.frontend: Still creating... (10s elapsed)
google_dns_record_set.frontend: Creation complete after 11s (ID: example-private/frontend.private.example.com./A)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

[drebes@infra]$ gcloud dns record-sets list --zone=example-private
NAME                           TYPE  TTL    DATA
private.example.com.           NS    21600  ns-gcp-private.googledomains.com.
private.example.com.           SOA   21600  ns-gcp-private.googledomains.com. cloud-dns-hostmaster.google.com. 1 21600 3600 259200 300
frontend.private.example.com.  A     300    172.16.1.10

And once terraform-provider-google-beta is launched with private zone support (this PR):

[drebes@infra]$ vi private-dns.tf 
resource "google_dns_managed_zone" "example-private" {
  provider    = "google-beta"
  name        = "example-private"
  dns_name    = "private.example.com."
  description = "A private zone"
  visibility  = "private"

  private_visibility_config {
    networks {
      network_url = "https://www.googleapis.com/compute/v1/projects/drebes-playground-infra-1871/global/networks/prod"
    }
  }
}

resource "google_dns_record_set" "frontend" {
  name = "frontend.${google_dns_managed_zone.example-private.dns_name}"
  type = "A"
  ttl  = 300

  managed_zone = "${google_dns_managed_zone.example-private.name}"

  rrdatas = ["172.16.1.10"]
}
[drebes@infra]$ terraform plan
…

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

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

[rileykarson]

I talked through this with the rest of the maintainers, and given that we still don't have a timeline for when 0.12's SDK update will be released, we're comfortable with merging this as-is.

A similar issue is present in an existing resource (hashicorp/terraform-provider-google#1696) and the impact seems to be low; we'll include workaround instructions in the field's documentation.

@drebes: Once the proposed description has been added & the downstreams generated I'll merge them + this upstream PR. This resource will be present in the 2.1.0 release.

[modular-magician]

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 9bfc274) have been included in your existing downstream PRs.