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.

Sign up for GitHub

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

Jump to bottom

Fix bigquery dataset access iam roles with primative equivalent #3471

Merged
merged 5 commits into from
May 6, 2020
Merged

Fix bigquery dataset access iam roles with primative equivalent #3471

merged 5 commits into from
May 6, 2020

Conversation

emilymye
Copy link
Contributor

@emilymye emilymye commented May 5, 2020

Release Note Template for Downstream PRs (will be copied)

bigquery: Fixed error where `google_bigquery_dataset_access` resources could not be found post-creation if role was set to a predefined IAM role with an equivalent primative role (e.g. `roles/bigquery.dataOwner` and `OWNER`)

Fixes hashicorp/terraform-provider-google#6175

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 24 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 1 file changed, 24 insertions(+), 3 deletions(-))

psvaiter
psvaiter reviewed May 5, 2020
View reviewed changes
templates/terraform/constants/bigquery_dataset_access.go Outdated
@@ -0,0 +1,12 @@
var bigqueryAccessRoleToPrimitiveMap = map[string]string {
"roles/bigQuery.dataOwner": "OWNER",
"roles/bigQuery.dataEditor": "EDITOR",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be WRITER as said in official doc?

Suggested change
"roles/bigQuery.dataEditor": "EDITOR",
"roles/bigQuery.dataEditor": "WRITER",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 4 files changed, 71 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 4 files changed, 71 insertions(+), 5 deletions(-))
Ansible: Diff ( 2 files changed, 3 insertions(+), 5 deletions(-))
Inspec: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

1 similar comment
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 4 files changed, 71 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 4 files changed, 71 insertions(+), 5 deletions(-))
Ansible: Diff ( 2 files changed, 3 insertions(+), 5 deletions(-))
Inspec: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

@emilymye emilymye requested a review from danawillow May 5, 2020 17:11
danawillow
danawillow approved these changes May 5, 2020
View reviewed changes
products/bigquery/terraform.yaml
@@ -91,6 +91,11 @@ overrides: !ruby/object:Overrides::ResourceOverrides
properties:
datasetId: !ruby/object:Overrides::Terraform::PropertyOverride
ignore_read: true
role: !ruby/object:Overrides::Terraform::PropertyOverride
diff_suppress_func: 'resourceBigQueryDatasetAccessRoleDiffSuppress'
custom_expand: "templates/terraform/custom_expand/bigquery_access_role.go.erb"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need the custom expander because that's what gets used to find the item in the list, right? If so can you add a comment explaining as much? It took me a second to figure it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 4 files changed, 71 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 4 files changed, 71 insertions(+), 5 deletions(-))
Ansible: Diff ( 2 files changed, 3 insertions(+), 5 deletions(-))
Inspec: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

@emilymye emilymye merged commit cbdd813 into GoogleCloudPlatform:master May 6, 2020
This was referenced May 6, 2020
Merged
Merged
Merged
Merged
nathkn pushed a commit to nathkn/magic-modules that referenced this pull request May 18, 2020
@emilymye
Fix bigquery dataset access iam roles with primative equivalent (Goog…
9f3bc26 
…leCloudPlatform#3471)

* account for iam vs primative bigquery roles

* add test, fix description

* s/EDITOR/WRITER

* fix test

* comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psvaiter psvaiter psvaiter left review comments

@danawillow danawillow danawillow approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Provider produced inconsistent result in bigquery_dataset_access
5 participants
@emilymye @modular-magician @danawillow @psvaiter @googlebot