feat: limit pivot role permissions #535
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dbalintx
reviewed
Jul 4, 2023
noah-paige
requested changes
Jul 6, 2023
This was referenced Jul 11, 2023
Merged
dlpzx
added a commit
that referenced
this pull request
Jul 14, 2023
### Feature or Bugfix - Feature - Bugfix ### Detail #### SSE-S3 for imported datasets With #515 we added the possibility to introduce the KMS key that encrypts the imported bucket when importing a dataset. This is a required field that cannot be empty. In this PR we allow customers to leave the field empty for the case in which no KMS key is used. We assume that if no key is defined, then the bucket is encrypted using SSE-S3 encryption. - changes in frontend views - changes in definition of RDS column values - added line in migration script to backfill `undefined` keys with the `undefined` value for previously imported datasets. #### Rename imported dataset resources In #535 the permissions given to the pivot role were restricted to more specific resources. Most policies where updated to access `resource-prefix` AWS resources only. For imported datasets some of the data.all created resources in the dataset stack (Glue crawler ....) do not follow this naming pattern. In this PR we update the name of those resources to include the prefix. This way the pivot role will be able to execute actions on them. - Added naming convention pattern - Enforce pattern in dataset import - Added migration script to update the names of existing imported datasets⚠️ After the migration script is executed, dataset stacks need to be updated for the AWS resources to be updated⚠️ ### Testing - in-progress I tested locally, but now I am also testing in a real deployment: - [ ] CICD pipeline completes successfully - [ ] KMS RDS info updated as expected - [ ] KMS RDS info of new created and imported datasets as expected - [ ] CloudFormation stacks of updated stacks of imported and created datasets successful (resources updated) - [ ] CloudFormation stacks of new stacks of imported and created datasets successful (resources prefixed) - [ ] Crawlers can be run for all dataset cases ### Relates - #535 - #515 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
dlpzx
added a commit
that referenced
this pull request
Jul 19, 2023
### Feature or Bugfix Release PR with the following list of features. Refer to each PR for the details ### Detail - #498 - #482 - #543 - #524 (which also solves #531) - #532 - #535 - #497 - #515 - #529 - #562 - #455 - #572 - #567 - #573 - #579 - #578 - #582 ### Breaking changes - release notes -⚠️ IMPORTANT: upgrade to a version >V1.5.0 before upgrading to V1.6 to avoid deletion of resources in custom resource deletion -⚠️ IMPORTANT: requires an update of environments and then datasets after upgrading. Either using cdk.json parameter `enable_update_dataall_stacks_in_cicd_pipeline`, waiting for overnight update stack task, or manually updating first environments and then datasets - CloudFront distribution replace for #529 - Additional EC2 permissions in CDK Synth CodeBuild stage for #543 --> this can be avoided by upgrading to v1.5.6 before upgrading to v1.6.0 - local development affected by more restrictive pivotRole trust policy ### Relates V1.6.0 release By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Gezim Musliaj <102723839+gmuslia@users.noreply.github.com> Co-authored-by: Noah Paige <69586985+noah-paige@users.noreply.github.com> Co-authored-by: nikpodsh <124577300+nikpodsh@users.noreply.github.com> Co-authored-by: chamcca <40579012+chamcca@users.noreply.github.com> Co-authored-by: Nikita Podshivalov <nikpodsh@amazon.com> Co-authored-by: dbalintx <132444646+dbalintx@users.noreply.github.com> Co-authored-by: mourya-33 <134511711+mourya-33@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature or Bugfix
Detail
Relates
Testing with
dataallPivotRole-cdkI verified that there are not error logs in CloudWatch for graphql, worker and ecs log-groups
dataallPivotRole-cdkgets created successfullyBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.