-
Notifications
You must be signed in to change notification settings - Fork 34
PLFM-9253: Changes to support using Code Pipeline with Synapse #1512
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
Changes from 28 commits
c55cf2d
da1dfef
5b04ff6
3b977ee
7eba7ec
2a39c53
ddc45d7
dedce36
81c6261
0bb1257
e3e4379
ef16296
e968c04
2425330
88f9cc9
6b52705
89fad2d
73fcb5c
43f17dc
c2f44d1
c35cf31
1ee5012
f36f051
5f185ef
f81ef23
0b05b58
6754dee
3e4880a
146cad8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -325,7 +325,9 @@ | |
| { "Fn::ImportValue": "us-east-1-accounts-AWSIAMAdminRoleArn" }, | ||
| { "Fn::GetAtt": [ "SynapseDeploymentRole", "Arn" ] }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/sagebase-github-oidc-sage-bionetworks-it" }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_Administrator_693a85eb20cd5043" } | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_Administrator_693a85eb20cd5043" }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/Synapse-Build-*-CodeBuildServiceRole" }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/Deployment-Pipeline-CodeBuildServiceRole" } | ||
| ] | ||
| } | ||
| } | ||
|
|
@@ -334,20 +336,7 @@ | |
| "Sid": "Allow root administration of the key", | ||
|
||
| "Effect": "Allow", | ||
| "Principal": { | ||
| "AWS": [ | ||
| { | ||
| "Fn::GetAtt": [ | ||
| "SynapseDeploymentRole", | ||
| "Arn" | ||
| ] | ||
| }, | ||
| { | ||
| "Fn::ImportValue": "us-east-1-accounts-AWSIAMAdminRoleArn" | ||
| }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:root" }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/sagebase-github-oidc-sage-bionetworks-it" }, | ||
| { "Fn::Sub": "arn:aws:iam::${AWS::AccountId}:role/aws-reserved/sso.amazonaws.com/AWSReservedSSO_Administrator_693a85eb20cd5043" } | ||
| ] | ||
| "AWS": "*" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this being changed to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question: In this policy the 'allow' and 'deny' sections have the same list of IAM entities, creating unnecessary duplication. By simplifying the 'allow' to '*' and leaving the 'deny' in place, we get the same level of 'least privilege' while simplifying the policy document. This begs the question, "why not keep the 'allow' and eliminate the 'deny' part of the policy?" The reason is because the 'deny' section allows the use of wildcards and we need to use the pattern, |
||
| }, | ||
| "Action": [ | ||
| "kms:*" | ||
|
|
||
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.
Really needed?