Storage Bucket IAM ancestry inaccurate

jsmilani commented 1 year ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave +1 or me too comments; they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment.
If the issue is assigned to a user, that user is claiming responsibility for the issue.

Terraform Validator version

terraform-validator: v0.13.0 AND gcloud terraform tools: v0.8.0

Affected Resource(s)

google_storage_bucket_iam_member

Terraform Plan JSON

{
  "format_version": "1.1",
  "terraform_version": "1.3.4",
  "variables": {},
  "planned_values": {
    "root_module": {
      "resources": [
        {
          "address": "google_storage_bucket.testdata-bucket",
          "mode": "managed",
          "type": "google_storage_bucket",
          "name": "testdata-bucket",
          "provider_name": "registry.terraform.io/hashicorp/google",
          "schema_version": 0,
          "values": {
            "bucket_policy_only": true,
            "cors": [],
            "default_event_based_hold": false,
            "encryption": [],
            "force_destroy": false,
            "id": "bkt-test-data-development-123",
            "labels": {},
            "lifecycle_rule": [],
            "location": "US-CENTRAL1",
            "logging": [],
            "name": "bkt-test-data-development-123",
            "project": "prj-test-123",
            "requester_pays": false,
            "retention_policy": [],
            "self_link": "https://www.googleapis.com/storage/v1/b/bkt-test-data-development-123",
            "storage_class": "STANDARD",
            "uniform_bucket_level_access": true,
            "url": "gs://bkt-test-data-development-123",
            "versioning": [
              {
                "enabled": true
              }
            ],
            "website": []
          },
          "sensitive_values": {
            "cors": [],
            "encryption": [],
            "labels": {},
            "lifecycle_rule": [],
            "logging": [],
            "retention_policy": [],
            "versioning": [
              {}
            ],
            "website": []
          }
        },
        {
          "address": "google_storage_bucket_iam_member.testdata-bucket-test",
          "mode": "managed",
          "type": "google_storage_bucket_iam_member",
          "name": "testdata-bucket-test",
          "provider_name": "registry.terraform.io/hashicorp/google",
          "schema_version": 0,
          "values": {
            "bucket": "bkt-test-data-development-123",
            "condition": [],
            "member": "group:grp-foobar@test.dev",
            "role": "roles/storage.objectAdmin"
          },
          "sensitive_values": {
            "condition": []
          }
        }
      ]
    }
  },
  "resource_changes": [
    {
      "address": "google_storage_bucket.testdata-bucket",
      "mode": "managed",
      "type": "google_storage_bucket",
      "name": "testdata-bucket",
      "provider_name": "registry.terraform.io/hashicorp/google",
      "change": {
        "actions": [
          "no-op"
        ],
        "before": {
        },
        "after": {
        },
        "after_unknown": {},
        "before_sensitive": {
        },
        "after_sensitive": {
        }
      }
    },
    {
      "address": "google_storage_bucket_iam_member.testdata-bucket-test",
      "mode": "managed",
      "type": "google_storage_bucket_iam_member",
      "name": "testdata-bucket-test",
      "provider_name": "registry.terraform.io/hashicorp/google",
      "change": {
        "actions": [
          "create"
        ],
        "before": null,
        "after": {
          "bucket": "bkt-test-data-development-123",
          "condition": [],
          "member": "group:grp-foobar@test.dev",
          "role": "roles/storage.objectAdmin"
        },
        "after_unknown": {
          "condition": [],
          "etag": true,
          "id": true
        },
        "before_sensitive": false,
        "after_sensitive": {
          "condition": []
        }
      }
    }
  ]
}

Debug Output

ancestors: ["projects/prj-cloudbuild-456", "folders/456", "organizations/123"]
ancestry_path: "organizations/123/folders/456/projects/prj-cloudbuild-456"

Expected Behavior

I would expect the ancestry to include the project the bucket is in and all parent folders of that project.

Actual Behavior

The ancestry is the path to the cloudbuild project for the service account running the job.

Steps to Reproduce

terraform-validator validate tfplan.json --policy-path=/to/policy

Important Factoids

I need to create a custom policy for the IAM members (eg. must match a pattern) that only applies to certain folders. I have a working policy if only the information was available so the match functionality in the Constraints file would work.

spec:
  match:
    target: # {"$ref":"#/definitions/io.k8s.cli.setters.target"}
      - organizations/*/folders/123/**

# OR

spec:
  match:
    ancestries:
     - folders/fldr-development

References

0000

jsmilani commented 1 year ago

I ran with gcloud beta terraform vet ... --verbosity=debug and saw this in the logs:

DEBUG: [google_storage_bucket_iam_member.testdata-bucket-test: Unable to fetch and merge remote storage.googleapis.com/Bucket asset due to unset or (known after apply) identity fields on the TF resource.].

The bucket already exists but the iam is new and tf plan showed:

  + resource "google_storage_bucket_iam_member" "testdata-bucket-test" {
      + bucket = "bkt-test-data-development-123"
      + etag   = (known after apply)
      + id     = (known after apply)
      + member = "group:grp-foobar@test.dev"
      + role   = "roles/storage.objectAdmin"
    }

I assume the debug message is implying that one of those known after apply fields, id or etag, are causing the bucket lookup to fail, but those are not arguments for the resource so I don't see how those fields can ever exist prior to creating the object. terraform-validator should be able to validate things prior to being created. Am I missing something or is there some other bug preventing the bucket info from being retrieved. Also if I tried to create the iam resource at the same time as creating bucket, it seems like there are likely to be even more problems attempting to figure out the ancestry. Is it even possible to ensure the ancestry is accurate at creation time? Is this likely to be a widespread issue for all IAM resources? I actually want to enforce this policy for more than just bucket iam, but the resource ancestry needs to be known for it to work correctly.

I am open to other solutions, for instance if I could conditionalize this from some other env var or known variable I can set elsewhere, that would work as an adequate alternative for my use case. I don't see any docs on doing that.

jsmilani commented 1 year ago

I was testing other terraform-validator versions and argument variations and realized I was passing in --project=prj-cloudbuild-456 as an argument so at least I know that wasn't guessed from the service account. Without that argument I get a different error:

ERROR: logging before flag.Parse: W1110 00:24:15.286505     478 convert.go:306] storage.googleapis.com/Bucket//storage.googleapis.com/bkt-test-data-development-123 did not return a value for ID field. Skipping asset fetch.
Error: converting tfplan to CAI assets: adding resource changes to converter: adding resource create/update/no-op augmenting asset: getting project for //storage.googleapis.com/bkt-test-data-development-123: required field 'project' is not set, you may use --project=my-project to provide a default project to resolve the issue

The bucket resource exists and has a project_id so I am guessing the google_storage_bucket_iam_member asset is not able to retrieve the correct project_id from the bucket resource (as seen in the terraform plan json in the description) and resorts to randomly picking the default. It still seems like a bug.

iyabchen commented 1 year ago

The code takes in tfplan data from the "change" field, hence the bucket or the bucket iam member does not know the project for the given tfplan. In those case, it falls back to use the value provide by the --project flag. To get the desired results, passing in --project prj-test-123 would help.

jsmilani commented 1 year ago

changing the --project arg only makes sense when you are only terraforming a single project. We terraform a set of projects and they might not even exist prior to running tf plan so that may be a work around under some circumstances but not ours.

The resource I am validating is google_storage_bucket_iam_member and that is getting created and it doesn't have a property to pass in for the project id of the bucket, just the bucket name itself. It should be able to infer the project id from the bucket name. The google_storage_bucket exists already and I am not validating that resource so I am not sure if that is where the change field you are referring to is coming from. I don't believe it makes sense to have the ancestry of one resource depend on the change status of another resource. I would expect correct and consistent ancestry as long as I am validating the same resource regardless of what other resources are changing.

iyabchen commented 1 year ago

@jsmilani the fix is merged in the main branch, could you see whether that can fix your issue?

GoogleCloudPlatform / terraform-validator