Allow user provided descriptions

[wiktorn]

Following modules create resources with hard-coded descriptions:

• modules/bigquery-dataset
• modules/data-catalog-policy-tag
• modules/logging-bucket
• modules/net-address (with exception of external addresses)
• modules/net-address
• modules/net-vpc-firewall
• modules/net-vpc
• modules/net-vpn-ha
• modules/net-vpc-swp (or net/swp)
  * [ ] modules/folder
  * [ ] modules/organization
  * [ ] modules/project

Modules should allow users to provide description of the resources, using current values as defaults.

Details:

$ find modules -type f -name '*tf' -not -name 'variables.tf' -not -name 'outputs.tf' -not -name 'variables-*.tf' | xargs grep  'description \+= \+"'  | sort
modules/bigquery-dataset/main.tf:  description         = "Terraform managed."
modules/bigquery-dataset/main.tf:  description         = "Terraform managed."
modules/data-catalog-policy-tag/main.tf:  description  = "${each.key} - Terraform managed.  "
modules/folder/logging.tf:  description = "${each.key} (Terraform-managed)."
modules/folder/logging.tf:    description = "Grants bucketWriter to ${google_logging_folder_sink.sink[each.key].writer_identity} used by log sink ${each.key} on ${local.folder.id}"
modules/logging-bucket/main.tf:  description = "Log Analytics Dataset"
modules/net-address/main.tf:  description  = "Terraform managed."
modules/net-vpc-firewall/default-rules.tf:  description   = "Access from the admin subnet to all subnets."
modules/net-vpc-firewall/default-rules.tf:  description   = "Allow http to machines with matching tags."
modules/net-vpc-firewall/default-rules.tf:  description   = "Allow http to machines with matching tags."
modules/net-vpc-firewall/default-rules.tf:  description   = "Allow SSH to machines with matching tags."
modules/net-vpc/routes.tf:  description         = "Terraform-managed."
modules/net-vpc/routes.tf:  description       = "Terraform-managed."
modules/net-vpc/routes.tf:  description      = "Terraform-managed."
modules/net-vpc/routes.tf:  description  = "Terraform-managed."
modules/net-vpc/routes.tf:  description = "Terraform-managed."
modules/net-vpn-ha/main.tf:  description     = "Terraform managed external VPN gateway"
modules/organization/iam.tf:  description = "Terraform-managed."
modules/organization/logging.tf:  description = "${each.key} (Terraform-managed)."
modules/organization/logging.tf:    description = "Grants bucketWriter to ${google_logging_organization_sink.sink[each.key].writer_identity} used by log sink ${each.key} on ${var.organization_id}"
modules/project/iam.tf:  description = "Terraform-managed."
modules/project/logging.tf:  description = "${each.key} (Terraform-managed)."
modules/project/logging.tf:    description = "Grants bucketWriter to ${google_logging_project_sink.sink[each.key].writer_identity} used by log sink ${each.key} on ${local.project.project_id}"

(Last update: 2023-08-01 16:05:50)

[skalolazka]

The module list needs updating. net-vpc-swp done (will be renamed to net-swp).

[ludoo]

I'm ambivalent on some of the changes here. For example in the project module, the custom_roles variable is currently

variable "custom_roles" {
  description = "Map of role name => list of permissions to create in this project."
  type        = map(list(string))
  default     = {}
  nullable    = false
}

Adding a description will either

• require a new variable just for custom roles description, "polluting" the variable namespace with something that will almost never be used, or
• change the type to an object, making module invocation more verbose

IMHO this falls into the "we cover 99% of use cases, the extra 1% can be easily covered by the user editing the module to support their specific requirements". @juliocc @wiktorn @apichick WDYT?

[wiktorn]

I tend to agree to leave it as it is for now, especially that this would be backward incompatible change.

OTOH custom_roles is one of the few things, where I find custom description useful.

[ludoo]

I tend to agree to leave it as it is for now, especially that this would be backward incompatible change.

OTOH custom_roles is one of the few things, where I find custom description useful.

Yep, but it drastically changes the interface. My preference would be to leave as is until someone really needs it, but if you and Julio strongly feel about this, let's. I would just tackle resource management modules in their own PR as they need other fixes (eg fw policies), and they are really critical for everything else.

[juliocc]

I tend to agree to leave it as it is for now, especially that this would be backward incompatible change.
OTOH custom_roles is one of the few things, where I find custom description useful.

Yep, but it drastically changes the interface. My preference would be to leave as is until someone really needs it, but if you and Julio strongly feel about this, let's. I would just tackle resource management modules in their own PR as they need other fixes (eg fw policies), and they are really critical for everything else.

I took a look at custom roles last week and I came to the same conclusion. There are two extra fields that we could potentially add (stage and description). That being said I like the simplicity of a map(list(string)).

I vote to leave it as it is for now.