Skip to content

BED-6542: Only Collect Management Groups from Current Tenant#185

Merged
StranDutton merged 4 commits intomainfrom
BED-6542-false-edges-across-tenants
Apr 13, 2026
Merged

BED-6542: Only Collect Management Groups from Current Tenant#185
StranDutton merged 4 commits intomainfrom
BED-6542-false-edges-across-tenants

Conversation

@StranDutton
Copy link
Copy Markdown
Contributor

@StranDutton StranDutton commented Apr 2, 2026

Ticket: BED-6542

Writeup (Notion, internal access only): here

Problem: AZContains edges are wrongly created across AZTenant environments, connecting AZManagementGroups of one AZTenant to AZSubscriptions of another AZTenant.

This PR filters out foreign-tenant AZManagementGroups collection.

When AzureHound collects from an account with cross-tenant visibility (like through Azure Lighthouse), the ARM API returns management groups from all accessible tenants. Without AzureHound filtering by tenant, management groups with the same name across tenants produce identical objectId paths, collapsing into a single node in BHE and creating false AZContains edges between tenants. This compares each management group's tenantId against the authenticated tenant and skips any that don't match.

Summary by CodeRabbit

  • Bug Fixes

    • Management groups from other tenants are now filtered out and no longer processed or emitted.
  • Tests

    • Added tests to verify foreign-tenant filtering and correct ordering of home-tenant management groups.
  • Documentation

    • Updated developer toolchain requirement in the README to require Go 1.24 or later.

@StranDutton StranDutton self-assigned this Apr 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 60e72b18-c2a4-48a3-990d-bd74d438cd7a

📥 Commits

Reviewing files that changed from the base of the PR and between 604b286 and 45e99e5.

📒 Files selected for processing (1)
  • README.md
✅ Files skipped from review due to trivial changes (1)
  • README.md

Walkthrough

listManagementGroups now captures the client's home tenant ID and skips management groups whose Properties.TenantId differs, logging skipped foreign groups at verbosity level 2. Tests were updated to stub TenantInfo() and a new test verifies foreign-tenant exclusion.

Changes

Cohort / File(s) Summary
Tenant Filtering Logic
cmd/list-management-groups.go
Capture client.TenantInfo().TenantId and compare against item.Ok.Properties.TenantId; skip and V(2)-log management groups from foreign tenants.
Test Coverage Updates
cmd/list-management-groups_test.go
Stubbed TenantInfo() and set Properties.TenantId on mocked groups; added TestListManagementGroups_FiltersForeignTenants to assert foreign-tenant groups are excluded.
Docs
README.md
Updated Go toolchain requirement from “Go 1.18 or later” to “Go 1.24 or later”.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I checked each group's tenant tag,
Sniffed out strangers—gave a wag.
Home IDs kept, the rest we drop,
Hopping past the non-home crop. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly and clearly describes the main change: filtering management groups to only collect those from the current tenant, which matches the core objective of preventing false edges across tenants.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch BED-6542-false-edges-across-tenants

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@StranDutton StranDutton merged commit 96436c7 into main Apr 13, 2026
10 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 13, 2026
@StranDutton StranDutton deleted the BED-6542-false-edges-across-tenants branch April 13, 2026 18:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants