Add back broad push access to multicodec

[rvagg]

Ref: #53

I think this gets us back to where we were but with the new much stricter master protection.

[github-actions]

Before merge, verify that all the following plans are correct. They will be applied as-is after the merge.

Terraform plans

multiformats

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  + create
  ~ update in-place

Terraform will perform the following actions:

  # github_branch_protection.this["multicodec:master"] will be updated in-place
  ~ resource "github_branch_protection" "this" {
        id                              = "MDIwOkJyYW5jaFByb3RlY3Rpb25SdWxlMzYwMTQzNA=="
        # (9 unchanged attributes hidden)

      ~ required_pull_request_reviews {
          ~ require_code_owner_reviews      = false -> true
            # (5 unchanged attributes hidden)
        }

        # (1 unchanged block hidden)
    }

  # github_repository_file.this["multicodec/codeowners"] will be created
  + resource "github_repository_file" "this" {
      + branch              = "master"
      + commit_author       = (known after apply)
      + commit_email        = (known after apply)
      + commit_message      = "chore: Update CODEOWNERS [skip ci]"
      + commit_sha          = (known after apply)
      + content             = <<-EOT
            * @darobin @rvagg @vmx
        EOT
      + file                = "CODEOWNERS"
      + id                  = (known after apply)
      + overwrite_on_create = false
      + repository          = "multicodec"
      + sha                 = (known after apply)
    }

  # github_team_repository.this["go team:multicodec"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "multicodec"
      + team_id    = "2210603"
    }

  # github_team_repository.this["javascript team:multicodec"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "multicodec"
      + team_id    = "2123131"
    }

Plan: 3 to add, 1 to change, 0 to destroy.

[galargh]

new much stricter master protection

I don't see any changes to branch protection being done here. Were they performed through the UI?

Otherwise, the change is correct 👍

[willscott]

@galargh - is there more that needs to happen for branch protection than what you referred to in #53 (comment) ?

[galargh]

@galargh - is there more that needs to happen for branch protection than what you referred to in #53 (comment) ?

The mentioned comment is just an information that the branch protection config is now available (and modifiable) through GitHub Management. I didn't change the branch protection settings. I think it should be up to the repo owners to decide.

[BigLep]

@galargh : I assume we need to do something similar for protecting master/main like we did in ipld/github-mgmt#51 for go-car. Is that right?

[willscott]

Can we merge this? and figure out branch protections in a follow-up?

Proposed the change and got agreement of relevant stakeholders both in #53 (comment) and in slack a week ago.

• I can't propose a PR directly in this repo to self-serve the permission change a week later.
• I don't have access to make a PR in https://github.com/multiformats/multicodec a week later and am still blocking on beginning the discussion I wanted to have at the outset of this.

[galargh]

@galargh : I assume we need to do something similar for protecting master/main like we did in ipld/github-mgmt#51 for go-car. Is that right?

That's one way to do it. It could look something like this: #68 However, personally, I don't have a strong opinion on this. It should be up to the maintainers to decide what setup they want there.

Can we merge this? and figure out branch protections in a follow-up?

I'm happy to merge this myself as soon as we get approval from @darobin and/or @rvagg. Personally, given the conversations both in #53 and in Slack, I'm uncomfortable merging it without their sign-off.

I don't have access to make a PR in https://github.com/multiformats/multicodec a week later and am still blocking on beginning the discussion I wanted to have at the outset of this.

The repo is public so one thing I could suggest to unblock would be to create a PR from a fork.

[BigLep]

@galargh : Thanks for the help. I was in conversation with @rvagg yesterday during IPLD triage (which is what triggered my comment above).

We want to keep master/main protected and we want to ensure that codeowners have looked at a PR before merges. We don't want to allow others who have admin permissions in this organization to be able to accidentally bypass these checks on master.

As a result, I think lets do the combination of this PR as it currently stands AND branch protection (ipld/github-mgmt#51 ) AND require codeowner review (#68 ). (That can all be in this PR.)

The one thing that isn't clear for me is whether we should be authoriting CODEOWNER files in github-mgmt or in the repo itself. I can see the value of authoring in github-mgmt so it's clear when trying to think about permissions, but we should add a comment in CODEOWNERS pointing people back to github-mgmt so they know not to make any changes in the repo itself.

Given @rvagg is Australian time, if you can prepare this earlier, I can review.

[galargh]

The one thing that isn't clear for me is whether we should be authoriting CODEOWNER files in github-mgmt or in the repo itself. I can see the value of authoring in github-mgmt so it's clear when trying to think about permissions, but we should add a comment in CODEOWNERS pointing people back to github-mgmt so they know not to make any changes in the repo itself.

That's the cool thing about file support in GitHub Management - it's completely fine with the files being change in the target repos. So, for example, we can author the CODEOWNERS file here. Once we merge the PR, a CODEOWNERS file will be created in the repo. Then, we can keep modifying it through GitHub Management. Or, we can modify it in the repo. If we do the latter, GitHub Management's Sync will notice that the file state drifted and will update the file's content in tf state and in the GitHub Management's config YAML.

Given the comments, I marked #68 as ready for review.

[BigLep]

I merged #68 here.
I think this PR is good to go now.

[BigLep]

@galargh : I think you're expertise will be needed here. It looks like this failed because one of the CODEOWNERS didn't approve it: https://github.com/multiformats/github-mgmt/actions/runs/4565781828/jobs/8057386933#step:6:32. I assume may just need to do this in two steps?

[galargh]

Yikes, that's because we don't allow bypassing branch protection rules in the target repo 🤦 To unblock, I allowed bypassing branch protection rules, recreated a plan in https://github.com/multiformats/github-mgmt/actions/runs/4567564588 and applied it in https://github.com/multiformats/github-mgmt/actions/runs/4567577215. All the changes have been introduced now.

[BigLep]

Thanks @galargh !