-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(certificatemanager): Allow opting out of transparency logging #21686
feat(certificatemanager): Allow opting out of transparency logging #21686
Conversation
@@ -72,6 +72,13 @@ export interface CertificateProps { | |||
* @default CertificateValidation.fromEmail() | |||
*/ | |||
readonly validation?: CertificateValidation; | |||
|
|||
/** | |||
* Enable or disable transparency logging for this certificate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add something here about how enabling/disabling this works? For example I think you can't change it after creation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well you technically can change it in the api, however it will not have an effect. When a certificate is registered/logged it cannot be undone.
* | ||
* @default TransparencyLoggingPreference.ENABLED | ||
*/ | ||
readonly certificateTransparencyLoggingPreference?: TransparencyLoggingPreference; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readonly certificateTransparencyLoggingPreference?: TransparencyLoggingPreference; | |
readonly transparencyLoggingEnabled?: boolean; |
Can we shorten the name to just transparencyLoggingEnabled
? Also, since it is just enabled/disabled we can make it a boolean
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok will do
exempting integration test since integration tests for certificatemanager require you to have a valid public domain (hence the lack of existing integration tests). |
67aaa4d
to
515c0cb
Compare
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ws#21686) Certificates created with AWS Certificate Manager are recorded in a certificate transparency log. ACM however allows you to opt of out of transparency logging. This feature enables certificates created in ACM through CDK to opt out of transparency logging. * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn 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*
Certificates created with AWS Certificate Manager are recorded in a certificate transparency log. ACM however allows you to opt of out of transparency logging. This feature enables certificates created in ACM through CDK to opt out of transparency logging.
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn 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