pabloem commented on code in PR #39177:
URL: https://github.com/apache/beam/pull/39177#discussion_r3807011103
##########
infra/enforcement/account_keys.py:
##########
@@ -376,13 +379,14 @@ def create_announcement(self, recipient: str) -> None:
self.logger.info("No compliance issues found, no announcement will
be created.")
return
- 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]
+ unmanaged_keys_issues = [issue for issue in diff if "IAC_DRIFT_SA_KEY"
in issue]
+ general_issues = [issue for issue in diff if "IAC_DRIFT_SA_KEY" not in
issue]
if general_issues:
self.logger.info(f"Found {len(general_issues)} general compliance
issues. Triggering announcement...")
- 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"
+ title = f"[SECURITY] Action Required: Unauthorized Service
Accounts Detected"
Review Comment:
change the title too to include `IAC_DRIFT_SA_KEY` - also, create a constant
at the top of the file for `IAC_DRIFT_SA_KEY`. Even better if we have a central
place for all the constants.
##########
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"
+ iam_drift_issues = [issue for issue in diff if "IAC_DRIFT_IAM_USER" in
issue]
+ general_issues = [issue for issue in diff if "IAC_DRIFT_IAM_USER" not
in issue]
Review Comment:
is it okay if general_issues only filters out `IAC_DRIFT_IAM_USER` but not
`IAC_DRIFT_SA_KEY`
##########
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"
+ iam_drift_issues = [issue for issue in diff if "IAC_DRIFT_IAM_USER" in
issue]
+ general_issues = [issue for issue in diff if "IAC_DRIFT_IAM_USER" 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 iam_drift_issues:
+ self.logger.info(f"Found {len(iam_drift_issues)} 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 iam_drift_issues:
+ 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(iam_drift_issues)} 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"
+ iam_drift_issues = [issue for issue in diff if "IAC_DRIFT_IAM_USER" in
issue]
+ general_issues = [issue for issue in diff if "IAC_DRIFT_IAM_USER" not
in issue]
Review Comment:
use a coinstant here please and make sure filtering is correct (do we only
filter out `IAC_DRIFT_IAM_USER` ? or should we filter out other IAC_DRIFT
constants?
##########
infra/enforcement/iam.py:
##########
@@ -223,7 +224,7 @@ def check_compliance(self) -> List[str]:
existing_user = existing_users.get(email)
if current_user and not existing_user:
- differences.append(f"User {email} not found in existing
policy.")
+ differences.append(f"IAC_DRIFT_IAM_USER: Unauthorized user
'{email}' detected in GCP but not found in existing policy.")
Review Comment:
create a constant for `IAC_DRIFT_IAM_USER`
--
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]
