Manage membership thru IaC #1793

austinlparker · 2023-11-14T21:08:59Z

Per #1596, I wanted to come up with a quick spike of what this would look like. This does have a slight flaw that we need to correct -- it doesn't have child teams right now. I'm also not entirely sure what'll happen when we run it...? Will it delete/re-create all the teams?

Anyway, wanted to get some code here for people to look at and bikeshed about. :)

svrnm · 2023-11-15T09:12:29Z

I'm also not entirely sure what'll happen when we run it...? Will it delete/re-create all the teams?

I would guess that it does look at the current state and sync it to what ever desired state you want, isn't that how terraform is suposed to work? Maybe we can create some fake organisation to do some testing/trials and if we are satisfied with the results we can apply them on the real organisation?

Looking through the groups, this may also be an opportunity to bring some structure into it, right now there are some SIGs that have teams with sub-teams, some do not have that structure, etc.

Is it possible to split iac/membership/terraform.tfvars.json into multiple files? At least put members and teams into their own files, but maybe even have a per-SIG structure, which we could combine with a CODEOWNERS entry (e.g. if there is a dotnet-teams.tfvars.json code-owned by dotnet-maintainers, the right maintainers (+TC+GC) get notified and asked for approval)

austinlparker · 2023-11-15T15:28:56Z

Is it possible to split iac/membership/terraform.tfvars.json into multiple files? At least put members and teams into their own files, but maybe even have a per-SIG structure, which we could combine with a CODEOWNERS entry (e.g. if there is a dotnet-teams.tfvars.json code-owned by dotnet-maintainers, the right maintainers (+TC+GC) get notified and asked for approval)

yes, although that does add an additional bit of baggage when you want to add a new one.

my 0.02 is that the goal would be two files; one that is just members, and one that is SIGs/repos/whatever.

members would be kinda what it is now ({'members': [...]}) but the other could get more complex:

