Remove `pl-deploy-bot` from the org

rjan90 commented 2 months ago

Summary

When reviewing the Lotus-Infra repository, which FilOz now maintains, we encountered the pl-deploy-bot user. We believe this bot was originally created as part of a GitOps contract between Protocol Labs and Weaveworks.

We've determined that the bot is no longer used in the lotus-infra repository. For security reasons, we recommend removing the pl-deploy-bot user from the organization entirely, and are opening this PR to propose this change and get feedback. If anyone is aware of any current uses for this bot within the organization, please let us know.

Reviewer's Checklist

[ ] It is clear where the request is coming from (if unsure, ask)
[ ] All the automated checks passed
[ ] The YAML changes reflect the summary of the request
[ ] The Terraform plan posted as a comment reflects the summary of the request

github-actions[bot] commented 2 months ago

The following access changes will be introduced as a result of applying the plan:

Access Changes

``` User pl-deploy-bot: - will lose push permission to sentinel-infra - will lose admin permission to wge-post-deployment-template-renderer ```

BigLep commented 2 months ago

@galargh : can you help educate me on how to handle this?

From https://github.com/filecoin-project/github-mgmt/actions/runs/10743694671/job/29805452133?pr=66

Error: Instance cannot be destroyed

on resources.tf line 1: 1: resource "github_membership" "this" {

Resource github_membership.this["pl-deploy-bot"] has lifecycle.prevent_destroy set, but the plan calls for this resource to be destroyed. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or reduce the scope of the plan using the -target flag.

I'm not seeing anything in https://github.com/orgs/filecoin-project/people/pl-deploy-bot that would prevent this.

My fallback is to remove the user from the UI and then merge this PR.

BigLep commented 2 months ago

Oh, it looks like we prevent member deletes per https://github.com/filecoin-project/github-mgmt/blob/master/terraform/resources.tf#L9

@galargh : is that intentional? What's the recommended way forward?

galargh commented 2 months ago

Oh, it looks like we prevent member deletes per master/terraform/resources.tf#L9

@galargh : is that intentional? What's the recommended way forward?

This is intentional. This is because org member removals are hard to revert. To re-invite someone, they have to accept the invitation. This is a security measure.

There are 2 intended ways forward for this.

Temporarily remove the protection. As in: 1. Remove the protection. 2. Remove the org member via github as code. 3. Reinstate the protection.
Discuss removal of the member in a PR but apply the change manually through the UI. The order being: 1. Discussion. 2. Manual removal. 3. Run sync in github as code.

github-actions[bot] commented 2 months ago

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

Terraform plans

filecoin-project

``` Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: - destroy Terraform will perform the following actions: # github_repository_collaborator.this["sentinel-infra:pl-deploy-bot"] will be destroyed # (because key ["sentinel-infra:pl-deploy-bot"] is not in for_each map) - resource "github_repository_collaborator" "this" { - id = "sentinel-infra:pl-deploy-bot" -> null - permission = "push" -> null - repository = "sentinel-infra" -> null - username = "pl-deploy-bot" -> null } # github_repository_collaborator.this["wge-post-deployment-template-renderer:pl-deploy-bot"] will be destroyed # (because key ["wge-post-deployment-template-renderer:pl-deploy-bot"] is not in for_each map) - resource "github_repository_collaborator" "this" { - id = "wge-post-deployment-template-renderer:pl-deploy-bot" -> null - permission = "admin" -> null - repository = "wge-post-deployment-template-renderer" -> null - username = "pl-deploy-bot" -> null } Plan: 0 to add, 0 to change, 2 to destroy. ```

BigLep commented 2 months ago

Oh, it looks like we prevent member deletes per master/terraform/resources.tf#L9 @galargh : is that intentional? What's the recommended way forward?

This is intentional. This is because org member removals are hard to revert. To re-invite someone, they have to accept the invitation. This is a security measure.

There are 2 intended ways forward for this.

Temporarily remove the protection. As in: 1. Remove the protection. 2. Remove the org member via github as code. 3. Reinstate the protection.

Discuss removal of the member in a PR but apply the change manually through the UI. The order being: 1. Discussion. 2. Manual removal. 3. Run sync in github as code.

Ack, got it. Here is my plan:

[x] I added a comment next to the user to delete saying they'll be manually deleted and referencing this PR
[x] I'm then going to merge this PR. That way the context for this change shows up in the commit log.
[x] Then I'll go delete in the UI
[x] Paste back screenshot of deletion
[x] Write a readme for how to delete a user

BigLep commented 2 months ago

I confirmed this PR was applied: https://github.com/filecoin-project/github-mgmt/actions/runs/10816661876

I removed the member from the UI: https://github.com/orgs/filecoin-project/people/pl-deploy-bot

BigLep commented 2 months ago

Docs for how to remove a member: https://github.com/filecoin-project/github-mgmt/pull/72

Here is the sync workflow run to update now that the user has been removed: https://github.com/filecoin-project/github-mgmt/actions/runs/10816816463

filecoin-project / github-mgmt