diff --git a/local-app/python-tools/gfl-resource-actions/aws_resource_management/aws_utils.py b/local-app/python-tools/gfl-resource-actions/aws_resource_management/aws_utils.py index 2052acce..adf8900f 100644 --- a/local-app/python-tools/gfl-resource-actions/aws_resource_management/aws_utils.py +++ b/local-app/python-tools/gfl-resource-actions/aws_resource_management/aws_utils.py @@ -503,7 +503,8 @@ def get_account_list() -> List[Dict[str, str]]: _session_cache[cache_key] = {"accounts": accounts, "timestamp": time.time()} return accounts - except Exception: + except Exception as e: + logger.warning(f"Failed to get accounts from Organizations API: {str(e)}") # Fall back to profiles accounts = _get_accounts_from_profiles() @@ -522,11 +523,26 @@ def get_organization_accounts() -> List[Dict[str, str]]: List of dictionaries with account information """ try: - accounts = _get_accounts_from_profiles() + # Use Organizations API to get accounts + session = boto3.Session() + org_client = session.client("organizations") + accounts = [] + + # Use pagination to get all accounts + paginator = org_client.get_paginator("list_accounts") + for page in paginator.paginate(): + for account in page["Accounts"]: + if account["Status"] == "ACTIVE": + accounts.append( + {"account_id": account["Id"], "account_name": account["Name"]} + ) + + logger.info(f"Found {len(accounts)} accounts in Organizations API") return accounts except Exception as e: - logger.error(f"Error getting accounts from profiles: {str(e)}") - return [] + logger.error(f"Error getting accounts from Organizations API: {str(e)}") + logger.info("Falling back to accounts from profiles") + return _get_accounts_from_profiles() def _get_accounts_from_profiles() -> List[Dict[str, str]]: diff --git a/local-app/python-tools/gfl-resource-actions/aws_resource_management/core.py b/local-app/python-tools/gfl-resource-actions/aws_resource_management/core.py index f83dd1a4..0e4ffcde 100644 --- a/local-app/python-tools/gfl-resource-actions/aws_resource_management/core.py +++ b/local-app/python-tools/gfl-resource-actions/aws_resource_management/core.py @@ -73,11 +73,18 @@ def process_accounts( exclude_regions = exclude_regions or [] stats = stats or initialize_stats() - # Get account list - either from profiles or Organizations API + # Get account list from the organization you're currently authenticated with accounts = self._get_accounts() + # Pre-filter accounts that we can authenticate with + valid_accounts = self._pre_validate_accounts(accounts) + # Log summary of accounts - logger.info(f"Found {len(accounts)} accounts to process") + logger.info(f"Found {len(valid_accounts)} accessible accounts to process") + if len(accounts) > len(valid_accounts): + logger.info( + f"Skipped {len(accounts) - len(valid_accounts)} inaccessible accounts" + ) if exclude_accounts: logger.info(f"Excluding {len(exclude_accounts)} accounts") @@ -87,7 +94,7 @@ def process_accounts( ) as executor: # Create futures for each account futures = [] - for account in accounts: + for account in valid_accounts: account_id = account.get("account_id") if account_id in exclude_accounts: stats["accounts_skipped"] += 1 @@ -132,13 +139,42 @@ def process_accounts( logger.warning("Account processing interrupted by user") raise + def _pre_validate_accounts( + self, accounts: List[Dict[str, str]] + ) -> List[Dict[str, str]]: + """ + Pre-validate which accounts we can actually authenticate with. + + Args: + accounts: List of account dictionaries with account_id and account_name + + Returns: + List of accounts we can authenticate with + """ + valid_accounts = [] + for account in accounts: + account_id = account.get("account_id") + try: + # Try to get credentials without actually making any API calls + credentials = get_credentials(account_id, self.profile_name) + if credentials: + valid_accounts.append(account) + else: + logger.debug( + f"No credentials available for account {account_id}, skipping" + ) + except Exception as e: + logger.debug(f"Cannot authenticate with account {account_id}: {str(e)}") + + return valid_accounts + def _get_accounts(self) -> List[Dict[str, str]]: - """Get list of accounts from profiles or Organizations API.""" + """Get list of accounts from the current organization only.""" + # Always use the Organizations API by default to get accounts from current org + # Only use profiles if explicitly requested (which should be rare) if self.use_profiles: logger.info("Using AWS SSO profiles for account discovery") - return ( - get_organization_accounts() - ) # Changed from get_accounts_from_profiles + return get_organization_accounts() else: logger.info("Using AWS Organizations API for account discovery") return get_account_list() @@ -368,15 +404,24 @@ def _process_resources( exclusion_tag = self.config.get("exclusion_tag", "DoNotStop") emr_tag = self.config.get("emr_tag", "aws:elasticmapreduce:job-flow-id") ec2_instances = resources.get("ec2_instances", []) + + # Fixed: Safely check for EKS tag prefixes in instance tags eks_managed = sum( 1 for i in ec2_instances - if any(tag.startswith(eks_tag) for tag in i.get("tags", {})) + if any( + isinstance(tag_key, str) and tag_key.startswith(eks_tag) + for tag_key in i.get("tags", {}) + ) ) + + # Fixed: Safely check for EMR tags in instance tags emr_managed = sum(1 for i in ec2_instances if emr_tag in i.get("tags", {})) + excluded_ec2 = sum( 1 for i in ec2_instances if exclusion_tag in i.get("tags", {}) ) + # Update resource counts stats["ec2_found"] += len(ec2_instances) stats["ec2_skipped"] += excluded_ec2 @@ -415,18 +460,26 @@ def _log_resource_counts( for i in resources.get("ec2_instances", []) if self.config.get("exclusion_tag") in i.get("tags", {}) ) + eks_tag = self.config.get("eks_tag", "kubernetes.io/cluster/") emr_tag = self.config.get("emr_tag", "aws:elasticmapreduce:job-flow-id") + + # Fixed: Safely check for EKS tag prefixes in instance tags eks_managed = sum( 1 for i in resources.get("ec2_instances", []) - if any(tag.startswith(eks_tag) for tag in i.get("tags", {})) + if any( + isinstance(tag_key, str) and tag_key.startswith(eks_tag) + for tag_key in i.get("tags", {}) + ) ) + emr_managed = sum( 1 for i in resources.get("ec2_instances", []) if emr_tag in i.get("tags", {}) ) + logger.info( f"Account {account_id} - Found {total_ec2} EC2 instances ({excluded_ec2} explicitly excluded, {eks_managed} EKS-managed, {emr_managed} EMR-managed)" ) @@ -451,7 +504,7 @@ def _create_resource_managers( # We're using a default region here just for manager initialization # Each method in the manager will use the appropriate region for the operation default_region = self._get_default_regions(credentials)[0] - + return { "ec2": EC2Manager(account_id, default_region), "rds": RDSManager(account_id, default_region), diff --git a/local-app/python-tools/gfl-resource-actions/aws_resource_management/discovery.py b/local-app/python-tools/gfl-resource-actions/aws_resource_management/discovery.py index f422f269..e8478781 100644 --- a/local-app/python-tools/gfl-resource-actions/aws_resource_management/discovery.py +++ b/local-app/python-tools/gfl-resource-actions/aws_resource_management/discovery.py @@ -19,9 +19,9 @@ from aws_resource_management.config_manager import get_config from aws_resource_management.logging_setup import log_with_context, setup_logging from aws_resource_management.managers import ( - BaseResourceManager, EC2Manager, RDSManager, + ResourceManager, ) from botocore.exceptions import ClientError, EndpointConnectionError @@ -509,6 +509,123 @@ def _discover_eks( logger.error(f"Error discovering EKS clusters in {region}: {e}") return [] + def _discover_ecr( + self, region: str, resource_ids: Optional[List[str]] = None + ) -> List[Dict[str, Any]]: + """ + Discover ECR repositories and images in a region. + + Args: + region: AWS region + resource_ids: Specific repository names to filter by (optional) + + Returns: + List of ECR image dictionaries + """ + # Create ECR client + ecr_client = boto3.client( + "ecr", + region_name=region, + aws_access_key_id=self.credentials.get("aws_access_key_id"), + aws_secret_access_key=self.credentials.get("aws_secret_access_key"), + aws_session_token=self.credentials.get("aws_session_token"), + ) + + try: + # Get all repositories + repositories = [] + response = ecr_client.describe_repositories() + repositories.extend(response.get("repositories", [])) + + # Handle pagination + while "nextToken" in response: + response = ecr_client.describe_repositories( + nextToken=response["nextToken"] + ) + repositories.extend(response.get("repositories", [])) + + # Filter by repository names if specified + if resource_ids: + repositories = [ + repo + for repo in repositories + if repo["repositoryName"] in resource_ids + ] + + # Process repositories and images + all_images = [] + for repo in repositories: + repo_name = repo["repositoryName"] + try: + # Get image details + image_ids = [] + images_response = ecr_client.list_images(repositoryName=repo_name) + image_ids.extend(images_response.get("imageIds", [])) + + # Handle pagination + while "nextToken" in images_response: + images_response = ecr_client.list_images( + repositoryName=repo_name, + nextToken=images_response["nextToken"], + ) + image_ids.extend(images_response.get("imageIds", [])) + + # Process images in chunks (ECR API has a limit) + chunk_size = 100 + for i in range(0, len(image_ids), chunk_size): + chunk = image_ids[i : i + chunk_size] + if not chunk: + continue + + try: + # Get details for all images in this chunk + image_details = ecr_client.describe_images( + repositoryName=repo_name, imageIds=chunk + ) + + # Create simplified image dictionaries + for image in image_details.get("imageDetails", []): + # Get digest or tag for ID + image_id = image.get("imageDigest", "") + if "imageTags" in image and image["imageTags"]: + primary_tag = image["imageTags"][0] + else: + primary_tag = "untagged" + + # Create a simplified image dictionary + image_dict = { + "id": image_id, + "name": f"{repo_name}:{primary_tag}", + "repositoryName": repo_name, + "region": region, + "tags": image.get("imageTags", []), + "sizeInMB": image.get("imageSizeInBytes", 0) + / (1024 * 1024), + "pushedAt": ( + image["imagePushedAt"].isoformat() + if "imagePushedAt" in image + else None + ), + "digest": image.get("imageDigest"), + } + all_images.append(image_dict) + + except Exception as e: + logger.warning( + f"Error describing images for repository {repo_name} chunk {i//chunk_size + 1}: {e}" + ) + + except Exception as e: + logger.warning( + f"Error listing images for repository {repo_name}: {e}" + ) + + return all_images + + except Exception as e: + logger.error(f"Error discovering ECR repositories in {region}: {e}") + return [] + # Backward compatibility wrapper for get_account_resources # This function accepts both positional and keyword arguments @@ -531,30 +648,30 @@ def get_account_resources(*args, **kwargs) -> Dict[str, List[Dict[str, Any]]]: # Handle both old-style positional calling and new-style keyword calling credentials = None regions = [] - + if len(args) >= 1: credentials = args[0] else: - credentials = kwargs.get('credentials') - + credentials = kwargs.get("credentials") + if len(args) >= 2: regions = args[1] else: - regions = kwargs.get('regions', []) + regions = kwargs.get("regions", []) # Extract keyword arguments with defaults - exclude_resources = kwargs.get('exclude_resources', []) - account_id = kwargs.get('account_id', 'unknown') - account_name = kwargs.get('account_name', 'Unknown') - emr_cluster_states = kwargs.get('emr_cluster_states') - + exclude_resources = kwargs.get("exclude_resources", []) + account_id = kwargs.get("account_id", "unknown") + account_name = kwargs.get("account_name", "Unknown") + emr_cluster_states = kwargs.get("emr_cluster_states") + return _get_account_resources_impl( credentials=credentials, regions=regions, exclude_resources=exclude_resources, account_id=account_id, account_name=account_name, - emr_cluster_states=emr_cluster_states + emr_cluster_states=emr_cluster_states, ) @@ -563,13 +680,13 @@ def _get_account_resources_impl( regions: List[str], *, # Force all following parameters to be keyword-only exclude_resources: List[str] = None, - account_id: str = 'unknown', + account_id: str = "unknown", account_name: Optional[str] = None, emr_cluster_states: Optional[List[str]] = None, ) -> Dict[str, List[Dict[str, Any]]]: """ Implementation of resource discovery for an account. - + Args: credentials: AWS credentials dictionary regions: List of AWS regions to search @@ -583,7 +700,7 @@ def _get_account_resources_impl( """ if exclude_resources is None: exclude_resources = [] - + resources = { "ec2_instances": [], "rds_instances": [], @@ -592,57 +709,59 @@ def _get_account_resources_impl( "ecr_images": [], "ecr_old_images": [], } - + try: # Create session or use provided credentials session = None if isinstance(credentials, dict): # Create a session from the credentials dictionary session = boto3.Session( - aws_access_key_id=credentials.get('aws_access_key_id'), - aws_secret_access_key=credentials.get('aws_secret_access_key'), - aws_session_token=credentials.get('aws_session_token') + aws_access_key_id=credentials.get("aws_access_key_id"), + aws_secret_access_key=credentials.get("aws_secret_access_key"), + aws_session_token=credentials.get("aws_session_token"), ) else: # Try to get a session for the account session = get_session_for_account(account_id) - + if not session: logger.error(f"Could not create session for account {account_id}") return resources - + # Extract credentials from session for the ResourceDiscovery creds = session.get_credentials() discovery_credentials = { "aws_access_key_id": creds.access_key, "aws_secret_access_key": creds.secret_key, - "aws_session_token": creds.token if hasattr(creds, 'token') and creds.token else None, + "aws_session_token": ( + creds.token if hasattr(creds, "token") and creds.token else None + ), } - + # Create resource discovery instance discovery = ResourceDiscovery(discovery_credentials, account_id) - + # Discover EC2 instances if "ec2" not in exclude_resources: logger.info( f"Discovering EC2 instances for account {account_id} in {len(regions)} regions" ) resources["ec2_instances"] = discovery.discover_resources("ec2", regions) - + # Discover RDS instances if "rds" not in exclude_resources: logger.info( f"Discovering RDS instances for account {account_id} in {len(regions)} regions" ) resources["rds_instances"] = discovery.discover_resources("rds", regions) - + # Discover EKS clusters if "eks" not in exclude_resources and EKSManager: logger.info( f"Discovering EKS clusters for account {account_id} in {len(regions)} regions" ) resources["eks_clusters"] = discovery.discover_resources("eks", regions) - + # Discover EMR clusters if "emr" not in exclude_resources and EMRManager: logger.info( @@ -652,7 +771,7 @@ def _get_account_resources_impl( resources["emr_clusters"] = discovery.discover_resources( "emr", regions, resource_ids=None ) - + # Discover ECR images if "ecr" not in exclude_resources and ECRManager: logger.info( @@ -670,8 +789,8 @@ def _get_account_resources_impl( resource["accountId"] = account_id if account_name: resource["accountName"] = account_name - + except Exception as e: logger.error(f"Error discovering resources for account {account_id}: {e}") - + return resources diff --git a/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/__init__.py b/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/__init__.py index da446c04..1033b4cd 100644 --- a/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/__init__.py +++ b/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/__init__.py @@ -3,7 +3,7 @@ """ # Import classes for easier access from the managers package -from aws_resource_management.managers.base import BaseResourceManager, ResourceManager +from aws_resource_management.managers.base import ResourceManager from aws_resource_management.managers.ec2 import EC2Manager from aws_resource_management.managers.rds import RDSManager @@ -26,7 +26,6 @@ # Export manager types for public use __all__ = [ "ResourceManager", - "BaseResourceManager", "EC2Manager", "RDSManager", ] diff --git a/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/base.py b/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/base.py index 2b9c6763..a532cbff 100644 --- a/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/base.py +++ b/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/base.py @@ -14,7 +14,7 @@ logger = logging.getLogger("aws_resource_management.managers.base") -class BaseResourceManager: +class ResourceManager: """Base class for all resource managers.""" def __init__(self, account_id: str, region: str): @@ -152,15 +152,15 @@ def discover_resources(self) -> List[Dict[str, Any]]: List of dictionaries containing resource information """ raise NotImplementedError("Subclasses must implement discover_resources method") - + def get_boto3_client(self, service_name: str, region: str) -> Any: """ Get a boto3 client for the specified service and region. - + Args: service_name: AWS service name (e.g., 'ec2', 'rds') region: AWS region - + Returns: Boto3 client for the specified service in the specified region """ @@ -168,13 +168,13 @@ def get_boto3_client(self, service_name: str, region: str) -> Any: # Get an account-specific session with proper role assumption if not self.session: self.session = get_session_for_account(self.account_id) - + # Create config with retry settings boto_config = Config( region_name=region, retries={"max_attempts": 3, "mode": "standard"}, ) - + # Create client with proper configuration client = self.session.client( service_name, region_name=region, config=boto_config @@ -183,19 +183,21 @@ def get_boto3_client(self, service_name: str, region: str) -> Any: except Exception as e: logger.error(f"Error creating {service_name} client in {region}: {str(e)}") return None - + # Alias for backward compatibility - many manager implementations use create_client() create_client = get_boto3_client - - def paginate_boto3(self, client: Any, operation: str, result_key: str) -> List[Dict[str, Any]]: + + def paginate_boto3( + self, client: Any, operation: str, result_key: str + ) -> List[Dict[str, Any]]: """ Paginate through AWS API responses. - + Args: client: Boto3 client operation: Operation name (e.g., 'describe_instances') result_key: Key in the response that contains the results - + Returns: Combined list of results from all pages """ @@ -209,13 +211,20 @@ def paginate_boto3(self, client: Any, operation: str, result_key: str) -> List[D except Exception as e: logger.error(f"Error paginating {operation}: {str(e)}") return [] - - def log_action(self, resource_id: str, region: str, action: str, - resource_name: str = None, details: str = None, - dry_run: bool = False, existing_schedule: str = None) -> None: + + def log_action( + self, + resource_id: str, + region: str, + action: str, + resource_name: str = None, + details: str = None, + dry_run: bool = False, + existing_schedule: str = None, + ) -> None: """ Log an action taken on a resource. - + Args: resource_id: Resource ID region: AWS region @@ -229,40 +238,41 @@ def log_action(self, resource_id: str, region: str, action: str, dry_run_prefix = "[DRY RUN] Would " if dry_run else "" details_str = f" - {details}" if details else "" schedule_str = f" [Schedule: {existing_schedule}]" if existing_schedule else "" - + logger.info( f"{dry_run_prefix}{action.capitalize()} {self.resource_type} {resource_id} {name_str} " f"in {region}{details_str}{schedule_str}" ) - + def should_exclude(self, resource: Dict[str, Any]) -> bool: """ Check if a resource should be excluded based on tags. - + Args: resource: Resource dictionary with tags - + Returns: True if the resource should be excluded, False otherwise """ from aws_resource_management.config_manager import get_config + config = get_config() - + tags = resource.get("tags", {}) exclusion_tag = config.get("exclusion_tag") - + if exclusion_tag and exclusion_tag in tags: return True - + return False - + def get_partition_for_region(self, region: str) -> str: """ Get the AWS partition for a region. - + Args: region: AWS region - + Returns: AWS partition (aws, aws-cn, aws-us-gov) """ @@ -272,7 +282,3 @@ def get_partition_for_region(self, region: str) -> str: return "aws-cn" else: return "aws" - - -# Create alias for backward compatibility -ResourceManager = BaseResourceManager diff --git a/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/emr.py b/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/emr.py index 3e082f12..d8f23c1c 100644 --- a/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/emr.py +++ b/local-app/python-tools/gfl-resource-actions/aws_resource_management/managers/emr.py @@ -4,10 +4,13 @@ from typing import Any, Dict, List, Optional +from aws_resource_management.aws_utils import ( + detect_partition_from_credentials, + get_session_for_account, +) from aws_resource_management.config_manager import get_config from aws_resource_management.logging_setup import setup_logging from aws_resource_management.managers.base import ResourceManager -from aws_resource_management.aws_utils import get_session_for_account, detect_partition_from_credentials from botocore.exceptions import ClientError logger = setup_logging() @@ -236,14 +239,16 @@ def validate_credentials(self, region: str = None) -> bool: if not session: logger.error(f"Could not create session for account {self.account_id}") return False - + # Determine partition from session credentials creds = session.get_credentials() if creds: credentials = { "aws_access_key_id": creds.access_key, "aws_secret_access_key": creds.secret_key, - "aws_session_token": creds.token if hasattr(creds, 'token') and creds.token else None, + "aws_session_token": ( + creds.token if hasattr(creds, "token") and creds.token else None + ), } partition = detect_partition_from_credentials(credentials) if partition == "aws-us-gov": @@ -256,7 +261,7 @@ def validate_credentials(self, region: str = None) -> bool: try: logger.info(f"Validating EMR credentials in region {region}") emr_client = self.get_boto3_client("emr", region) - + # Just list a small number of clusters to verify permissions response = emr_client.list_clusters(MaxResults=1) logger.info(f"EMR credentials valid in region {region}")