Skip to content
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

GitHub as code with Terraform #9

Merged
merged 3 commits into from Nov 13, 2020
Merged

GitHub as code with Terraform #9

merged 3 commits into from Nov 13, 2020

Conversation

gianarb
Copy link
Contributor

@gianarb gianarb commented Nov 11, 2020

Description

This PR kicks out the work required to manage the Tinkerbell GitHub organization as code.
For now I would like to start syncing and managing:

  • membership
  • team
  • repository permissions

This PR needs CI/CD as well. The terraform state will be stored in Terraform Cloud, CI/CD will work via GitHub Aciton.

  • Setup CI/CD

NB: I have the state file locally, and terraform plan is in sync. I will move it to terraform soon but I think I need some help @mmlb @displague

Why is this needed

Managing the setting for this organization as code will decrease the chance for undocumented changes. Fewest people will have admin access and everything will have to happen as a pull request, with code review and so on.

As an open community and as members of CNCF we want to be as open as possible, and this is another step in that direction.

@@ -0,0 +1,4 @@
provider "github" {
token = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we define a variable like variable "token" { description = "GitHub Token"} we can change this to token = var.token. By then setting the environment variable TF_VAR_token=$GH_TOKEN within the GitHub action (or using the Terraform "-var" arguments) we will have this value specified.

@@ -0,0 +1,171 @@
resource "github_membership" "amenowanna" {
Copy link
Member

@displague displague Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can create a variable for this:

variable "admins" {
  type = list(string)
  value = [
    "amenowanna",
    "benr", 
     ...
  ]
}

(I may have gotten the type wrong here, you probably don't need the type. Variables definitions should be kept in variables.tf.)

And then:

resource "github_membership" "admin" {
    count = len(var.admin)
    username = var.admin[count.index]
    role = "admin"
}

There is also a for/each way to do this, but I am not as familiar with that. (maybe we need to use a for each to avoid a rebuild when the list is modified from the beginning or middle)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, please! :D Do not ask me to make this dynamic now... it is already a pain to sync all the resources with the local state. I will leave the prettification of this for somebody else or at least for phase 2 when we will reorder all teams and members. Does it sound good?

Pleasee!!

@displague
Copy link
Member

displague commented Nov 11, 2020

This PR needs CI/CD as well. The terraform state will be stored in Terraform Cloud, CI/CD will work via GitHub Aciton.

An S3 bucket can also be used for storage.
https://www.terraform.io/docs/backends/types/s3.html

@gianarb gianarb force-pushed the feat/terraform-org branch 2 times, most recently from 7a8f87f to ff31659 Compare November 12, 2020 09:00
Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
@gianarb gianarb force-pushed the feat/terraform-org branch 2 times, most recently from 1417c28 to d2a3bf7 Compare November 12, 2020 09:20
@gianarb
Copy link
Contributor Author

gianarb commented Nov 12, 2020

An S3 bucket can also be used for storage.
https://www.terraform.io/docs/backends/types/s3.html

At the current time, we do not have an easy way to set up an infrastructure that is not managed by a team in Equinix Metal. And as part of CNCF now we should try to mitigate the dependencies with Equinix Metal as a team, not as a product because we know a lot of the compute power CNCF has come from Equinix Metal!

At the current stage just to keep things going I set up a new organization in Terraform Cloud called Tinkerbell, and that's where the status gets stored.

@rawkode is helping us to figure out how to have a way to declare infrastructure with Pulumi, and as soon as we onboard more services from CNCF I presume we will be able to move to AWS S3 in the right account, or to Minio in the Tinkerbell infrastructure as soon as we will have one.

I also saw that 1Password has a provider for Terraform, not sure how it works but we have an account there as you know and as a follow up to this I would like to investigate if we can control users and access to 1Password from Terraform and also if we can remove secret drift out from GitHub secret (now that's how I store the Terraform Cloud Token) centralizing them to 1Password.

@gianarb gianarb force-pushed the feat/terraform-org branch 2 times, most recently from a6fc5f1 to b98faad Compare November 12, 2020 13:55
@mmlb
Copy link
Contributor

mmlb commented Nov 12, 2020

How about using https://probot.github.io/apps/settings/ ? Managing teams in tf has been a bit of a pain internally and I've been thinking about trying ^ out. Its pretty nice in that we can also extend parts of it in each repo pretty easily.

@gianarb
Copy link
Contributor Author

gianarb commented Nov 12, 2020

will waste some time with probot

@gianarb gianarb closed this Nov 12, 2020
@gianarb gianarb deleted the feat/terraform-org branch November 12, 2020 15:15
@gianarb gianarb restored the feat/terraform-org branch November 12, 2020 15:26
@gianarb
Copy link
Contributor Author

gianarb commented Nov 12, 2020

After an evaluation settings is cool but it has the repository as its scope. I don't need that. I want something that helps us to manage openly the entire organisation. You can not create teams or manage users. And I want to do just that.

The idea to use probot was also good because it didn't require infrastructure to manage, but with GitHub Actions and Terraform Cloud we do not really need a lot more. We will also need something to manage DNS or other services moving forward, so at some point, something like Terraform/Pulumi will come back.

@gianarb gianarb reopened this Nov 12, 2020
@mmlb
Copy link
Contributor

mmlb commented Nov 12, 2020

Ah yep, totally right that probot/settings won't create teams and either way we'll want to be able to manager other bits of infra outside of github too anyway.

Comment on lines 11 to 14
variable "github_token" {
description = "GitHub token"
type = string
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GITHUB_TOKEN is already looked for by the provider by default so we can just get rid of this variable all together

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes. I thought I had added it to line-range when making my comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the original value token = "" work? (does the provider use empty values as a signal to use the GITHUB_TOKEN variable?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I fixed that few days ago when you left the comment :) it was a leftover from my side

Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
@displague
Copy link
Member

How will this run?

I haven't made workflows in a .github directory before, but I would expect tinkerbell/.github/.github/workflows/terraform.yml, rather than tinkerbell/.github/terraform/.github/workflows/terraform.yml

@gianarb gianarb force-pushed the feat/terraform-org branch 2 times, most recently from c13d87b to ce12c98 Compare November 13, 2020 21:21
@@ -1,14 +0,0 @@
branches:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we removing this file?


# .tfstate files
*.tfstate
*.tfstate.*
Copy link
Member

@displague displague Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

but this pr includes a .tfstate_back file (which doesn't match your patterns)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I renamed that when moving state to terraform cloud. we should be good now Thanks

}

provider "github" {
token = var.github_token
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this variable is no longer defined, so I expect this to fail

Copy link
Member

@displague displague Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.terraform.io/docs/providers/github/index.html#token - it is optional, we can remove it.

As @mmlb pointed out, GH Actions will provide the token we need with the environment variable that this TF provider expects.

@gianarb gianarb force-pushed the feat/terraform-org branch 3 times, most recently from 8d96801 to c2439f2 Compare November 13, 2020 21:36
displague
displague previously approved these changes Nov 13, 2020
Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
@gianarb gianarb removed the request for review from mmlb November 13, 2020 21:43
if: contains(github.event.pull_request.labels.*.name, 'ci-check/terraform') && steps.plan.outcome == 'failure'
run: exit 1
# We will introduce it as soon as we validate a few PR and see what plan has to say!
#- name: Terraform Apply
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this latest change, @gianarb! Starting with a terraform plan-only approach is smart thinking.

@gianarb gianarb merged commit 6c0a1d9 into master Nov 13, 2020
@displague displague deleted the feat/terraform-org branch November 13, 2020 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants