Skip to content

Fix: skip unchanged GitHub team memberships in provisioner #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Alexanderamiri merged 2 commits into from
Mar 17, 2026
Merged

Fix: skip unchanged GitHub team memberships in provisioner #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
201bf0e
Update CLAUDE.md: team-prefixed naming, boundary, tags, audit findings
Alexanderamiri Mar 17, 2026
e874f6c
Skip unchanged GitHub team memberships in provisioner
Alexanderamiri Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ Migration happens later per-app at developer's pace — apps move from old ALB/E
- **Clean, generalized implementations**: Always patternize. If something will be used more than once, make it a module, script, or reusable component from the start. No hacky workarounds.
- **IAM least-privilege**: Scope `Resource` to specific ARNs/regions. If `*` is required by AWS, add a comment explaining why.
- **Secrets via SSM**: Webhook URLs and secrets in SSM Parameter Store under `/javabin/`. Lambdas read at runtime via `ssm:GetParameter`. Never in env vars, TF variables, or code.
- **Tags via provider**: All resources get `default_tags` from `providers.tf`. Don't manually add tags that are already in defaults.
- **Tags via provider**: All resources get `default_tags` from `providers.tf`. Don't manually add tags that are already in defaults. 5 static tags (team, service, repo, environment, managed-by) are set at deploy time; 2 dynamic tags (created-by, commit) are added by the resource-tagger Lambda via EventBridge.
- **Team-prefixed naming**: App resources use `{team}-{service}` naming. The permission boundary enforces this — apps can only create resources whose names start with their team prefix.
- **Permission boundary is human-applied**: The boundary lives in `terraform/org/boundary.tf` and is applied manually (not via CI) because its self-protection prevents CI from modifying it.
- **Pattern matching, not lists**: When categorizing AWS services, use keyword matching. Don't hardcode service name lists.
- **No `.zip` files in git**: Lambda zips are build artifacts from `archive_file`. They're in `.gitignore`.
- **Terraform-first**: Everything lives in Terraform from the first resource. No "set up manually, migrate later." Only exception: bootstrap script for state bucket.
Expand Down Expand Up @@ -125,6 +127,7 @@ terraform/platform/
terraform/org/
main.tf AWS Organizations, SCPs
identity-center.tf IAM Identity Center, permission sets, ABAC (team attribute from SAML)
boundary.tf Permission boundary policy (human-applied, self-protecting)
cloudtrail.tf CloudTrail trail + S3 bucket
providers.tf Provider config
variables.tf Variables
Expand Down Expand Up @@ -207,14 +210,19 @@ terraform/state/

| Type | Pattern |
|------|---------|
| Resources | `javabin-{purpose}` |
| IAM roles | `javabin-ci-{purpose}` (CI — 6 roles including `ci-infra` / `ci-infra-plan` split), `javabin-{service}` (runtime) |
| Platform resources | `javabin-{purpose}` |
| App resources | `{team}-{service}` or `{team}-{service}-{suffix}` |
| IAM CI roles | `javabin-ci-{purpose}` |
| App IAM roles | `{team}-{service}` |
| Lambdas | `javabin-{function}` |
| S3 buckets | `javabin-{purpose}-{account_id}` |
| SSM params | `/javabin/{namespace}/{name}` |
| S3 buckets (platform) | `javabin-{purpose}-{account_id}` |
| S3 buckets (app) | `{team}-{purpose}-{account_id}` |
| SSM params (platform) | `/javabin/{namespace}/{name}` |
| SSM params (app) | `/javabin/apps/{team}/{service}/{name}` |
| ECS cluster | `javabin-platform` |
| ECR repos | `{service-name}` |
| SNS topics | `javabin-{alerts,security}` |
| ECR repos | `{team}-{service}` |
| SNS topics | `javabin-{alerts,security,budget-enforcement}` |
| Log groups | `/ecs/{team}/{service}` |

## Alert Routing

