search

GSA-TTS / tts-tech-operations

TTS Technology Operations
https://handbook.tts.gsa.gov/tech-operations/
Other
6 stars 0 forks source link

Terraform cannot commit GitHub repository templates directly to `main` #1513

Open adborden opened 3 years ago

adborden commented 3 years ago

Background Information

Terraform fails to create issue templates for 18f/glossary because branch protection is enabled. I think this is good, any automation should create pull requests instead of committing directly to the repository. This also simplifies compliance, because we say "all changes to main must be reviewed by a team member".

Error: PUT https://api.github.com/repos/18f/glossary/contents/.github/ISSUE_TEMPLATE/general.md: 409 Could not create file: At least 1 approving review is required by reviewers with write access. []

Implementation Steps

Acceptance Criteria

adborden commented 3 years ago

https://github.com/gruntwork-io/git-xargs is probably a better tool for this. We can still create the issue templates via CI, but git-xargs will automate pull request creation in the event of the template needing changes.

adborden commented 3 years ago

As part of https://github.com/18F/tts-tech-portfolio/issues/1441, I'm going to run into this issue to update the new workflows across GH repos. I tested out git-xargs in https://github.com/18F/tts-tech-portfolio/pull/1536 which generated an example pull request https://github.com/18F/ghad/pull/59.

There's not a lot of code introduced, git-xargs does the heavy lifting and I'm mostly just shepparding data around (list of templates, list of repos, and commit and pull request descriptions. However, there are two sore spots:

It is a little redundant to create identical pull requests. At least it's a one-person job to review+merge (since tts-bot would be the PR author). From a compliance standpoint, you could argue that this is a feature, and each change goes through PR, just like dependabot updates. From the terraform perspective, by reviewing the terraform plan, you're essentially doing what you'd be doing by reviewing each PR individually, but for terraform it's all a single PR in 18F/tts-tech-portfolio.

git-xargs needs a list of repos, and so does terraform. It's not a major issue, but it does mean using csv/json/yaml for the repo list and reading it into terraform and the git-xargs script.

Getting back to terraform... we could disable "Include admins" for the branch protection, but since all of Tech Portfolio are admins, it makes branch protection must less effective. I could also see us creating new GH accounts for our admin privileges separate from our day-to-day existing accounts. This might be a good security practice regardless.

I will keep 🤔

adborden commented 3 years ago

Thinking about this a little more, I feel like the redundant reviews is okay and git-xargs feels like the right solution. As we grow into multiple teams where different teams manage their own repos, creating A PR feels good for visibility. Dependabot PRs also fall in this category, where we see similar PRs across multiple repos (although it's usually only a few at a time). I think a workflow of reviewing automated PRs is still a good place to aim, Ops Rotation is already in that vein of reviewing and merging PRs.