Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve diff output when changing google_bigquery_dataset.access #4085

Closed
swenzel opened this issue Jul 23, 2019 · 6 comments
Closed

Improve diff output when changing google_bigquery_dataset.access #4085

swenzel opened this issue Jul 23, 2019 · 6 comments
Assignees
Labels

Comments

@swenzel
Copy link

swenzel commented Jul 23, 2019

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 an issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to "hashibot", a community member has claimed the issue already.

Terraform Version

google provider and google provider beta are at 2.11.0
terraform version is 0.12.5

Affected Resource(s)

  • google_bigquery_dataset

Terraform Configuration Files

click to expand

bigquery.tf

locals {
  warehouse_access = [
    {
      role          = "READER"
      user_by_email = google_service_account.object_viewer.email
    },
    {
      user_by_email = google_service_account.object_writer.email
      role          = "WRITER"
    },
    {
      user_by_email = google_service_account.object_owner.email
      role          = "OWNER"
    }
  ]
}

resource "google_service_account" "object_viewer" {
  account_id = "object-viewer"
}

resource "google_service_account" "object_writer" {
  account_id = "object-writer"
}

resource "google_service_account" "object_owner" {
  account_id = "object-owner"
}

resource "google_service_account" "new_object_owner" {
  account_id = "new-object-owner"
}

resource "google_bigquery_dataset" "warehouse" {
  dataset_id = "warehouse"
  location   = var.project_location
  dynamic "access" {
    for_each = local.warehouse_access
    content {
      role           = access.value.role
      group_by_email = lookup(access.value, "group_by_email", null)
      user_by_email  = lookup(access.value, "user_by_email", null)
    }
  }
}

main.tf

data "terraform_remote_state" "terraform_states" {
  backend = "gcs"

  config = {
    bucket = "${var.project_id}-tf"
    prefix = "test/"
  }
}

provider "google" {
  project = var.project_id
  region  = var.project_region
  zone    = var.project_zone
  version = "~>2"
}

provider "google-beta" {
  project = var.project_id
  region  = var.project_region
  zone    = var.project_zone
}

variables.tf

variable project_id {
  type    = string
  default = "myproject"
}

variable project_location {
  type    = string
  default = "EU"
}
variable project_region {
  type    = string
  default = "europe-west3"
}

variable project_zone {
  type    = string
  default = "europe-west3-b"
}

Debug Output

Don't think this is necessary, but I will provide it if requested.

Expected Behavior

