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

fix(certificate-manager): Wait for all subject alternative resource records to be available. #7996

Closed
wants to merge 1 commit into from

Conversation

magJ
Copy link

@magJ magJ commented May 15, 2020

Commit Message

Wait for all subject alternative resource records to be available.

The DNS validated certificate handler, only waits for the first certificate ResourceRecord to be available.
This can be a problem when using subject alternative names, as the resource records arent necessarily all made available at the same time.

Thus we must wait for them all to be provided, before proceeding.

End Commit Message

Fixes #7995


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@mergify
Copy link
Contributor

mergify bot commented May 15, 2020

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@magJ magJ changed the title Wait for all subject alternative resource records to be available. fix: Wait for all subject alternative resource records to be available. May 15, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: d4d9ccb
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@SomayaB SomayaB changed the title fix: Wait for all subject alternative resource records to be available. fix(certificate-manager): Wait for all subject alternative resource records to be available. May 15, 2020
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @magJ !

Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build failure is because the body of the custom resource changed, and thus the names of the parameters representing the assets for the custom resource also changed in the @aws-cdk/aws-ecs module's tests (they are named based on the hash of the source of the custom resource).

Can you update those @magJ ? I'll approve the change after the build passes.

Thanks again!

jogold added a commit to jogold/aws-cdk that referenced this pull request Jun 15, 2020
…icate

Automatically adding Amazon Route 53 CNAME records for DNS validation is
now natively supported by CloudFormation.

Add a `validation` prop to `Certificate` to handle both email and DNS
validation. Deprecate `DnsValidatedCertificate`.

The default remains email validation (non-breaking).

Closes aws#5831
Closes aws#5835
Closes aws#6081
Closes aws#6516
Closes aws#7150
Closes aws#7941
Closes aws#7995
Closes aws#7996
jogold added a commit to jogold/aws-cdk that referenced this pull request Jun 15, 2020
…cate

Automatically adding Amazon Route 53 CNAME records for DNS validation is
now natively supported by CloudFormation.

Add a `validation` prop to `Certificate` to handle both email and DNS
validation. Deprecate `DnsValidatedCertificate`.

The default remains email validation (non-breaking).

Closes aws#5831
Closes aws#5835
Closes aws#6081
Closes aws#6516
Closes aws#7150
Closes aws#7941
Closes aws#7995
Closes aws#7996
@magJ
Copy link
Author

magJ commented Jun 19, 2020

I'll just close this in favor of #8552, getting the entire build/tests of this project working locally takes a lot of screwing around, and the docker build doesn't seem to be maintained(there are numerous issues open for this).

@magJ magJ closed this Jun 19, 2020
mergify bot pushed a commit that referenced this pull request Jul 10, 2020
…cate (#8552)

Automatically adding Amazon Route 53 CNAME records for DNS validation is
now natively supported by CloudFormation.

Add a `validation` prop to `Certificate` to handle both email and DNS
validation. `DnsValidatedCertificate` is now only useful for cross-region
certificate creation.

The default remains email validation (non-breaking).

Closes #5831
Closes #5835
Closes #6081
Closes #6516
Closes #7150
Closes #7941
Closes #7995
Closes #7996
Closes #8282 
Closes #8659
Closes #8783

----

*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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DNS validated certificate handler sometimes fails with subject alternative names.
3 participants