Skip to content

Commit 2b6c2da

Browse files
authored
fix(acm): DnsValidatedCertificate intermittently fails with "Cannot read property 'Name' of undefined" (#18033)
There have been about a dozen reports of "Cannot read property 'Name' of undefined" errors from the `DnsValidatedCertificate` over the last two years. The most likely culprit seems to be a partial response from the ACM DescribeCertificates API, where one ResourceRecord entry is present, but not the others. Updated the wait condition to verify that all records are present. fixes #8282 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent ed19828 commit 2b6c2da

File tree

2 files changed

+123
-1
lines changed
  • packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler

2 files changed

+123
-1
lines changed

packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/lib/index.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ const requestCertificate = async function (requestId, domainName, subjectAlterna
116116
CertificateArn: reqCertResponse.CertificateArn
117117
}).promise();
118118
const options = Certificate.DomainValidationOptions || [];
119-
if (options.length > 0 && options[0].ResourceRecord) {
119+
// Ensure all records are ready; there is (at least a theory there's) a chance of a partial response here in rare cases.
120+
if (options.length > 0 && options.every(opt => opt && !!opt.ResourceRecord)) {
120121
// some alternative names will produce the same validation record
121122
// as the main domain (eg. example.com + *.example.com)
122123
// filtering duplicates to avoid errors with adding the same record

packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/test/handler.test.js

+121
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('DNS Validated Certificate Handler', () => {
4747
handler.resetSleep();
4848
handler.resetMaxAttempts();
4949
AWS.restore();
50+
nock.cleanAll();
5051
console.log = origLog;
5152
spySleep.resetHistory();
5253
});
@@ -380,6 +381,126 @@ describe('DNS Validated Certificate Handler', () => {
380381
});
381382
});
382383

384+
test('Create operation with `SubjectAlternativeNames` gracefully handles partial results from DescribeCertificate', () => {
385+
const requestCertificateFake = sinon.fake.resolves({
386+
CertificateArn: testCertificateArn,
387+
});
388+
389+
const describeCertificateFake = sinon.stub();
390+
describeCertificateFake.onFirstCall().resolves({
391+
Certificate: {
392+
CertificateArn: testCertificateArn,
393+
DomainValidationOptions: [
394+
{
395+
ValidationStatus: 'PENDING_VALIDATION',
396+
ResourceRecord: {
397+
Name: testRRName,
398+
Type: 'CNAME',
399+
Value: testRRValue
400+
}
401+
},
402+
{
403+
ValidationStatus: 'PENDING_VALIDATION',
404+
},
405+
]
406+
}
407+
});
408+
describeCertificateFake.resolves({
409+
Certificate: {
410+
CertificateArn: testCertificateArn,
411+
DomainValidationOptions: [
412+
{
413+
ValidationStatus: 'SUCCESS',
414+
ResourceRecord: {
415+
Name: testRRName,
416+
Type: 'CNAME',
417+
Value: testRRValue
418+
}
419+
},
420+
{
421+
ValidationStatus: 'SUCCESS',
422+
ResourceRecord: {
423+
Name: testAltRRName,
424+
Type: 'CNAME',
425+
Value: testAltRRValue
426+
}
427+
},
428+
]
429+
}
430+
});
431+
432+
const addTagsToCertificateFake = sinon.fake.resolves({
433+
Certificate: testCertificateArn,
434+
Tags: testTags,
435+
});
436+
437+
const changeResourceRecordSetsFake = sinon.fake.resolves({
438+
ChangeInfo: {
439+
Id: 'bogus'
440+
}
441+
});
442+
443+
AWS.mock('ACM', 'requestCertificate', requestCertificateFake);
444+
AWS.mock('ACM', 'describeCertificate', describeCertificateFake);
445+
AWS.mock('ACM', 'addTagsToCertificate', addTagsToCertificateFake);
446+
AWS.mock('Route53', 'changeResourceRecordSets', changeResourceRecordSetsFake);
447+
448+
const request = nock(ResponseURL).put('/', body => {
449+
return body.Status === 'SUCCESS';
450+
}).reply(200);
451+
452+
return LambdaTester(handler.certificateRequestHandler)
453+
.event({
454+
RequestType: 'Create',
455+
RequestId: testRequestId,
456+
ResourceProperties: {
457+
DomainName: testDomainName,
458+
HostedZoneId: testHostedZoneId,
459+
Region: 'us-east-1',
460+
Tags: testTags,
461+
}
462+
})
463+
.expectResolve(() => {
464+
sinon.assert.calledWith(requestCertificateFake, sinon.match({
465+
DomainName: testDomainName,
466+
ValidationMethod: 'DNS'
467+
}));
468+
sinon.assert.calledWith(changeResourceRecordSetsFake, sinon.match({
469+
ChangeBatch: {
470+
Changes: [
471+
{
472+
Action: 'UPSERT',
473+
ResourceRecordSet: {
474+
Name: testRRName,
475+
Type: 'CNAME',
476+
TTL: 60,
477+
ResourceRecords: [{
478+
Value: testRRValue
479+
}]
480+
}
481+
}, {
482+
Action: 'UPSERT',
483+
ResourceRecordSet: {
484+
Name: testAltRRName,
485+
Type: 'CNAME',
486+
TTL: 60,
487+
ResourceRecords: [{
488+
Value: testAltRRValue
489+
}]
490+
}
491+
}
492+
]
493+
},
494+
HostedZoneId: testHostedZoneId
495+
}));
496+
sinon.assert.calledWith(addTagsToCertificateFake, sinon.match({
497+
"CertificateArn": testCertificateArn,
498+
"Tags": testTagsValue,
499+
}));
500+
expect(request.isDone()).toBe(true);
501+
});
502+
});
503+
383504
test('Create operation fails after more than 60s if certificate has no DomainValidationOptions', () => {
384505
handler.withRandom(() => 0);
385506
const requestCertificateFake = sinon.fake.resolves({

0 commit comments

Comments
 (0)