-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
bigquery dataset view access creates circular dependency #2686
bigquery dataset view access creates circular dependency #2686
Comments
@chrismorgan-hb @danawillow I would really appreciate your comments, or close if this is not a bug, thanks. |
I think having it as a separate resource is going to be the cleanest solution to this. When Terraform is running, it creates the dependency graph and then creates/updates each resource individually in order. So, if you were to create everything without the view and then manually edit your config to add the view later, that should work just fine (as long as you aren't introducing a circular dependency via interpolation). However, I agree that users should be able to set up their infrastructure by running Terraform just once, so this should be fixed. Since this seems like something that can be worked around, I don't think we can realistically prioritize fixing it immediately, but if you open a PR for the fix someone will look at it. |
Thank you Dana for your comments. If I get around to learning golang then sure I would love to submit a PR, but realistically that's unlikely in the short term. In the meantime yes I've worked around this using the above python script called by a data.external resource where I have defined explicit depends_on. I agree its probably the least common use case for adding access so not a high priority especially given the workaround. One important thing to note if anyone does get around to this is that "access entries" for bigquery datasets are overwrite-only. For example, if you define a dataset with access for a bunch of things, and then later try to update it with view access, it will overwrite the access to just views. In the script above I have considered this and pre-fetch the existing entries before appending the view access and overwriting. |
The "overwrite-only" nature won't be a problem - that's a common pattern in bigquery IAM. Added the help-wanted label to solicit PRs for this one. :) |
One side effect I found when I "create everything without the view and then manually edit your config to add the view later" is that it ends up always thinking there is a change in the dataset resource.
The next time I run
It's not a huge deal but does result in a short period of time where view access is revoked, which is not ideal. For now I've added a |
That particular behavior is intended- one of Terraform's features is detecting drift. So if you create the resource with Terraform, change it later, and then run Terraform again, it'll try to change it back to what you've specified in your config. |
trying to figure out how to find a way to solve this issue, I found this issue hashicorp/terraform#4149 |
Also maybe this terraform cloud feature may help, https://www.hashicorp.com/blog/creating-infrastructure-pipelines-with-terraform-cloud-run-triggers/ ? |
Now that GoogleCloudPlatform/magic-modules#3312 is merged, can we move forward to fined grained access control such as #5520? |
@hden, that PR added fine-grained access control. |
I meant as a terraform resource, or did I missed the document somewhere? |
Once it gets released (it should appear in version 3.17.0) it'll be usable as a |
Hi - just curious, is there an expected release date for 3.17.0? |
Monday! |
That is awesome! I could have guessed that based on your history of releasing weekly! Big impediment removed! Thank you. |
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! |
Community Note
Terraform Version
Terraform v0.11.11
provider.google v1.19.1
Affected Resource(s)
google_bigquery_dataset
google_bigquery_table
Terraform Configuration Files
Debug Output
Error: Error applying plan:
2 error(s) occurred:
google_bigquery_table.filtered_real_table: googleapi: Error 404: Not found: Dataset project_example:private, notFound
google_bigquery_dataset.private: 1 error(s) occurred:
google_bigquery_dataset.private: googleapi: Error 400: View project_example:public.filtered_real_table to authorize not found., invalid
Panic Output
NA
Expected Behavior
Either the bigquery dataset's "access entries" for view access should be deferred until after the datasets, tables and views are created, or we need an explicit new resource for adding view access to datasets, called something like
google_bigquery_dataset_access_entries
Actual Behavior
The view can't be created because it depends on another dataset,
That other dataset can't be created because it depends on the view
A circular dependency exists.
Steps to Reproduce
Run the example provided above.
Note it is incomplete, it needs a
var.project
and an existing GCP project, I'm not defining projects in tf, although you could modify it to also create a project.It's also using
var.external_group_email
to illustrate the use case, however, this access to thepublic
dataset can just be deleted, it's not related to the error.Important Factoids
Is this a bug, or perhaps the docs are wrong and I can't use this for my use case? I think it is the former, based on what other GCP clients allow (see python example below)
In the documentation for google_bigquery_dataset it says:
In the test defined for this, testAccBigQueryDatasetWithViewAccess it is not actually testing a view that references tables in the dataset. The test uses a view that appears to be referencing some other project:
SELECT state FROM [lookerdata:cdc.project_tycho_reports]
I'm not sure with the current setup that it is possible to create a dataset with view access for a view that "will have read access to tables in this dataset", without creating a circular dependency, as I have experienced.
I think the access entries need to be applied separately.
In my example this would result in a dag with this dependency order
References
#2051 is a request to add IAM bindings and other IAM things for bigquery, which is somewhat related. View access to a dataset is not part of IAM though.
Test for the
bigquery_dataset
view access : https://github.com/terraform-providers/terraform-provider-google/blob/2.0.0/google/resource_bigquery_dataset_test.go#644 original issue requesting this
access control
functionalityFunctionality added in this PR: #1931
Workaround
I am using this Python script to add dependencies after the datasets, views and tables are created. Sorry it's not golang:
In your tf file, create a
data external
resource , and make sure it has explicitdepends_on
defined for all the views you want access entries created for and also the dataset the views need access to.This will apply permission to ALL views in the dataset passed in to the
query.public
attribute. If you want to only specify specific view permission you'll have to modify the script and allow a list to be passed in to thequery
parameter.If you define access blocks in the dataset (e.g. for roles or special groups) then subsequent runs of this workaround will lead to the resource definition for the dataset removing the view access which will then be re-added by the python script. i.e. it will cause changes to permissions even though technically nothing has changed. One potential workaround to that is use a
lifecycle { ignore_changes = ["access"] }
block in the dataset resource and make it ignore changes to access. That ignore would need to be temporarily removed if you do want to make real access changes.bq_private_dataset_access_entries.py
The text was updated successfully, but these errors were encountered: