Skip to content

Commit be84c79

Browse files
Fix ROSA cluster tagging for both SSO and IAM access key deployments (#1007)
* Fix ROSA cluster tagging for both SSO and IAM access key deployments ROSA instances are created by AWS/Red Hat service accounts, hiding the actual user in RunInstances CloudTrail events. This adds multi-strategy username resolution that traces ownership through cluster IAM roles and CloudTrail events, with a fallback that handles deleted roles via direct CloudTrail CreateRole event search (90-day retention). Co-authored-by: Cursor <cursoragent@cursor.com> * Coderabbit fixes * Add unit tests for ROSA cluster tagging username resolution Cover CloudTrail fallback lookups, automation-user exclusion, strategy ordering, and ambiguous cluster role matching introduced in PR #1007. Co-authored-by: Cursor <cursoragent@cursor.com> * Coderabbit comments * Use generic test usernames and fix automation exclusion coverage Replace real IAM/AD identifiers with fictional names in ROSA tagging unit tests, and include the automation user in iam_users so exclude logic is actually exercised. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix non-cluster tag cleanup after resource-events username lookup Tagging can resolve owners via get_username_from_resource_events while teardown still used CloudTrail-only lookup, leaving partial tags that caused validate_existing_tag to skip re-applying Budget and LaunchTime. Align removal with tagging and require input tags in validation. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b4bcb9b commit be84c79

8 files changed

Lines changed: 596 additions & 9 deletions

File tree

cloud_governance/common/clouds/aws/cloudtrail/cloudtrail_operations.py

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,195 @@ def get_username_by_instance_id_and_time(self, resource_id: str, resource_type:
253253
username, event = self.__get_user_by_resource_id(start_time, end_time, resource_id, resource_type, event_type)
254254
return self.__check_filter_username(username, event)
255255

256+
def get_username_from_resource_events(self, resource_id: str, iam_users: list,
257+
start_time: datetime = None, end_time: datetime = None,
258+
exclude_users: set = None):
259+
"""
260+
Fallback username lookup: search ALL CloudTrail events for a resource
261+
and return the first username matching a known IAM user.
262+
Handles managed services (ROSA, EKS, etc.) where RunInstances is
263+
performed by a service account but subsequent events (CreateTags, etc.)
264+
may be performed by the actual user via SSO or IAM credentials.
265+
@param resource_id: The AWS resource ID to look up
266+
@param iam_users: List of known IAM usernames to validate against
267+
@param start_time: Optional search window start (defaults to LOOKBACK_DAYS ago)
268+
@param end_time: Optional search window end (defaults to now)
269+
@param exclude_users: Optional set of usernames to skip (automation accounts)
270+
@return: matching IAM username or empty string
271+
"""
272+
if exclude_users is None:
273+
exclude_users = set()
274+
try:
275+
if not start_time:
276+
start_time = datetime.now() - timedelta(days=self.LOOKBACK_DAYS)
277+
if not end_time:
278+
end_time = datetime.now()
279+
responses = self.get_full_responses(
280+
StartTime=start_time, EndTime=end_time,
281+
LookupAttributes=[{
282+
'AttributeKey': 'ResourceName',
283+
'AttributeValue': resource_id
284+
}])
285+
for event in responses:
286+
username, _ = self.__check_event_is_assumed_role(
287+
event.get('CloudTrailEvent', ''))
288+
if username and username in iam_users and username not in exclude_users:
289+
logger.info(f'Found username {username} from {event.get("EventName")} '
290+
f'event on resource {resource_id}')
291+
return username
292+
event_username = event.get('Username', '')
293+
if event_username and event_username in iam_users and event_username not in exclude_users:
294+
logger.info(f'Found username {event_username} from {event.get("EventName")} '
295+
f'event on resource {resource_id}')
296+
return event_username
297+
return ''
298+
except Exception as err:
299+
logger.error(f'Error in get_username_from_resource_events: {err}')
300+
return ''
301+
302+
def __get_username_from_role_cloudtrail(self, role: dict, iam_users: list):
303+
"""
304+
Look up CloudTrail CreateRole event for a given IAM role and return
305+
the username if it matches a known IAM user.
306+
"""
307+
role_name = role['RoleName']
308+
role_arn = role['Arn']
309+
create_date = role.get('CreateDate')
310+
if not create_date:
311+
return ''
312+
end_time = create_date + timedelta(seconds=self.SEARCH_SECONDS)
313+
start_time = create_date - timedelta(seconds=self.SEARCH_SECONDS)
314+
responses = self.__get_cloudtrail_responses(
315+
start_time=start_time, end_time=end_time,
316+
resource_arn=role_arn)
317+
for event in responses:
318+
if event.get('EventName') == 'CreateRole':
319+
username, _ = self.__check_event_is_assumed_role(
320+
event.get('CloudTrailEvent', ''))
321+
if username and username in iam_users:
322+
logger.info(f'Found cluster owner {username} from CreateRole '
323+
f'event on role {role_name}')
324+
return username
325+
event_username = event.get('Username', '')
326+
if event_username and event_username in iam_users:
327+
logger.info(f'Found cluster owner {event_username} from CreateRole '
328+
f'event on role {role_name}')
329+
return event_username
330+
return ''
331+
332+
def __get_username_from_create_role_events(self, start_time: datetime,
333+
end_time: datetime,
334+
iam_users: list):
335+
"""
336+
Search CloudTrail for CreateRole events of OpenShift operator roles
337+
within a time window. Handles the case where roles were deleted from
338+
IAM but creation events still exist in CloudTrail (90-day retention).
339+
"""
340+
try:
341+
events = []
342+
kwargs = {
343+
'LookupAttributes': [{
344+
'AttributeKey': 'EventName',
345+
'AttributeValue': 'CreateRole'
346+
}],
347+
'StartTime': start_time, 'EndTime': end_time,
348+
'MaxResults': 50
349+
}
350+
while True:
351+
response = self.__global_cloudtrail.lookup_events(**kwargs)
352+
events.extend(response.get('Events', []))
353+
next_token = response.get('NextToken')
354+
if not next_token:
355+
break
356+
kwargs['NextToken'] = next_token
357+
for event in events:
358+
resources = event.get('Resources', [])
359+
role_names = [r.get('ResourceName', '') for r in resources
360+
if r.get('ResourceType') == 'AWS::IAM::Role']
361+
if not any('openshift' in n.lower() for n in role_names):
362+
continue
363+
username, _ = self.__check_event_is_assumed_role(
364+
event.get('CloudTrailEvent', ''))
365+
if username and username in iam_users:
366+
logger.info(f'Found cluster owner {username} from '
367+
f'CloudTrail CreateRole event (deleted role)')
368+
return username
369+
event_username = event.get('Username', '')
370+
if event_username and event_username in iam_users:
371+
logger.info(f'Found cluster owner {event_username} from '
372+
f'CloudTrail CreateRole event (deleted role)')
373+
return event_username
374+
except Exception as err:
375+
logger.error(f'Error searching CreateRole events: {err}')
376+
return ''
377+
378+
def get_username_from_cluster_role(self, cluster_id: str, iam_users: list,
379+
launch_time: datetime = None):
380+
"""
381+
Trace a cluster's ownership through its IAM roles.
382+
Uses three strategies:
383+
1. Direct match: find roles whose names contain the cluster ID
384+
(works for IPI/UPI clusters)
385+
2. Temporal match: find ROSA/OpenShift operator roles created
386+
shortly before the cluster's instance launch time
387+
(works for ROSA STS where roles use cluster name, not infra ID)
388+
3. CloudTrail fallback: search CreateRole events directly for
389+
OpenShift roles in the time window (handles deleted roles)
390+
Works for both SSO (AssumedRole) and IAM user identities.
391+
@param cluster_id: The cluster infra ID (e.g. 'u0s3a7y7o5y9c6w-x56mc')
392+
@param iam_users: List of known IAM usernames to validate against
393+
@param launch_time: Instance launch time for temporal role matching
394+
@return: matching IAM username or empty string
395+
"""
396+
ROSA_ROLE_WINDOW_SECONDS = 21600 # 6 hours before launch
397+
try:
398+
paginator = self.__iam_client.get_paginator('list_roles')
399+
roles = []
400+
for page in paginator.paginate(PaginationConfig={'PageSize': 1000}):
401+
roles.extend(page.get('Roles', []))
402+
cluster_roles = [r for r in roles if cluster_id in r.get('RoleName', '')]
403+
if cluster_roles:
404+
for role in cluster_roles:
405+
username = self.__get_username_from_role_cloudtrail(role, iam_users)
406+
if username:
407+
return username
408+
409+
if not launch_time:
410+
return ''
411+
candidate_roles = []
412+
for role in roles:
413+
role_name = role.get('RoleName', '')
414+
if 'openshift' not in role_name.lower():
415+
continue
416+
create_date = role.get('CreateDate')
417+
if not create_date:
418+
continue
419+
launch_naive = launch_time.replace(tzinfo=None)
420+
create_naive = create_date.replace(tzinfo=None)
421+
diff_seconds = (launch_naive - create_naive).total_seconds()
422+
if 0 <= diff_seconds <= ROSA_ROLE_WINDOW_SECONDS:
423+
candidate_roles.append(role)
424+
if candidate_roles:
425+
candidate_roles.sort(key=lambda r: r.get('CreateDate', ''), reverse=True)
426+
resolved_users = set()
427+
for role in candidate_roles[:6]:
428+
username = self.__get_username_from_role_cloudtrail(role, iam_users)
429+
if username:
430+
resolved_users.add(username)
431+
if len(resolved_users) == 1:
432+
return resolved_users.pop()
433+
434+
ct_start = launch_time - timedelta(seconds=ROSA_ROLE_WINDOW_SECONDS)
435+
username = self.__get_username_from_create_role_events(
436+
start_time=ct_start, end_time=launch_time,
437+
iam_users=iam_users)
438+
if username:
439+
return username
440+
return ''
441+
except Exception as err:
442+
logger.error(f'Error in get_username_from_cluster_role: {err}')
443+
return ''
444+
256445
def get_stop_time(self, resource_id: str, event_name: str):
257446
"""
258447
This method return the time of when instance is stopped

cloud_governance/policy/policy_operations/aws/tag_cluster/tag_cluster_operations.py

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from cloud_governance.common.clouds.aws.ec2.ec2_operations import EC2Operations
77
from cloud_governance.common.clouds.aws.iam.iam_operations import IAMOperations
88
from cloud_governance.common.clouds.aws.utils.utils import Utils
9+
from cloud_governance.common.logger.init_logger import logger
910

1011

1112
class TagClusterOperations:
@@ -29,9 +30,25 @@ def __init__(self, region: str, input_tags: dict = None, cluster_name: str = No
2930
self.cluster_name = cluster_name
3031
self.input_tags = input_tags
3132
self.cloudtrail = CloudTrailOperations(region_name='us-east-1')
32-
self._get_username_from_instance_id_and_time = CloudTrailOperations(region_name=region).get_username_by_instance_id_and_time
33+
self._regional_cloudtrail = CloudTrailOperations(region_name=region)
34+
self._get_username_from_instance_id_and_time = self._regional_cloudtrail.get_username_by_instance_id_and_time
3335
self.dry_run = dry_run
3436
self.iam_users = self.iam_operations.get_iam_users_list()
37+
self._automation_user = self._get_automation_username()
38+
39+
@staticmethod
40+
def _get_automation_username():
41+
"""
42+
Identify the current caller (the automation/service account running
43+
this policy) so it can be excluded from CloudTrail fallback searches.
44+
"""
45+
try:
46+
sts = boto3.client('sts')
47+
identity = sts.get_caller_identity()
48+
arn = identity.get('Arn', '')
49+
return arn.split('/')[-1] if '/' in arn else ''
50+
except Exception:
51+
return ''
3552

3653
def _input_tags_list_builder(self):
3754
"""
@@ -89,12 +106,61 @@ def get_user_name_from_name_tag(self, tags: list):
89106
return user
90107
return None
91108

92-
def get_username(self, start_time: datetime, resource_id: str, resource_type: str, tags: list):
109+
def get_username(self, start_time: datetime, resource_id: str, resource_type: str,
110+
tags: list, cluster_id: str = ''):
93111
"""
94-
This method returns the username
112+
This method returns the username using multiple strategies:
113+
1. Check existing tags (User tag, Name tag)
114+
2. CloudTrail - resource creation event (RunInstances, etc.)
115+
3. CloudTrail - any event on this resource by a known IAM user
116+
(handles managed services like ROSA where service accounts create
117+
resources but user events like CreateTags may exist)
118+
4. CloudTrail - trace cluster IAM roles to find who created them
119+
(handles ROSA/OSD where instances are created by service accounts
120+
but IAM roles are created by the user)
95121
:return:
96122
"""
123+
exclude = {self._automation_user} if self._automation_user else set()
97124
iam_username = self.get_user_name_from_name_tag(tags=tags)
98125
if not iam_username:
99-
return self._get_username_from_instance_id_and_time(start_time=start_time, resource_id=resource_id, resource_type=resource_type)
126+
ct_username = self._get_username_from_instance_id_and_time(
127+
start_time=start_time, resource_id=resource_id,
128+
resource_type=resource_type)
129+
if ct_username == 'AutoScaling':
130+
return ct_username
131+
if ct_username and ct_username in self.iam_users and ct_username not in exclude:
132+
return ct_username
133+
if cluster_id:
134+
iam_username = self.cloudtrail.get_username_from_cluster_role(
135+
cluster_id=cluster_id, iam_users=self.iam_users,
136+
launch_time=start_time)
137+
if iam_username:
138+
logger.info(f'Resolved cluster owner {iam_username} via IAM '
139+
f'role lookup for cluster {cluster_id}')
140+
return iam_username
141+
iam_username = self._regional_cloudtrail.get_username_from_resource_events(
142+
resource_id=resource_id, iam_users=self.iam_users,
143+
start_time=start_time, exclude_users=exclude)
100144
return iam_username
145+
146+
def get_username_from_cluster_instances(self, resources: list, cluster_name: str):
147+
"""
148+
Search all instances in a cluster for a valid User tag.
149+
If any instance already has a tagged owner, return that username.
150+
Handles managed services (ROSA, EKS) where CloudTrail cannot attribute
151+
the creator - if at least one instance was tagged, propagate to siblings.
152+
@param resources: list of instance groups from the cluster
153+
@param cluster_name: the cluster identifier to match
154+
@return: username string or None
155+
"""
156+
for instance_group in resources:
157+
for item in instance_group:
158+
tags = item.get('Tags', [])
159+
for tag in tags:
160+
if any(prefix in tag.get('Key', '') for prefix in self.cluster_prefix):
161+
if cluster_name in tag.get('Key', ''):
162+
user = self.ec2_operations.get_tag_value_from_tags(
163+
tags=tags, tag_name='User')
164+
if user and user != 'NA' and user in self.iam_users:
165+
return user
166+
return None

cloud_governance/policy/policy_operations/aws/tag_cluster/tag_cluster_resouces.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,11 @@ def update_cluster_tags(self, resources: list):
312312
else:
313313
username = self.get_username(start_time=item.get('LaunchTime'),
314314
resource_id=instance_id,
315-
resource_type='AWS::EC2::Instance', tags=tags)
315+
resource_type='AWS::EC2::Instance', tags=tags,
316+
cluster_id=cluster_name)
317+
if not username:
318+
username = self.get_username_from_cluster_instances(
319+
resources=resources, cluster_name=cluster_name)
316320
if username:
317321
if username == 'AutoScaling':
318322
add_tags.extend(self._fill_na_tags(user=username))

cloud_governance/policy/policy_operations/aws/tag_non_cluster/non_cluster_operations.py

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,17 @@ def __init__(self, region: str = 'us-east-2', dry_run: str = 'yes', input_tags:
2424
self.ec2_operations = EC2Operations(region=region)
2525
self.utils = Utils(region=region)
2626
self.iam_users = self.iam_client.get_iam_users_list()
27+
self._automation_user = self._get_automation_username()
28+
29+
@staticmethod
30+
def _get_automation_username():
31+
try:
32+
sts = boto3.client('sts')
33+
identity = sts.get_caller_identity()
34+
arn = identity.get('Arn', '')
35+
return arn.split('/')[-1] if '/' in arn else ''
36+
except Exception:
37+
return ''
2738

2839
def _get_instances_data(self, instance_id: str = ''):
2940
"""
@@ -209,14 +220,27 @@ def get_user_name_from_name_tag(self, tags: list = None, resource_name: str = No
209220

210221
def get_username(self, start_time: datetime, resource_id: str, resource_type: str, tags: list, resource_name: str = '', end_time: datetime = None):
211222
"""
212-
This method returns the username
223+
This method returns the username using multiple strategies:
224+
1. Check existing tags (Name tag)
225+
2. CloudTrail - resource creation event
226+
3. CloudTrail - any event on this resource by a known IAM user
227+
(handles managed services where service accounts create resources)
213228
:return:
214229
"""
215230
iam_username = self.get_user_name_from_name_tag(tags=tags, resource_name=resource_name)
216231
if not iam_username:
217232
iam_username = self.get_user_name_from_name_tag(resource_name=resource_name)
218233
if not iam_username:
219-
return self._get_username_from_cloudtrail(start_time=start_time, resource_id=resource_id, resource_type=resource_type, end_time=end_time)
234+
ct_username = self._get_username_from_cloudtrail(
235+
start_time=start_time, resource_id=resource_id,
236+
resource_type=resource_type, end_time=end_time)
237+
exclude = {self._automation_user} if self._automation_user else set()
238+
if ct_username and ct_username in self.iam_users and ct_username not in exclude:
239+
return ct_username
240+
iam_username = self.cloudtrail.get_username_from_resource_events(
241+
resource_id=resource_id, iam_users=self.iam_users,
242+
start_time=start_time, end_time=end_time,
243+
exclude_users=exclude)
220244
return iam_username
221245

222246
def validate_existing_tag(self, tags: list):
@@ -225,7 +249,9 @@ def validate_existing_tag(self, tags: list):
225249
@param tags:
226250
@return:
227251
"""
228-
check_tags = ['User', 'Project', 'Manager', 'Owner', 'Email']
252+
check_tags = ['User', 'Project', 'Manager', 'Owner', 'Email', 'LaunchTime']
253+
if self.input_tags:
254+
check_tags.extend(key for key in self.input_tags if key not in check_tags)
229255
tag_count = 0
230256
if tags:
231257
for tag in tags:

cloud_governance/policy/policy_operations/aws/tag_non_cluster/remove_non_cluster_tags.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ def __get_instance_tags(self, launch_time: datetime, instance_id: str, tags: lis
1717
@param tags:
1818
@return:
1919
"""
20-
username = self._get_username_from_cloudtrail(start_time=launch_time, resource_id=instance_id, resource_type='AWS::EC2::Instance')
20+
username = self.ec2_operations.get_tag_value_from_tags(tags=tags or [], tag_name='User')
21+
if not username or username == self.NA_VALUE:
22+
username = self.get_username(
23+
start_time=launch_time, resource_id=instance_id,
24+
resource_type='AWS::EC2::Instance', tags=tags or [])
2125
search_tags = []
2226
user_tags = self.iam_client.get_user_tags(username=username)
2327
if not username:

0 commit comments

Comments
 (0)