Skip to content

Make group sync additive-only — never remove members#34

Merged
Alexanderamiri merged 1 commit into
mainfrom
fix/additive-only-group-sync
Mar 12, 2026
Merged

Make group sync additive-only — never remove members#34
Alexanderamiri merged 1 commit into
mainfrom
fix/additive-only-group-sync

Conversation

@Alexanderamiri
Copy link
Copy Markdown
Member

Summary

Critical fix — prevents accidental mass-removal of group members.

  • Lambda handle_sync_groups_and_heros: remove deletion logic, only add missing members. Members added manually via Google Admin UI are preserved. Extra members are logged but not removed.
  • provision-groups.py: remove implicit helter logic, all groups use explicit memberships
  • sync-heros.py: always include helter in mapped groups

Why

When the registry restructure PR merged, the empty heros.yaml (members: []) triggered provisioning which resolved all groups to 0 members. The Lambda then removed existing members from Google groups.

Test plan

  • Merge and apply
  • Verify Lambda only adds members, never removes
  • Verify extra members in Google groups are preserved and logged

- Lambda: remove deletion logic from handle_sync_groups_and_heros, only add
  missing members. Members added manually via Google Admin UI are preserved.
  Extra members are logged but not removed.
- provision-groups.py: remove implicit helter logic, all groups use explicit
  memberships from heros.yaml
- sync-heros.py: always include helter in mapped groups

This prevents accidental mass-removal of group members when heros.yaml
is empty or incomplete.
@github-actions
Copy link
Copy Markdown

Terraform Plan

Changes detected — review required.

