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

Expose Route53 zone nameservers for parent zone NS record #1525

Merged
merged 7 commits into from
Apr 22, 2015

Conversation

lamdor
Copy link
Contributor

@lamdor lamdor commented Apr 14, 2015

No description provided.

@knuckolls
Copy link
Contributor

👍 lgtm

@@ -90,6 +96,8 @@ func resourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) error
return err
}

d.Set("delegation_set_name_servers", zone.DelegationSet.NameServers)
Copy link
Contributor

Choose a reason for hiding this comment

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

d.Set can return an error, can you check to make sure it's nil? We've seen bugs go by silently by not checking for errors when setting a non-primitive type.

if err := d.Set("delegation_set_name_servers", zone.DelegationSet.NameServers); err != nil{

or similar, like in https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_route53_record.go#L199

@@ -90,6 +97,11 @@ func resourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) error
return err
}

ns := zone.DelegationSet.NameServers
Copy link
Contributor

Choose a reason for hiding this comment

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

We should sort this. Or, use a set.

@pmoust
Copy link
Contributor

pmoust commented Apr 14, 2015

This closes #462, another WIP was #1214

@catsby
Copy link
Contributor

catsby commented Apr 14, 2015

I just closed #1525

@lamdor
Copy link
Contributor Author

lamdor commented Apr 15, 2015

All changes have been made. cc @catsby @mitchellh @pmoust

--- PASS: TestAccRoute53Zone (135.52s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    135.539s

@lamdor
Copy link
Contributor Author

lamdor commented Apr 15, 2015

hold off on this. there's one more change i think i need to make.

@lamdor
Copy link
Contributor Author

lamdor commented Apr 15, 2015

The problem I'm running into is dealing with the computed attribute. I want to modify another record with the name servers (basically an NS record on the "parent" zone). But I can't for the life of me figure out how to get it to resolve right.

So I have this configuration (simplified a bit):

variable "parent_route53_zone_id" { }

provider "aws" {
  region = "us-east-1"
}

resource "aws_route53_zone" "foo" {
  name = "foo.bar.com"
}

resource "aws_route53_record" "ns_in_bar" {
  zone_id = "${var.parent_route53_zone_id}"
  name = "bar.com"
  type = "NS"
  ttl = "30"
  records = [
    "${aws_route53_zone.foo.name_servers.*}"
  ]
}

I've also tried it with just an echo in a null_resource like:

resource "null_resource" "echo_ns" {
  provisioner "local-exec" {
    command = "echo ${join("--", aws_route53_zone.bar.name_servers.*)}"
  }
}

But I keep getting: * Resource 'aws_route53_zone.bar' does not have attribute 'name_servers.*' for variable 'aws_route53_zone.bar.name_servers.*

I then tried to see if I gave the name_servers more structure if they'd work. I changed the schema to:

            "name_servers": &schema.Schema{
                Type:     schema.TypeSet,
                Computed: true,
                Elem: &schema.Resource{
                    Schema: map[string]*schema.Schema{
                        "name": &schema.Schema{
                            Type: schema.TypeString,
                        },
                    },
                },
                Set: func(v interface{}) int {
                    m := v.(map[string]interface{})
                    return hashcode.String(m["name"].(string))
                },
            },

and set the resource data correctly with map's instead.

I then changed the configuration to:

variable "parent_route53_zone_id" { }

provider "aws" {
  region = "us-east-1"
}

resource "aws_route53_zone" "foo" {
  name = "foo.bar.com"
}

resource "aws_route53_record" "ns_in_bar" {
  zone_id = "${var.parent_route53_zone_id}"
  name = "bar.com"
  type = "NS"
  ttl = "30"
  records = [
    "${aws_route53_zone.foo.name_servers.*.name}"
  ]
}

Still no dice: * Resource 'aws_route53_zone.bar' does not have attribute 'name_servers.*.name' for variable 'aws_route53_zone.bar.name_servers.*.name for both the ns record and the null_resource.

Any ideas on how I can make this work?

@mitchellh
Copy link
Contributor

LGTM. @rubbish That's not currently possible, though I think this should work: ${join("--", aws_route53_zone.foo.nameservers)}

mitchellh added a commit that referenced this pull request Apr 22, 2015
providers/aws: Expose Route53 zone nameservers for parent zone NS record
@mitchellh mitchellh merged commit 8ca42f8 into hashicorp:master Apr 22, 2015
@ghost
Copy link

ghost commented May 3, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

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

Successfully merging this pull request may close these issues.

5 participants