Merge pull request #1919 from guardicore/1636-long-aws-check

1636 long aws check

CHANGELOG.md

@@ -85,6 +85,7 @@ Changelog](https://keepachangelog.com/en/1.0.0/).
 - A bug where windows executable was not self deleting. #1763
 - Incorrect line number in the telemetry overview window on the Map page. #1850
 - Automatic jumping to the bottom in the telemetry overview windows. #1850
+- 2-second delay when the Island server starts, and it's not running on AWS. #1636
 
 ### Security
 - Change SSH exploiter so that it does not set the permissions of the agent

docs/content/usage/integrations/aws-run-on-ec2-machine.md

@@ -50,6 +50,10 @@ If your EC2 instances don't have the _SSM agent_ installed, they will not be abl
 
 See [Amazon's documentation about working with SSM agents](https://docs.aws.amazon.com/systems-manager/latest/userguide/ssm-agent.html) for more details on how to check if you have an SSM agent and how to manually install one if you don't yet have it.
 
+### Firewall rules
+
+Make sure that all machines that will run the Monkey agent can access the Island(port 5000).
+
 ## Usage
 
 ### Running the Infection Monkey

monkey/common/aws/aws_instance.py

@@ -1,11 +1,11 @@
 import json
 import logging
 import re
+from dataclasses import dataclass
+from typing import Optional, Tuple
 
 import requests
 
-from common.cloud.instance import CloudInstance
-
 AWS_INSTANCE_METADATA_LOCAL_IP_ADDRESS = "169.254.169.254"
 AWS_LATEST_METADATA_URI_PREFIX = "http://{0}/latest/".format(AWS_INSTANCE_METADATA_LOCAL_IP_ADDRESS)
 ACCOUNT_ID_KEY = "accountId"
@@ -15,26 +15,50 @@ logger = logging.getLogger(__name__)
 AWS_TIMEOUT = 2
 
 
-class AwsInstance(CloudInstance):
+@dataclass
+class AwsInstanceInfo:
+    instance_id: Optional[str] = None
+    region: Optional[str] = None
+    account_id: Optional[str] = None
+
+
+class AwsInstance:
     """
     Class which gives useful information about the current instance you're on.
     """
 
-    def is_instance(self):
-        return self.instance_id is not None
-
     def __init__(self):
-        self.instance_id = None
-        self.region = None
-        self.account_id = None
+        self._is_instance, self._instance_info = AwsInstance._fetch_instance_info()
 
+    @property
+    def is_instance(self) -> bool:
+        return self._is_instance
+
+    @property
+    def instance_id(self) -> str:
+        return self._instance_info.instance_id
+
+    @property
+    def region(self) -> str:
+        return self._instance_info.region
+
+    @property
+    def account_id(self) -> str:
+        return self._instance_info.account_id
+
+    @staticmethod
+    def _fetch_instance_info() -> Tuple[bool, AwsInstanceInfo]:
         try:
             response = requests.get(
                 AWS_LATEST_METADATA_URI_PREFIX + "meta-data/instance-id",
                 timeout=AWS_TIMEOUT,
             )
-            self.instance_id = response.text if response else None
-            self.region = self._parse_region(
+            if not response:
+                return False, AwsInstanceInfo()
+
+            info = AwsInstanceInfo()
+            info.instance_id = response.text if response else False
+            info.region = AwsInstance._parse_region(
                 requests.get(
                     AWS_LATEST_METADATA_URI_PREFIX + "meta-data/placement/availability-zone",
                     timeout=AWS_TIMEOUT,
@@ -42,9 +66,10 @@ class AwsInstance(CloudInstance):
             )
         except (requests.RequestException, IOError) as e:
             logger.debug("Failed init of AwsInstance while getting metadata: {}".format(e))
+            return False, AwsInstanceInfo()
 
         try:
-            self.account_id = self._extract_account_id(
+            info.account_id = AwsInstance._extract_account_id(
                 requests.get(
                     AWS_LATEST_METADATA_URI_PREFIX + "dynamic/instance-identity/document",
                     timeout=AWS_TIMEOUT,
@@ -54,6 +79,9 @@ class AwsInstance(CloudInstance):
             logger.debug(
                 "Failed init of AwsInstance while getting dynamic instance data: {}".format(e)
             )
+            return False, AwsInstanceInfo()
+
+        return True, info
 
     @staticmethod
     def _parse_region(region_url_response):
@@ -68,12 +96,6 @@ class AwsInstance(CloudInstance):
         else:
             return None
 
-    def get_instance_id(self):
-        return self.instance_id
-
-    def get_region(self):
-        return self.region
-
     @staticmethod
     def _extract_account_id(instance_identity_document_response):
         """

monkey/common/cloud/aws/aws_service.py

@@ -1,74 +0,0 @@
-import logging
-
-import boto3
-import botocore
-
-from common.cloud.aws.aws_instance import AwsInstance
-
-INSTANCE_INFORMATION_LIST_KEY = "InstanceInformationList"
-INSTANCE_ID_KEY = "InstanceId"
-COMPUTER_NAME_KEY = "ComputerName"
-PLATFORM_TYPE_KEY = "PlatformType"
-IP_ADDRESS_KEY = "IPAddress"
-
-logger = logging.getLogger(__name__)
-
-
-def filter_instance_data_from_aws_response(response):
-    return [
-        {
-            "instance_id": x[INSTANCE_ID_KEY],
-            "name": x[COMPUTER_NAME_KEY],
-            "os": x[PLATFORM_TYPE_KEY].lower(),
-            "ip_address": x[IP_ADDRESS_KEY],
-        }
-        for x in response[INSTANCE_INFORMATION_LIST_KEY]
-    ]
-
-
-class AwsService(object):
-    """
-    A wrapper class around the boto3 client and session modules, which supplies various AWS
-    services.
-
-    This class will assume:
-        1. That it's running on an EC2 instance
-        2. That the instance is associated with the correct IAM role. See
-        https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html#iam-role
-        for details.
-    """
-
-    region = None
-
-    @staticmethod
-    def set_region(region):
-        AwsService.region = region
-
-    @staticmethod
-    def get_client(client_type, region=None):
-        return boto3.client(
-            client_type, region_name=region if region is not None else AwsService.region
-        )
-
-    @staticmethod
-    def get_instances():
-        """
-        Get the information for all instances with the relevant roles.
-
-        This function will assume that it's running on an EC2 instance with the correct IAM role.
-        See https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html#iam
-        -role for details.
-
-        :raises: botocore.exceptions.ClientError if can't describe local instance information.
-        :return: All visible instances from this instance
-        """
-        current_instance = AwsInstance()
-        local_ssm_client = boto3.client("ssm", current_instance.get_region())
-        try:
-            response = local_ssm_client.describe_instance_information()
-
-            filtered_instances_data = filter_instance_data_from_aws_response(response)
-            return filtered_instances_data
-        except botocore.exceptions.ClientError as e:
-            logger.warning("AWS client error while trying to get instances: " + e)
-            raise e

monkey/common/cloud/instance.py

@@ -1,9 +0,0 @@
-class CloudInstance(object):
-    """
-    This is an abstract class which represents a cloud instance.
-
-    The current machine can be a cloud instance (for example EC2 instance or Azure VM).
-    """
-
-    def is_instance(self) -> bool:
-        raise NotImplementedError()

monkey/common/utils/code_utils.py

@@ -1,7 +1,3 @@
-# abstract, static method decorator
-# noinspection PyPep8Naming
-
-
 class abstractstatic(staticmethod):
     __slots__ = ()
 
@@ -10,3 +6,12 @@ class abstractstatic(staticmethod):
         function.__isabstractmethod__ = True
 
     __isabstractmethod__ = True
+
+
+class Singleton(type):
+    _instances = {}
+
+    def __call__(cls, *args, **kwargs):
+        if cls not in cls._instances:
+            cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
+        return cls._instances[cls]

monkey/infection_monkey/utils/aws_environment_check.py

@@ -1,6 +1,6 @@
 import logging
 
-from common.cloud.aws.aws_instance import AwsInstance
+from common.aws.aws_instance import AwsInstance
 from infection_monkey.telemetry.aws_instance_telem import AWSInstanceTelemetry
 from infection_monkey.telemetry.messengers.legacy_telemetry_messenger_adapter import (
     LegacyTelemetryMessengerAdapter,
@@ -10,18 +10,14 @@ from infection_monkey.utils.threading import create_daemon_thread
 logger = logging.getLogger(__name__)
 
 
-def _running_on_aws(aws_instance: AwsInstance) -> bool:
-    return aws_instance.is_instance()
-
-
 def _report_aws_environment(telemetry_messenger: LegacyTelemetryMessengerAdapter):
     logger.info("Collecting AWS info")
 
     aws_instance = AwsInstance()
 
-    if _running_on_aws(aws_instance):
+    if aws_instance.is_instance:
         logger.info("Machine is an AWS instance")
-        telemetry_messenger.send_telemetry(AWSInstanceTelemetry(aws_instance.get_instance_id()))
+        telemetry_messenger.send_telemetry(AWSInstanceTelemetry(aws_instance.instance_id))
     else:
         logger.info("Machine is NOT an AWS instance")
 

monkey/monkey_island/cc/app.py

@@ -48,7 +48,6 @@ from monkey_island.cc.resources.zero_trust.finding_event import ZeroTrustFinding
 from monkey_island.cc.resources.zero_trust.zero_trust_report import ZeroTrustReport
 from monkey_island.cc.server_utils.consts import MONKEY_ISLAND_ABS_PATH
 from monkey_island.cc.server_utils.custom_json_encoder import CustomJSONEncoder
-from monkey_island.cc.services.remote_run_aws import RemoteRunAwsService
 from monkey_island.cc.services.representations import output_json
 
 HOME_FILE = "index.html"
@@ -103,10 +102,6 @@ def init_app_services(app):
     with app.app_context():
         database.init()
 
-    # If running on AWS, this will initialize the instance data, which is used "later" in the
-    # execution of the island.
-    RemoteRunAwsService.init()
-
 
 def init_app_url_rules(app):
     app.add_url_rule("/", "serve_home", serve_home)

monkey/monkey_island/cc/resources/remote_run.py

@@ -4,8 +4,8 @@ import flask_restful
 from botocore.exceptions import ClientError, NoCredentialsError
 from flask import jsonify, make_response, request
 
-from common.cloud.aws.aws_service import AwsService
 from monkey_island.cc.resources.auth.auth import jwt_required
+from monkey_island.cc.services import aws_service
 from monkey_island.cc.services.remote_run_aws import RemoteRunAwsService
 
 CLIENT_ERROR_FORMAT = (
@@ -19,10 +19,6 @@ NO_CREDS_ERROR_FORMAT = (
 
 
 class RemoteRun(flask_restful.Resource):
-    def __init__(self):
-        super(RemoteRun, self).__init__()
-        RemoteRunAwsService.init()
-
     def run_aws_monkeys(self, request_body):
         instances = request_body.get("instances")
         island_ip = request_body.get("island_ip")
@@ -32,11 +28,11 @@ class RemoteRun(flask_restful.Resource):
     def get(self):
         action = request.args.get("action")
         if action == "list_aws":
-            is_aws = RemoteRunAwsService.is_running_on_aws()
+            is_aws = aws_service.is_on_aws()
             resp = {"is_aws": is_aws}
             if is_aws:
                 try:
-                    resp["instances"] = AwsService.get_instances()
+                    resp["instances"] = aws_service.get_instances()
                 except NoCredentialsError as e:
                     resp["error"] = NO_CREDS_ERROR_FORMAT.format(e)
                     return jsonify(resp)
@@ -52,7 +48,6 @@ class RemoteRun(flask_restful.Resource):
         body = json.loads(request.data)
         resp = {}
         if body.get("type") == "aws":
-            RemoteRunAwsService.update_aws_region_authless()
             result = self.run_aws_monkeys(body)
             resp["result"] = result
             return jsonify(resp)

monkey/monkey_island/cc/server_setup.py

@@ -3,6 +3,7 @@ import json
 import logging
 import sys
 from pathlib import Path
+from threading import Thread
 
 import gevent.hub
 from gevent.pywsgi import WSGIServer
@@ -131,7 +132,8 @@ def _configure_gevent_exception_handling(data_dir):
 def _start_island_server(
     should_setup_only: bool, config_options: IslandConfigOptions, container: DIContainer
 ):
-    populate_exporter_list()
+    # AWS exporter takes a long time to load
+    Thread(target=populate_exporter_list, name="Report exporter list", daemon=True).start()
     app = init_app(mongo_setup.MONGO_URL, container)
 
     if should_setup_only:

monkey/monkey_island/cc/server_utils/aws_cmd_runner.py

@@ -1,10 +1,10 @@
 import logging
 import time
 
-from common.cloud.aws.aws_service import AwsService
-from common.cmd.aws.aws_cmd_result import AwsCmdResult
 from common.cmd.cmd_runner import CmdRunner
 from common.cmd.cmd_status import CmdStatus
+from monkey_island.cc.server_utils.aws_cmd_result import AwsCmdResult
+from monkey_island.cc.services import aws_service
 
 logger = logging.getLogger(__name__)
 
@@ -18,7 +18,7 @@ class AwsCmdRunner(CmdRunner):
         super(AwsCmdRunner, self).__init__(is_linux)
         self.instance_id = instance_id
         self.region = region
-        self.ssm = AwsService.get_client("ssm", region)
+        self.ssm = aws_service.get_client("ssm", region)
 
     def query_command(self, command_id):
         time.sleep(2)

monkey/monkey_island/cc/services/aws_service.py

@@ -0,0 +1,99 @@
+import logging
+from functools import wraps
+from threading import Event
+from typing import Callable, Optional
+
+import boto3
+import botocore
+
+from common.aws.aws_instance import AwsInstance
+
+INSTANCE_INFORMATION_LIST_KEY = "InstanceInformationList"
+INSTANCE_ID_KEY = "InstanceId"
+COMPUTER_NAME_KEY = "ComputerName"
+PLATFORM_TYPE_KEY = "PlatformType"
+IP_ADDRESS_KEY = "IPAddress"
+
+logger = logging.getLogger(__name__)
+
+
+def filter_instance_data_from_aws_response(response):
+    return [
+        {
+            "instance_id": x[INSTANCE_ID_KEY],
+            "name": x[COMPUTER_NAME_KEY],
+            "os": x[PLATFORM_TYPE_KEY].lower(),
+            "ip_address": x[IP_ADDRESS_KEY],
+        }
+        for x in response[INSTANCE_INFORMATION_LIST_KEY]
+    ]
+
+
+aws_instance: Optional[AwsInstance] = None
+AWS_INFO_FETCH_TIMEOUT = 10.0  # Seconds
+init_done = Event()
+
+
+def initialize():
+    global aws_instance
+    aws_instance = AwsInstance()
+    init_done.set()
+
+
+def wait_init_done(fnc: Callable):
+    @wraps(fnc)
+    def inner():
+        awaited = init_done.wait(AWS_INFO_FETCH_TIMEOUT)
+        if not awaited:
+            logger.error(
+                f"AWS service couldn't initialize in time! "
+                f"Current timeout is {AWS_INFO_FETCH_TIMEOUT}, "
+                f"but AWS info took longer to fetch from metadata server."
+            )
+            return
+        fnc()
+
+    return inner
+
+
+@wait_init_done
+def is_on_aws():
+    return aws_instance.is_instance
+
+
+@wait_init_done
+def get_region():
+    return aws_instance.region
+
+
+@wait_init_done
+def get_account_id():
+    return aws_instance.account_id
+
+
+@wait_init_done
+def get_client(client_type):
+    return boto3.client(client_type, region_name=aws_instance.region)
+
+
+@wait_init_done
+def get_instances():
+    """
+    Get the information for all instances with the relevant roles.
+
+    This function will assume that it's running on an EC2 instance with the correct IAM role.
+    See https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html#iam
+    -role for details.
+
+    :raises: botocore.exceptions.ClientError if can't describe local instance information.
+    :return: All visible instances from this instance
+    """
+    local_ssm_client = boto3.client("ssm", aws_instance.region)
+    try:
+        response = local_ssm_client.describe_instance_information()
+
+        filtered_instances_data = filter_instance_data_from_aws_response(response)
+        return filtered_instances_data
+    except botocore.exceptions.ClientError as e:
+        logger.warning("AWS client error while trying to get instances: " + e)
+        raise e

monkey/monkey_island/cc/services/initialize.py

@@ -1,7 +1,8 @@
 from pathlib import Path
+from threading import Thread
 
 from common import DIContainer
-from monkey_island.cc.services import DirectoryFileStorageService, IFileStorageService
+from monkey_island.cc.services import DirectoryFileStorageService, IFileStorageService, aws_service
 from monkey_island.cc.services.post_breach_files import PostBreachFilesService
 from monkey_island.cc.services.run_local_monkey import LocalMonkeyRunService
 
@@ -14,6 +15,9 @@ def initialize_services(data_dir: Path) -> DIContainer:
         IFileStorageService, DirectoryFileStorageService(data_dir / "custom_pbas")
     )
 
+    # Takes a while so it's best to start it in the background
+    Thread(target=aws_service.initialize, name="AwsService initialization", daemon=True).start()
+
     # This is temporary until we get DI all worked out.
     PostBreachFilesService.initialize(container.resolve(IFileStorageService))
     LocalMonkeyRunService.initialize(data_dir)

monkey/monkey_island/cc/services/remote_run_aws.py

@@ -1,38 +1,13 @@
 import logging
 
-from common.cloud.aws.aws_instance import AwsInstance
-from common.cloud.aws.aws_service import AwsService
-from common.cmd.aws.aws_cmd_runner import AwsCmdRunner
 from common.cmd.cmd import Cmd
 from common.cmd.cmd_runner import CmdRunner
+from monkey_island.cc.server_utils.aws_cmd_runner import AwsCmdRunner
 
 logger = logging.getLogger(__name__)
 
 
 class RemoteRunAwsService:
-    aws_instance = None
-
-    def __init__(self):
-        pass
-
-    @staticmethod
-    def init():
-        """
-        Initializes service. Subsequent calls to this function have no effect.
-        Must be called at least once (in entire monkey lifetime) before usage of functions
-        :return: None
-        """
-        if RemoteRunAwsService.aws_instance is None:
-            RemoteRunAwsService.try_init_aws_instance()
-
-    @staticmethod
-    def try_init_aws_instance():
-        # noinspection PyBroadException
-        try:
-            RemoteRunAwsService.aws_instance = AwsInstance()
-        except Exception:
-            logger.error("Failed init aws instance. Exception info: ", exc_info=True)
-
     @staticmethod
     def run_aws_monkeys(instances, island_ip):
         """
@@ -51,17 +26,6 @@ class RemoteRunAwsService:
             lambda _, result: result.is_success,
         )
 
-    @staticmethod
-    def is_running_on_aws():
-        return RemoteRunAwsService.aws_instance.is_instance()
-
-    @staticmethod
-    def update_aws_region_authless():
-        """
-        Updates the AWS region without auth params (via IAM role)
-        """
-        AwsService.set_region(RemoteRunAwsService.aws_instance.region)
-
     @staticmethod
     def _run_aws_monkey_cmd_async(instance_id, is_linux, island_ip):
         """

monkey/monkey_island/cc/services/reporting/aws_exporter.py

@@ -5,7 +5,7 @@ from datetime import datetime
 import boto3
 from botocore.exceptions import UnknownServiceError
 
-from common.cloud.aws.aws_instance import AwsInstance
+from monkey_island.cc.services import aws_service
 from monkey_island.cc.services.reporting.exporter import Exporter
 
 __authors__ = ["maor.rayzin", "shay.nehmad"]
@@ -35,8 +35,7 @@ class AWSExporter(Exporter):
             logger.info("No issues were found by the monkey, no need to send anything")
             return True
 
-        # Not suppressing error here on purpose.
-        current_aws_region = AwsInstance().get_region()
+        current_aws_region = aws_service.get_region()
 
         for machine in issues_list:
             for issue in issues_list[machine]:
@@ -90,7 +89,7 @@ class AWSExporter(Exporter):
         )
         instance_arn = "arn:aws:ec2:" + str(region) + ":instance:{instance_id}"
         # Not suppressing error here on purpose.
-        account_id = AwsInstance().get_account_id()
+        account_id = aws_service.get_account_id()
         logger.debug("aws account id acquired: {}".format(account_id))
 
         aws_finding = {

monkey/monkey_island/cc/services/reporting/exporter_init.py

@@ -1,6 +1,6 @@
 import logging
 
-from monkey_island.cc.services.remote_run_aws import RemoteRunAwsService
+from monkey_island.cc.services import aws_service
 from monkey_island.cc.services.reporting.aws_exporter import AWSExporter
 from monkey_island.cc.services.reporting.report_exporter_manager import ReportExporterManager
 
@@ -22,8 +22,7 @@ def populate_exporter_list():
 def try_add_aws_exporter_to_manager(manager):
     # noinspection PyBroadException
     try:
-        RemoteRunAwsService.init()
-        if RemoteRunAwsService.is_running_on_aws():
+        if aws_service.is_on_aws():
             manager.add_exporter_to_list(AWSExporter)
     except Exception:
         logger.error("Failed adding aws exporter to manager. Exception info:", exc_info=True)

monkey/monkey_island/cc/services/reporting/report_exporter_manager.py

@@ -1,17 +1,10 @@
 import logging
 
+from common.utils.code_utils import Singleton
+
 logger = logging.getLogger(__name__)
 
 
-class Singleton(type):
-    _instances = {}
-
-    def __call__(cls, *args, **kwargs):
-        if cls not in cls._instances:
-            cls._instances[cls] = super(Singleton, cls).__call__(*args, **kwargs)
-        return cls._instances[cls]
-
-
 class ReportExporterManager(object, metaclass=Singleton):
     def __init__(self):
         self._exporters_set = set()

monkey/monkey_island/monkey_island.spec

@@ -4,7 +4,6 @@ import platform
 import sys
 
 
-
 block_cipher = None
 
 
@@ -13,7 +12,8 @@ def main():
     # The format of the tuples is (src, dest_dir). See https://pythonhosted.org/PyInstaller/spec-files.html#adding-data-files
     added_datas = [
         ("../common/BUILD", "/common"),
-        ("../monkey_island/cc/setup/mongo/attack_mitigations.json", "/monkey_island/cc/setup/mongo/attack_mitigations.json")
+        ("../monkey_island/cc/setup/mongo/attack_mitigations.json",
+         "/monkey_island/cc/setup/mongo/attack_mitigations.json")
     ]
 
     a = Analysis(['main.py'],

monkey/tests/unit_tests/common/cloud/aws/test_aws_instance.py

@@ -1,8 +1,10 @@
+from types import MappingProxyType
+
 import pytest
 import requests
 import requests_mock
 
-from common.cloud.aws.aws_instance import AWS_LATEST_METADATA_URI_PREFIX, AwsInstance
+from common.aws.aws_instance import AWS_LATEST_METADATA_URI_PREFIX, AwsInstance
 
 INSTANCE_ID_RESPONSE = "i-1234567890abcdef0"
 
@@ -37,8 +39,8 @@ EXPECTED_ACCOUNT_ID = "123456789012"
 
 
 def get_test_aws_instance(
-    text={"instance_id": None, "region": None, "account_id": None},
-    exception={"instance_id": None, "region": None, "account_id": None},
+    text=MappingProxyType({"instance_id": None, "region": None, "account_id": None}),
+    exception=MappingProxyType({"instance_id": None, "region": None, "account_id": None}),
 ):
     with requests_mock.Mocker() as m:
         # request made to get instance_id
@@ -64,124 +66,97 @@ def get_test_aws_instance(
 # all good data
 @pytest.fixture
 def good_data_mock_instance():
-    return get_test_aws_instance(
+    instance = get_test_aws_instance(
         text={
             "instance_id": INSTANCE_ID_RESPONSE,
             "region": AVAILABILITY_ZONE_RESPONSE,
             "account_id": INSTANCE_IDENTITY_DOCUMENT_RESPONSE,
         }
     )
+    yield instance
+    del instance
 
 
 def test_is_instance_good_data(good_data_mock_instance):
-    assert good_data_mock_instance.is_instance()
+    assert good_data_mock_instance.is_instance
 
 
-def test_get_instance_id_good_data(good_data_mock_instance):
-    assert good_data_mock_instance.get_instance_id() == EXPECTED_INSTANCE_ID
+def test_instance_id_good_data(good_data_mock_instance):
+    assert good_data_mock_instance.instance_id == EXPECTED_INSTANCE_ID
 
 
-def test_get_region_good_data(good_data_mock_instance):
-    assert good_data_mock_instance.get_region() == EXPECTED_REGION
+def test_region_good_data(good_data_mock_instance):
+    assert good_data_mock_instance.region == EXPECTED_REGION
 
 
-def test_get_account_id_good_data(good_data_mock_instance):
-    assert good_data_mock_instance.get_account_id() == EXPECTED_ACCOUNT_ID
+def test_account_id_good_data(good_data_mock_instance):
+    assert good_data_mock_instance.account_id == EXPECTED_ACCOUNT_ID
 
 
 # 'region' bad data
 @pytest.fixture
 def bad_region_data_mock_instance():
-    return get_test_aws_instance(
+    instance = get_test_aws_instance(
         text={
             "instance_id": INSTANCE_ID_RESPONSE,
             "region": "in-a-different-world",
             "account_id": INSTANCE_IDENTITY_DOCUMENT_RESPONSE,
         }
     )
+    yield instance
+    del instance
 
 
 def test_is_instance_bad_region_data(bad_region_data_mock_instance):
-    assert bad_region_data_mock_instance.is_instance()
+    assert bad_region_data_mock_instance.is_instance
 
 
-def test_get_instance_id_bad_region_data(bad_region_data_mock_instance):
-    assert bad_region_data_mock_instance.get_instance_id() == EXPECTED_INSTANCE_ID
+def test_instance_id_bad_region_data(bad_region_data_mock_instance):
+    assert bad_region_data_mock_instance.instance_id == EXPECTED_INSTANCE_ID
 
 
-def test_get_region_bad_region_data(bad_region_data_mock_instance):
-    assert bad_region_data_mock_instance.get_region() is None
+def test_region_bad_region_data(bad_region_data_mock_instance):
+    assert bad_region_data_mock_instance.region is None
 
 
-def test_get_account_id_bad_region_data(bad_region_data_mock_instance):
-    assert bad_region_data_mock_instance.get_account_id() == EXPECTED_ACCOUNT_ID
+def test_account_id_bad_region_data(bad_region_data_mock_instance):
+    assert bad_region_data_mock_instance.account_id == EXPECTED_ACCOUNT_ID
 
 
 # 'account_id' bad data
 @pytest.fixture
 def bad_account_id_data_mock_instance():
-    return get_test_aws_instance(
+    instance = get_test_aws_instance(
         text={
             "instance_id": INSTANCE_ID_RESPONSE,
             "region": AVAILABILITY_ZONE_RESPONSE,
             "account_id": "who-am-i",
         }
     )
+    yield instance
+    del instance
 
 
 def test_is_instance_bad_account_id_data(bad_account_id_data_mock_instance):
-    assert bad_account_id_data_mock_instance.is_instance()
+    assert not bad_account_id_data_mock_instance.is_instance
 
 
-def test_get_instance_id_bad_account_id_data(bad_account_id_data_mock_instance):
-    assert bad_account_id_data_mock_instance.get_instance_id() == EXPECTED_INSTANCE_ID
+def test_instance_id_bad_account_id_data(bad_account_id_data_mock_instance):
+    assert bad_account_id_data_mock_instance.instance_id is None
 
 
-def test_get_region_bad_account_id_data(bad_account_id_data_mock_instance):
-    assert bad_account_id_data_mock_instance.get_region() == EXPECTED_REGION
+def test_region_bad_account_id_data(bad_account_id_data_mock_instance):
+    assert bad_account_id_data_mock_instance.region is None
 
 
-def test_get_account_id_data_bad_account_id_data(bad_account_id_data_mock_instance):
-    assert bad_account_id_data_mock_instance.get_account_id() is None
-
-
-# 'instance_id' bad requests
-@pytest.fixture
-def bad_instance_id_request_mock_instance(instance_id_exception):
-    return get_test_aws_instance(
-        text={
-            "instance_id": None,
-            "region": AVAILABILITY_ZONE_RESPONSE,
-            "account_id": INSTANCE_IDENTITY_DOCUMENT_RESPONSE,
-        },
-        exception={"instance_id": instance_id_exception, "region": None, "account_id": None},
-    )
-
-
-@pytest.mark.parametrize("instance_id_exception", [requests.RequestException, IOError])
-def test_is_instance_bad_instance_id_request(bad_instance_id_request_mock_instance):
-    assert bad_instance_id_request_mock_instance.is_instance() is False
-
-
-@pytest.mark.parametrize("instance_id_exception", [requests.RequestException, IOError])
-def test_get_instance_id_bad_instance_id_request(bad_instance_id_request_mock_instance):
-    assert bad_instance_id_request_mock_instance.get_instance_id() is None
-
-
-@pytest.mark.parametrize("instance_id_exception", [requests.RequestException, IOError])
-def test_get_region_bad_instance_id_request(bad_instance_id_request_mock_instance):
-    assert bad_instance_id_request_mock_instance.get_region() is None
-
-
-@pytest.mark.parametrize("instance_id_exception", [requests.RequestException, IOError])
-def test_get_account_id_bad_instance_id_request(bad_instance_id_request_mock_instance):
-    assert bad_instance_id_request_mock_instance.get_account_id() == EXPECTED_ACCOUNT_ID
+def test_account_id_data_bad_account_id_data(bad_account_id_data_mock_instance):
+    assert bad_account_id_data_mock_instance.account_id is None
 
 
 # 'region' bad requests
 @pytest.fixture
 def bad_region_request_mock_instance(region_exception):
-    return get_test_aws_instance(
+    instance = get_test_aws_instance(
         text={
             "instance_id": INSTANCE_ID_RESPONSE,
             "region": None,
@@ -189,59 +164,28 @@ def bad_region_request_mock_instance(region_exception):
         },
         exception={"instance_id": None, "region": region_exception, "account_id": None},
     )
+    yield instance
+    del instance
 
 
 @pytest.mark.parametrize("region_exception", [requests.RequestException, IOError])
 def test_is_instance_bad_region_request(bad_region_request_mock_instance):
-    assert bad_region_request_mock_instance.is_instance()
+    assert not bad_region_request_mock_instance.is_instance
 
 
 @pytest.mark.parametrize("region_exception", [requests.RequestException, IOError])
-def test_get_instance_id_bad_region_request(bad_region_request_mock_instance):
-    assert bad_region_request_mock_instance.get_instance_id() == EXPECTED_INSTANCE_ID
+def test_instance_id_bad_region_request(bad_region_request_mock_instance):
+    assert bad_region_request_mock_instance.instance_id is None
 
 
 @pytest.mark.parametrize("region_exception", [requests.RequestException, IOError])
-def test_get_region_bad_region_request(bad_region_request_mock_instance):
-    assert bad_region_request_mock_instance.get_region() is None
+def test_region_bad_region_request(bad_region_request_mock_instance):
+    assert bad_region_request_mock_instance.region is None
 
 
 @pytest.mark.parametrize("region_exception", [requests.RequestException, IOError])
-def test_get_account_id_bad_region_request(bad_region_request_mock_instance):
-    assert bad_region_request_mock_instance.get_account_id() == EXPECTED_ACCOUNT_ID
-
-
-# 'account_id' bad requests
-@pytest.fixture
-def bad_account_id_request_mock_instance(account_id_exception):
-    return get_test_aws_instance(
-        text={
-            "instance_id": INSTANCE_ID_RESPONSE,
-            "region": AVAILABILITY_ZONE_RESPONSE,
-            "account_id": None,
-        },
-        exception={"instance_id": None, "region": None, "account_id": account_id_exception},
-    )
-
-
-@pytest.mark.parametrize("account_id_exception", [requests.RequestException, IOError])
-def test_is_instance_bad_account_id_request(bad_account_id_request_mock_instance):
-    assert bad_account_id_request_mock_instance.is_instance()
-
-
-@pytest.mark.parametrize("account_id_exception", [requests.RequestException, IOError])
-def test_get_instance_id_bad_account_id_request(bad_account_id_request_mock_instance):
-    assert bad_account_id_request_mock_instance.get_instance_id() == EXPECTED_INSTANCE_ID
-
-
-@pytest.mark.parametrize("account_id_exception", [requests.RequestException, IOError])
-def test_get_region_bad_account_id_request(bad_account_id_request_mock_instance):
-    assert bad_account_id_request_mock_instance.get_region() == EXPECTED_REGION
-
-
-@pytest.mark.parametrize("account_id_exception", [requests.RequestException, IOError])
-def test_get_account_id_bad_account_id_request(bad_account_id_request_mock_instance):
-    assert bad_account_id_request_mock_instance.get_account_id() is None
+def test_account_id_bad_region_request(bad_region_request_mock_instance):
+    assert bad_region_request_mock_instance.account_id is None
 
 
 # not found request
@@ -261,20 +205,21 @@ def not_found_request_mock_instance():
         m.get(url)
 
         not_found_aws_instance_object = AwsInstance()
-        return not_found_aws_instance_object
+        yield not_found_aws_instance_object
+        del not_found_aws_instance_object
 
 
 def test_is_instance_not_found_request(not_found_request_mock_instance):
-    assert not_found_request_mock_instance.is_instance() is False
+    assert not_found_request_mock_instance.is_instance is False
 
 
-def test_get_instance_id_not_found_request(not_found_request_mock_instance):
-    assert not_found_request_mock_instance.get_instance_id() is None
+def test_instance_id_not_found_request(not_found_request_mock_instance):
+    assert not_found_request_mock_instance.instance_id is None
 
 
-def test_get_region_not_found_request(not_found_request_mock_instance):
-    assert not_found_request_mock_instance.get_region() is None
+def test_region_not_found_request(not_found_request_mock_instance):
+    assert not_found_request_mock_instance.region is None
 
 
-def test_get_account_id_not_found_request(not_found_request_mock_instance):
-    assert not_found_request_mock_instance.get_account_id() is None
+def test_account_id_not_found_request(not_found_request_mock_instance):
+    assert not_found_request_mock_instance.account_id is None

monkey/tests/unit_tests/monkey_island/cc/services/test_aws_service.py

@@ -1,7 +1,7 @@
 import json
 from unittest import TestCase
 
-from common.cloud.aws.aws_service import filter_instance_data_from_aws_response
+from monkey_island.cc.services.aws_service import filter_instance_data_from_aws_response
 
 
 class TestAwsService(TestCase):