-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
[WIP] provider/aws: aws_route53_vpc_association_authorization resource #12553
Conversation
Adds the aws_route53_vpc_association_authorization resource which allows you to create cross account route53 zone associations.
Not sure if it is possible to create cross account tests yet, saw that it was an issue in #11505 as well. |
Right now getting this working with aws_route53_zone_association is blocked by a reliable way to retrieve a list of hosted zones associations when you don't have access to the zone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking the aws_route53_vpc_association_authorization and aws_route53_zone_association resources.
|
||
resource "aws_route53_zone_association" "foobar" { | ||
zone_id = "${aws_route53_zone.foo.id}" | ||
vpc_id = "${aws_vpc.peer.id}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is needed/required that the association comes shortly after the authorisation. I'm not fully clear how long, but I did some tests using the aws
command to do the authorisation and then run TF to do the association. It can't have been more than half a minute or so between the two, but very often the association failed. Making sure the time between the two was less than half a minute (a few seconds perhaps, five-ten is a guesstimate), then it worked.
So I suggest a auth_id = "${aws_route53_vpc_association_authorization.peer.id}"
to "link" the two resources in a way that TF creates the authorisation "just before" doing the association. I'm not sure a depends_on
is enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I ran into this but if that's true I feel like it should be fixed by AWS (or at least documented so we understand the issue). If we end up tying the two resources together it would prevent you from using it when you don't have access to both accounts. This is also why I mentioned the blocker above, if we wanted to require access to both accounts we could work around it. I feel like that would be a fairly big limitation though considering a multi account setup is usually for isolation.
I personally don't need this resource anymore, I would still like to finish it up if I get time but right now I'm not sure when that will be. If anyone is interested in this feel free to take over.
I've applied this PR onto latest master, rebuilt and installed. Now I'm trying to modify my manifest to use
Gives me:
And if I add a
I'm the admin, with access to everything (as far as I know)... If I list the request (with |
resource "aws_route53_vpc_association_authorization" "peer" { | ||
zone_id = "${aws_route53_zone.foo.id}" | ||
vpc_id = "${aws_vpc.peer.id}" | ||
region = "us-east-2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that supposed to fail?
* module.mymodule.aws_route53_vpc_association_authorization.local2remote: : invalid or unknown key: region
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be using option vpc_region
according to the code: https://github.com/hashicorp/terraform/pull/12553/files#diff-4d2ea30d2ceccbd84af4d620b8533ab3R32
resource "aws_route53_zone_association" "foobar" { | ||
zone_id = "${aws_route53_zone.foo.id}" | ||
vpc_id = "${aws_vpc.peer.id}" | ||
region = "us-east-2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto:
* module.mymodule.aws_route53_zone_association.local2remote: : invalid or unknown key: region
@RyanJarv Any news on getting this finished? |
I'll try to take a look at this again soon. Right now I'm thinking we would want to try to implement this using access to both accounts but just make sure we can easily break it up later if/when AWS supports verifying the Route53 server for a VPC from the client account only. |
@RyanJarv I guess the way providers work in Terraform, it's not possible to use an account that can assume the role of the other account for this? Also, I guess this wouldn't help in the case where allowing that access isn't feasible? |
That's a good question, will have to look into that.
Ok typed the paragraph below out but then realized that didn't really address this question. Yeah pretty much, but thinking about this some more I'm wondering how much this matters. Mainly I wanted the access just to verify the current state of the association from the client account, not necessarily to create the association since you can configure different providers to be used with specific resources in the tf config (https://www.terraform.io/docs/providers/aws/r/vpc_peering_accepter.html would be an good example of that). For the case where access to the other account isn't available I was thinking that it would lead to situations where the client doesn't know the real state of the route53 association. So I'm wondering if it's enough to just have it stored in the state file and accept that you may have to make manual changes to the state in certain cases, like if the authorization expired before the association was made (not sure if that's possible), or if the association was removed on the route53 side and we wanted to recreate the authorization (assuming the first one isn't valid anymore). Would be curious if any one else has any thoughts on this or if other resources have the same issue (this is the first one I've worked on). @FransUrbo Cool, yeah sorry didn't get a chance to see how you did this before. Do you mind making a PR to my branch or just pasting a link to something you have that is working? |
@RyanJarv I guess part of the issue is philosophical, I don't know tf well enough to know whether using assume role is something that's typically done. As a user, I personally would find it easier to grant those permissions, use the same account for auth, and provide some way of providing some detail (an account number or ARN, presumably) that would let a single provider handle the association end-to-end. Or maybe just let you specify a second AWS provider and account number in the same resource? But I don't know if setting up a second provider and using two separate resources is a more 'Terraformish' solution. In my limited testing using the CLI to do similar associations, if there's already a pending association that wasn't authorized, that can cause issues. So may be worth making sure to test what happens if the authorization is created multiple times without the association being made. |
The assume role part is already handled by the AWS provider (you can just specify the role and source account instead of keys directly), so the main difference here would be just having a single resource handle the functions of route53_vpc_association_authorization and route53_vpc_association. From my experience the provider connection info seems to be kinda transparent to the provider so we would have to look into if managing two different contexts in the same resource is something that makes sense. This also wouldn't be consistent with how it's handled with the vpc_peering_accepter resource but other then those to things I think this would work. |
If I understand the documentation for CreateVPCAssociationAuthorization correctly, it NEVER (!!) expires the request... It say several times in the documentation that "if you want to delete the authorisation request, delete it by ....". |
@RyanJarv There's no need for a PR. TF will take care of all of this automatically. I just put a
The first one succeeded just fine, but not the second one. Maybe because TF didn't understand the auth request (or didn't find it, not sure what). (update) This is the exact message I get when applying the code with those two resources:
So your new resource works perfectly with (update) You say it yourself in the PR intro: (update) So I've just proven that you can tick of that last TODO: |
Ok so if the authorization never expires I think the only gotcha to what we have now is if the authorization is modified out side of terraform. Assuming that's the only issue I think ideally we would want to detect that but probably fine to just document it for now. When I made this originally we didn't have a way to do cross account unit testing which is what that last check box is about, not sure if that's changed since. |
@RyanJarv I think "they" do that for VPC Peering. I think I remember I created an issue about that, and when it was updated (the issue/PR), there where tests for cross-account VPC peering added. Can't remember the issue number though.. |
Is this likely to progress? I would be really happy to have a vpc_association_authorization resource! I already use the peering resources cross account and have a module that takes advantage of being able to create an additional provider to accept the connection in the other account. It sounds like this would work the same way. Currently I have to use a provisioner which calls the AWS CLI and I'd really like to be able to remove that dependency. 😄 |
@antonosmond Curious, would pushing this forward in the current state with documentation and/or a warning about the incompatibilities of aws_route53_vpc_association_authorization and aws_route53_zone_association address your use case? (see hashicorp/terraform-provider-aws#384) |
@RyanJarv if we could create the authorization & association on first apply using this then we would probably make it a module which we can target once to set it up and wouldn't need to re-run it so the fact that subsequent apply's would fail wouldn't really matter. Obviously it'd be great if they worked, but I think it'd still be useful to us in it's current state. |
@antonosmond Correct me if I'm wrong but I think you may be confused about the current state. This works however there would be no way to currently use aws_route53_zone_association with it. If you did use this the association would need to be external to terraform because the aws_route53_zone_association resource makes the assumption the VPC and Zone are owned by the same account. |
Aaaa ok thanks for the clarification @RyanJarv. I think it'd still be useful - I guess I could use a provisioner which does the association so if the authorization was already done by terraform it'd be one less thing for the provisioner to do. |
Opened a PR for this on the new terraform-providers repo: hashicorp/terraform-provider-aws#2005 |
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. |
Description
Adds the aws_route53_vpc_association_authorization resource which allows
you to create cross account route53 zone associations.
Related Issues
#10208
TODO