From 090e8415a3729da6248d25bb874c8a864cb2d2e1 Mon Sep 17 00:00:00 2001 From: Thirumalesh Aaraveti <97395760+athiruma@users.noreply.github.com> Date: Tue, 29 Oct 2024 17:13:28 +0530 Subject: [PATCH] Add the cost-savings tag (#856) --- .../common/clouds/aws/iam/iam_operations.py | 20 +++++++- .../common/clouds/aws/rds/rds_operations.py | 14 ++++++ .../common/clouds/aws/utils/common_methods.py | 1 + .../policy/aws/cleanup/database_idle.py | 5 ++ .../policy/aws/cleanup/instance_idle.py | 3 ++ .../policy/aws/cleanup/unattached_volume.py | 3 ++ .../policy/aws/cleanup/unused_nat_gateway.py | 6 +++ .../policy/aws/zombie_snapshots.py | 5 ++ .../helpers/aws/aws_policy_operations.py | 50 +++++++++++++++---- .../policy/aws/cleanup/test_database_idle.py | 2 + .../policy/aws/cleanup/test_instance_idle.py | 10 ++-- .../aws/cleanup/test_unattached_volume.py | 7 ++- .../aws/cleanup/test_unused_nat_gateway.py | 5 ++ .../policy/aws/test_zombie_snapshots.py | 8 ++- 14 files changed, 120 insertions(+), 19 deletions(-) diff --git a/cloud_governance/common/clouds/aws/iam/iam_operations.py b/cloud_governance/common/clouds/aws/iam/iam_operations.py index 5aef03da..b4f5ac3c 100644 --- a/cloud_governance/common/clouds/aws/iam/iam_operations.py +++ b/cloud_governance/common/clouds/aws/iam/iam_operations.py @@ -38,7 +38,8 @@ def get_roles(self): This method returns all roles @return: """ - return self.utils.get_details_resource_list(func_name=self.iam_client.list_roles, input_tag='Roles', check_tag='Marker') + return self.utils.get_details_resource_list(func_name=self.iam_client.list_roles, input_tag='Roles', + check_tag='Marker') def get_users(self): """ @@ -113,7 +114,8 @@ def list_attached_role_policies(self, role_name: str): """ attached_policies = [] try: - attached_policies = self.iam_client.list_attached_role_policies(RoleName=role_name).get('AttachedPolicies', []) + attached_policies = self.iam_client.list_attached_role_policies(RoleName=role_name).get('AttachedPolicies', + []) except Exception as err: logger.error(err) return attached_policies @@ -142,3 +144,17 @@ def tag_role(self, role_name: str, tags: list): return True except Exception as err: raise err + + def untag_role(self, role_name: str, tags: list): + """ + This method untags the iam role + :param role_name: + :param tags: + :return: + """ + try: + self.iam_client.untag_role(RoleName=role_name, + TagKeys=[key for tag in tags for key, _ in tag.items() if key == 'Key']) + return True + except Exception as err: + raise err diff --git a/cloud_governance/common/clouds/aws/rds/rds_operations.py b/cloud_governance/common/clouds/aws/rds/rds_operations.py index 3001b52b..3ab34552 100644 --- a/cloud_governance/common/clouds/aws/rds/rds_operations.py +++ b/cloud_governance/common/clouds/aws/rds/rds_operations.py @@ -41,3 +41,17 @@ def add_tags_to_resource(self, resource_arn: str, tags: list): logger.info(f"Tags are updated to the resource: {resource_arn}") except Exception as err: logger.error(f"Something went wrong in add/ update tags: {err}") + + def remove_tags_from_resource(self, resource_arn: str, tags: list): + """ + This method deletes the tags of the rds resource + :param resource_arn: + :param tags: + :return: + """ + try: + tags_keys = [key for tag in tags for key, _ in tag.items() if key == 'Key'] + self._db_client.remove_tags_from_resource(ResourceName=resource_arn, Tags=tags_keys) + logger.info(f"Tags: {tags_keys} are deleted to the resource: {resource_arn}") + except Exception as err: + logger.error(f"Something went wrong in add/ update tags: {err}") diff --git a/cloud_governance/common/clouds/aws/utils/common_methods.py b/cloud_governance/common/clouds/aws/utils/common_methods.py index f43061a1..76088803 100644 --- a/cloud_governance/common/clouds/aws/utils/common_methods.py +++ b/cloud_governance/common/clouds/aws/utils/common_methods.py @@ -20,6 +20,7 @@ def get_tag_value_from_tags(tags: list, tag_name: str, cast_type: str = 'str', :rtype: """ if tags: + tag_name = tag_name.lower().replace("_", '').replace("-", '').strip() for tag in tags: key = tag.get('Key').lower().replace("_", '').replace("-", '').strip() if key == tag_name.lower(): diff --git a/cloud_governance/policy/aws/cleanup/database_idle.py b/cloud_governance/policy/aws/cleanup/database_idle.py index a7f34157..368c6b16 100644 --- a/cloud_governance/policy/aws/cleanup/database_idle.py +++ b/cloud_governance/policy/aws/cleanup/database_idle.py @@ -54,7 +54,12 @@ def run_policy_operations(self): resource_state=db.get('DBInstanceStatus') if not cleanup_result else "Deleted" ) + if not cleanup_result: + self.update_resource_tags(resource_id=resource_arn, tags=tags + self.cost_savings_tag) idle_dbs.append(resource_data) + else: + cleanup_days = 0 + self.delete_resource_tags(resource_id=resource_arn, tags=self.cost_savings_tag) if not cleanup_result: self.update_resource_day_count_tag(resource_id=resource_arn, cleanup_days=cleanup_days, tags=tags) diff --git a/cloud_governance/policy/aws/cleanup/instance_idle.py b/cloud_governance/policy/aws/cleanup/instance_idle.py index b7327462..05be4ef9 100644 --- a/cloud_governance/policy/aws/cleanup/instance_idle.py +++ b/cloud_governance/policy/aws/cleanup/instance_idle.py @@ -60,7 +60,10 @@ def run_policy_operations(self): if self._force_delete and self._dry_run == 'no': resource_data.update({'ForceDeleted': str(self._force_delete)}) idle_instances.append(resource_data) + if not cleanup_result: + self.update_resource_tags(resource_id=instance_id, tags=tags + self.cost_savings_tag) else: + self.delete_resource_tags(resource_id=instance_id, tags=self.cost_savings_tag) cleanup_days = 0 if not cleanup_result: self.update_resource_day_count_tag(resource_id=instance_id, cleanup_days=cleanup_days, tags=tags) diff --git a/cloud_governance/policy/aws/cleanup/unattached_volume.py b/cloud_governance/policy/aws/cleanup/unattached_volume.py index 8579537c..772c60c2 100644 --- a/cloud_governance/policy/aws/cleanup/unattached_volume.py +++ b/cloud_governance/policy/aws/cleanup/unattached_volume.py @@ -48,8 +48,11 @@ def run_policy_operations(self): volume_size=f"{volume.get('Size')} GB" ) unattached_volumes.append(resource_data) + if not cleanup_result: + self.update_resource_tags(resource_id=resource_id, tags=tags + self.cost_savings_tag) else: cleanup_days = 0 + self.delete_resource_tags(resource_id=resource_id, tags=self.cost_savings_tag) if not cleanup_result: self.update_resource_day_count_tag(resource_id=resource_id, cleanup_days=cleanup_days, tags=tags) diff --git a/cloud_governance/policy/aws/cleanup/unused_nat_gateway.py b/cloud_governance/policy/aws/cleanup/unused_nat_gateway.py index fd62bcad..221ee3f7 100644 --- a/cloud_governance/policy/aws/cleanup/unused_nat_gateway.py +++ b/cloud_governance/policy/aws/cleanup/unused_nat_gateway.py @@ -89,6 +89,12 @@ def run_policy_operations(self): unit_price=unit_price if not cleanup_result else "Deleted") unused_nat_gateways.append(resource_data) + if not cleanup_result: + self.update_resource_tags(resource_id=resource_id, tags=tags + self.cost_savings_tag) + else: + cleanup_days = 0 + self.delete_resource_tags(resource_id=resource_id, tags=self.cost_savings_tag) + if not cleanup_result: self.update_resource_day_count_tag(resource_id=resource_id, cleanup_days=cleanup_days, tags=tags) diff --git a/cloud_governance/policy/aws/zombie_snapshots.py b/cloud_governance/policy/aws/zombie_snapshots.py index d4b6e9b5..6b651909 100644 --- a/cloud_governance/policy/aws/zombie_snapshots.py +++ b/cloud_governance/policy/aws/zombie_snapshots.py @@ -63,6 +63,11 @@ def run(self): resource_state='Backup' if not cleanup_result else "Deleted" ) zombie_snapshots.append(resource_data) + if not cleanup_result: + self.update_resource_tags(resource_id=resource_id, tags=tags + self.cost_savings_tag) + else: + cleanup_days = 0 + self.delete_resource_tags(resource_id=resource_id, tags=self.cost_savings_tag) if not cleanup_result: self.update_resource_day_count_tag(resource_id=resource_id, cleanup_days=cleanup_days, tags=tags) return zombie_snapshots diff --git a/cloud_governance/policy/helpers/aws/aws_policy_operations.py b/cloud_governance/policy/helpers/aws/aws_policy_operations.py index 998345b4..017536ab 100644 --- a/cloud_governance/policy/helpers/aws/aws_policy_operations.py +++ b/cloud_governance/policy/helpers/aws/aws_policy_operations.py @@ -27,6 +27,7 @@ def __init__(self): self._ec2_operations = EC2Operations(region=self._region) self._cloudwatch = CloudWatchOperations(region=self._region) self._resource_pricing = ResourcesPricing() + self.cost_savings_tag = [{'Key': 'cost-savings', 'Value': 'true'}] def get_tag_name_from_tags(self, tags: list, tag_name: str) -> str: """ @@ -117,22 +118,31 @@ def _update_tag_value(self, tags: list, tag_name: str, tag_value: str): tags = self.__remove_tag_key_aws(tags=tags) return tags - def update_resource_day_count_tag(self, resource_id: str, cleanup_days: int, tags: list, - force_tag_update: str = ''): + def delete_resource_tags(self, tags: list, resource_id: str): """ - This method updates the resource tags - :param force_tag_update: - :type force_tag_update: + This method deletes the tag of a resource + :param tags: + :param resource_id: + :return: + """ + try: + if self._policy == 'empty_roles': + self._iam_operations.untag_role(role_name=resource_id, tags=tags) + elif self._policy in ('ip_unattached', 'unused_nat_gateway', 'zombie_snapshots', 'unattached_volume', + 'instance_run', 'instance_idle'): + self._ec2_client.delete_tags(Resources=[resource_id], Tags=tags) + elif self._policy == 'database_idle': + self._rds_operations.remove_tags_from_resource(resource_arn=resource_id, tags=tags) + except Exception as err: + logger.info(f'Exception raised: {err}: {resource_id}') + + def update_resource_tags(self, tags: list, resource_id: str): + """ + This method updates the tags of the resource :param tags: - :type tags: - :param cleanup_days: - :type cleanup_days: :param resource_id: - :type resource_id: :return: - :rtype: """ - tags = self._update_tag_value(tags=tags, tag_name='DaysCount', tag_value=str(cleanup_days)) try: if self._policy == 's3_inactive': self._s3_client.put_bucket_tagging(Bucket=resource_id, Tagging={'TagSet': tags}) @@ -146,6 +156,24 @@ def update_resource_day_count_tag(self, resource_id: str, cleanup_days: int, tag except Exception as err: logger.info(f'Exception raised: {err}: {resource_id}') + def update_resource_day_count_tag(self, resource_id: str, cleanup_days: int, tags: list, + force_tag_update: str = ''): + """ + This method updates the resource tags + :param force_tag_update: + :type force_tag_update: + :param tags: + :type tags: + :param cleanup_days: + :type cleanup_days: + :param resource_id: + :type resource_id: + :return: + :rtype: + """ + tags = self._update_tag_value(tags=tags, tag_name='DaysCount', tag_value=str(cleanup_days)) + self.update_resource_tags(tags=tags, resource_id=resource_id) + def _get_all_instances(self): """ This method updates the instance type count to the elasticsearch diff --git a/tests/unittest/cloud_governance/policy/aws/cleanup/test_database_idle.py b/tests/unittest/cloud_governance/policy/aws/cleanup/test_database_idle.py index a6ac3b10..d09b8229 100644 --- a/tests/unittest/cloud_governance/policy/aws/cleanup/test_database_idle.py +++ b/tests/unittest/cloud_governance/policy/aws/cleanup/test_database_idle.py @@ -36,6 +36,8 @@ def test_database_idle(): assert len(running_instances_data) == 1 assert get_tag_value_from_tags(tags=rds_client.describe_db_instances()['DBInstances'][0]['TagList'], tag_name='DaysCount') == f"{current_date.date()}@1" + assert get_tag_value_from_tags(tags=rds_client.describe_db_instances()['DBInstances'][0]['TagList'], + tag_name='cost-savings') == "true" @mock_cloudwatch diff --git a/tests/unittest/cloud_governance/policy/aws/cleanup/test_instance_idle.py b/tests/unittest/cloud_governance/policy/aws/cleanup/test_instance_idle.py index 73057d76..6aa50f6e 100644 --- a/tests/unittest/cloud_governance/policy/aws/cleanup/test_instance_idle.py +++ b/tests/unittest/cloud_governance/policy/aws/cleanup/test_instance_idle.py @@ -5,6 +5,7 @@ import boto3 from moto import mock_ec2, mock_cloudwatch +from cloud_governance.common.clouds.aws.utils.common_methods import get_tag_value_from_tags from cloud_governance.main.environment_variables import environment_variables from cloud_governance.policy.aws.cleanup.instance_idle import InstanceIdle from tests.unittest.configs import AWS_DEFAULT_REGION, DRY_RUN_YES, DEFAULT_AMI_ID, INSTANCE_TYPE_T2_MICRO, \ @@ -15,20 +16,23 @@ @mock_ec2 def test_instance_idle__instance_age_less_than_7(): """ - This method tests instance_idle of less than DEFAULT_DAYA 7 + This method tests instance_idle of less than DEFAULT_DAYs 7 :return: :rtype: """ environment_variables.environment_variables_dict['dry_run'] = DRY_RUN_YES environment_variables.environment_variables_dict['policy'] = 'instance_idle' + environment_variables.environment_variables_dict['AWS_DEFAULT_REGION'] = AWS_DEFAULT_REGION ec2_client = boto3.client('ec2', region_name=AWS_DEFAULT_REGION) tags = [{'Key': 'User', 'Value': TEST_USER_NAME}] - ec2_client.run_instances(ImageId=DEFAULT_AMI_ID, InstanceType=INSTANCE_TYPE_T2_MICRO, MaxCount=1, MinCount=1, + ec2_client.run_instances(ImageId=DEFAULT_AMI_ID, InstanceType=INSTANCE_TYPE_T2_MICRO, MaxCount=1, + MinCount=1, TagSpecifications=[{'ResourceType': 'instance', 'Tags': tags}]) instance_idle = InstanceIdle() response = instance_idle.run() assert len(response) == 0 + assert not get_tag_value_from_tags(instance_idle._get_all_instances()[0]['Tags'], tag_name='cost-savings') def mock_describe_instances(*args, **kwargs): @@ -81,7 +85,7 @@ def test_instance_idle__check_not_idle(): MockCloudWatchMetric(metrics=[5, 4, 8, 10]).create_metric(), MockCloudWatchMetric(metrics=[5000, 2000, 4000, 8000]).create_metric(), MockCloudWatchMetric(metrics=[1000, 200, 500]).create_metric() - ] + ] instance_idle = InstanceIdle() response = instance_idle.run() assert len(response) == 0 diff --git a/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py b/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py index e8cbcf60..678f5c6f 100644 --- a/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py +++ b/tests/unittest/cloud_governance/policy/aws/cleanup/test_unattached_volume.py @@ -1,6 +1,7 @@ import boto3 from moto import mock_ec2 +from cloud_governance.common.clouds.aws.utils.common_methods import get_tag_value_from_tags from cloud_governance.main.environment_variables import environment_variables from cloud_governance.policy.aws.cleanup.unattached_volume import UnattachedVolume @@ -21,6 +22,8 @@ def test_unattached_volume_dry_run_yes_0_unattached(): volume_run = UnattachedVolume() response = volume_run.run() assert len(response) == 0 + assert not get_tag_value_from_tags(tags=volume_run._get_all_volumes()[0]['Tags'], + tag_name='cost-savings') @mock_ec2 @@ -36,6 +39,8 @@ def test_unattached_volume_dry_run_yes(): response = response[0] assert response.get('ResourceAction') == 'False' assert response.get('SkipPolicy') == 'NA' + assert get_tag_value_from_tags(tags=volume_run._get_all_volumes()[0]['Tags'], + tag_name='cost-savings') == 'true' @mock_ec2 @@ -103,7 +108,7 @@ def test_check_exists_cluster(): ec2_client = boto3.client('ec2', region_name=region_name) default_ami_id = 'ami-03cf127a' ec2_client.run_instances(ImageId=default_ami_id, InstanceType='t2.micro', MaxCount=1, - MinCount=1, TagSpecifications=[{ 'ResourceType': 'instance','Tags': tags}]) + MinCount=1, TagSpecifications=[{'ResourceType': 'instance', 'Tags': tags}]) ec2_client.create_volume(AvailabilityZone=f'{region_name}a', Size=10, TagSpecifications=[{ 'ResourceType': 'volume', 'Tags': tags diff --git a/tests/unittest/cloud_governance/policy/aws/cleanup/test_unused_nat_gateway.py b/tests/unittest/cloud_governance/policy/aws/cleanup/test_unused_nat_gateway.py index 118bb50b..6b0b3db9 100644 --- a/tests/unittest/cloud_governance/policy/aws/cleanup/test_unused_nat_gateway.py +++ b/tests/unittest/cloud_governance/policy/aws/cleanup/test_unused_nat_gateway.py @@ -3,6 +3,7 @@ import boto3 from moto import mock_ec2, mock_cloudwatch +from cloud_governance.common.clouds.aws.utils.common_methods import get_tag_value_from_tags from cloud_governance.main.environment_variables import environment_variables from cloud_governance.policy.aws.cleanup.unused_nat_gateway import UnUsedNatGateway from tests.unittest.configs import AWS_DEFAULT_REGION, NAT_GATEWAY_NAMESPACE @@ -25,6 +26,8 @@ def test_unused_nat_gateway_dry_run_yes(): response = unused_nat_gateway.run() assert len(response) == 1 assert response[0]['CleanUpDays'] == 0 + assert get_tag_value_from_tags(tags=unused_nat_gateway._ec2_operations.get_nat_gateways()[0]['Tags'], + tag_name='cost-savings') == 'true' @mock_cloudwatch @@ -62,6 +65,8 @@ def test_unused_nat_gateway_dry_run_yes_collect_none(): unused_nat_gateway = UnUsedNatGateway() response = unused_nat_gateway.run() assert len(response) == 0 + assert get_tag_value_from_tags(tags=unused_nat_gateway._ec2_operations.get_nat_gateways()[0]['Tags'], + tag_name='cost-savings') == '' @mock_ec2 diff --git a/tests/unittest/cloud_governance/policy/aws/test_zombie_snapshots.py b/tests/unittest/cloud_governance/policy/aws/test_zombie_snapshots.py index 7793b51d..2eca57f5 100644 --- a/tests/unittest/cloud_governance/policy/aws/test_zombie_snapshots.py +++ b/tests/unittest/cloud_governance/policy/aws/test_zombie_snapshots.py @@ -11,7 +11,6 @@ TEST_USER_NAME, DRY_RUN_NO - @mock_ec2 def test_zombie_snapshots(): """ @@ -52,8 +51,11 @@ def test_zombie_snapshots(): assert len(response) == 1 assert response[0]['CleanUpDays'] == 0 assert get_tag_value_from_tags(tags=ec2_client.describe_snapshots(OwnerIds=['self'], - SnapshotIds=[snapshot_id])['Snapshots'][0]['Tags'], + SnapshotIds=[snapshot_id])['Snapshots'][0][ + 'Tags'], tag_name='DaysCount') + assert get_tag_value_from_tags(tags=zombie_snapshots._ec2_operations.get_snapshots()[0]['Tags'], + tag_name='cost-savings') == 'true' @mock_ec2 @@ -136,6 +138,8 @@ def test_zombie_snapshots_skip(): response = zombie_snapshots.run() assert len(ec2_client.describe_snapshots(OwnerIds=['self'])['Snapshots']) == 1 assert len(response) == 0 + assert get_tag_value_from_tags(tags=zombie_snapshots._ec2_operations.get_snapshots()[0]['Tags'], + tag_name='cost-savings') == '' @mock_ec2