docs: explain differences to CDKs own key pair construct #278
Conversation
WalkthroughThe update enhances the codebase by integrating support for creating EC2 Key Pairs directly via CloudFormation, a feature previously unavailable. Despite this new CloudFormation capability, the custom construct for Key Pairs remains valuable. It offers unique benefits like key storage in AWS Secrets Manager, KMS encryption, and public key exposure for other resources. The documentation also includes refined compatibility notes for different CDK versions, ensuring users are well-informed about version-specific considerations. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (1)
- README.md (7 hunks)
Additional comments: 4
README.md (4)
- 49-51: The code example provided for creating a Key Pair is clear and well-documented. However, it's worth mentioning that the
storePublicKey: trueoption is crucial for scenarios where the public key is needed for other resources, as highlighted earlier in the documentation. This clarification could help users understand the significance of this option in practical use cases.Consider adding a comment in the code example to highlight the importance of
storePublicKey: truefor certain use cases.
- 55-55: The method
grantReadOnPrivateKey(someRole)is correctly documented with a semicolon at the end, aligning with JavaScript/TypeScript syntax standards. This change ensures consistency and correctness in the documentation.- 58-58: Similarly, the method
grantReadOnPublicKey(anotherRole)is documented with a semicolon, maintaining consistency and adherence to syntax standards. This change is approved.- 62-64: The example usage of the Key Pair on an EC2 instance is clear and demonstrates how to integrate the Key Pair construct with other AWS resources. This example is valuable for users looking to understand how to apply the construct in a practical scenario.
| > ⚠️ Please be aware, CloudFormation now natively supports creating EC2 Key Pairs via [AWS::EC2::KeyPair](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-keypair.html), so you can generally use [CDK's own KeyPair construct](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.KeyPair.html). There are a few differences though and this is the reason why this custom construct is still in existence: | ||
| > | ||
| > - Instead of SSM Parameter Store, keys are stored in [AWS Secrets Manager] | ||
| > - Secrets can be **KMS encrypted** - even different KMS keys for the private and public keys. Of course, SSM parameters _can_ be encrypted too, CloudFormation just doesn't do it | ||
| > - Optionally, this construct can store and expose the public key, enabling the user to directly use it as input for other resources, e.g. for CloudFront signed urls |
There was a problem hiding this comment.
The documentation clearly outlines the advantages of using the custom Key Pair construct over CloudFormation's direct support. However, consider rephrasing "this is the reason why this custom construct is still in existence" to something more concise and positive, such as "this is why the custom construct remains valuable." This change would enhance the readability and tone of the documentation.
- this is the reason why this custom construct is still in existence:
+ this is why the custom construct remains valuable:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| > ⚠️ Please be aware, CloudFormation now natively supports creating EC2 Key Pairs via [AWS::EC2::KeyPair](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-keypair.html), so you can generally use [CDK's own KeyPair construct](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.KeyPair.html). There are a few differences though and this is the reason why this custom construct is still in existence: | |
| > | |
| > - Instead of SSM Parameter Store, keys are stored in [AWS Secrets Manager] | |
| > - Secrets can be **KMS encrypted** - even different KMS keys for the private and public keys. Of course, SSM parameters _can_ be encrypted too, CloudFormation just doesn't do it | |
| > - Optionally, this construct can store and expose the public key, enabling the user to directly use it as input for other resources, e.g. for CloudFront signed urls | |
| > ⚠️ Please be aware, CloudFormation now natively supports creating EC2 Key Pairs via [AWS::EC2::KeyPair](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-keypair.html), so you can generally use [CDK's own KeyPair construct](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.KeyPair.html). There are a few differences though and this is why the custom construct remains valuable: | |
| > | |
| > - Instead of SSM Parameter Store, keys are stored in [AWS Secrets Manager] | |
| > - Secrets can be **KMS encrypted** - even different KMS keys for the private and public keys. Of course, SSM parameters _can_ be encrypted too, CloudFormation just doesn't do it | |
| > - Optionally, this construct can store and expose the public key, enabling the user to directly use it as input for other resources, e.g. for CloudFront signed urls |
| @@ -29,7 +33,7 @@ For TypeScript/NodeJS, add these to your `dependencies` in `package.json`. For P | |||
| ## CDK compatibility | |||
|
|
|||
| - Version 3.x is compatible with the CDK v2. | |||
| - Version 2.x is compatible with the CDK v1. There won't be regular updates for this. | |||
| - Version 2.x is compatible with the CDK v1. There won't be updates for this. | |||
There was a problem hiding this comment.
The compatibility notes for CDK versions are clear and concise. However, it might be beneficial to add a note or link on how users can check their current CDK version or upgrade to a newer version if needed. This addition would provide valuable guidance to users who are unsure about their CDK version compatibility.
Would you like me to draft a brief guide on checking and upgrading CDK versions for inclusion in this section?
| const key = new KeyPair(this, 'Signing-Key-Pair', { | ||
| name: 'CFN-signing-key', | ||
| exposePublicKey: true, | ||
| storePublicKey: true, | ||
| publicKeyFormat: PublicKeyFormat.PEM, | ||
| }); | ||
|
|
||
| const pubKey = new cloudfront.PublicKey(this, 'Signing-Public-Key', { | ||
| encodedKey: key.publicKeyValue, | ||
| }); | ||
| const trustedKeyGroupForCF = new cloudfront.KeyGroup( | ||
| this, | ||
| 'Signing-Key-Group', | ||
| { | ||
| items: [pubKey], | ||
| }, | ||
| ); |
There was a problem hiding this comment.
The documentation on using the key pair for CloudFront signed URLs/cookies is informative and provides a clear example of how to set up the necessary resources. However, ensure that the term "URL" is consistently capitalized throughout the documentation to maintain professionalism and adherence to standard terminology.
- for CloudFront signed urls/cookies
+ for CloudFront signed URLs/cookiesCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const key = new KeyPair(this, 'Signing-Key-Pair', { | |
| name: 'CFN-signing-key', | |
| exposePublicKey: true, | |
| storePublicKey: true, | |
| publicKeyFormat: PublicKeyFormat.PEM, | |
| }); | |
| const pubKey = new cloudfront.PublicKey(this, 'Signing-Public-Key', { | |
| encodedKey: key.publicKeyValue, | |
| }); | |
| const trustedKeyGroupForCF = new cloudfront.KeyGroup( | |
| this, | |
| 'Signing-Key-Group', | |
| { | |
| items: [pubKey], | |
| }, | |
| ); | |
| const key = new KeyPair(this, 'Signing-Key-Pair', { | |
| name: 'CFN-signing-key', | |
| exposePublicKey: true, | |
| storePublicKey: true, | |
| publicKeyFormat: PublicKeyFormat.PEM, | |
| }); | |
| const pubKey = new cloudfront.PublicKey(this, 'Signing-Public-Key', { | |
| encodedKey: key.publicKeyValue, | |
| }); | |
| const trustedKeyGroupForCF = new cloudfront.KeyGroup( | |
| this, | |
| 'Signing-Key-Group', | |
| { | |
| items: [pubKey], | |
| }, | |
| ); |
No description provided.