fix: implement automatic user ID resolution in governance module #2
Conversation
- Add data.github_user resource to fetch numeric IDs for referenced users - Extract user logins from repository permissions and environment reviewers - Build github_user_ids map from individual user data sources - Update README to document central ID resolution strategy - Update repository module to require all IDs from parent (no data sources) - Update all examples with ID resolution (simple and complete) - Document breaking changes in repository module README Resolves user ID calculation for governance module examples. Previous approach using github_organization data source returned GraphQL string IDs instead of numeric IDs. New approach uses individual github_user lookups which provide correct numeric IDs with acceptable API call trade-off.
There was a problem hiding this comment.
Pull Request Overview
This PR implements automatic user ID resolution in the governance module to fix an issue where the previous approach returned GraphQL string IDs instead of numeric IDs. The new implementation extracts user logins from repository configurations and uses individual data.github_user lookups to obtain correct numeric IDs.
Key Changes:
- Added automatic extraction of user logins from repository permissions and environment reviewers in the governance module
- Removed data source lookups from the repository module, making it dependent on parent module for all IDs
- Updated examples to demonstrate standalone ID resolution pattern
- Documented breaking changes and new ID resolution strategy
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| data.tf | Added user login extraction logic and data.github_user lookups to build central user ID map |
| modules/repository/main.tf | Removed all data sources; added precondition validations requiring IDs from parent module |
| modules/repository/variables.tf | Updated variable descriptions to indicate IDs must be provided by parent module |
| modules/repository/examples/simple/main.tf | Added example ID resolution pattern using data sources |
| modules/repository/examples/complete/main.tf | Added example ID resolution pattern for standalone usage |
| modules/repository/README.md | Added breaking changes documentation and updated performance optimization section |
| README.md | Updated high-level documentation about central ID resolution strategy |
| examples/complete/tfplan | Binary terraform plan file (auto-generated) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
data.tf
Outdated
|
|
||
| # Fetch user data for referenced users (only if not provided) | ||
| data "github_user" "referenced_users" { | ||
| for_each = length(var.github_user_ids) == 0 ? toset(local.all_user_logins) : [] |
There was a problem hiding this comment.
The for_each argument expects a set or map, but the false branch returns [] (a list/tuple). While Terraform may coerce an empty list in practice, it's better to be explicit and use toset([]) instead of [] for type consistency and clarity.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <[email protected]>
Co-authored-by: vmvarela <[email protected]>
Fix type consistency in for_each empty set handling
|
🎉 This PR is included in version 0.1.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Resolves user ID calculation for governance module examples.
Previous approach using github_organization data source returned GraphQL
string IDs instead of numeric IDs. New approach uses individual github_user
lookups which provide correct numeric IDs with acceptable API call trade-off.