Create KMS and service account resources for TUF-on-CI #944
Conversation
This adds a service account (which will be granted access to a workload identity pool in the private repo), a KMS keyring and key, and grants signerVerifier to the service account. Signed-off-by: Hayden Blauzvern <[email protected]>
haydentherapper
force-pushed
the
tuf-terraform
branch
from
7b49973 to
cbe6750
Compare
terraform/gcp/modules/tuf/kms.tf
Outdated
| @@ -0,0 +1,40 @@ | |||
| /** | |||
| * Copyright 2023 The Sigstore Authors | |||
There was a problem hiding this comment.
| * Copyright 2023 The Sigstore Authors | |
| * Copyright 2024 The Sigstore Authors |
| @@ -0,0 +1,21 @@ | |||
| /** | |||
| * Copyright 2023 The Sigstore Authors | |||
There was a problem hiding this comment.
| * Copyright 2023 The Sigstore Authors | |
| * Copyright 2024 The Sigstore Authors |
There was a problem hiding this comment.
Looks good to me. I'm ok with merging this but I have two potential suggestions for consideration:
- are the variables useful (What's the advantage of choosing non-default sa name or keyring name) ? I left some specific suggestions in code
- this one I can do in a future PR, no need to include in this PR: It would be useful to add public key viewer permissions, something like
(member could be limited to all project members if there's a group like that, but I think allUsers should be fine too...)
resource "google_kms_key_ring_iam_member" "tuf-everyone-key-iam" { key_ring_id = google_kms_key_ring.tuf-keyring.id role = "roles/cloudkms.publicKeyViewer" member = "allUsers" depends_on = [google_kms_key_ring.tuf-keyring] }
| */ | ||
|
|
||
| resource "google_service_account" "tuf-sa" { | ||
| account_id = var.tuf_service_account_name |
There was a problem hiding this comment.
can we get rid of the variable and parametrize on cluster_name like rekor does:
account_id = format("%s-tuf-sa", var.cluster_name)
There was a problem hiding this comment.
cluster_name is related to GKE. I'm not sure what we could parameterize on beyond project name. I decided to add variables for all new fields just in case someone was opinionated on names, but there's a default value for each so no configuration is needed.
| tuf_service_account_name = var.tuf_service_account_name | ||
|
|
||
| tuf_keyring_name = var.tuf_keyring_name | ||
| tuf_key_name = var.tuf_key_name | ||
| kms_location = var.tuf_kms_location |
There was a problem hiding this comment.
Could we simplify and not expose any of these as variables (except kms_location that seems potentially useful)?
- service account can be parametrized on cluster name (see other comment)
- key and keyring name could just be hard coded -- at least I can't see the value of changing them
There was a problem hiding this comment.
Given that there's defaults for these, most deployers won't have to think about setting the variables. My thought is that this is just in case someone is opinionated.
| resource "google_kms_key_ring_iam_member" "tuf-sa-key-iam" { | ||
| key_ring_id = google_kms_key_ring.tuf-keyring.id | ||
| role = "roles/cloudkms.signerVerifier" | ||
| member = format("serviceAccount:%s@%s.iam.gserviceaccount.com", var.tuf_service_account_name, var.project_id) | ||
| depends_on = [google_kms_key_ring.tuf-keyring, google_service_account.tuf-sa] | ||
| } |
There was a problem hiding this comment.
this looks good to me.
If you decide to remove the service account variable as suggested in other comment, member here would need changing to something like
member = "serviceAccount:${google_service_account.tuf-sa.email}"
|
Some details for others following this:
|
Left comments in line, not a must to have, just was easy to add in case someone wants to customize names
For opening up viewer permissions on the key, I've added a variable to specify members who can view the key. |
Signed-off-by: Hayden Blauzvern <[email protected]>
This adds a service account (which will be granted access to a workload identity pool in the private repo), a KMS keyring and key, and grants signerVerifier to the service account.
Summary
Release Note
Documentation