fix(cloudfront): requirement of domainNames prevents moving a domain name between distributions #29329
Conversation
github-actions
bot
added
the
valued-contributor
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
|
Exemption Request There are no integ tests for a distribution with a certificate. |
aws-cdk-automation
added
pr-linter/exemption-requested
|
@Tietew We are keen to see this change completed - is there anything we can do? It looks like it's waiting on a reviewer. |
|
@Tietew Could you attach a screenshot of this behavior? Given there are no integ tests with certificates. |
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
paulhcsun
added
the
pr-linter/exempt-integ-test
paulhcsun
added
pr-linter/do-not-close
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
paulhcsun
changed the title
paulhcsun
changed the title
There was a problem hiding this comment.
Hey @Tietew, apologies for not getting around to this. I've reopened the PR and added the do-not-close label to it.
We require that every fix be associated to an issue so could you also create an issue for this and link it in the PR description body? Also as @msambol mentioned would you be able to attach some screenshots of the behaviour please?
| @@ -79,6 +79,9 @@ be retrieved from `distribution.distributionDomainName`. CloudFront distribution | |||
| default. If you want to use your own domain name, such as `www.example.com`, you must associate a certificate with your distribution that contains | |||
| your domain name, and provide one (or more) domain names from the certificate for the distribution. | |||
|
|
|||
| When you want to move a domain name between distributions, you can associate a certificate and specify no domain names. | |||
There was a problem hiding this comment.
| When you want to move a domain name between distributions, you can associate a certificate and specify no domain names. | |
| When you want to move a domain name between distributions, you can associate a certificate without specifying any domain names. |
| @@ -129,7 +129,7 @@ export interface DistributionProps { | |||
| * | |||
| * If you want to use your own domain name, such as www.example.com, instead of the cloudfront.net domain name, | |||
| * you can add an alternate domain name to your distribution. If you attach a certificate to the distribution, | |||
| * you must add (at least one of) the domain names of the certificate to this list. | |||
There was a problem hiding this comment.
Since this is no longer something required, could you also add a sentence at the end explaining the case where we don't need to provide a domain name?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aws-cdk-automation
removed
the
pr/needs-community-review
TheRealAmazonKendra
removed
pr-linter/do-not-close
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
Reason for this change
When I want to move a domain name from a distribution to another distribution,
I must create a distribution with a certificate associated but no domain names.
Description of changes
This PR removes the validation that
domainNamesmust not be blank when a certificate is associated.Description of how you validated changes
Updated a unit test to allow absent
domainNameswhen a certificate is associated.See AWS Documentation for details: Using custom URLs by adding alternate domain names (CNAMEs) > Moving an alternate domain name to a different distribution
https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/CNAMEs.html#alternate-domain-names-move
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license