Added name field to both custom project and org roles #3370

upodroid · 2020-04-12T13:06:04Z

I updated the custom roles to export the name attribute so it can be used with iam role bindings.

I also updated the project_iam_policy article to include a note about importing the policy first before applying the change.

Fixes: hashicorp/terraform-provider-google#6089

https://cloud.google.com/iam/docs/reference/rest/v1/projects.roles#resource:-role

Release Note Template for Downstream PRs (will be copied)

iam: Added `name` field to `google_project_iam_custom_role`
iam: Added `name` field to `google_organization_iam_custom_role`

googlebot · 2020-04-12T13:06:09Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

upodroid · 2020-04-12T13:07:46Z

@googlebot I signed it!

googlebot · 2020-04-12T13:07:49Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

modular-magician · 2020-04-12T13:08:17Z

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@c2thorn, please review this PR or find an appropriate assignee.

modular-magician · 2020-04-12T13:11:17Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 9 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 9 insertions(+), 2 deletions(-))

c2thorn

Hi @upodroid, thanks for your submission! A few things to address here:
In your issue, it looked like you ran into a problem referencing role_id instead of id. id is an inherit attribute that most resources have, so it usually isn't called out specifically in docs. But it certainly won't hurt to add.

Your submission adds name, which is a valid attribute since it is returned from the API, but doesn't really seem to solve the original issue you had since id already exists. I added a comment in the name description to try to make it more clear that they are the same.

c2thorn · 2020-04-13T21:09:28Z

third_party/terraform/resources/resource_google_organization_iam_custom_role.go

