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.

Sign up for GitHub

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

Jump to bottom

(aws-cloudfront): sslSupportMethod is not defined #19476

Closed
dayjournal opened this issue Mar 20, 2022 · 3 comments · Fixed by #19737
Closed

(aws-cloudfront): sslSupportMethod is not defined #19476

dayjournal opened this issue Mar 20, 2022 · 3 comments · Fixed by #19737
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@dayjournal
Copy link
Contributor

dayjournal commented Mar 20, 2022 •
edited by ryparker

What is the problem?

Cannot specify sslSupportMethod for CloudFront Distribution in distribution.ts.
I can specify minimumProtocolVersion but not sslSupportMethod.

Example of minimumProtocolVersion
https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-cloudfront/lib/distribution.ts#L222
https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-cloudfront/lib/distribution.ts#L537

Example of sslSupportMethod
https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-cloudfront/lib/distribution.ts#L528

Reproduction Steps

minimumProtocolVersion: cloudfront.SecurityPolicyProtocol.TLS_V1_2_2021,
sslSupportMethod: cloudfront.SSLMethod.SNI,

What did you expect to happen?

I want to set sslSupportMethod.

What actually happened?

I can't configure it with the new method CloudFront Distribution.
It seems to be possible with the old method CloudFrontWebDistribution.

CDK CLI Version

v2.15.0

Framework Version

v2.15.0

Node.js Version

v16.10.0

OS

M1 Mac (macOS Monterey v12.2.1)

Language

Typescript

Language Version

v3.9.7

Other information

No response
@dayjournal dayjournal added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 20, 2022
@github-actions github-actions bot added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Mar 20, 2022
@ryparker
Copy link
Contributor

ryparker commented Mar 21, 2022 •
edited

Hey @dayjournal 👋🏻

Thanks for opening the issue and for linking the relevant CDK code. I'm going to convert this into a feature-request as it is not currently supported in the Distribution construct. This would be a great first issue if anyone is interested to pick this up and submit a PR (please make sure to follow our contribution guidelines.) 🙏

Meanwhile if you're in need of a workaround you can utilize an escape hatch to override the final CloudFormation template that CDK outputs.

e.g.

// Override `SslSupportMethod` until Issue #19476 is resolved.
const cfnDistribution = distribution.node.defaultChild as CfnDistribution;
cfnDistribution.addPropertyOverride(
  "DistributionConfig.ViewerCertificate.SslSupportMethod",
  SSLMethod.VIP
);

@ryparker ryparker added good first issue Related to contributions. See CONTRIBUTING.md p1 feature-request A feature should be added or improved. effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 21, 2022
@comcalvi comcalvi removed their assignment Apr 1, 2022
dayjournal added a commit to dayjournal/aws-cdk that referenced this issue Apr 3, 2022
@dayjournal
Add sslSupportMethod
2bed3c5 
- Modified to allow selection of sslSupportMethod

closes aws#19476
@dayjournal dayjournal mentioned this issue Apr 3, 2022
Merged
4 tasks
@dayjournal
Copy link
Contributor Author

@ryparker
Hi, Created pull request!
#19737

dayjournal added a commit to dayjournal/aws-cdk that referenced this issue Apr 7, 2022
@dayjournal
Add sslSupportMethod
285f3ea 
- Add test for sslSupportMethod
- Corrected comment

closes aws#19476
@mergify mergify bot closed this as completed in #19737 Apr 7, 2022
mergify bot pushed a commit that referenced this issue Apr 7, 2022
@dayjournal
fix(aws-cloudfront): Add sslSupportMethod (#19737)
c5a9679 
- Modified to allow selection of sslSupportMethod

closes #19476

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

otaviomacedo pushed a commit that referenced this issue Apr 11, 2022
@dayjournal @otaviomacedo
fix(aws-cloudfront): Add sslSupportMethod (#19737)
4794a34 
- Modified to allow selection of sslSupportMethod

closes #19476

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this issue Apr 27, 2022
@dayjournal
fix(aws-cloudfront): Add sslSupportMethod (aws#19737)
35fa494 
- Modified to allow selection of sslSupportMethod

closes aws#19476

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*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
Assignees
No one assigned
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(aws-cloudfront): Add sslSupportMethod dayjournal/aws-cdk
3 participants
@dayjournal @ryparker @comcalvi