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

aws_route53: Allow updates to existing records in a single operation #26754

Open
1 of 2 tasks
christow-aws opened this issue Aug 14, 2023 · 2 comments
Open
1 of 2 tasks
Labels
@aws-cdk/aws-route53 Related to Amazon Route 53 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p3

Comments

@christow-aws
Copy link

christow-aws commented Aug 14, 2023

Describe the feature

Currently, in order to update a Route53 record's routing policy using CDK (which uses an underlying lambda to process events), we have to do multiple passes at a record's fields over the course of multiple deployments. This operation is supported in the console with 1 click. For example, we have to do the following:

(initial code - uses simple routing policy)

const aRecord = new ARecord(this, 'RecordSet', {
    target: RecordTarget.fromAlias(new ApiGatewayDomain(domain)),
    zone: hostedZone,
})

(first pass - changing routing policy to weighted)

// Delete this for round 2 of deployments
const aRecord = new ARecord(this, 'RecordSet', {
    target: RecordTarget.fromAlias(new ApiGatewayDomain(domain)),
    zone: hostedZone,
})

const aRecord2 = new ARecord(this, 'RecordSet2', {
    target: RecordTarget.fromAlias(new ApiGatewayDomain(domain)),
    zone: hostedZone,
    deleteExisting: true, // Delete this for round 2 of deployments
})
const recordSet = aRecord2.node.defaultChild as CfnRecordSet
recordSet.weight = 100
recordSet.setIdentifier = 'API-Gateway-A-Record-weighted'
aRecord2.node.addDependency(aRecord)

(second pass - cleanup)

const aRecord2 = new ARecord(this, 'RecordSet2', {
    target: RecordTarget.fromAlias(new ApiGatewayDomain(domain)),
    zone: hostedZone,
})
const recordSet = aRecord2.node.defaultChild as CfnRecordSet
recordSet.weight = 100
recordSet.setIdentifier = 'API-Gateway-A-Record-weighted'

Use Case

Modifying record routing policies.

Proposed Solution

Involves updating the custom resource lambda, of which I haven't poked around in.

Other Information

We're also required to use the deleteExisting field, which theoretically isn't necessary (but is super dangerous). R53 appears to allow the entire operation to be done at once (rm old, insert new) with validation at API-call-time that the new, to-be-inserted is valid before deleting the old one. This means that there shouldn't be a case where the existing record is deleted and the new one cannot be inserted.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2, TypeScript

Environment details (OS name and version, etc.)

AL2 on x86

@christow-aws christow-aws added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 14, 2023
@github-actions github-actions bot added the @aws-cdk/aws-route53 Related to Amazon Route 53 label Aug 14, 2023
@pahud pahud self-assigned this Aug 14, 2023
@pahud
Copy link
Contributor

pahud commented Aug 14, 2023

Yes we probably should create some patterns in aws-route53-patterns to address that.

Check out this workaround if you need to create RecordSets for weighted ARecords.

Let me know if this sample works for you.

export class DemoStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const hostedZone = new route53.HostedZone(this, 'Zone', { zoneName: 'pahuddemo.com' });

    const recordWeight: { ip: string, weight: number }[] = [
      {ip: '1.2.3.4', weight: 10},
      {ip: '5.6.7.8', weight: 20},
    ]
    
    recordWeight.forEach(rs => {
      this.createdWeightedRecordSet(rs.ip, rs.weight, hostedZone, 'test');
    })
  }
  private createdWeightedRecordSet(ip: string, weight: number, hostedZone: route53.HostedZone, recordName: string) {
    const arecord = new route53.ARecord(this, `RecordSet${weight}`, {
      target: route53.RecordTarget.fromIpAddresses(ip),
      zone: hostedZone,
      recordName,
    })
    const cfnRecordSet = arecord.node.tryFindChild('Resource') as route53.CfnRecordSet
    cfnRecordSet.addPropertyOverride('Weight', weight);
    cfnRecordSet.addPropertyOverride('SetIdentifier', `RecordSet${weight}`);
  }
}

@pahud pahud added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 14, 2023
@pahud pahud removed their assignment Aug 14, 2023
@christow-aws
Copy link
Author

christow-aws commented Aug 14, 2023

Sorry, the example given isn't really a workaround for this issue. Or, if it is, I don't get it. It seems to be a minimal example of how to create a weighted record, which is well understood and not the purpose of this issue. (Also, for future reference, SetIdentifier needs to be unique per record of same type and routing policy, and the way the example has it seeded means that there couldn't be two records with the same weight, even if they pointed to different resources. That's perfectly legal, and normal as far as use cases go. Pretty sure the ARecord node name when new is called would also cause problems due to repeats.)

Back to the topic at hand...
This won't work in cases that a record of the same type with the same name already exists, and you want to change the routing policy. For example, going from simple routing to weighted.

The custom resource lambda seems to try to create the new record in one action, and delete the old one in another. I say "try" because this doesn't work in practice. R53 barks that a record of the same type, name, and routing policy already exists, and there can't be another that differs in routing policies. This behavior should actually be all-at-once. Deleting a record in one operation and recreating in another introduces an availability risk, even if it's done within the TTL of the record. Some clients may expire during the cutover, or there may be new clients that don't have the DNS record cached.

In the console, an API call appears to be made that deletes the old record AND creates the new one in one go. So, unlike the lambda which has atomic operations (one thing at a time, for sure), the console uses an atomic transaction (do everything I ask, or none of it). This removes the availability risk, as the work is done all at once, so any resolved DNS calls either point to the old resource before the update, or the new one after it is complete. Yay for atomic transactions!

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Aug 15, 2023
@pahud pahud added p3 and removed p2 labels Jun 11, 2024
lukepafford pushed a commit to lukepafford/aws-cdk that referenced this issue Aug 23, 2024
\### Issue # (if applicable)

Closes aws#15213
Addresses aws#26754

\### Reason for this change

\### Description of changes

\### Description of how you validated changes

\### Checklist

- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

---

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-route53 Related to Amazon Route 53 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p3
Projects
None yet
Development

No branches or pull requests

2 participants