-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(stepfunctions): add support for EncryptionConfiguration #30959
feat(stepfunctions): add support for EncryptionConfiguration #30959
Conversation
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.
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.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
tools/@aws-cdk/spec2cdk/temporary-schemas/us-east-1/aws-stepfunctions-activity.json
Outdated
Show resolved
Hide resolved
tools/@aws-cdk/spec2cdk/temporary-schemas/us-east-1/aws-stepfunctions-statemachine.json
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-stepfunctions/lib/encryption-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-stepfunctions/lib/encryption-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-stepfunctions/lib/encryption-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-stepfunctions/lib/encryption-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-stepfunctions/lib/encryption-configuration.ts
Outdated
Show resolved
Hide resolved
ce4774b
to
a3fecf0
Compare
a3fecf0
to
09350fd
Compare
Can you describe how the StateMachine and Activity constructs are actually being changed in the Perhaps an example of these properties in use as well. |
09350fd
to
971290d
Compare
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.
Comments on docs
Co-authored-by: Adam Wong <[email protected]>
Co-authored-by: Adam Wong <[email protected]>
* @default Duration.seconds(300) | ||
*/ | ||
public readonly kmsDataKeyReusePeriodSeconds?; | ||
constructor(kmsKey: kms.IKey, kmsDataKeyReusePeriodSeconds?: cdk.Duration) { |
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.
nit: new lines?
Co-authored-by: Adam Wong <[email protected]>
Co-authored-by: Adam Wong <[email protected]>
…nagedEncryptionConfiguration & AwsOwnedEncryptionConfiguration extend - Created new class CustomerManagedEncryptionConfiguration - Created new class AwsOwnedEncryptionConfiguration - Updated unit and integ tests to use either CustomerManagedEncryptionConfiguration or AwsOwnedEncryptionConfiguration when setting encryptionConfiguration - Updated README to use CustomerManagedEncryptionConfiguration or AwsOwnedEncryptionConfiguration in the code samples - Updated README to include specific comment on encrypting log group with link to relevant documentation - Added example in README for switching between CustomerManagedEncryptionConfiguration and AwsOwnedEncryptionConfiguration
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.
A few more small changes. Also please add a line break to the end of the files that do not have one.
packages/aws-cdk-lib/aws-stepfunctions/lib/aokencryptionconfiguration.ts
Outdated
Show resolved
Hide resolved
- Moved util.ts to a private directory - Updated buildEncryptionConfiguration to to accept type EncryptionConfiguration - Rename ckmencryptionconfiguration to customer-managed-key-encryption-configuration - Rename aokencryptionconfiguration to aws-owned-key-encryption-configuration - Rename encryptionconfiguration to encryption-configuration
Pull request has been modified.
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. |
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.
Changes have been appsec approved. Great work on this @VaidSaraswat!
Thanks to everyone for helping with the review on this!
@Mergifyio update |
☑️ Nothing to do
|
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). |
Comments on closed issues and PRs are hard for our team to see. |
Reason for this change
Allow customers to specify a customer managed KMS key and data key reuse period to encrypt state machine definition and execution history and activity inputs. The underlying
AWS::StepFunctions::StateMachine
andAWS::StepFunctions::Activity
resources currently expose this through an optionalEncryptionConfiguration
property.Description of changes
Activity and StateMachine accept a new field called encryptionConfiguration of type
EncryptionConfiguration
in their respective props. We have two separate classes which inherit the base class: 1.CustomerManagedEncryptionConfiguration
2.AwsOwnedEncryptionConfiguration
CustomerManagedEncryptionConfiguration
:kmsKey
IKey
undefined
kmsDataKeyReusePeriodSeconds
:Duration
GenerateDataKey
. Must be a value between 60 and 900 seconds.AwsOwnedEncryptionConfiguration
Permission Changes
Activity:
kmsKey?
the key policy will be updated with the following policy statement:StateMachine:
kmsKey?
the key policy will be updated with the following policy statement:kmsKey?
andlogs?
prop, the following key policy statement will be added to the key used by the StateMachine:In addition the execution role will be updated to include a separate policy that includes kms actions and encryption context for logging (otherwise customer will not see logs)
Description of how you validated changes
Unit Test (scenarios):
kmsDataKeyReusePeriodSeconds
defaults to 300 secs'kms:Decrypt'
,'kms:GenerateDataKey'
actions on the associated KMS key.kmsDataKeyReusePeriodSeconds
throws an errorAwsOwnedEncryptionConfiguration
usesAWS_OWNED_KEY
encryption type.'kms:Decrypt'
,'kms:GenerateDataKey'
actions on the key.logs?
andkmsKey?
:'kms:Decrypt'
,'kms:GenerateDataKey'
actions on the Activity KMS keykmsDataKeyReusePeriodSeconds
defaults to 300 secskmsDataKeyReusePeriodSeconds
throws a validation errorAwsOwnedEncryptionConfiguration
usesAWS_OWNED_KEY
encryption type.Integration tests
getActivityTask
APICode samples
Creating an Activity with Encryption using a Customer Managed Key
Creating a StateMachine with Encryption using a Customer Managed Key
Creating a StateMachine with CWL Encryption using a Customer Managed Key
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license