Plan output
Acquiring state lock. This may take a few moments...
module.lambdas.data.archive_file.daily_cost_check: Reading...
module.lambdas.data.archive_file.compliance_reporter: Reading...
module.lambdas.data.archive_file.override_cleanup: Reading...
module.lambdas.data.archive_file.apply_gate: Reading...
module.lambdas.data.archive_file.override_cleanup: Read complete after 0s [id=08e23a8ca152c50d8321f7b9f15d3ebbdc97849d]
module.lambdas.data.archive_file.compliance_reporter: Read complete after 0s [id=323449bf04f46a46a9d9c440212010f551146129]
module.lambdas.data.archive_file.daily_cost_check: Read complete after 0s [id=da9c5f6e85719534eb3d93b02eca8a30fbbfeb34]
module.lambdas.data.archive_file.apply_gate: Read complete after 0s [id=064e560a455ec8b2a30253cd815a820ad002376f]
module.lambdas.data.archive_file.slack_alert: Reading...
module.lambdas.data.archive_file.cost_report: Reading...
module.lambdas.data.archive_file.team_provisioner: Reading...
module.lambdas.data.archive_file.cost_report: Read complete after 0s [id=9844b77d6a3a4efa27510589543ad38c835cc662]
module.lambdas.data.archive_file.slack_alert: Read complete after 0s [id=fab67fddf674e36bfa0602611c3c7a00edf7af0c]
module.lambdas.data.archive_file.team_provisioner: Read complete after 0s [id=48532bcb70c17a4018c8247b7fe51947e36f6dd9]
module.lambdas.aws_cloudwatch_event_rule.compliance_reporter_trigger: Refreshing state... [id=javabin-compliance-reporter-trigger]
module.monitoring.aws_sns_topic.security: Refreshing state... [id=arn:aws:sns:eu-central-1:553637109631:javabin-security]
module.monitoring.aws_guardduty_detector.main: Refreshing state... [id=f1df02cf279e4b5986ce1e9bcb3af9c5]
module.monitoring.aws_cloudwatch_event_rule.resource_modification: Refreshing state... [id=javabin-resource-modification]
module.lambdas.aws_cloudwatch_event_rule.override_cleanup_schedule: Refreshing state... [id=javabin-override-cleanup-schedule]
module.ingress.data.aws_route53_zone.main: Reading...
module.monitoring.aws_cloudwatch_event_rule.console_login: Refreshing state... [id=javabin-console-login]
module.monitoring.aws_securityhub_account.main: Refreshing state... [id=553637109631]
module.iam.aws_iam_role.ecs_execution: Refreshing state... [id=javabin-ecs-execution]
module.lambdas.aws_iam_role.apply_gate: Refreshing state... [id=javabin-apply-gate]
module.monitoring.aws_cloudwatch_event_rule.iam_changes: Refreshing state... [id=javabin-iam-changes]
module.monitoring.aws_sns_topic.alerts: Refreshing state... [id=arn:aws:sns:eu-central-1:553637109631:javabin-alerts]
module.ingress.data.aws_route53_zone.main: Read complete after 1s [id=Z09335963LMV0Z5QB9L45]
module.monitoring.aws_s3_bucket.config_logs: Refreshing state... [id=javabin-config-553637109631]
module.monitoring.aws_cloudwatch_event_rule.resource_creation: Refreshing state... [id=javabin-resource-creation]
module.iam.data.aws_iam_openid_connect_provider.github: Reading...
module.monitoring.aws_cloudwatch_event_rule.guardduty_findings: Refreshing state... [id=javabin-guardduty-findings]
module.lambdas.aws_iam_role.override_cleanup: Refreshing state... [id=javabin-override-cleanup]
module.iam.data.aws_iam_openid_connect_provider.github: Read complete after 0s [id=arn:aws:iam::553637109631:oidc-provider/token.actions.githubusercontent.com]
module.lambdas.aws_cloudwatch_event_rule.cost_report_schedule: Refreshing state... [id=javabin-cost-report-schedule]
module.ingress.aws_acm_certificate.wildcard: Refreshing state... [id=arn:aws:acm:eu-central-1:553637109631:certificate/9b79f56a-3719-4c62-8970-6f08985a7e5b]
module.monitoring.aws_cloudwatch_event_rule.config_compliance: Refreshing state... [id=javabin-config-compliance-change]
module.monitoring.aws_ce_anomaly_monitor.main: Refreshing state... [id=arn:aws:ce::553637109631:anomalymonitor/3609b3f1-c834-444e-a218-02ac6da1cb4d]
module.compute.aws_ecr_repository.ci["jvm"]: Refreshing state... [id=javabin-ci-jvm]
module.compute.aws_ecr_repository.ci["platform"]: Refreshing state... [id=javabin-ci-platform]
module.compute.aws_ecr_repository.ci["ts"]: Refreshing state... [id=javabin-ci-ts]
module.compute.aws_ecs_cluster.main: Refreshing state... [id=arn:aws:ecs:eu-central-1:553637109631:cluster/javabin-platform]
module.networking.aws_eip.nat: Refreshing state... [id=eipalloc-0764f0a1a3c80dce1]
module.lambdas.aws_iam_role.daily_cost_check: Refreshing state... [id=javabin-daily-cost-check]
module.iam.aws_iam_policy.developer_boundary: Refreshing state... [id=arn:aws:iam::553637109631:policy/javabin-developer-boundary]
module.monitoring.aws_iam_role.config_role: Refreshing state... [id=javabin-config-role]
module.networking.data.aws_availability_zones.available: Reading...
module.lambdas.aws_iam_role.team_provisioner: Refreshing state... [id=javabin-team-provisioner]
module.lambdas.aws_iam_role.compliance_reporter: Refreshing state... [id=javabin-compliance-reporter]
module.lambdas.aws_iam_role.cost_report: Refreshing state... [id=javabin-cost-report]
module.lambdas.aws_cloudwatch_event_rule.daily_cost_check_schedule: Refreshing state... [id=javabin-daily-cost-check-schedule]
module.networking.aws_vpc.main: Refreshing state... [id=vpc-0cd3502de2527a310]
module.identity.aws_cognito_user_pool.internal: Refreshing state... [id=eu-central-1_Icikv3dtD]
module.monitoring.aws_cloudwatch_event_rule.securityhub_findings: Refreshing state... [id=javabin-securityhub-findings]
module.lambdas.aws_iam_role.slack_alert: Refreshing state... [id=javabin-slack-alert]
module.identity.aws_cognito_user_pool.external: Refreshing state... [id=eu-central-1_gdFOsE4EM]
module.iam.aws_iam_role_policy.ecs_execution_secrets: Refreshing state... [id=javabin-ecs-execution:secrets-read]
module.networking.data.aws_availability_zones.available: Read complete after 0s [id=eu-central-1]
module.iam.aws_iam_role_policy_attachment.ecs_execution_base: Refreshing state... [id=javabin-ecs-execution-20260307162856804400000004]
module.lambdas.aws_iam_role_policy.apply_gate: Refreshing state... [id=javabin-apply-gate:javabin-apply-gate]
module.lambdas.aws_iam_role_policy_attachment.apply_gate_logs: Refreshing state... [id=javabin-apply-gate-20260310000556680800000001]
module.lambdas.aws_lambda_function.apply_gate: Refreshing state... [id=javabin-apply-gate]
module.monitoring.aws_securityhub_standards_subscription.aws_foundational: Refreshing state... [id=arn:aws:securityhub:eu-central-1:553637109631:subscription/aws-foundational-security-best-practices/v/1.0.0]
module.monitoring.aws_guardduty_detector_feature.runtime_monitoring: Refreshing state... [id=f1df02cf279e4b5986ce1e9bcb3af9c5/RUNTIME_MONITORING]
module.monitoring.aws_cloudwatch_event_target.iam_changes_sns: Refreshing state... [id=javabin-iam-changes-send-to-security-sns]
module.monitoring.aws_sns_topic_policy.security: Refreshing state... [id=arn:aws:sns:eu-central-1:553637109631:javabin-security]
module.monitoring.aws_cloudwatch_event_target.console_login_sns: Refreshing state... [id=javabin-console-login-send-to-security-sns]
module.monitoring.aws_cloudwatch_event_target.resource_creation_sns: Refreshing state... [id=javabin-resource-creation-send-to-security-sns]
module.monitoring.aws_cloudwatch_event_target.resource_modification_sns: Refreshing state... [id=javabin-resource-modification-send-to-security-sns]
module.lambdas.aws_iam_role_policy_attachment.override_cleanup_logs: Refreshing state... [id=javabin-override-cleanup-20260307162858005200000007]
module.lambdas.aws_lambda_function.override_cleanup: Refreshing state... [id=javabin-override-cleanup]
module.lambdas.aws_iam_role_policy.override_cleanup: Refreshing state... [id=javabin-override-cleanup:javabin-override-cleanup]
module.monitoring.aws_sns_topic_policy.alerts: Refreshing state... [id=arn:aws:sns:eu-central-1:553637109631:javabin-alerts]
module.monitoring.aws_cloudwatch_event_target.guardduty_findings_sns: Refreshing state... [id=javabin-guardduty-findings-send-to-security-sns]
module.monitoring.aws_cloudwatch_event_target.config_compliance_sns: Refreshing state... [id=javabin-config-compliance-change-send-to-security-sns]
module.monitoring.aws_ce_anomaly_subscription.alerts: Refreshing state... [id=arn:aws:ce::553637109631:anomalysubscription/f6b079c9-5174-43b7-85f3-dde533995482]
module.iam.aws_iam_role.ci_registry: Refreshing state... [id=javabin-ci-registry]
module.iam.aws_iam_role.ci_infra: Refreshing state... [id=javabin-ci-infra]
module.iam.aws_iam_role.ci_override_approver: Refreshing state... [id=javabin-ci-override-approver]
module.iam.aws_iam_role.ci_apply_gate: Refreshing state... [id=javabin-ci-apply-gate]
module.iam.aws_iam_role.ci_deploy["platform-test-app"]: Refreshing state... [id=javabin-ci-deploy-platform-test-app]
module.lambdas.aws_lambda_function.daily_cost_check: Refreshing state... [id=javabin-daily-cost-check]
module.lambdas.aws_iam_role_policy.daily_cost_check: Refreshing state... [id=javabin-daily-cost-check:javabin-daily-cost-check]
module.lambdas.aws_iam_role_policy_attachment.daily_cost_check_logs: Refreshing state... [id=javabin-daily-cost-check-20260307162856210400000002]
module.ingress.aws_route53_record.acm_validation["*.javazone.no"]: Refreshing state... [id=Z09335963LMV0Z5QB9L45__b68529ef50ff68d6cf320ff0e9c5c80a.javazone.no._CNAME]
module.monitoring.aws_iam_role_policy_attachment.config_role: Refreshing state... [id=javabin-config-role-20260307162900971300000009]
module.monitoring.aws_config_configuration_recorder.main: Refreshing state... [id=javabin-recorder]
module.compute.aws_ecs_cluster_capacity_providers.main: Refreshing state... [id=javabin-platform]
module.compute.aws_ecr_lifecycle_policy.ci["platform"]: Refreshing state... [id=javabin-ci-platform]
module.compute.aws_ecr_lifecycle_policy.ci["ts"]: Refreshing state... [id=javabin-ci-ts]
module.compute.aws_ecr_lifecycle_policy.ci["jvm"]: Refreshing state... [id=javabin-ci-jvm]
module.lambdas.aws_iam_role_policy_attachment.team_provisioner_logs: Refreshing state... [id=javabin-team-provisioner-20260307162856464600000003]
module.lambdas.aws_iam_role_policy_attachment.compliance_reporter_logs: Refreshing state... [id=javabin-compliance-reporter-20260307162857302300000005]
module.lambdas.aws_lambda_function.compliance_reporter: Refreshing state... [id=javabin-compliance-reporter]
module.lambdas.aws_iam_role_policy.compliance_reporter: Refreshing state... [id=javabin-compliance-reporter:javabin-compliance-reporter]
module.lambdas.aws_iam_role_policy_attachment.cost_report_logs: Refreshing state... [id=javabin-cost-report-20260307162857662100000006]
module.lambdas.aws_iam_role_policy.cost_report: Refreshing state... [id=javabin-cost-report:javabin-cost-report]
module.lambdas.aws_lambda_function.cost_report: Refreshing state... [id=javabin-cost-report]
module.monitoring.aws_cloudwatch_event_target.securityhub_findings_sns: Refreshing state... [id=javabin-securityhub-findings-send-to-security-sns]
module.lambdas.aws_iam_role_policy.slack_alert: Refreshing state... [id=javabin-slack-alert:javabin-slack-alert]
module.lambdas.aws_iam_role_policy_attachment.slack_alert_logs: Refreshing state... [id=javabin-slack-alert-20260307162858376500000008]
module.identity.aws_cognito_user_pool_domain.internal: Refreshing state... [id=javabin-internal]
module.iam.aws_iam_role_policy.ci_registry_lambda: Refreshing state... [id=javabin-ci-registry:invoke-team-provisioner]
module.lambdas.aws_lambda_permission.override_cleanup_schedule: Refreshing state... [id=AllowEventBridge]
module.lambdas.aws_cloudwatch_event_target.override_cleanup: Refreshing state... [id=javabin-override-cleanup-schedule-invoke-override-cleanup]
module.monitoring.aws_s3_bucket_server_side_encryption_configuration.config_logs: Refreshing state... [id=javabin-config-553637109631]
module.monitoring.aws_s3_bucket_public_access_block.config_logs: Refreshing state... [id=javabin-config-553637109631]
module.monitoring.aws_s3_bucket_policy.config_logs: Refreshing state... [id=javabin-config-553637109631]
module.iam.aws_iam_role_policy.ci_override_approver: Refreshing state... [id=javabin-ci-override-approver:invoke-apply-gate]
module.iam.aws_iam_role_policy.ci_infra_allow: Refreshing state... [id=javabin-ci-infra:infra-management]
module.iam.aws_iam_role_policy.ci_infra_deny: Refreshing state... [id=javabin-ci-infra:deny-dangerous-operations]
module.iam.aws_iam_role_policy.ci_apply_gate: Refreshing state... [id=javabin-ci-apply-gate:invoke-gate-and-read-plans]
module.ingress.aws_acm_certificate_validation.wildcard: Refreshing state... [id=2026-03-07 16:29:14.551 +0000 UTC]
module.iam.aws_iam_role.ci_app["platform-test-app"]: Refreshing state... [id=javabin-ci-app-platform-test-app]
module.lambdas.aws_lambda_function.slack_alert: Refreshing state... [id=javabin-slack-alert]
module.networking.aws_subnet.private_b: Refreshing state... [id=subnet-09ee21336f809f3c9]
module.networking.aws_security_group.ecs_tasks: Refreshing state... [id=sg-0df9a0a3a22548c62]
module.networking.aws_subnet.public_b: Refreshing state... [id=subnet-0eb818326ee94a266]
module.networking.aws_subnet.public_a: Refreshing state... [id=subnet-0f6bfec917146b856]
module.networking.aws_security_group.alb: Refreshing state... [id=sg-061000c0fa68a41b7]
module.networking.aws_internet_gateway.main: Refreshing state... [id=igw-07b193bea823a7f69]
module.networking.aws_subnet.private_a: Refreshing state... [id=subnet-0329ad20dc025c693]
module.iam.aws_iam_role_policy.ci_deploy_ecr["platform-test-app"]: Refreshing state... [id=javabin-ci-deploy-platform-test-app:ecr-push]
module.iam.aws_iam_role_policy.ci_deploy_ecs["platform-test-app"]: Refreshing state... [id=javabin-ci-deploy-platform-test-app:ecs-deploy]
module.iam.aws_iam_role_policy.ci_deploy_ssm["platform-test-app"]: Refreshing state... [id=javabin-ci-deploy-platform-test-app:ssm-read-overrides]
module.iam.aws_iam_role_policy.ci_deploy_logs["platform-test-app"]: Refreshing state... [id=javabin-ci-deploy-platform-test-app:cloudwatch-logs]
module.lambdas.aws_lambda_permission.daily_cost_check_schedule: Refreshing state... [id=AllowEventBridge]
module.lambdas.aws_cloudwatch_event_target.daily_cost_check: Refreshing state... [id=javabin-daily-cost-check-schedule-invoke-daily-cost-check]
module.monitoring.aws_config_delivery_channel.main: Refreshing state... [id=javabin-config-channel]
module.monitoring.aws_config_config_rule.required_tags: Refreshing state... [id=javabin-required-tags]
module.lambdas.aws_iam_role_policy.team_provisioner: Refreshing state... [id=javabin-team-provisioner:javabin-team-provisioner]
module.lambdas.aws_lambda_function.team_provisioner: Refreshing state... [id=javabin-team-provisioner]
module.lambdas.aws_lambda_permission.compliance_reporter_eventbridge: Refreshing state... [id=AllowEventBridge]
module.lambdas.aws_cloudwatch_event_target.compliance_reporter: Refreshing state... [id=javabin-compliance-reporter-trigger-invoke-compliance-reporter]
module.lambdas.aws_lambda_permission.cost_report_schedule: Refreshing state... [id=AllowEventBridge]
module.lambdas.aws_cloudwatch_event_target.cost_report: Refreshing state... [id=javabin-cost-report-schedule-invoke-cost-report]
module.networking.aws_vpc_security_group_egress_rule.ecs_all: Refreshing state... [id=sgr-0266cfa56e8feab14]
module.iam.aws_iam_role_policy.ci_app_deny["platform-test-app"]: Refreshing state... [id=javabin-ci-app-platform-test-app:deny-platform-operations]
module.iam.aws_iam_role_policy.ci_app_allow["platform-test-app"]: Refreshing state... [id=javabin-ci-app-platform-test-app:app-management]
module.networking.aws_vpc_security_group_ingress_rule.ecs_from_alb: Refreshing state... [id=sgr-064d01025000f601e]
module.networking.aws_vpc_security_group_ingress_rule.alb_http: Refreshing state... [id=sgr-07c58f16ef7496031]
module.networking.aws_vpc_security_group_egress_rule.alb_all: Refreshing state... [id=sgr-021faee81305c6e28]
module.networking.aws_vpc_security_group_ingress_rule.alb_https: Refreshing state... [id=sgr-00b490b07c35193b7]
module.networking.aws_route_table.public: Refreshing state... [id=rtb-01c9642f019d36b1f]
module.networking.aws_nat_gateway.main: Refreshing state... [id=nat-0e9cc9e27cc6598db]
module.monitoring.aws_config_configuration_recorder_status.main: Refreshing state... [id=javabin-recorder]
module.lambdas.aws_lambda_permission.slack_alert_security: Refreshing state... [id=AllowSNSSecurity]
module.lambdas.aws_lambda_permission.slack_alert_alerts: Refreshing state... [id=AllowSNSAlerts]
module.lambdas.aws_sns_topic_subscription.slack_alert_security: Refreshing state... [id=arn:aws:sns:eu-central-1:553637109631:javabin-security:0bda8a22-7a50-4a9d-9285-6b1fc1f75376]
module.lambdas.aws_sns_topic_subscription.slack_alert_alerts: Refreshing state... [id=arn:aws:sns:eu-central-1:553637109631:javabin-alerts:380384a2-0cac-48c9-b2d9-2a0aae6968cd]
module.ingress.aws_lb.main: Refreshing state... [id=arn:aws:elasticloadbalancing:eu-central-1:553637109631:loadbalancer/app/javabin-platform-alb/bec1dd43ab8341b9]
module.networking.aws_route_table_association.public_b: Refreshing state... [id=rtbassoc-0186c3a7f0279e344]
module.networking.aws_route_table_association.public_a: Refreshing state... [id=rtbassoc-07ff2e0bfa1578067]
module.networking.aws_route_table.private: Refreshing state... [id=rtb-0b0b4c643592a7db0]
module.networking.aws_route_table_association.private_a: Refreshing state... [id=rtbassoc-0b9248495de9f7316]
module.networking.aws_route_table_association.private_b: Refreshing state... [id=rtbassoc-005259f36758e089e]
module.ingress.aws_lb_listener.https: Refreshing state... [id=arn:aws:elasticloadbalancing:eu-central-1:553637109631:listener/app/javabin-platform-alb/bec1dd43ab8341b9/500c9c2b4186bf45]
module.ingress.aws_lb_listener.http_redirect: Refreshing state... [id=arn:aws:elasticloadbalancing:eu-central-1:553637109631:listener/app/javabin-platform-alb/bec1dd43ab8341b9/1d92e19ae75aa59b]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.lambdas.aws_lambda_function.team_provisioner will be updated in-place
  ~ resource "aws_lambda_function" "team_provisioner" {
        id                             = "javabin-team-provisioner"
      ~ last_modified                  = "2026-03-10T22:00:12.000+0000" -> (known after apply)
      ~ source_code_hash               = "9kaU/prBC1arXBNH+XNrrfe3IDAUktS55nm9sdfadzM=" -> "Y3Hx6LpaN1PjyKolpeMoe6/pLEzmRq/wj306/jnLC2o="
        tags                           = {}
        # (21 unchanged attributes hidden)

        # (4 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

─────────────────────────────────────────────────────────────────────────────

Saved the plan to: tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "tfplan"

@github-actions
Copy link
Copy Markdown

LLM Plan Review

Risk: 🟢 LOW

Routine Lambda function code update for team_provisioner with source code hash change.

  • [routine] Lambda function team_provisioner source code updated (hash changed from 9kaU/prBC1arXBNH+XNrrfe3IDAUktS55nm9sdfadzM= to Y3Hx6LpaN1PjyKolpeMoe6/pLEzmRq/wj306/jnLC2o=). This is an in-place update with no infrastructure changes.
  • [routine] Only one resource being modified - no additions or deletions. No security groups, IAM policies, or critical infrastructure affected.
  • [routine] Last modified timestamp will update automatically upon apply. No force-replacement or data loss risk.

@Alexanderamiri Alexanderamiri merged commit ae4cd67 into main Mar 12, 2026
4 checks passed
@Alexanderamiri Alexanderamiri deleted the fix/additive-only-group-sync branch March 12, 2026 09:31
Alexanderamiri added a commit that referenced this pull request May 9, 2026
## Summary

**Critical fix** — prevents mass-removal of group members.

- Lambda `handle_sync_groups_and_heros`: remove deletion logic, only add
missing members. Members added manually via Google Admin UI are
preserved. Extra members are logged but not removed.
- `provision-groups.py`: remove implicit helter logic, all groups use
explicit memberships
- `sync-heros.py`: always include `helter` in mapped groups

## Why

When the registry restructure PR merged, the empty `heros.yaml`
(members: []) triggered provisioning which resolved all groups to 0
members. The Lambda then removed existing members from Google groups.

## Test plan

- [ ] Merge and apply
- [ ] Verify Lambda only adds members, never removes
- [ ] Verify extra members in Google groups are preserved and logged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant