env0 / terratag

Terratag is a CLI tool that enables users of Terraform to automatically create and maintain tags across their entire set of AWS, Azure, and GCP resources
https://terratag.io
Mozilla Public License 2.0
936 stars 44 forks source link

Terratag tags `compute_global_address` but only Beta provider supports labels/tags. #171

Closed away168 closed 1 year ago

away168 commented 1 year ago

https://registry.terraform.io/providers/hashicorp/google/4.60.2/docs/resources/compute_global_address

roni-frantchi commented 1 year ago

Internally, Terratag uses tfschema which outputs the schema of the queried resource type and look for labels/tags.

GCP's documentation of that resource does show a labels attribute, though it says it is in beta.

It’s a little strange that GCP has two distinct providers as listed here, though the schema of the resources is shared between and just labeled on the user documentation as a property in beta.

That is what throws tfschema and consequentially Terratag off, a google provider matches schema with google-beta provider for some reason, and only mentions that difference in their shared generated doc.

Workaround

To work around this, one can use the google-beta provider for the failing resource only as well like so:

provider "google" {
  project     = "my-project-id"
  region      = "us-central1"
}

provider "google-beta" {
  project     = "my-project-id"
  region      = "us-central1"
}

resource "google_compute_instance" "labeled-instance" {
  provider = google-beta
  # ... terratag will add labels
}

For us to resolve this issue, we would need to add this resource to a deny list, however doing so will also make Terratag not tag it for google-beta users and future versions of google, and so we should probably not fix this one as the workaround is simple enough.

urie-tsec commented 1 year ago

Checking locally I don't see that tfschema gets the label paramter.

tfschema resource show google_compute_global_address
+--------------------+--------+----------+----------+----------+-----------+
| ATTRIBUTE          | TYPE   | REQUIRED | OPTIONAL | COMPUTED | SENSITIVE |
+--------------------+--------+----------+----------+----------+-----------+
| address            | string | false    | true     | true     | false     |
| address_type       | string | false    | true     | false    | false     |
| creation_timestamp | string | false    | false    | true     | false     |
| description        | string | false    | true     | false    | false     |
| id                 | string | false    | true     | true     | false     |
| ip_version         | string | false    | true     | false    | false     |
| name               | string | true     | false    | false    | false     |
| network            | string | false    | true     | false    | false     |
| prefix_length      | number | false    | true     | false    | false     |
| project            | string | false    | true     | true     | false     |
| purpose            | string | false    | true     | false    | false     |
| self_link          | string | false    | false    | true     | false     |
+--------------------+--------+----------+----------+----------+-----------+

block_type: timeouts, nesting: NestingSingle, min_items: 0, max_items: 0
+-----------+--------+----------+----------+----------+-----------+
| ATTRIBUTE | TYPE   | REQUIRED | OPTIONAL | COMPUTED | SENSITIVE |
+-----------+--------+----------+----------+----------+-----------+
| create    | string | false    | true     | false    | false     |
| delete    | string | false    | true     | false    | false     |
+-----------+--------+----------+----------+----------+-----------+ 

And looking at tfschema's code it should detect the provider name from the name of the resource that should be google in most cases and so it shouldn't take the beta provider. @roni-frantchi Looking at the code if I understand correctly it parses the output of the terraform providers schema -json and looks for the resource there. In that case that we have multiple providers with the same resource type it would choose one of them in an arbitrary measure I think.

urie-tsec commented 1 year ago

Looking even further it looks like it's due to a recent change that changed the logic. And considering the timeframe It roughly make sense since we didn't have the issue before https://github.com/env0/terratag/pull/167

roni-frantchi commented 1 year ago

Thank you @urie-tsec for the detailed triage!

I have completely missed that mentioned change - reopening the issue so we can look into it.

urie-tsec commented 1 year ago

I opened https://github.com/env0/terratag/issues/172 with more precise details

TomerHeber commented 1 year ago

@urie-tsec - thank you for the details. I'm working on PR in #172 I'll close this one.