Skip to content

Commit

Permalink
fix: Enforce a CNAME DNS entry has a single answer
Browse files Browse the repository at this point in the history
There are currently two ways to create `Guardian::DNS::RecordSet` resource:
  1. `GuCname`
  2. `GuDnsRecordSet`

A CNAME should not return multiple answers without "the correct filters in place (such as SELECT_FIRST_N 1) to limit them to a single answer at resolution time".

`Guardian::DNS::RecordSet` does not create any filters (and it's unlikely to).

The props of `GuCname` enforce this single answer restriction,
however if you're still able to use `GuDnsRecordSet` to create a CNAME that violates this restriction.

In this change we throw if a CNAME is being created with multiple answers.
  • Loading branch information
akash1810 committed Feb 3, 2022
1 parent 1bf660d commit 3681aac
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 5 deletions.
15 changes: 15 additions & 0 deletions src/constructs/dns/dns-records.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ describe("The GuDnsRecordSet construct", () => {
});
expect(stack).toHaveResourceOfTypeAndLogicalId("Guardian::DNS::RecordSet", "ThisExactLogicalId");
});

it("should throw if a CNAME is created with multiple answers", () => {
const stack = simpleGuStackForTesting();

expect(() => {
new GuDnsRecordSet(stack, "ThisExactLogicalId", {
name: "banana.example.com",
recordType: RecordType.CNAME,
resourceRecords: ["apple.example.com", "banana.example.com"],
ttl: Duration.hours(1),
});
}).toThrowError(
"According to RFC, a CNAME record should not return multiple answers. Doing so may cause problems during resolution."
);
});
});

describe("The GuCname construct", () => {
Expand Down
31 changes: 26 additions & 5 deletions src/constructs/dns/dns-records.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,37 @@ export interface GuDnsRecordSetProps {
*/
export class GuDnsRecordSet {
constructor(scope: GuStack, id: string, props: GuDnsRecordSetProps) {
const { name, recordType, resourceRecords, ttl } = props;
const { stage } = scope;

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- more `RecordType`s will be added soon!
if (recordType === RecordType.CNAME) {
/*
If you try to create a CNAME with multiple records within NS1, you are greeted with:
According to RFC, a CNAME record should not return multiple answers.
Doing so may cause problems during resolution.
If you want to use multiple answers, you should ensure you have the correct filters in place (such as SELECT_FIRST_N 1) to limit them to a single answer at resolution time.
`Guardian::DNS::RecordSet` does not implement "correct filters", so fail fast by throwing.
*/
if (resourceRecords.length !== 1) {
throw new Error(
"According to RFC, a CNAME record should not return multiple answers. Doing so may cause problems during resolution."
);
}
}

// The spec for this private resource type can be found here:
// https://github.com/guardian/cfn-private-resource-types/tree/main/dns/guardian-dns-record-set-type/docs#syntax
new CfnResource(scope, id, {
type: "Guardian::DNS::RecordSet",
properties: {
Name: props.name,
ResourceRecords: props.resourceRecords,
RecordType: props.recordType,
TTL: props.ttl.toSeconds(),
Stage: scope.stage,
Name: name,
ResourceRecords: resourceRecords,
RecordType: recordType,
TTL: ttl.toSeconds(),
Stage: stage,
},
});
}
Expand Down

0 comments on commit 3681aac

Please sign in to comment.