pabloem commented on code in PR #39177:
URL: https://github.com/apache/beam/pull/39177#discussion_r3787711333
##########
infra/enforcement/account_keys.py:
##########
@@ -406,23 +410,44 @@ def print_announcement(self, recipient: str) -> None:
"""
if not self.sending_client:
raise ValueError("SendingClient is required for printing
announcements")
-
+
diff = self.check_compliance()
if not diff:
self.logger.info("No compliance issues found, no announcement will
be printed.")
return
- title = f"Account Keys Compliance Issue Detected"
- body = f"Account keys for project {self.project_id} are not compliant
with the defined policies on {self.service_account_keys_file}\n\n"
- for issue in diff:
- body += f"- {issue}\n"
+ unmanaged_keys_issues = [issue for issue in diff if "SECURITY ALERT"
in issue]
+ general_issues = [issue for issue in diff if "SECURITY ALERT" not in
issue]
Review Comment:
SECURITY ALERT is too general to differentiate issues. We need something
more specific. Let's add a specific string for this. Let's say
'iac_drift_for_iam' - this refers to the difference between the IaC config and
the real state ('drift'). Use that string to match. And do not write SECURITY
ALERT, because it seems like a more serious problem than it currently is.
##########
infra/enforcement/iam.py:
##########
@@ -247,51 +248,86 @@ def create_announcement(self, recipient: str) -> None:
"""
if not self.sending_client:
raise ValueError("SendingClient is required for creating
announcements")
-
diff = self.check_compliance()
if not diff:
self.logger.info("No compliance issues found, no announcement will
be created.")
return
- title = f"IAM Policy Non-Compliance Detected"
- body = f"IAM policy for project {self.project_id} is not compliant
with the defined policies on {self.users_file}\n\n"
- for issue in diff:
- body += f"- {issue}\n"
+ security_alerts = [issue for issue in diff if "SECURITY ALERT" in
issue]
+ general_issues = [issue for issue in diff if "SECURITY ALERT" not in
issue]
Review Comment:
SECURITY ALERT is too general to differentiate issues. We need something
more specific. Let's add a specific string for this. Let's say
'iac_drift_for_iam' - this refers to the difference between the IaC config and
the real state ('drift'). Use that string to match. And do not write SECURITY
ALERT, because it seems like a more serious problem than it currently is.
##########
infra/enforcement/iam.py:
##########
@@ -247,51 +248,86 @@ def create_announcement(self, recipient: str) -> None:
"""
if not self.sending_client:
raise ValueError("SendingClient is required for creating
announcements")
-
diff = self.check_compliance()
if not diff:
self.logger.info("No compliance issues found, no announcement will
be created.")
return
- title = f"IAM Policy Non-Compliance Detected"
- body = f"IAM policy for project {self.project_id} is not compliant
with the defined policies on {self.users_file}\n\n"
- for issue in diff:
- body += f"- {issue}\n"
+ security_alerts = [issue for issue in diff if "SECURITY ALERT" in
issue]
+ general_issues = [issue for issue in diff if "SECURITY ALERT" not in
issue]
+
+ if general_issues:
+ self.logger.info(f"Found {len(general_issues)} general IAM
compliance issues. Triggering announcement...")
+ title = f"IAM Policy Non-Compliance Detected"
+ body = f"IAM policy for project {self.project_id} is not compliant
with the defined policies on {self.users_file}\n\n"
+ for issue in general_issues:
+ body += f"- {issue}\n"
+
+ announcement = f"Dear team,\n\nThis is an automated notification
about compliance issues detected in the IAM policy for project
{self.project_id}.\n\n"
+ announcement += f"We found {len(general_issues)} compliance
issue(s) that need your attention.\n"
+ announcement += f"\nPlease check the GitHub issue for detailed
information and take appropriate action to resolve these compliance violations."
+
+ self.sending_client.create_announcement(title, body, recipient,
announcement)
- announcement = f"Dear team,\n\nThis is an automated notification about
compliance issues detected in the IAM policy for project {self.project_id}.\n\n"
- announcement += f"We found {len(diff)} compliance issue(s) that need
your attention.\n"
- announcement += f"\nPlease check the GitHub issue for detailed
information and take appropriate action to resolve these compliance violations."
+ if security_alerts:
+ self.logger.info(f"Found {len(security_alerts)} critical IAM
security alerts. Dispatching to GitHub security issue...")
+ title = f"[SECURITY] Action Required: Unauthorized IAM Users
Detected"
+ body = f"Critical security violations detected in IAM policies for
project {self.project_id}:\n\n"
+ for issue in security_alerts:
+ body += f"- {issue}\n"
- self.sending_client.create_announcement(title, body, recipient,
announcement)
+ announcement = f"URGENT: Dear team,\n\nThis is an automated
security alert regarding unauthorized IAM access in project
{self.project_id}.\n\n"
+ announcement += f"We found {len(security_alerts)} critical
security alert(s) that require IMMEDIATE attention.\n"
+ announcement += f"\nPlease check the GitHub issue for detailed
information and revoke unauthorized access immediately."
+
+ self.sending_client.create_announcement(title, body, recipient,
announcement)
def print_announcement(self, recipient: str) -> None:
"""
Prints announcement details instead of sending them (for testing
purposes).
-
+
Args:
recipient (str): The email address of the announcement recipient.
"""
if not self.sending_client:
raise ValueError("SendingClient is required for printing
announcements")
-
+
diff = self.check_compliance()
if not diff:
self.logger.info("No compliance issues found, no announcement will
be printed.")
return
- title = f"IAM Policy Non-Compliance Detected"
- body = f"IAM policy for project {self.project_id} is not compliant
with the defined policies on {self.users_file}\n\n"
- for issue in diff:
- body += f"- {issue}\n"
+ security_alerts = [issue for issue in diff if "SECURITY ALERT" in
issue]
+ general_issues = [issue for issue in diff if "SECURITY ALERT" not in
issue]
Review Comment:
SECURITY ALERT is too general to differentiate issues. We need something
more specific. Let's add a specific string for this. Let's say
'iac_drift_for_iam' - this refers to the difference between the IaC config and
the real state ('drift'). Use that string to match. And do not write SECURITY
ALERT, because it seems like a more serious problem than it currently is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