{
  'sigs': [
    {
      'name': 'my-cool-sig',
      'repo-name': 'my-cool-sig',
      'teams': [//array of teams and team members],
      // other stuff we'd like to control via IAC?
    }, ...
  ]
}

austinlparker · 2023-11-15T15:33:25Z

re: 'what will happen when we run this' i actually dunno, there's gonna be subtle differences in team descriptions or whatever and the actual ID of certain things might change (not sure how the github tf provider handles this) so it's entirely possible that everyone's membership and teams will get blown away and will need to be re-added? but also maybe not? do we know anyone that's gone through a migration like this?

svrnm · 2023-11-15T15:42:16Z

re: 'what will happen when we run this' i actually dunno, there's gonna be subtle differences in team descriptions or whatever and the actual ID of certain things might change (not sure how the github tf provider handles this) so it's entirely possible that everyone's membership and teams will get blown away and will need to be re-added? but also maybe not? do we know anyone that's gone through a migration like this?

We can ask via the discussion, and/or we can setup a sample github org, setup some teams & repos, etc and see what is happening.

svrnm · 2023-11-15T15:44:37Z

Is it possible to split iac/membership/terraform.tfvars.json into multiple files? At least put members and teams into their own files, but maybe even have a per-SIG structure, which we could combine with a CODEOWNERS entry (e.g. if there is a dotnet-teams.tfvars.json code-owned by dotnet-maintainers, the right maintainers (+TC+GC) get notified and asked for approval)

yes, although that does add an additional bit of baggage when you want to add a new one.

my 0.02 is that the goal would be two files; one that is just members, and one that is SIGs/repos/whatever.

If it adds to much complexity, I think the added value (having SIGs in the CODEOWNER for those files) is not big enough, so 2 files sounds good to me 👍

yurishkuro · 2023-11-15T16:42:08Z

I am hoping in the final version the input files can be YAML, not JSON.

yurishkuro · 2023-11-15T16:43:30Z

iac/membership/main.tf

@@ -0,0 +1,23 @@
+resource "github_membership" "member" {


nit: I would put this under tools/..., e.g. tools/org_settings, I think iac is unnecessarily cryptic

yurishkuro · 2023-11-15T16:46:56Z

Design question - on the original ticket someone mentioned that TF needs a state storage. Is that true? What's the approach we want to take w.r.t. such storage? I think it would be ideal if the whole thing could run on standard GH runners, not via some additional CNCF compute (which is harder to manage). A storage on local disk committed back to the repo could work (would need a different repo then).

austinlparker · 2023-11-15T17:02:29Z

Design question - on the original ticket someone mentioned that TF needs a state storage. Is that true? What's the approach we want to take w.r.t. such storage? I think it would be ideal if the whole thing could run on standard GH runners, not via some additional CNCF compute (which is harder to manage). A storage on local disk committed back to the repo could work (would need a different repo then).

TF does require state storage, but there are free SaaS options we could pursue or automation to commit that state back to the GitHub repo. I would probably elect to use the SaaS option just to reduce overhead/maintenance burden.

danielgblanco · 2023-11-15T17:10:23Z

Regarding what would happen if we run this: my guess is that it'd fail to create teams and members because those already exist with the same names (and I believe you can't have two teams/members with the same name). Starting from scratch, Terraform state will contain nothing so there's nothing that Terraform is tracking internally. When it does the refresh at the beginning of a plan or apply it will only refresh state from GH to whatever is in the state, which will be empty at that point.

To import resources into the tf state (which as @austinlparker mentioned we'd preferably manage with a SaaS), we'd have to do an terraform import from the current members and teams into the new empty state. After that point, Terraform will ensure that things are kept in sync.

See the import for teams for example which can work with team name or ID

https://registry.terraform.io/providers/integrations/github/latest/docs/resources/team#import

austinlparker · 2023-11-15T17:26:11Z

Annoyingly, there doesn't appear to be a group import so we'll need to do some sort of scripting to import everything one by one.

austinlparker · 2023-11-15T17:51:52Z

So we could do import blocks (https://developer.hashicorp.com/terraform/language/import) but there's no way to loop it that I can find, so it'll be a pretty fun set of PRs.

Write a script to generate the tf files with import statements for all existing resources
Set up state management and put the org under tf control
Remove the import blocks

?

Given the level of pain involved here, it might be worth doing this in one bite (so putting members, teams, and repos under tf control at once).

austinlparker · 2023-11-16T16:52:43Z

After thinking on it a bit and chatting with some infra folks internally, I actually think it makes more sense to more closely align the variable files with the resources. This would give us a 'members.auto.tfvars', 'repositories.auto.tfvars', 'teams.auto.tfvars', etc. It would require a little more work for people (i.e., you would need to add yourself to members and to teams) but it would make the resulting tf code much easier to maintain and reason about. Putting everything in a big yaml file is gonna require a lot of preprocessing that will be, in short, "gnarly".

jaronoff97 · 2023-11-16T17:57:49Z

In theory the auto files go away after this initial PR though right? In the future, any new repos, teams, memberships should only be generated through terraform. I would also recommend maybe making some modules to manage these resources, that way you could define a user with their memberships in one place, not two. I wouldn't recommend import blocks, a script to go through it all is going to be your best bet.

jaronoff97 · 2023-11-16T18:00:41Z

iac/membership/main.tf

+  for_each = toset(var.members)
+
+  username = each.value
+  role     = "member"  // or "admin"


creating a module where the variables would be:

username = var.username role = var.role // default role = member team_memberships = [ { name = "<team>", role = "<member> // default is member }, ... ]

Could be very helpful here to make this a bit easier on the structure/configuration.

austinlparker · 2023-11-16T18:04:31Z

In theory the auto files go away after this initial PR though right? In the future, any new repos, teams, memberships should only be generated through terraform. I would also recommend maybe making some modules to manage these resources, that way you could define a user with their memberships in one place, not two. I wouldn't recommend import blocks, a script to go through it all is going to be your best bet.

Can Spacelift or whatever run an import script as part of state setup? We'll need to keep some sort of canonical reference (the auto.tfvars) - what would go away would be the import blocks. I think the whole end-to-end process would look something like:

Run a script that talks to GH API and creates a few different files - tfvars for repos, members, etc. as well as import blocks for all of those existing resources.
Test this locally until we're at 0 changes during plan.
Apply via Spacelift
Make a new PR to remove all the import blocks, merge it.
All changes to org resources are now managed through tf.

Modules

Change to have a list of maintainers and owners, add working groups

Use a members set

austinlparker · 2023-11-21T04:21:02Z

Jacob and I have done a bit more work on this and I think the structure is, more or less, good... there's some things to consider, though. First, I'm not entirely sure if the membership import stuff here works correctly (honestly, I will create a dummy org to test this out on before running it against the real one). Two, we've added some abstractions around users and groups in order to support future development. It might be a little less clear, but the workfow would basically be 'add yourself to a variables file for membership, and modify the sig block in main.tf to change team roles'.

jaronoff97 · 2023-11-21T15:28:32Z

@austinlparker Sounds good to me RE: testing on a dummy org, lmk if there are extra changes we should make. Also, it looks like we need to copy the output JSON to .auto.tfvars, right now there are no members or owners set.

austinlparker · 2023-11-21T15:45:42Z

@austinlparker Sounds good to me RE: testing on a dummy org, lmk if there are extra changes we should make. Also, it looks like we need to copy the output JSON to .auto.tfvars, right now there are no members or owners set.

Yeah, I've been passing it in manually.

Did some testing this morning in a dummy org (thanks @svrnm!) and discovered the following:

As long as the input mirrors the current state, then memberships will be created transparently in the tfstate without anything changing from the user perspective
However, teams will fail to create due to name collisions.

It seems like we'll need to do some imports there, working to figure those out.

austinlparker · 2023-11-21T15:53:43Z

Ok, did some more testing; Team memberships can be re-created in place, but teams themselves cannot, so for each team we'll need to do something like

tofu import 'module.<sig_name>.github_team.maintainers' <sig name>
tofu import 'module.<sig_name>.github_team.approvers[0]' <sig name>
tofu import 'module.<sig_name>.github_team.triagers[0]' <sig name>
tofu import 'module.<wg_name>.github_team.working_group' <wg name

Add import print statement

austinlparker · 2023-11-22T23:22:20Z

Quick update here - we need to split repo out into its own module, which I've done and not pushed yet, but I need more permissions to actually run the script. cc: @open-telemetry/technical-committee

svrnm · 2024-01-19T08:13:32Z

what's needed to push this forward? I still think implementing this will have a lot of benefits!

jpkrohling · 2024-04-23T07:31:37Z

tools/membership/main.tf

+    maintainers = ["MadVikingGod", "pellared", "MrAlias"]
+}
+
+module "technical-committee_wg" {


I feel like there's a good reason for this, but can't we have this consistent, with either - or _ ?

jpkrohling · 2024-04-23T07:33:48Z

tools/membership/main.tf

+    source = "./modules/sig"
+    name = "ruby"
+    triagers = []
+    approvers = ["robbkidd", "ericmustin", "arielvalentin", "ahayworth", "plantfansam"]


This seems to be missing @kaylareopelle

Thanks for calling this out, @jpkrohling!

I'm not really familiar with who counts as a SIG member. Should the list include everyone with a specific role on the opentelemetry-ruby repo? Should it also include folks with roles in the opentelemetry-ruby-contrib repo?

This PR is very old and doesn't include changes made to members/groups over the past several months; Don't worry, people won't get missed.

jpkrohling · 2024-04-23T07:37:38Z

tools/membership/main.tf

+
+module "collector-contrib-maintainer_wg" {
+    source = "./modules/wg"
+    name = "collector-contrib-maintainer"


What's the difference between this and the previous one?

jpkrohling · 2024-04-23T07:46:13Z

tools/org_helpers/getmembers.py

+
+# Replace these variables with your GitHub organization and personal access token
+ORG_NAME = 'open-telemetry'
+ACCESS_TOKEN = subprocess.run(["gh", "auth", "token"], capture_output=True).stdout.decode('utf-8').strip()


is it possible to use an env var, and if missing, call gh auth token?

jpkrohling · 2024-04-23T07:46:44Z

tools/org_helpers/getmembers.py

+import pprint
+
+# Replace these variables with your GitHub organization and personal access token
+ORG_NAME = 'open-telemetry'


I would love to be able to easily override this as well, so that I could test on my own org

jpkrohling · 2024-04-23T07:49:56Z

tools/org_helpers/getmembers.py

+    #     print(f"tofu import 'module.{group_name}_{suffix}.github_team_membership.sig_{singular}[\"{m}\"]' {group_name}-{member_group}:{m}")
+
+# GENERATE TF FILE
+with open('output.tf', 'w') as f:


Nice, I was wondering what was the origin of the members file :-) Perhaps it needs an update? Would this script be around once we make the switch?

Small nit: can we order the modules alphabetically?

jpkrohling · 2024-04-23T07:52:22Z

tools/org_helpers/output.tf

@@ -0,0 +1,502 @@
+
+module "memberships" {


Is this a duplicate of tools/membership/main.tf?

initial work for iac management (open-telemetry#1596)

5ff8058

austinlparker requested review from alolita, danielgblanco, dyladan, jpkrohling, mtwo, svrnm, tedsuo and trask as code owners November 14, 2023 21:08

austinlparker added 2 commits November 14, 2023 16:09

Merge branch 'main' into tofuMembershipSpike

98e80ca

fix to set child teams

15e9ec1

fix validation errors

0b1387e

yurishkuro reviewed Nov 15, 2023

View reviewed changes

jaronoff97 reviewed Nov 16, 2023

View reviewed changes

jaronoff97 and others added 2 commits November 17, 2023 12:29

new method of doing things

5eed485

Merge pull request #1 from jaronoff97/modules

3f71e95

Modules

austinlparker and others added 9 commits November 17, 2023 16:18

add provider to modules

a8abcab

better structure for members and wg

e33cb0f

use better names

87afde0

fix membership

2111844

Merge pull request #2 from jaronoff97/tofuMembershipSpike

ad136ba

Change to have a list of maintainers and owners, add working groups

missed a file

6e72c52

Merge pull request #3 from jaronoff97/tofuMembershipSpike

346d2e7

Use a members set

modularize tf files

9d77c77

add import script

217a79f

jaronoff97 and others added 3 commits November 21, 2023 11:36

add print for imports

cd92726

removed membership imports and sorted outputs for consistency

0238ac1

Merge pull request #4 from jaronoff97/tofuMembershipSpike

b7f120e

Add import print statement

svrnm mentioned this pull request Nov 21, 2023

Replace CODEOWNERS with component owners open-telemetry/opentelemetry.io#3581

Open

danielgblanco mentioned this pull request Dec 13, 2023

Add GC onboarding issue template #1836

Merged

svrnm mentioned this pull request Feb 6, 2024

Add an issue template for maintainer onboarding #1924

Open

austinlparker added 2 commits April 17, 2024 13:40

Merge branch 'open-telemetry:main' into tofuMembershipSpike

8ec0c8b

Merge branch 'main' into tofuMembershipSpike

40b1396

jpkrohling reviewed Apr 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manage membership thru IaC #1793

Manage membership thru IaC #1793

austinlparker commented Nov 14, 2023

svrnm commented Nov 15, 2023

austinlparker commented Nov 15, 2023

austinlparker commented Nov 15, 2023

svrnm commented Nov 15, 2023

svrnm commented Nov 15, 2023

yurishkuro commented Nov 15, 2023

yurishkuro Nov 15, 2023

yurishkuro commented Nov 15, 2023

austinlparker commented Nov 15, 2023

danielgblanco commented Nov 15, 2023 •

edited

austinlparker commented Nov 15, 2023

austinlparker commented Nov 15, 2023

austinlparker commented Nov 16, 2023

jaronoff97 commented Nov 16, 2023

jaronoff97 Nov 16, 2023 •

edited

austinlparker commented Nov 16, 2023

austinlparker commented Nov 21, 2023

jaronoff97 commented Nov 21, 2023 •

edited

austinlparker commented Nov 21, 2023

austinlparker commented Nov 21, 2023

austinlparker commented Nov 22, 2023

svrnm commented Jan 19, 2024

jpkrohling Apr 23, 2024

jpkrohling Apr 23, 2024

kaylareopelle Apr 27, 2024

austinlparker Apr 27, 2024

jpkrohling Apr 23, 2024

jpkrohling Apr 23, 2024

jpkrohling Apr 23, 2024

jpkrohling Apr 23, 2024

jpkrohling Apr 23, 2024

Manage membership thru IaC #1793

Are you sure you want to change the base?

Manage membership thru IaC #1793

Conversation

austinlparker commented Nov 14, 2023

svrnm commented Nov 15, 2023

austinlparker commented Nov 15, 2023

austinlparker commented Nov 15, 2023

svrnm commented Nov 15, 2023

svrnm commented Nov 15, 2023

yurishkuro commented Nov 15, 2023

Choose a reason for hiding this comment

yurishkuro commented Nov 15, 2023

austinlparker commented Nov 15, 2023

danielgblanco commented Nov 15, 2023 • edited

austinlparker commented Nov 15, 2023

austinlparker commented Nov 15, 2023

austinlparker commented Nov 16, 2023

jaronoff97 commented Nov 16, 2023

jaronoff97 Nov 16, 2023 • edited

Choose a reason for hiding this comment

austinlparker commented Nov 16, 2023

austinlparker commented Nov 21, 2023

jaronoff97 commented Nov 21, 2023 • edited

austinlparker commented Nov 21, 2023

austinlparker commented Nov 21, 2023

austinlparker commented Nov 22, 2023

svrnm commented Jan 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielgblanco commented Nov 15, 2023 •

edited

jaronoff97 Nov 16, 2023 •

edited

jaronoff97 commented Nov 21, 2023 •

edited