{"action":"created","comment":{"url":"https://api.github.com/repos/awslabs/aws-cdk/pulls/comments/227275046","pull_request_review_id":167276613,"id":227275046,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIyNzI3NTA0Ng==","diff_hunk":"@@ -197,12 +197,20 @@ export class PolicyStatement extends Token {\n    * Indicates if this permission has a \"Principal\" section.\n    */\n   public get hasPrincipal() {\n-    return this.principal && this.principal.length > 0;\n+    return Object.keys(this.principal).length > 0;\n   }\n \n   public addPrincipal(principal: PolicyPrincipal): PolicyStatement {\n     const fragment = principal.policyFragment();\n-    this.principal.push(fragment.principalJson);\n+    for (const key of Object.keys(fragment.principalJson)) {","path":"packages/@aws-cdk/aws-iam/lib/policy-document.ts","position":38,"original_position":38,"commit_id":"c40ab7917b4e927b8736b2fa37e1c76fbb02a9d3","original_commit_id":"c40ab7917b4e927b8736b2fa37e1c76fbb02a9d3","user":{"login":"rix0rrr","id":524162,"node_id":"MDQ6VXNlcjUyNDE2Mg==","avatar_url":"https://avatars2.githubusercontent.com/u/524162?v=4","gravatar_id":"","url":"https://api.github.com/users/rix0rrr","html_url":"https://github.com/rix0rrr","followers_url":"https://api.github.com/users/rix0rrr/followers","following_url":"https://api.github.com/users/rix0rrr/following{/other_user}","gists_url":"https://api.github.com/users/rix0rrr/gists{/gist_id}","starred_url":"https://api.github.com/users/rix0rrr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rix0rrr/subscriptions","organizations_url":"https://api.github.com/users/rix0rrr/orgs","repos_url":"https://api.github.com/users/rix0rrr/repos","events_url":"https://api.github.com/users/rix0rrr/events{/privacy}","received_events_url":"https://api.github.com/users/rix0rrr/received_events","type":"User","site_admin":false},"body":"I'm not sure this is correct in general. I believe the '{ AWS: ... }` part of a principal is a sort of type tag, and merging them does not make sense.\r\n\r\nFor example, merging the following two principals:\r\n\r\n```\r\n{ AWS: '123456789012' }\r\n{ GCP: 'booh' }  // even though this doesn't exist\r\n```\r\n\r\nto come up with:\r\n\r\n```\r\n{\r\n  AWS: '123456789012',\r\n  GCP: 'booh' \r\n}\r\n```\r\n\r\nIs NOT a valid principal. \r\n\r\nThe correct behavior would be to merge if the one and only key in the principal is the same, and to throw an error otherwise.","created_at":"2018-10-23T08:17:10Z","updated_at":"2018-10-23T08:20:11Z","html_url":"https://github.com/awslabs/aws-cdk/pull/983#discussion_r227275046","pull_request_url":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/awslabs/aws-cdk/pulls/comments/227275046"},"html":{"href":"https://github.com/awslabs/aws-cdk/pull/983#discussion_r227275046"},"pull_request":{"href":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983"}}},"pull_request":{"url":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983","id":224677679,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI0Njc3Njc5","html_url":"https://github.com/awslabs/aws-cdk/pull/983","diff_url":"https://github.com/awslabs/aws-cdk/pull/983.diff","patch_url":"https://github.com/awslabs/aws-cdk/pull/983.patch","issue_url":"https://api.github.com/repos/awslabs/aws-cdk/issues/983","number":983,"state":"open","locked":false,"title":"fix(iam): Merge multiple principals correctly","user":{"login":"RomainMuller","id":411689,"node_id":"MDQ6VXNlcjQxMTY4OQ==","avatar_url":"https://avatars2.githubusercontent.com/u/411689?v=4","gravatar_id":"","url":"https://api.github.com/users/RomainMuller","html_url":"https://github.com/RomainMuller","followers_url":"https://api.github.com/users/RomainMuller/followers","following_url":"https://api.github.com/users/RomainMuller/following{/other_user}","gists_url":"https://api.github.com/users/RomainMuller/gists{/gist_id}","starred_url":"https://api.github.com/users/RomainMuller/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/RomainMuller/subscriptions","organizations_url":"https://api.github.com/users/RomainMuller/orgs","repos_url":"https://api.github.com/users/RomainMuller/repos","events_url":"https://api.github.com/users/RomainMuller/events{/privacy}","received_events_url":"https://api.github.com/users/RomainMuller/received_events","type":"User","site_admin":false},"body":"When adidng multiple principals, an array of principals was created, but\r\nIAM documents expect an object with different keys, for which values may\r\nbe arrays. This corrects the merging process so it produces valid\r\nstatements.\r\n\r\nThis restores the fix for #924 that was introduced in #916 and reverted\r\nin #958 due to an other feature part of the same commit.","created_at":"2018-10-22T13:16:36Z","updated_at":"2018-10-23T08:20:11Z","closed_at":null,"merged_at":null,"merge_commit_sha":"3b9b724a1558d35e14169b107a6efae2f402a6bb","assignee":null,"assignees":[],"requested_reviewers":[{"login":"eladb","id":598796,"node_id":"MDQ6VXNlcjU5ODc5Ng==","avatar_url":"https://avatars3.githubusercontent.com/u/598796?v=4","gravatar_id":"","url":"https://api.github.com/users/eladb","html_url":"https://github.com/eladb","followers_url":"https://api.github.com/users/eladb/followers","following_url":"https://api.github.com/users/eladb/following{/other_user}","gists_url":"https://api.github.com/users/eladb/gists{/gist_id}","starred_url":"https://api.github.com/users/eladb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eladb/subscriptions","organizations_url":"https://api.github.com/users/eladb/orgs","repos_url":"https://api.github.com/users/eladb/repos","events_url":"https://api.github.com/users/eladb/events{/privacy}","received_events_url":"https://api.github.com/users/eladb/received_events","type":"User","site_admin":false}],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983/commits","review_comments_url":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983/comments","review_comment_url":"https://api.github.com/repos/awslabs/aws-cdk/pulls/comments{/number}","comments_url":"https://api.github.com/repos/awslabs/aws-cdk/issues/983/comments","statuses_url":"https://api.github.com/repos/awslabs/aws-cdk/statuses/c40ab7917b4e927b8736b2fa37e1c76fbb02a9d3","head":{"label":"awslabs:rmuller/fix-multiple-principals","ref":"rmuller/fix-multiple-principals","sha":"c40ab7917b4e927b8736b2fa37e1c76fbb02a9d3","user":{"login":"awslabs","id":3299148,"node_id":"MDEyOk9yZ2FuaXphdGlvbjMyOTkxNDg=","avatar_url":"https://avatars0.githubusercontent.com/u/3299148?v=4","gravatar_id":"","url":"https://api.github.com/users/awslabs","html_url":"https://github.com/awslabs","followers_url":"https://api.github.com/users/awslabs/followers","following_url":"https://api.github.com/users/awslabs/following{/other_user}","gists_url":"https://api.github.com/users/awslabs/gists{/gist_id}","starred_url":"https://api.github.com/users/awslabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/awslabs/subscriptions","organizations_url":"https://api.github.com/users/awslabs/orgs","repos_url":"https://api.github.com/users/awslabs/repos","events_url":"https://api.github.com/users/awslabs/events{/privacy}","received_events_url":"https://api.github.com/users/awslabs/received_events","type":"Organization","site_admin":false},"repo":{"id":105808767,"node_id":"MDEwOlJlcG9zaXRvcnkxMDU4MDg3Njc=","name":"aws-cdk","full_name":"awslabs/aws-cdk","private":false,"owner":{"login":"awslabs","id":3299148,"node_id":"MDEyOk9yZ2FuaXphdGlvbjMyOTkxNDg=","avatar_url":"https://avatars0.githubusercontent.com/u/3299148?v=4","gravatar_id":"","url":"https://api.github.com/users/awslabs","html_url":"https://github.com/awslabs","followers_url":"https://api.github.com/users/awslabs/followers","following_url":"https://api.github.com/users/awslabs/following{/other_user}","gists_url":"https://api.github.com/users/awslabs/gists{/gist_id}","starred_url":"https://api.github.com/users/awslabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/awslabs/subscriptions","organizations_url":"https://api.github.com/users/awslabs/orgs","repos_url":"https://api.github.com/users/awslabs/repos","events_url":"https://api.github.com/users/awslabs/events{/privacy}","received_events_url":"https://api.github.com/users/awslabs/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/awslabs/aws-cdk","description":"The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code","fork":false,"url":"https://api.github.com/repos/awslabs/aws-cdk","forks_url":"https://api.github.com/repos/awslabs/aws-cdk/forks","keys_url":"https://api.github.com/repos/awslabs/aws-cdk/keys{/key_id}","collaborators_url":"https://api.github.com/repos/awslabs/aws-cdk/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/awslabs/aws-cdk/teams","hooks_url":"https://api.github.com/repos/awslabs/aws-cdk/hooks","issue_events_url":"https://api.github.com/repos/awslabs/aws-cdk/issues/events{/number}","events_url":"https://api.github.com/repos/awslabs/aws-cdk/events","assignees_url":"https://api.github.com/repos/awslabs/aws-cdk/assignees{/user}","branches_url":"https://api.github.com/repos/awslabs/aws-cdk/branches{/branch}","tags_url":"https://api.github.com/repos/awslabs/aws-cdk/tags","blobs_url":"https://api.github.com/repos/awslabs/aws-cdk/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/awslabs/aws-cdk/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/awslabs/aws-cdk/git/refs{/sha}","trees_url":"https://api.github.com/repos/awslabs/aws-cdk/git/trees{/sha}","statuses_url":"https://api.github.com/repos/awslabs/aws-cdk/statuses/{sha}","languages_url":"https://api.github.com/repos/awslabs/aws-cdk/languages","stargazers_url":"https://api.github.com/repos/awslabs/aws-cdk/stargazers","contributors_url":"https://api.github.com/repos/awslabs/aws-cdk/contributors","subscribers_url":"https://api.github.com/repos/awslabs/aws-cdk/subscribers","subscription_url":"https://api.github.com/repos/awslabs/aws-cdk/subscription","commits_url":"https://api.github.com/repos/awslabs/aws-cdk/commits{/sha}","git_commits_url":"https://api.github.com/repos/awslabs/aws-cdk/git/commits{/sha}","comments_url":"https://api.github.com/repos/awslabs/aws-cdk/comments{/number}","issue_comment_url":"https://api.github.com/repos/awslabs/aws-cdk/issues/comments{/number}","contents_url":"https://api.github.com/repos/awslabs/aws-cdk/contents/{+path}","compare_url":"https://api.github.com/repos/awslabs/aws-cdk/compare/{base}...{head}","merges_url":"https://api.github.com/repos/awslabs/aws-cdk/merges","archive_url":"https://api.github.com/repos/awslabs/aws-cdk/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/awslabs/aws-cdk/downloads","issues_url":"https://api.github.com/repos/awslabs/aws-cdk/issues{/number}","pulls_url":"https://api.github.com/repos/awslabs/aws-cdk/pulls{/number}","milestones_url":"https://api.github.com/repos/awslabs/aws-cdk/milestones{/number}","notifications_url":"https://api.github.com/repos/awslabs/aws-cdk/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/awslabs/aws-cdk/labels{/name}","releases_url":"https://api.github.com/repos/awslabs/aws-cdk/releases{/id}","deployments_url":"https://api.github.com/repos/awslabs/aws-cdk/deployments","created_at":"2017-10-04T19:22:36Z","updated_at":"2018-10-23T06:50:22Z","pushed_at":"2018-10-23T08:09:14Z","git_url":"git://github.com/awslabs/aws-cdk.git","ssh_url":"git@github.com:awslabs/aws-cdk.git","clone_url":"https://github.com/awslabs/aws-cdk.git","svn_url":"https://github.com/awslabs/aws-cdk","homepage":"https://awslabs.github.io/aws-cdk","size":127902,"stargazers_count":1013,"watchers_count":1013,"language":"TypeScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":78,"mirror_url":null,"archived":false,"open_issues_count":264,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":78,"open_issues":264,"watchers":1013,"default_branch":"master"}},"base":{"label":"awslabs:master","ref":"master","sha":"33c32a8b88f4b2d749519ba15ab17084b3f4c822","user":{"login":"awslabs","id":3299148,"node_id":"MDEyOk9yZ2FuaXphdGlvbjMyOTkxNDg=","avatar_url":"https://avatars0.githubusercontent.com/u/3299148?v=4","gravatar_id":"","url":"https://api.github.com/users/awslabs","html_url":"https://github.com/awslabs","followers_url":"https://api.github.com/users/awslabs/followers","following_url":"https://api.github.com/users/awslabs/following{/other_user}","gists_url":"https://api.github.com/users/awslabs/gists{/gist_id}","starred_url":"https://api.github.com/users/awslabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/awslabs/subscriptions","organizations_url":"https://api.github.com/users/awslabs/orgs","repos_url":"https://api.github.com/users/awslabs/repos","events_url":"https://api.github.com/users/awslabs/events{/privacy}","received_events_url":"https://api.github.com/users/awslabs/received_events","type":"Organization","site_admin":false},"repo":{"id":105808767,"node_id":"MDEwOlJlcG9zaXRvcnkxMDU4MDg3Njc=","name":"aws-cdk","full_name":"awslabs/aws-cdk","private":false,"owner":{"login":"awslabs","id":3299148,"node_id":"MDEyOk9yZ2FuaXphdGlvbjMyOTkxNDg=","avatar_url":"https://avatars0.githubusercontent.com/u/3299148?v=4","gravatar_id":"","url":"https://api.github.com/users/awslabs","html_url":"https://github.com/awslabs","followers_url":"https://api.github.com/users/awslabs/followers","following_url":"https://api.github.com/users/awslabs/following{/other_user}","gists_url":"https://api.github.com/users/awslabs/gists{/gist_id}","starred_url":"https://api.github.com/users/awslabs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/awslabs/subscriptions","organizations_url":"https://api.github.com/users/awslabs/orgs","repos_url":"https://api.github.com/users/awslabs/repos","events_url":"https://api.github.com/users/awslabs/events{/privacy}","received_events_url":"https://api.github.com/users/awslabs/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/awslabs/aws-cdk","description":"The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code","fork":false,"url":"https://api.github.com/repos/awslabs/aws-cdk","forks_url":"https://api.github.com/repos/awslabs/aws-cdk/forks","keys_url":"https://api.github.com/repos/awslabs/aws-cdk/keys{/key_id}","collaborators_url":"https://api.github.com/repos/awslabs/aws-cdk/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/awslabs/aws-cdk/teams","hooks_url":"https://api.github.com/repos/awslabs/aws-cdk/hooks","issue_events_url":"https://api.github.com/repos/awslabs/aws-cdk/issues/events{/number}","events_url":"https://api.github.com/repos/awslabs/aws-cdk/events","assignees_url":"https://api.github.com/repos/awslabs/aws-cdk/assignees{/user}","branches_url":"https://api.github.com/repos/awslabs/aws-cdk/branches{/branch}","tags_url":"https://api.github.com/repos/awslabs/aws-cdk/tags","blobs_url":"https://api.github.com/repos/awslabs/aws-cdk/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/awslabs/aws-cdk/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/awslabs/aws-cdk/git/refs{/sha}","trees_url":"https://api.github.com/repos/awslabs/aws-cdk/git/trees{/sha}","statuses_url":"https://api.github.com/repos/awslabs/aws-cdk/statuses/{sha}","languages_url":"https://api.github.com/repos/awslabs/aws-cdk/languages","stargazers_url":"https://api.github.com/repos/awslabs/aws-cdk/stargazers","contributors_url":"https://api.github.com/repos/awslabs/aws-cdk/contributors","subscribers_url":"https://api.github.com/repos/awslabs/aws-cdk/subscribers","subscription_url":"https://api.github.com/repos/awslabs/aws-cdk/subscription","commits_url":"https://api.github.com/repos/awslabs/aws-cdk/commits{/sha}","git_commits_url":"https://api.github.com/repos/awslabs/aws-cdk/git/commits{/sha}","comments_url":"https://api.github.com/repos/awslabs/aws-cdk/comments{/number}","issue_comment_url":"https://api.github.com/repos/awslabs/aws-cdk/issues/comments{/number}","contents_url":"https://api.github.com/repos/awslabs/aws-cdk/contents/{+path}","compare_url":"https://api.github.com/repos/awslabs/aws-cdk/compare/{base}...{head}","merges_url":"https://api.github.com/repos/awslabs/aws-cdk/merges","archive_url":"https://api.github.com/repos/awslabs/aws-cdk/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/awslabs/aws-cdk/downloads","issues_url":"https://api.github.com/repos/awslabs/aws-cdk/issues{/number}","pulls_url":"https://api.github.com/repos/awslabs/aws-cdk/pulls{/number}","milestones_url":"https://api.github.com/repos/awslabs/aws-cdk/milestones{/number}","notifications_url":"https://api.github.com/repos/awslabs/aws-cdk/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/awslabs/aws-cdk/labels{/name}","releases_url":"https://api.github.com/repos/awslabs/aws-cdk/releases{/id}","deployments_url":"https://api.github.com/repos/awslabs/aws-cdk/deployments","created_at":"2017-10-04T19:22:36Z","updated_at":"2018-10-23T06:50:22Z","pushed_at":"2018-10-23T08:09:14Z","git_url":"git://github.com/awslabs/aws-cdk.git","ssh_url":"git@github.com:awslabs/aws-cdk.git","clone_url":"https://github.com/awslabs/aws-cdk.git","svn_url":"https://github.com/awslabs/aws-cdk","homepage":"https://awslabs.github.io/aws-cdk","size":127902,"stargazers_count":1013,"watchers_count":1013,"language":"TypeScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":78,"mirror_url":null,"archived":false,"open_issues_count":264,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"forks":78,"open_issues":264,"watchers":1013,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983"},"html":{"href":"https://github.com/awslabs/aws-cdk/pull/983"},"issue":{"href":"https://api.github.com/repos/awslabs/aws-cdk/issues/983"},"comments":{"href":"https://api.github.com/repos/awslabs/aws-cdk/issues/983/comments"},"review_comments":{"href":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983/comments"},"review_comment":{"href":"https://api.github.com/repos/awslabs/aws-cdk/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/awslabs/aws-cdk/pulls/983/commits"},"statuses":{"href":"https://api.github.com/repos/awslabs/aws-cdk/statuses/c40ab7917b4e927b8736b2fa37e1c76fbb02a9d3"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"rix0rrr"}}