diff --git a/senza/components/elastigroup.py b/senza/components/elastigroup.py index c70400e6..0df304aa 100644 --- a/senza/components/elastigroup.py +++ b/senza/components/elastigroup.py @@ -13,9 +13,8 @@ from senza.components.auto_scaling_group import normalize_network_threshold from senza.components.taupage_auto_scaling_group import check_application_id, check_application_version, \ check_docker_image_exists, generate_user_data -from senza.utils import ensure_keys, CROSS_STACK_POLICY_NAME +from senza.utils import ensure_keys from senza.spotinst import MissingSpotinstAccount -import senza.manaus.iam ELASTIGROUP_RESOURCE_TYPE = 'Custom::elastigroup' SPOTINST_LAMBDA_FORMATION_ARN = 'arn:aws:lambda:{}:178579023202:function:spotinst-cloudformation' @@ -29,88 +28,6 @@ ELASTIGROUP_DEFAULT_PRODUCT = "Linux/UNIX" -def get_instance_profile_from_definition(definition, elastigroup_config): - launch_spec = elastigroup_config["compute"]["launchSpecification"] - - if "iamRole" not in launch_spec: - return None - - if "name" in launch_spec["iamRole"]: - if isinstance(launch_spec["iamRole"]["name"], dict): - instance_profile_id = launch_spec["iamRole"]["name"]["Ref"] - instance_profile = definition["Resources"].get(instance_profile_id, None) - if instance_profile is None: - raise click.UsageError("Instance Profile referenced is not present in Resources") - - if instance_profile["Type"] != "AWS::IAM::InstanceProfile": - raise click.UsageError( - "Instance Profile references a Resource that is not of type 'AWS::IAM::InstanceProfile'") - - return instance_profile - - return None - - -def get_instance_profile_role(instance_profile, definition): - roles = instance_profile["Properties"]["Roles"] - if isinstance(roles[0], dict): - role_id = roles[0]["Ref"] - role = definition["Resources"].get(role_id, None) - if role is None: - raise click.UsageError("Instance Profile references a Role that is not present in Resources") - - if role["Type"] != "AWS::IAM::Role": - raise click.UsageError("Instance Profile Role references a Resource that is not of type 'AWS::IAM::Role'") - - return role - - return None - - -def create_cross_stack_policy_document(): - return { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "cloudformation:SignalResource", - "cloudformation:DescribeStackResource" - ], - "Resource": "*" - } - ] - } - - -def find_or_create_cross_stack_policy(): - return senza.manaus.iam.find_or_create_policy(policy_name=CROSS_STACK_POLICY_NAME, - policy_document=create_cross_stack_policy_document(), - description="Required permissions for EC2 instances created by " - "Spotinst to signal CloudFormation") - - -def patch_cross_stack_policy(definition, elastigroup_config): - """ - This function will make sure that the role used in the Instance Profile includes the Cross Stack API - requests policy, needed for Elastigroups to run as expected. - """ - instance_profile = get_instance_profile_from_definition(definition, elastigroup_config) - if instance_profile is None: - return - - instance_profile_role = get_instance_profile_role(instance_profile, definition) - if instance_profile_role is None: - return - - cross_stack_policy = find_or_create_cross_stack_policy() - - role_properties = instance_profile_role["Properties"] - managed_policies_set = set(role_properties.get("ManagedPolicyArns", [])) - managed_policies_set.add(cross_stack_policy["Arn"]) - role_properties["ManagedPolicyArns"] = list(managed_policies_set) - - def component_elastigroup(definition, configuration, args, info, force, account_info): """ This component creates a Spotinst Elastigroup CloudFormation custom resource template. @@ -145,7 +62,6 @@ def component_elastigroup(definition, configuration, args, info, force, account_ extract_auto_scaling_rules(configuration, elastigroup_config) extract_block_mappings(configuration, elastigroup_config) extract_instance_profile(args, definition, configuration, elastigroup_config) - patch_cross_stack_policy(definition, elastigroup_config) # cfn definition access_token = _extract_spotinst_access_token(definition) config_name = configuration["Name"] diff --git a/senza/manaus/iam.py b/senza/manaus/iam.py index e6967774..6a811ab2 100644 --- a/senza/manaus/iam.py +++ b/senza/manaus/iam.py @@ -14,7 +14,6 @@ from typing import Any, Dict, Iterator, Optional, Union import boto3 -import json from botocore.exceptions import ClientError from .boto_proxy import BotoClientProxy @@ -162,42 +161,3 @@ def get_certificates( continue yield certificate - - -def _get_policy_by_name(policy_name, iam_client): - """ - This function goes through all the policies in the AWS account and return the first one matching the policy_name - input parameter - """ - paginator = iam_client.get_paginator("list_policies") - - page_iterator = paginator.paginate() - - for page in page_iterator: - if "Policies" in page: - for policy in page["Policies"]: - if policy["PolicyName"] == policy_name: - return policy - - return None - - -def find_or_create_policy(policy_name, policy_document, description): - """ - This function will look for a policy name with `policy_name`. - If not found, it will create the policy using the provided `policy_name` and `policy_document`. - - :return: Policy object - """ - iam_client = boto3.client("iam") - - policy = _get_policy_by_name(policy_name, iam_client) - if policy is None: - response = iam_client.create_policy( - PolicyName=policy_name, - PolicyDocument=json.dumps(policy_document), - Description=description - ) - policy = response["Policy"] - - return policy diff --git a/senza/templates/_helper.py b/senza/templates/_helper.py index a8b7d563..62979bbc 100644 --- a/senza/templates/_helper.py +++ b/senza/templates/_helper.py @@ -8,8 +8,6 @@ from click import confirm from clickclick import Action from senza.aws import get_account_alias, get_account_id, get_security_group -from senza.utils import CROSS_STACK_POLICY_NAME -import senza.manaus.iam from ..manaus.boto_proxy import BotoClientProxy @@ -148,33 +146,6 @@ def create_mint_read_policy_document(application_id: str, bucket_name: str, regi } -def create_cross_stack_policy_document(): - return { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "cloudformation:SignalResource", - "cloudformation:DescribeStackResource" - ], - "Resource": "*" - } - ] - } - - -def check_cross_stack_policy(iam, role_name: str): - try: - iam.get_role_policy( - RoleName=role_name, - PolicyName=CROSS_STACK_POLICY_NAME - ) - return True - except botocore.exceptions.ClientError: - return False - - def check_iam_role(application_id: str, bucket_name: str, region: str): role_name = "app-{}".format(application_id) with Action("Checking IAM role {}..".format(role_name)): @@ -229,33 +200,6 @@ def check_iam_role(application_id: str, bucket_name: str, region: str): PolicyDocument=json.dumps(mint_read_policy), ) - attach_cross_stack_policy(exists, create, role_name, iam) - - -def find_or_create_cross_stack_policy(): - return senza.manaus.iam.find_or_create_policy(policy_name=CROSS_STACK_POLICY_NAME, - policy_document=create_cross_stack_policy_document(), - description="Required permissions for EC2 instances created by " - "Spotinst to signal CloudFormation") - - -def attach_cross_stack_policy(pre_existing_role, role_created, role_name, iam_client): - if not pre_existing_role and not role_created: - return - - cross_stack_policy_exists = False - if pre_existing_role: - cross_stack_policy_exists = check_cross_stack_policy(iam_client, role_name) - - if role_created or not cross_stack_policy_exists: - with Action("Updating IAM role policy of {}..".format(role_name)): - policy = find_or_create_cross_stack_policy() - - iam_client.attach_role_policy( - RoleName=role_name, - PolicyArn=policy["Arn"], - ) - def check_s3_bucket(bucket_name: str, region: str): s3 = boto3.resource("s3", region) diff --git a/senza/utils.py b/senza/utils.py index 063ffe4a..d0cf2f93 100644 --- a/senza/utils.py +++ b/senza/utils.py @@ -6,8 +6,6 @@ import re import pystache -CROSS_STACK_POLICY_NAME = "system-cf-notifications" - def named_value(dictionary): """ diff --git a/tests/test_elastigroup.py b/tests/test_elastigroup.py index d5b52218..3babc55e 100644 --- a/tests/test_elastigroup.py +++ b/tests/test_elastigroup.py @@ -12,7 +12,7 @@ ensure_default_product, fill_standard_tags, extract_subnets, extract_load_balancer_name, extract_public_ips, extract_image_id, extract_security_group_ids, extract_instance_types, - extract_instance_profile, patch_cross_stack_policy) + extract_instance_profile) def test_component_elastigroup_defaults(monkeypatch): @@ -799,142 +799,6 @@ def test_extract_instance_profile(monkeypatch): assert test_case["expected_config"] == got -def test_patch_cross_stack_policy(monkeypatch): - test_cases = [ - { # No instance profile in definition - "elastigroup_config": {"compute": {"launchSpecification": {}}}, - "definition": {}, - "expected_output": {} - }, - { # Instance profile definition references a managed instance profile - "elastigroup_config": {"compute": {"launchSpecification": {"iamRole": { - "arn": "arn:aws:iam::12345667:instance-profile/foo"}}}}, - "definition": {}, - "expected_output": {} - }, - { # Instance profile Role definition references a managed role - "elastigroup_config": {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile"}}}}}, - "definition": {"Resources": {"my-instance-profile": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": ['a-managed-role']}}}}, - "expected_output": {"Resources": {"my-instance-profile": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": ['a-managed-role']}}}} - }, - { # Policy not in policies list of role - "elastigroup_config": {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile1"}}}}}, - "definition": {"Resources": { - "my-instance-profile1": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role1"}]} - }, - "my-role1": { - "Type": "AWS::IAM::Role", - "Properties": {} - } - }}, - "expected_output": {"Resources": { - "my-instance-profile1": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role1"}]} - }, - "my-role1": { - "Type": "AWS::IAM::Role", - "Properties": {"ManagedPolicyArns": ['arn:aws:iam::aws:policy/zed']} - } - }} - }, - { # Policy already in policies list of role - "elastigroup_config": {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile2"}}}}}, - "definition": {"Resources": { - "my-instance-profile2": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role2"}]} - }, - "my-role2": { - "Type": "AWS::IAM::Role", - "Properties": {"ManagedPolicyArns": ['arn:aws:iam::aws:policy/zed']} - } - }}, - "expected_output": {"Resources": { - "my-instance-profile2": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role2"}]} - }, - "my-role2": { - "Type": "AWS::IAM::Role", - "Properties": {"ManagedPolicyArns": ['arn:aws:iam::aws:policy/zed']} - } - }} - } - ] - - cross_stack_policy_mock = MagicMock() - cross_stack_policy_mock.return_value = {"PolicyName": "zed", "Arn": "arn:aws:iam::aws:policy/zed"} - monkeypatch.setattr("senza.manaus.iam.find_or_create_policy", cross_stack_policy_mock) - - for test_case in test_cases: - definition = test_case["definition"] - patch_cross_stack_policy(definition, test_case["elastigroup_config"]) - - assert definition == test_case["expected_output"] - - -def test_patch_cross_stack_policy_errors(): - # Error case 1 :: Instance profile not in Resources - with pytest.raises(click.UsageError): - elastigroup_config = {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile"}}}}} - definition = {"Resources": {}} - - patch_cross_stack_policy(definition, elastigroup_config) - - # Error case 2 :: Instance profile not of type AWS::IAM::InstanceProfile - with pytest.raises(click.UsageError): - elastigroup_config = {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile"}}}}} - definition = {"Resources": { - "my-instance-profile": { - "Type": "AWS::IAM::SomeOtherResource", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role"}]} - }}} - - patch_cross_stack_policy(definition, elastigroup_config) - - # Error case 3 :: Instance profile Role not in Resources - with pytest.raises(click.UsageError): - elastigroup_config = {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile"}}}}} - definition = {"Resources": { - "my-instance-profile": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role"}]} - } - }} - - patch_cross_stack_policy(definition, elastigroup_config) - - # Error case 4 :: Instance profile Role not of type AWS::IAM::Role - with pytest.raises(click.UsageError): - elastigroup_config = {"compute": {"launchSpecification": {"iamRole": { - "name": {"Ref": "my-instance-profile"}}}}} - definition = {"Resources": { - "my-instance-profile": { - "Type": "AWS::IAM::InstanceProfile", - "Properties": {"Path": "/", "Roles": [{"Ref": "my-role"}]} - }, - "my-role": { - "Type": "AWS::IAM::SomeOtherResource", - "Properties": {"ManagedPolicyArns": ['arn:aws:iam::aws:policy/zed']} - } - }} - - patch_cross_stack_policy(definition, elastigroup_config) - - def test_multiple_elastigroups(monkeypatch): config1 = { "Name": "eg1", diff --git a/tests/test_manaus/test_iam.py b/tests/test_manaus/test_iam.py index d2251500..7a2e97bc 100644 --- a/tests/test_manaus/test_iam.py +++ b/tests/test_manaus/test_iam.py @@ -3,7 +3,7 @@ import pytest from botocore.exceptions import ClientError -from senza.manaus.iam import IAM, IAMServerCertificate, find_or_create_policy +from senza.manaus.iam import IAM, IAMServerCertificate IAM_CERT1 = {'CertificateBody': 'body', 'CertificateChain': 'chain', @@ -258,65 +258,3 @@ def test_equality(monkeypatch): assert certificate1 == certificate1_exp # only the arn is compared assert certificate1 != certificate2 - - -def test_find_or_create_policy(monkeypatch): - policy_name = 'somePolicy' - policy_document = { - "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "*" - ], - "Resource": "*" - } - ] - } - description = 'A general description of the policy' - - iam = MagicMock() - iam.return_value = iam - - paginator_mock = MagicMock() - - monkeypatch.setattr('boto3.client', iam) - - # Test case 1 :: Policy does not exist, policy is created - paginator_mock.paginate.return_value = [{'Policies': [{'PolicyName': 'foo', 'Arn': 'arn:aws:iam::aws:policy/foo'}, - {'PolicyName': 'bar', 'Arn': 'arn:aws:iam::aws:policy/bar'}]}, - {'Policies': [{'PolicyName': 'zed', 'Arn': 'arn:aws:iam::aws:policy/zed'}]}] - - iam.get_paginator.return_value = paginator_mock - - iam.create_policy.return_value = {'Policy': {'PolicyName': policy_name, - 'Arn': 'arn:aws:iam::aws:policy/' + policy_name}} - - policy = find_or_create_policy(policy_name, policy_document, description) - - assert iam.get_paginator.call_count == 1 - assert iam.create_policy.call_count == 1 - assert policy["PolicyName"] == policy_name - - # Test case 2 :: Policy exists, policy creation is skipped - iam.reset_mock() - - paginator_mock.paginate.return_value = [{'Policies': [{'PolicyName': 'foo', 'Arn': 'arn:aws:iam::aws:policy/foo'}, - {'PolicyName': 'bar', 'Arn': 'arn:aws:iam::aws:policy/bar'}]}, - { - 'Policies': [ - {'PolicyName': 'zed', 'Arn': 'arn:aws:iam::aws:policy/zed'}, - {'PolicyName': policy_name, - 'Arn': 'arn:aws:iam::aws:policy/' + policy_name} - ] - } - ] - - iam.get_paginator.return_value = paginator_mock - - policy = find_or_create_policy(policy_name, policy_document, description) - - assert iam.get_paginator.call_count == 1 - assert iam.create_policy.call_count == 0 - assert policy["PolicyName"] == policy_name diff --git a/tests/test_templates.py b/tests/test_templates.py index 30f445ce..7265a215 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -1,14 +1,12 @@ import click -import botocore from unittest.mock import MagicMock from senza.templates._helper import (create_mint_read_policy_document, get_mint_bucket_name, - check_value, prompt, choice, attach_cross_stack_policy) + check_value, prompt, choice) from senza.templates.postgresapp import (ebs_optimized_supported, generate_random_password, set_default_variables, generate_definition, get_latest_image) -from senza.utils import CROSS_STACK_POLICY_NAME def test_template_helper_get_mint_bucket_name(monkeypatch): @@ -77,70 +75,6 @@ def test_template_helper_check_value(): assert False, 'check_value doesnot return with a raise' -def test_attach_cross_stack_policy(monkeypatch): - role_name = "myrole" - - iam = MagicMock() - iam.return_value = iam - - cross_stack_policy_mock = MagicMock() - cross_stack_policy_mock.return_value = {'PolicyName': CROSS_STACK_POLICY_NAME, 'Arn': 'arn:aws:iam::aws:policy/' + - CROSS_STACK_POLICY_NAME} - - monkeypatch.setattr('boto3.client', iam) - monkeypatch.setattr('senza.manaus.iam.find_or_create_policy', cross_stack_policy_mock) - - # Test case 1 :: role already exists, policy is not yet attached - get_role_policy_error_response = {'Error': {'Code': 'Error getting the role policy'}} - iam.get_role_policy.side_effect = botocore.exceptions.ClientError(get_role_policy_error_response, 'get_role_policy') - - iam.attach_role_policy.return_value = {'ResponseMetadata': {'key': 'value'}} - - role_exists = True - role_created = False - attach_cross_stack_policy(role_exists, role_created, role_name, iam) - - assert iam.get_role_policy.call_count == 1 - assert iam.attach_role_policy.call_count == 1 - - # Test case 2 :: role already exists, policy is already attached - iam.reset_mock() - - iam.get_role_policy.side_effect = {'RoleName': 'myrolepolicy'} - - role_exists = True - role_created = False - attach_cross_stack_policy(role_exists, role_created, role_name, iam) - - assert iam.get_role_policy.call_count == 1 - assert iam.attach_role_policy.call_count == 0 - - # Test case 3 :: role was just created, policy is not yet attached - iam.reset_mock() - - get_role_policy_error_response = {'Error': {'Code': 'Error getting the role policy'}} - iam.get_role_policy.side_effect = botocore.exceptions.ClientError(get_role_policy_error_response, 'get_role_policy') - - iam.attach_role_policy.return_value = {'ResponseMetadata': {'key': 'value'}} - - role_exists = False - role_created = True - attach_cross_stack_policy(role_exists, role_created, role_name, iam) - - assert iam.get_role_policy.call_count == 0 - assert iam.attach_role_policy.call_count == 1 - - # Test case 3 :: role does not exist and was also not created - iam.reset_mock() - - role_exists = False - role_created = False - attach_cross_stack_policy(role_exists, role_created, role_name, iam) - - assert iam.get_role_policy.call_count == 0 - assert iam.attach_role_policy.call_count == 0 - - def test_choice_callable_default(monkeypatch): mock = MagicMock() monkeypatch.setattr('clickclick.choice', mock)