Skip to content

Commit

Permalink
Merge pull request #1072 from guardian/aa-validate-cname
Browse files Browse the repository at this point in the history
fix: Enforce a CNAME DNS entry has a single answer
  • Loading branch information
akash1810 authored Feb 3, 2022
2 parents 1bf660d + 3681aac commit 9ad3b0d
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 9ad3b0d

Please sign in to comment.