When changing local.warehouse_access and then running terraform plan or terraform apply I want to see the actual changes:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # google_bigquery_dataset.warehouse will be updated in-place
  ~ resource "google_bigquery_dataset" "warehouse" {
        creation_time                   = 1563906029592
        dataset_id                      = "warehouse"
        default_partition_expiration_ms = 0
        default_table_expiration_ms     = 0
        etag                            = "JDkdDcON73asdfsbAhT+oQ=="
        id                              = "myproject:warehouse"
        labels                          = {}
        last_modified_time              = 1563906282924
        location                        = "EU"
        project                         = "myproject"
        self_link                       = "https://www.googleapis.com/bigquery/v2/projects/myproject/datasets/warehouse"

      + access {
          + role          = "OWNER"
          + user_by_email = "[email protected]"
        }
      - access {
          - role          = "OWNER" -> null
          - user_by_email = "[email protected]" -> null
        }
        access {
            role          = "WRITER"
            user_by_email = "[email protected]"
        }
        access {
            role          = "READER"
            user_by_email = "[email protected]"
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Actual Behavior

terraform plan suggests that it will delete and recreate all other access entries too:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # google_bigquery_dataset.warehouse will be updated in-place
  ~ resource "google_bigquery_dataset" "warehouse" {
        creation_time                   = 1563906029592
        dataset_id                      = "warehouse"
        default_partition_expiration_ms = 0
        default_table_expiration_ms     = 0
        etag                            = "JDkdasdfswujxubAhT+oQ=="
        id                              = "myproject:warehouse"
        labels                          = {}
        last_modified_time              = 1563906282924
        location                        = "EU"
        project                         = "myproject"
        self_link                       = "https://www.googleapis.com/bigquery/v2/projects/myproject/datasets/warehouse"

      + access {
          + role          = "OWNER"
          + user_by_email = "[email protected]"
        }
      - access {
          - role          = "OWNER" -> null
          - user_by_email = "[email protected]" -> null
        }
      - access {
          - role          = "WRITER" -> null
          - user_by_email = "[email protected]" -> null
        }
      - access {
          - role          = "READER" -> null
          - user_by_email = "[email protected]" -> null
        }
      + access {
          + role          = "WRITER"
          + user_by_email = "[email protected]"
        }
      + access {
          + role          = "READER"
          + user_by_email = "[email protected]"
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Steps to Reproduce

  1. terraform apply
  2. In bigquery.tf change
    user_by_email = google_service_account.object_owner.email
    to
    user_by_email = google_service_account.new_object_owner.email
  3. terraform plan

Important Factoids

none

References

@ghost ghost added the bug label Jul 23, 2019
@emilymye
Copy link
Contributor

Hmm - this might actually just be an upstream issue, since we don't have any control over how the diffs are generated. On the other hand, it might be that the hashkey for the access item in the set changes when you remove one, which shouldn't be happening and would be something to fix on our end?

TF 0.12 did also change the way set items are output (instead of outputting with the hashkey like access.1234567.role: < some diff>) so it could just be that the new diff just outputs all elements in the set when the set changes.

I'll need to repro to confirm whether it's an issue on our end or not, so I'll get back to you with a potential fix or file a bug against upstream.

@swenzel
Copy link
Author

swenzel commented Jul 25, 2019

Hey, thanks for looking into this!
I hope you can find my example to be easily reproducible with a new cloud project. This issue is bugging me for a while now since our warehouse ACL changes quite frequently and its hard to see the actual changes with all that noise.

We only recently switched to 0.12 and we had that issue in 0.11 already. There I could see that it was like you described, the hash key changed. Which is strange because from what I can see the algos check out and the entries are hashed individually. But I'm not a go programmer and cannot properly debug this...

@swenzel
Copy link
Author

swenzel commented Jul 31, 2019

I found hashicorp/terraform#21901
where I read hashicorp/terraform#21901 (comment)
Then I tried setting the unused attributes to an empty string instead of 'null' and then it actually works!

So instead of:

resource "google_bigquery_dataset" "warehouse" {
  dataset_id = "warehouse"
  location   = var.project_location
  dynamic "access" {
    for_each = local.warehouse_access
    content {
      role           = access.value.role
      group_by_email = lookup(access.value, "group_by_email", null)
      user_by_email  = lookup(access.value, "user_by_email", null)
    }
  }
}

I do

resource "google_bigquery_dataset" "warehouse" {
  dataset_id = "warehouse"
  location   = var.project_location
  dynamic "access" {
    for_each = local.warehouse_access
    content {
      role           = access.value.role
      group_by_email = lookup(access.value, "group_by_email", "")
      user_by_email  = lookup(access.value, "user_by_email", "")
      special_group  = ""
      domain         = ""
      dynamic "view" {
        for_each = []
        content {
          project_id = ""
          dataset_id = ""
          table_id   = ""
        }
      }
    }
  }
}

to get the desired output.

@edwardmedia edwardmedia self-assigned this Jan 8, 2020
@edwardmedia
Copy link
Contributor

Error occurs during the first tf apply. Below is the pair of request and its response.
Terraform version: 0.12.20
provider.google v3.8.0

: ---[ REQUEST ]---------------------------------------
: POST /bigquery/v2/projects/myproject/datasets?alt=json HTTP/1.1
: Host: www.googleapis.com
: User-Agent: HashiCorp Terraform/0.12.20 (+https://www.terraform.io) Terraform Plugin SDK/1.4.0 terraform-provider-google/3.8.0
: Content-Length: 165
: Content-Type: application/json
: Accept-Encoding: gzip
: 
: {
:  "access": [
:   {
:    "role": "OWNER"
:   },
:   {
:    "role": "READER"
:   },
:   {
:    "role": "WRITER"
:   }
:  ],
:  "datasetReference": {
:   "datasetId": "warehouse"
:  },
:  "defaultEncryptionConfiguration": null,
:  "location": "EU"
: }
: 
: -----------------------------------------------------
: 2020/02/10 15:33:50 [DEBUG] Google API Response Details:
: ---[ RESPONSE ]--------------------------------------
: HTTP/2.0 400 Bad Request
: Alt-Svc: quic=":443"; ma=2592000; v="46,43",h3-Q050=":443"; ma=2592000,h3-Q049=":443"; ma=2592000,h3-Q048=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000
: Cache-Control: private
: Content-Type: application/json; charset=UTF-8
: Date: Mon, 10 Feb 2020 23:33:50 GMT
: Server: ESF
: Vary: Origin
: Vary: X-Origin
: Vary: Referer
: X-Content-Type-Options: nosniff
: X-Frame-Options: SAMEORIGIN
: X-Google-Backends: [::1]:4156,/bns/pv/borg/pv/bns/cloud-dataengine-routing/prod-routing-global.router/111.esf,acseac16-v6:443
: X-Google-Dos-Service-Trace: main:bigquery-prod-routing-router
: X-Google-Esf-Cloud-Client-Params: backend_service_name: "bigquery.googleapis.com" backend_fully_qualified_method: "google.cloud.bigquery.v2.DatasetService.InsertDataset"
: X-Google-Gfe-Request-Trace: acseac16-v6:443,/bns/pv/borg/pv/bns/cloud-dataengine-routing/prod-routing-global.router/111.esf,acseac16-v6:443
: X-Google-Gfe-Response-Body-Transformations: chunked
: X-Google-Gfe-Response-Code-Details-Trace: response_code_set_by_backend
: X-Google-Gfe-Service-Trace: bigquery-prod-routing-router
: X-Google-Netmon-Label: /bns/pv/borg/pv/bns/cloud-dataengine-routing/prod-routing-global.router/111
: X-Google-Security-Signals: FRAMEWORK=HTTPSERVER2
: X-Google-Service: bigquery-prod-routing-router
: X-Google-Session-Info: CI_ahsPRHRoEGBAoCygBOjcSF2JpZ3F1ZXJ5Lmdvb2dsZWFwaXMuY29tGLqWm4HzBSIVMTEyNjIzOTc5OTIzNjA4OTY5NzU1Shg6Fk5PVF9BX1BFUlNJU1RFTlRfVE9LRU4
: X-Google-Shellfish-Status: CA0gBEBG
: X-Xss-Protection: 0
: 
: {
:   "error": {
:     "code": 400,
:     "message": "An access entry must have exactly one of userByEmail, groupByEmail, domain, specialGroup defined, or view.",
:     "errors": [
:       {
:         "message": "An access entry must have exactly one of userByEmail, groupByEmail, domain, specialGroup defined, or view.",
:         "domain": "global",
:         "reason": "invalid",
elixDatasetService$ServiceParameters$9.handleBlockingRequest(HelixDataset.java:16585)\n\tat com.google.cloud.helix.proto.proto2api.HelixDataset$HelixDatasetService$ServiceParameters$9.handleBlockingRequest(HelixDataset.java:16583)\n\tat com.google.net.rpc3.impl.server.RpcBlockingApplicationHandler.handleRequest(RpcBlockingApplicationHandler.java:28)\n\tat com.google.net.rpc3.impl.server.RpcServerInternalContext.runRpcInApplication(RpcServerInternalContext.java:619)\n\tat com.google.net.rpc3.impl.server.RpcServerChannel$4.apply(RpcServerChannel.java:975)\n\tat com.google.net.rpc3.impl.server.RpcServerChannel$4.apply(RpcServerChannel.java:960)\n\tat com.google.common.util.concurrent.AbstractTransformFuture$TransformFuture.doTransform(AbstractTransformFuture.java:242)\n\tat com.google.common.util.concurrent.AbstractTransformFuture$TransformFuture.doTransform(AbstractTransformFuture.java:232)\n\tat com.google.common.util.concurrent.AbstractTransformFuture.run(AbstractTransformFuture.java:118)\n\tat com.google.common.context.ContextRunnable.runInContext(ContextRunnable.java:89)\n\tat com.google.common.context.ContextRunnable$1.run(ContextRunnable.java:78)\n\tat io.grpc.Context.run(Context.java:602)\n\tat com.google.tracing.GenericContextCallback.runInInheritedContext(GenericContextCallback.java:75)\n\tat com.google.common.context.ContextRunnable.run(ContextRunnable.java:74)\n\tat com.google.common.util.concurrent.MoreExecutors$5$1.run(MoreExecutors.java:1158)\n\tat com.google.common.context.ContextRunnable.runInContext(ContextRunnable.java:89)\n\tat com.google.common.context.ContextRunnable$1.run(ContextRunnable.java:78)\n\tat com.google.tracing.GenericContextCallback.runInInheritedContext(GenericContextCallback.java:77)\n\tat com.google.common.context.ContextRunnable.run(ContextRunnable.java:74)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat java.lang.Thread.run(Thread.java:748)\n"
2-10T15:33:50.668-0800 [DEBUG] plugin.terraform-provider-google_v3.8.0_x5:       }
:     ],
:     "status": "INVALID_ARGUMENT"
:   }
: }
: 
: -----------------------------------------------------

@chrisst
Copy link
Contributor

chrisst commented Feb 18, 2020

@swenzel You appear to have found the root cause already :) This is definitely not something we are able to address within the provider code given the current state of the SDK. The upstream issue will possibly fix this but I suspect we might need to wait until the next version of the SDK is released first.

I'm going to close this as a duplicate of #3929 so that we can concentrate discussions in one place.

@edwardmedia I think what you are seeing isn't related to the original issue here. If you have found a bug can you open a new issue?

@chrisst chrisst closed this as completed Feb 18, 2020
@ghost
Copy link

ghost commented Mar 20, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants