Skip to content

Commit

Permalink
[iam.role] Fix IAM Role creation with Boto3 (#517)
Browse files Browse the repository at this point in the history
It turns out that in Boto3 you _must_ supply the
AssumeRolePolicyDocument - AWS will not create the default document for
you. For that reason, we change our default behavior here to set the
`assume_role_policy_document` setting to the default EC2 policy
document.
  • Loading branch information
diranged authored Mar 4, 2022
1 parent 5d637a9 commit e06bd57
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 12 deletions.
46 changes: 36 additions & 10 deletions kingpin/actors/aws/iam/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ def _ensure_entity(self, name, state):
raise gen.Return()

@gen.coroutine
def _create_entity(self, name):
def _create_entity(self, name, **kwargs):
"""Creates an IAM Entity.
If the entity exists, we just warn and move on.
Expand All @@ -411,7 +411,7 @@ def _create_entity(self, name):

try:
ret = yield self.api_call(
self.create_entity, **{self.entity_kwarg_name: name}
self.create_entity, **{self.entity_kwarg_name: name, **kwargs}
)
except ClientError as e:
if "EntityAlreadyExists" in str(e):
Expand Down Expand Up @@ -836,7 +836,21 @@ class Role(EntityBaseActor):
causes Kingpin to ignore the value, and Amazon defaults the role to an
'EC2' style rule. Supplying the document will cause Kingpin to ensure the
assume role policy is correct.
Default: None
Default:
.. code-block:: json
{ "Version": "2012-10-17",
"Statement": [
{ "Effect": "Allow",
"Principal": {
"Service": "ec2.amazonaws.com"
},
"Action": "sts:AssumeRole"
}
]
}
**Example**
Expand Down Expand Up @@ -872,7 +886,7 @@ class Role(EntityBaseActor):
),
"assume_role_policy_document": (
str,
None,
'{"Version": "2012-10-17", "Statement": [{"Effect": "Allow", "Principal": {"Service": "ec2.amazonaws.com"}, "Action": "sts:AssumeRole" }]}',
("The policy that grants an entity" "permission to assume the role"),
),
}
Expand All @@ -895,10 +909,9 @@ def __init__(self, *args, **kwargs):
self._parse_inline_policies(self.option("inline_policies"))

# Pre-parse the Assume Role Policy Document if it was supplied
if self.option("assume_role_policy_document") is not None:
self.assume_role_policy_doc = self._parse_policy_json(
self.option("assume_role_policy_document")
)
self.assume_role_policy_doc = self._parse_policy_json(
self.option("assume_role_policy_document")
)

@gen.coroutine
def _ensure_assume_role_doc(self, name):
Expand Down Expand Up @@ -944,6 +957,20 @@ def _ensure_assume_role_doc(self, name):
**{self.entity_kwarg_name: name, "PolicyDocument": json.dumps(new)},
)

@gen.coroutine
def _create_entity(self, name):
"""Creates an IAM Role.
If the entity exists, we just warn and move on.
args:
name: The IAM Entity Name
"""
yield super(Role, self)._create_entity(
name,
AssumeRolePolicyDocument=json.dumps(self.assume_role_policy_doc),
)

@gen.coroutine
def _execute(self):
name = self.option("name")
Expand All @@ -956,8 +983,7 @@ def _execute(self):
if self.option("inline_policies") is not None:
yield self._ensure_inline_policies(name)

if self.option("assume_role_policy_document") is not None:
yield self._ensure_assume_role_doc(name)
yield self._ensure_assume_role_doc(name)

raise gen.Return()

Expand Down
26 changes: 25 additions & 1 deletion kingpin/actors/aws/iam/test/test_entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ def test_execute_no_policy(self):
self.actor._ensure_assume_role_doc.side_effect = [tornado_value(None)]
yield self.actor._execute()
self.assertTrue(self.actor._ensure_entity.called)
self.assertFalse(self.actor._ensure_assume_role_doc.called)
self.assertTrue(self.actor._ensure_assume_role_doc.called)
self.iam_stubber.assert_no_pending_responses()

@testing.gen_test
Expand All @@ -1025,6 +1025,30 @@ def test_execute(self):
self.assertTrue(self.actor._ensure_inline_policies.called)
self.assertTrue(self.actor._ensure_assume_role_doc.called)

@testing.gen_test
def test_create_entity(self):
self.actor.assume_role_policy_doc = "{}"
self.actor._dry = False
self.iam_stubber.add_response(
# API Call
"create_role",
# Response,
{
"Role": {
"Arn": "arn:.................",
"Path": "/",
"RoleName": "test",
"RoleId": "AQ..............C...",
"CreateDate": datetime(2015, 1, 1),
}
},
# Call Params
{"RoleName": "test", "AssumeRolePolicyDocument": '"{}"'},
)
self.iam_stubber.activate()
yield self.actor._create_entity("test")
self.iam_stubber.assert_no_pending_responses()


class TestInstanceProfile(testing.AsyncTestCase):
def setUp(self):
Expand Down
2 changes: 1 addition & 1 deletion kingpin/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
# Copyright 2018 Nextdoor.com, Inc


__version__ = "2.0.1"
__version__ = "2.0.2"

0 comments on commit e06bd57

Please sign in to comment.