From 797482a172d2c9fc89b99a19a224440fe55f35d4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Thu, 28 Apr 2022 08:43:38 -0400 Subject: [PATCH] Common: Replace protected attributes with read-only properties --- monkey/common/cloud/aws/aws_instance.py | 26 +++--- monkey/common/cloud/aws/aws_service.py | 2 +- .../utils/aws_environment_check.py | 2 +- .../cc/services/remote_run_aws.py | 2 +- .../cc/services/reporting/aws_exporter.py | 2 +- .../common/cloud/aws/test_aws_instance.py | 84 +++++++++---------- 6 files changed, 60 insertions(+), 58 deletions(-) diff --git a/monkey/common/cloud/aws/aws_instance.py b/monkey/common/cloud/aws/aws_instance.py index 8f00cc399..ced7d6ab5 100644 --- a/monkey/common/cloud/aws/aws_instance.py +++ b/monkey/common/cloud/aws/aws_instance.py @@ -33,16 +33,24 @@ class AwsInstance(CloudInstance): __metaclass__ = Singleton def __init__(self): - self._is_instance, instance_info = AwsInstance._fetch_instance_info() - - self._instance_id = instance_info.instance_id - self._region = instance_info.region - self._account_id = instance_info.account_id + 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: @@ -93,12 +101,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): """ @@ -117,4 +119,4 @@ class AwsInstance(CloudInstance): :return: the AWS account ID which "owns" this instance. See https://docs.aws.amazon.com/general/latest/gr/acct-identifiers.html """ - return self._account_id + return self.account_id diff --git a/monkey/common/cloud/aws/aws_service.py b/monkey/common/cloud/aws/aws_service.py index 4a9ded280..3cacc1a8f 100644 --- a/monkey/common/cloud/aws/aws_service.py +++ b/monkey/common/cloud/aws/aws_service.py @@ -63,7 +63,7 @@ class AwsService(object): :return: All visible instances from this instance """ current_instance = AwsInstance() - local_ssm_client = boto3.client("ssm", current_instance.get_region()) + local_ssm_client = boto3.client("ssm", current_instance.region) try: response = local_ssm_client.describe_instance_information() diff --git a/monkey/infection_monkey/utils/aws_environment_check.py b/monkey/infection_monkey/utils/aws_environment_check.py index af84354e8..f508135ca 100644 --- a/monkey/infection_monkey/utils/aws_environment_check.py +++ b/monkey/infection_monkey/utils/aws_environment_check.py @@ -21,7 +21,7 @@ def _report_aws_environment(telemetry_messenger: LegacyTelemetryMessengerAdapter if _running_on_aws(aws_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") diff --git a/monkey/monkey_island/cc/services/remote_run_aws.py b/monkey/monkey_island/cc/services/remote_run_aws.py index 3c0ecb9d2..26ec58e5c 100644 --- a/monkey/monkey_island/cc/services/remote_run_aws.py +++ b/monkey/monkey_island/cc/services/remote_run_aws.py @@ -57,7 +57,7 @@ class RemoteRunAwsService: """ Updates the AWS region without auth params (via IAM role) """ - AwsService.set_region(RemoteRunAwsService.aws_instance._region) + AwsService.set_region(RemoteRunAwsService.aws_instance.region) @staticmethod def _run_aws_monkey_cmd_async(instance_id, is_linux, island_ip): diff --git a/monkey/monkey_island/cc/services/reporting/aws_exporter.py b/monkey/monkey_island/cc/services/reporting/aws_exporter.py index 7c6d0903d..ce3f6a953 100644 --- a/monkey/monkey_island/cc/services/reporting/aws_exporter.py +++ b/monkey/monkey_island/cc/services/reporting/aws_exporter.py @@ -36,7 +36,7 @@ class AWSExporter(Exporter): return True # Not suppressing error here on purpose. - current_aws_region = AwsInstance().get_region() + current_aws_region = AwsInstance().region for machine in issues_list: for issue in issues_list[machine]: diff --git a/monkey/tests/unit_tests/common/cloud/aws/test_aws_instance.py b/monkey/tests/unit_tests/common/cloud/aws/test_aws_instance.py index 2981a7bd1..54d8e942a 100644 --- a/monkey/tests/unit_tests/common/cloud/aws/test_aws_instance.py +++ b/monkey/tests/unit_tests/common/cloud/aws/test_aws_instance.py @@ -77,16 +77,16 @@ def test_is_instance_good_data(good_data_mock_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 @@ -105,16 +105,16 @@ def test_is_instance_bad_region_data(bad_region_data_mock_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 @@ -133,16 +133,16 @@ def test_is_instance_bad_account_id_data(bad_account_id_data_mock_instance): assert 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 == EXPECTED_INSTANCE_ID -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 == EXPECTED_REGION -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 +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 # 'instance_id' bad requests @@ -164,18 +164,18 @@ def test_is_instance_bad_instance_id_request(bad_instance_id_request_mock_instan @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 +def test_instance_id_bad_instance_id_request(bad_instance_id_request_mock_instance): + assert bad_instance_id_request_mock_instance.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 +def test_region_bad_instance_id_request(bad_instance_id_request_mock_instance): + assert bad_instance_id_request_mock_instance.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_bad_instance_id_request(bad_instance_id_request_mock_instance): + assert bad_instance_id_request_mock_instance.account_id == EXPECTED_ACCOUNT_ID # 'region' bad requests @@ -197,18 +197,18 @@ def test_is_instance_bad_region_request(bad_region_request_mock_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 == EXPECTED_INSTANCE_ID @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 +def test_account_id_bad_region_request(bad_region_request_mock_instance): + assert bad_region_request_mock_instance.account_id == EXPECTED_ACCOUNT_ID # 'account_id' bad requests @@ -230,18 +230,18 @@ def test_is_instance_bad_account_id_request(bad_account_id_request_mock_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 +def test_instance_id_bad_account_id_request(bad_account_id_request_mock_instance): + assert bad_account_id_request_mock_instance.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 +def test_region_bad_account_id_request(bad_account_id_request_mock_instance): + assert bad_account_id_request_mock_instance.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_account_id_request(bad_account_id_request_mock_instance): + assert bad_account_id_request_mock_instance.account_id is None # not found request @@ -268,13 +268,13 @@ def test_is_instance_not_found_request(not_found_request_mock_instance): 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