Add additional encryption options #27
Conversation
| @@ -74,7 +76,8 @@ locals { | |||
| } | |||
|
|
|||
| resource "google_pubsub_topic" "dataflow_input_pubsub_topic" { | |||
| name = local.dataflow_input_topic_name | |||
| name = local.dataflow_input_topic_name | |||
| kms_key_name = var.pubsub_kms_key_name | |||
There was a problem hiding this comment.
Will kms_key_name be ignored if user doesn't provide this input?
| @@ -34,6 +35,12 @@ resource "google_storage_bucket" "dataflow_job_temp_bucket" { | |||
| name = local.dataflow_temporary_gcs_bucket_name | |||
| location = var.region | |||
| storage_class = "REGIONAL" | |||
| dynamic "encryption" { | |||
There was a problem hiding this comment.
I can see the intent behind this dynamic block. However it's not easy to read at all. Is there another better way to apply a conditional here? Maybe two bucket resources with conditional/count, one encrypted, the other is not enccypted.
Also, what is driving this requirement given this is a temporary bucket?
There was a problem hiding this comment.
There could be organization policy applied that you are not able to create any buckets despite of their content without CMEK for example.
There was a problem hiding this comment.
Thx for clarifying what's driving this PR. Org policies requiring use of CMEK for other services (GCS, PubSub or Dataflow) makes sense.
|
|
||
| variable "gcs_kms_key_name" { | ||
| type = string | ||
| description = "(Optional) The `id` of a Cloud KMS key that will be used to encrypt objects inserted into temporary bucket. User is responsible for permissions to this key for Cloud Storage Service Account." |
There was a problem hiding this comment.
Would be helpful to document the permissions required in the README, even if they are not managed by the module. Same for KMS key for PubSub topic.
ilakhtenkov
force-pushed
the
feature/add-additional-encryption-options
branch
from
553f9ed to
688e5de
Compare
ilakhtenkov
force-pushed
the
feature/add-additional-encryption-options
branch
from
688e5de to
4916f78
Compare
|
I will move it to draft as need to fix some points. |
Added options to set up kms keys for Pub/Sub and GCS