-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
Export Teams and team memberships to Terraform #206
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
=====================================
Coverage 6.37% 6.37%
=====================================
Files 5 5
Lines 251 251
=====================================
Hits 16 16
Misses 235 235 ☔ View full report in Codecov by Sentry. |
src/cmd/terraform.go
Outdated
` | ||
resp, err := c.ListTeams(nil) | ||
teams := resp.Nodes | ||
cobra.CheckErr(err) | ||
for _, team := range teams { | ||
teamBody := "" | ||
|
||
aliases := flattenAliases(team.Aliases) | ||
if len(aliases) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of 12 days ago team aliases
are required so this len()
check should be removed.
src/cmd/terraform.go
Outdated
` | ||
resp, err := c.ListTeams(nil) | ||
teams := resp.Nodes | ||
cobra.CheckErr(err) | ||
for _, team := range teams { | ||
teamBody := "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please introduce strings.Builder
here?
We can set teamBody := strings.Builder{}
just above this for loop then:
- sb.WriteString("strings below")
- sb.String() // to return the whole string to
config.WriteString(templateConfig()
below - sb.Reset() // to reset the underlying buffer at the end of each loop iteration
Running this command is surprisingly slow and this would be a small step in a more optimized direction IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added!
src/cmd/terraform.go
Outdated
}` | ||
membersOutput += fmt.Sprintf(memberConfig, member.User.Email, member.Role) | ||
} | ||
if len(membersOutput) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if len()
check is safe to remove since appending an empty string doesn't really change teamBody
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually where membersOutput
starts down to here seems like a good candidate for a small function. Lmk if you would like to discuss :)
Something loosely like membersOutput := getMembershipsAsTerraformConfig(team.Memberships.Nodes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented these suggestions
src/cmd/terraform.go
Outdated
func getMembershipsAsTerraformConfig(members []opslevel.TeamMembership) string { | ||
membersBody := strings.Builder{} | ||
for _, member := range members { | ||
memberConfig := ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can memberConfig
be moved out of the for loop?
Overall this looks good! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🎉
Issues
After shipping OpsLevel/terraform-provider-opslevel#144 some customers can be in a state where their terraform Team configs are not using the new
member { }
blocks. This results in Team Memberships not being able to reconcile without kicking all members out of the terraform managed teams.Also, the terraform export output is not compatible with the latest TF schema version.
Changelog
changie
entryTophatting
Before:
After (places a line space between resources):