Expand Down Expand Up @@ -276,7 +284,7 @@ The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation tar
| 1 | Identity (Google + Identity Center + Cognito) | **Deployed** — GCP SA with domain-wide delegation, Identity Center with ABAC + 3 permission sets in `terraform/org/`. Google Workspace SAML IdP for SSO (auto-provisions users, groups synced via CI/team-provisioner). Cognito pool TF exists but not yet applied (needs Google OAuth client). |
| 2a | Networking | **Deployed** — VPC, subnets, NAT |
| 2b | Ingress | **Deployed** — ALB + ACM cert |
| 2c | IAM / OIDC | **Deployed** — 6 CI roles (infra, infra-plan, per-app, deploy, override-approver, registry) |
| 2c | IAM / OIDC | **Deployed** — 6 CI roles (infra, infra-plan, per-app, deploy, override-approver, registry), team-prefixed naming + permission boundary |
| 2d | Compute | **Deployed** — ECS cluster + ECR repos |
| 2e | Monitoring | **Deployed** — GuardDuty, Security Hub, Config, SNS |
| 2f | Lambda Functions | **Deployed** — 11 functions (budget-enforcer, resource-tagger, ci-broker added; Google/GitHub/Budget/Cognito/Identity Center sync live) |
Expand All @@ -287,13 +295,16 @@ The SA JSON key is at `/javabin/platform/google-admin-sa`, the impersonation tar
| 3d | Registry Repo | **Working** — repo exists, dispatch uses GitHub App token, team provisioner invoked |
| 3e | javabin CLI | **Code done** — 4 commands (register, init, status, whoami) in javaBin/javabin-cli |
| 3f | CI Images + Supporting Repos | Not started |
| 3g | Tags, Naming, ABAC | **Done** — 5 static + 2 dynamic tags, team-prefixed naming enforced by permission boundary, resource-tagger Lambda for created-by/commit |
| 4 | App Onboarding | **Partially working** — platform-test-app full pipeline passes (plan → review → apply → docker-build), ECS deploy fails on service stabilization |

### Known Issues
- **ECS deploy stabilization**: platform-test-app task registers but service fails health check
- **Cognito pools not yet applied**: TF exists but needs Google OAuth client credentials
- **Team provisioner Lambda**: All sync functions working (Google/GitHub/Budget/Cognito/Identity Center). Password-set flow deployed.
- **`registered_app_repos` manually managed**: Being replaced with team-scoped IAM roles (repo→team resolved via GitHub API at runtime)
- **Cost allocation tags pending activation**: `repo`, `created-by`, `commit` tags need activation in Billing console (requires billing data to appear first)
- **Platform-test-app naming migration**: Existing resources have old `javabin-` prefix names, needs state migration to `{team}-{service}` naming

## Agent Guidelines

Expand Down
43 changes: 25 additions & 18 deletions terraform/lambda-src/team_provisioner/handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,23 +568,26 @@ def sync_github_team(team):
else:
logger.info("Created GitHub team %s", team_slug)

# Current team members (paginated — GitHub returns max 100 per page)
current_members = set()
page = 1
while True:
members_resp = _github_api(
"GET",
f"/orgs/{GITHUB_ORG}/teams/{team_slug}/members?per_page=100&page={page}",
token,
)
if not isinstance(members_resp, list) or not members_resp:
break
current_members.update(m["login"].lower() for m in members_resp)
if len(members_resp) < 100:
break
page += 1

# Desired members
# Current team members with roles (paginated — GitHub returns max 100 per page)
# We need roles to avoid unnecessary PUT calls that can change membership state.
current_members = {} # login -> role
for role_filter in ("maintainer", "member"):
page = 1
while True:
members_resp = _github_api(
"GET",
f"/orgs/{GITHUB_ORG}/teams/{team_slug}/members?role={role_filter}&per_page=100&page={page}",
token,
)
if not isinstance(members_resp, list) or not members_resp:
break
for m in members_resp:
current_members[m["login"].lower()] = role_filter
if len(members_resp) < 100:
break
page += 1

# Desired members — only call PUT if role needs changing
desired_users = set()
for member in team.get("members", []):
m = _normalize_member(member)
Expand All @@ -601,6 +604,10 @@ def sync_github_team(team):
if github_role not in ("maintainer", "member"):
logger.warning("Invalid role '%s' for %s — defaulting to member", github_role, github_user)
github_role = "member"
current_role = current_members.get(github_user)
if current_role == github_role:
logger.info("Skipping %s in team %s — already %s", github_user, team_slug, github_role)
continue
_github_api(
"PUT",
f"/orgs/{GITHUB_ORG}/teams/{team_slug}/memberships/{github_user}",
Expand All @@ -610,7 +617,7 @@ def sync_github_team(team):
logger.info("Set %s as %s in team %s", github_user, github_role, team_slug)

# Remove members no longer in the team definition
for login in current_members - desired_users:
for login in set(current_members.keys()) - desired_users:
_github_api(
"DELETE",
f"/orgs/{GITHUB_ORG}/teams/{team_slug}/memberships/{login}",
Expand Down