@@ -122,6 +122,7 @@ func resourceGoogleOrganizationIamCustomRoleRead(d *schema.ResourceData, meta in
 	d.Set("role_id", parsedRoleName.Name)
 	d.Set("org_id", parsedRoleName.OrgId)
 	d.Set("title", role.Title)
+	d.Set("name", role.Name)


name won't appear in state until the field is established as part of the schema. It will need to be set to Computed: true as well.

Do I set this in the schema definition on resourceGoogleProjectIamCustomRole function?

Yep, within the schema map. Just needs to be set to a string and computed.

c2thorn · 2020-04-13T21:17:56Z

third_party/terraform/website/docs/r/google_organization_iam_custom_role.html.markdown

@@ -60,6 +60,8 @@ exported:

 * `deleted` - (Optional) The current deleted state of the role.

+* `name` - The name of the role which can be used with iam role bindings in the format `organizations/{{org_id}}/roles/{{role_id}}`


Let's separate out the definition and make the recommendation a bit more generic. Something like:

Suggested change

* `name` - The name of the role which can be used with iam role bindings in the format `organizations/{{org_id}}/roles/{{role_id}}`

* `name` - The name of the role in the format `organizations/{{org_id}}/roles/{{role_id}}`. Like `id`, this field can be used as a reference in other resources such as IAM role bindings.

c2thorn · 2020-04-13T21:18:41Z

third_party/terraform/website/docs/r/google_project_iam.html.markdown

@@ -28,7 +28,8 @@ Four different resources help you manage your IAM policy for a project. Each of
   from anyone without organization-level access to the project. Proceed with caution.
   It's not recommended to use `google_project_iam_policy` with your provider project
   to avoid locking yourself out, and it should generally only be used with projects
-   fully managed by Terraform.
+   fully managed by Terraform. If you do use this resource, **import** the policy before


Suggested change

fully managed by Terraform. If you do use this resource, **import** the policy before

fully managed by Terraform. If you do use this resource, it is recommended to **import** the policy before

nitpick here

c2thorn · 2020-04-13T21:19:56Z

third_party/terraform/website/docs/r/google_project_iam_custom_role.html.markdown

@@ -49,7 +49,7 @@ The following arguments are supported:

 * `stage` - (Optional) The current launch stage of the role.
    Defaults to `GA`.
-    List of possible stages is [here](https://cloud.google.com/iam/reference/rest/v1/organizations.roles#Role.RoleLaunchStage).
+    List of possible stages is [here](https://cloud.google.com/iam/reference/rest/v1/projects.roles#Role.RoleLaunchStage).


Looks like only the "organizations" version of the link works, this version doesn't point to role launch stages

c2thorn · 2020-04-13T21:21:23Z

third_party/terraform/website/docs/r/google_project_iam_custom_role.html.markdown

@@ -60,6 +60,8 @@ exported:

 * `deleted` - (Optional) The current deleted state of the role.

+ * `name` - The name of the role which can be used with iam role bindings in the format `projects/{{project}}/roles/{{role_id}}`


See the suggestion for google_organization_iam_custom_role

c2thorn · 2020-04-13T21:21:43Z

third_party/terraform/resources/resource_google_project_iam_custom_role.go

@@ -126,6 +126,7 @@ func resourceGoogleProjectIamCustomRoleRead(d *schema.ResourceData, meta interfa

 	d.Set("role_id", GetResourceNameFromSelfLink(role.Name))
 	d.Set("title", role.Title)
+	d.Set("name", role.Name)


Same as the previous comment

c2thorn

Meant to request changes. Please read my review above.

upodroid · 2020-04-13T22:32:44Z

I fixed all the issues in the comments. I also added the id field to make it clear for new terraform users. I knew the about the id field because I heard that it will be replacing selflinks in the future.

modular-magician · 2020-04-13T22:36:37Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 20 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 20 insertions(+), 1 deletion(-))

c2thorn

One more change and it should be good to go

third_party/terraform/resources/resource_google_organization_iam_custom_role.go

third_party/terraform/resources/resource_google_project_iam_custom_role.go

…am_custom_role.go Co-Authored-By: Cameron Thornton <[email protected]>

…stom_role.go Co-Authored-By: Cameron Thornton <[email protected]>

upodroid · 2020-04-13T22:54:20Z

Oopsie, copy and paste

modular-magician · 2020-04-13T22:58:57Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 20 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 20 insertions(+), 1 deletion(-))

c2thorn

Looks great! For some reason our build process is failing, but I think it's a temporary issue. I'll rerun it and merge once it shows successful. Thanks @upodroid!

modular-magician · 2020-04-13T23:10:13Z

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

Diff report:

Terraform GA: Diff ( 5 files changed, 20 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 5 files changed, 20 insertions(+), 1 deletion(-))

…tform#3370) * added name field to both custom project and org roles * patched the schema to include computed values * Update third_party/terraform/resources/resource_google_organization_iam_custom_role.go Co-Authored-By: Cameron Thornton <[email protected]> * Update third_party/terraform/resources/resource_google_project_iam_custom_role.go Co-Authored-By: Cameron Thornton <[email protected]> Co-authored-by: Cameron Thornton <[email protected]>

added name field to both custom project and org roles

43396b7

googlebot added the cla: no label Apr 12, 2020

googlebot added cla: yes and removed cla: no labels Apr 12, 2020

modular-magician added the community label Apr 12, 2020

c2thorn approved these changes Apr 13, 2020

View reviewed changes

c2thorn requested changes Apr 13, 2020

View reviewed changes

patched the schema to include computed values

0d06b29

upodroid requested a review from c2thorn April 13, 2020 22:48

c2thorn requested changes Apr 13, 2020

View reviewed changes

third_party/terraform/resources/resource_google_organization_iam_custom_role.go Outdated Show resolved Hide resolved

third_party/terraform/resources/resource_google_project_iam_custom_role.go Outdated Show resolved Hide resolved

upodroid and others added 2 commits April 13, 2020 23:53

Update third_party/terraform/resources/resource_google_organization_i…

e290faf

…am_custom_role.go Co-Authored-By: Cameron Thornton <[email protected]>

Update third_party/terraform/resources/resource_google_project_iam_cu…

72c1289

…stom_role.go Co-Authored-By: Cameron Thornton <[email protected]>

c2thorn approved these changes Apr 13, 2020

View reviewed changes

c2thorn merged commit 84c4adc into GoogleCloudPlatform:master Apr 13, 2020

This was referenced Apr 13, 2020

Added name field to both custom project and org roles hashicorp/terraform-provider-google#6111

Merged

Added name field to both custom project and org roles hashicorp/terraform-provider-google-beta#1951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added name field to both custom project and org roles #3370

Added name field to both custom project and org roles #3370

upodroid commented Apr 12, 2020 •

edited

Loading

googlebot commented Apr 12, 2020

upodroid commented Apr 12, 2020

googlebot commented Apr 12, 2020

modular-magician commented Apr 12, 2020

modular-magician commented Apr 12, 2020

c2thorn left a comment

c2thorn Apr 13, 2020

upodroid Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn Apr 13, 2020

c2thorn left a comment

upodroid commented Apr 13, 2020 •

edited

Loading

modular-magician commented Apr 13, 2020

c2thorn left a comment

upodroid commented Apr 13, 2020

modular-magician commented Apr 13, 2020

c2thorn left a comment

modular-magician commented Apr 13, 2020

		@@ -60,6 +60,8 @@ exported:

		* `deleted` - (Optional) The current deleted state of the role.

		* `name` - The name of the role which can be used with iam role bindings in the format `organizations/{{org_id}}/roles/{{role_id}}`

	* `name` - The name of the role which can be used with iam role bindings in the format `organizations/{{org_id}}/roles/{{role_id}}`
	* `name` - The name of the role in the format `organizations/{{org_id}}/roles/{{role_id}}`. Like `id`, this field can be used as a reference in other resources such as IAM role bindings.

	fully managed by Terraform. If you do use this resource, import the policy before
	fully managed by Terraform. If you do use this resource, it is recommended to import the policy before

Added name field to both custom project and org roles #3370

Added name field to both custom project and org roles #3370

Conversation

upodroid commented Apr 12, 2020 • edited Loading

googlebot commented Apr 12, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

upodroid commented Apr 12, 2020

googlebot commented Apr 12, 2020

modular-magician commented Apr 12, 2020

modular-magician commented Apr 12, 2020

Diff report:

c2thorn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c2thorn left a comment

Choose a reason for hiding this comment

upodroid commented Apr 13, 2020 • edited Loading

modular-magician commented Apr 13, 2020

Diff report:

c2thorn left a comment

Choose a reason for hiding this comment

upodroid commented Apr 13, 2020

modular-magician commented Apr 13, 2020

Diff report:

c2thorn left a comment

Choose a reason for hiding this comment

modular-magician commented Apr 13, 2020

Diff report:

upodroid commented Apr 12, 2020 •

edited

Loading

upodroid commented Apr 13, 2020 •

edited

Loading