Skip to content

Conversation

@ramonvermeulen
Copy link
Contributor

@ramonvermeulen ramonvermeulen commented May 6, 2025

Closes hashicorp/terraform-provider-google#22589

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

`google_iam_workforce_pool_iam_member`
`google_iam_workforce_pool_iam_policy`
`google_iam_workforce_pool_iam_binding`

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 6, 2025
@ramonvermeulen ramonvermeulen force-pushed the ramon/22589-google-iam-workfoce-pool branch 3 times, most recently from a4a17ad to 961a9ca Compare May 6, 2025 18:30
@ramonvermeulen
Copy link
Contributor Author

ramonvermeulen commented May 6, 2025

I put exclude_test: true and implemented custom iam tests because the generated test gave me an error.
Would love to hear if there is a better way to solve this.

Some background:

When I do not set exclude_test: true the generated code gives an error on a generated line that uses IamImportQualifiersForTest:

ImportStateId: fmt.Sprintf("{{ $.IamImportFormat }} {{ $.IamPolicy.AllowedIamRole }}", {{ if ne $.IamImportQualifiersForTest "" }}{{ $.IamImportQualifiersForTest }}, {{ end }}{{ $example.PrimaryResourceName }}),

Because the format string contains two times %s (for location and workforce_pool_id, based on the configured import_format) but only one argument is provided.

image

func (r Resource) IamImportQualifiersForTest() string {

When I look into this method, it only has if branches for specific keys, such as:

project
zone
region
location
universe_domain

A fix could be to add another if branch, or in the final else append importQualifiers with context["%s"] and inject the param via a format string:

else if param == "workforce_pool_id" {
    importQualifiers = append(importQualifiers, `context["workforce_pool_id"]`)
}

However, I don't know the background of this code well enough, so I don't know what is desired here. Would love to open the discussion and/or get some guidance on this!

@ramonvermeulen
Copy link
Contributor Author

ramonvermeulen commented May 6, 2025

Another thing, while implementing I ran into another issue. The API seems to give 500s when trying to set the IAM policy on the resource (get the same error when I try this directly via the endpoint).

POST /v1/locations/global/workforcePools/my-test-pool:setIamPolicy?alt=json HTTP/1.1

Body:

{
  "policy": {
    "bindings": [
      {
        "members": [
          "user:[email protected]"
        ],
        "role": "roles/iam.workforcePoolAdmin"
      }
    ],
    "etag": "BwY0e7cRtGo=",
    "version": 2
  }
}

Response:

{
  "error": {
    "code": 500,
    "message": "Internal error encountered.",
    "status": "INTERNAL"
  }
}

@ramonvermeulen ramonvermeulen marked this pull request as ready for review May 6, 2025 19:00
@github-actions github-actions bot requested a review from BBBmau May 6, 2025 19:01
@github-actions
Copy link

github-actions bot commented May 6, 2025

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@SirGitsalot, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@modular-magician modular-magician added service/iam-workforce and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels May 7, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 7 files changed, 682 insertions(+), 155 deletions(-))
google-beta provider: Diff ( 7 files changed, 682 insertions(+), 155 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 345 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_iam_workforce_pool_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_binding" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Resource: google_iam_workforce_pool_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_member" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Missing doc report (experimental)

The following resources have fields missing in documents.

  • google_iam_workforce_pool_iam_binding
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]
  • google_iam_workforce_pool_iam_member
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]
  • google_iam_workforce_pool_iam_policy
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 20
Passed tests: 17
Skipped tests: 0
Affected tests: 3

Click here to see the affected service packages
  • iamworkforcepool

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccIAMWorkforcePoolIamBinding
  • TestAccIAMWorkforcePoolIamMember
  • TestAccIAMWorkforcePoolIamPolicy

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccIAMWorkforcePoolIamBinding [Debug log]
TestAccIAMWorkforcePoolIamMember [Debug log]
TestAccIAMWorkforcePoolIamPolicy [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@ramonvermeulen ramonvermeulen force-pushed the ramon/22589-google-iam-workfoce-pool branch from 6ae618d to ceb4486 Compare May 9, 2025 06:38
@github-actions github-actions bot requested a review from BBBmau May 9, 2025 06:38
@ramonvermeulen ramonvermeulen force-pushed the ramon/22589-google-iam-workfoce-pool branch from ceb4486 to 372d2dc Compare May 9, 2025 06:39
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 9, 2025
@ramonvermeulen ramonvermeulen force-pushed the ramon/22589-google-iam-workfoce-pool branch from 372d2dc to 5e6f2ff Compare May 9, 2025 06:39
@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label May 9, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 651 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 6 files changed, 651 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 345 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_iam_workforce_pool_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_binding" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Resource: google_iam_workforce_pool_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_member" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Missing doc report (experimental)

The following resources have fields missing in documents.

  • google_iam_workforce_pool_iam_binding
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]
  • google_iam_workforce_pool_iam_member
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]
  • google_iam_workforce_pool_iam_policy
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 22
Passed tests: 19
Skipped tests: 0
Affected tests: 3

Click here to see the affected service packages
  • iamworkforcepool

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccIAMWorkforcePoolWorkforcePoolIamBindingGenerated
  • TestAccIAMWorkforcePoolWorkforcePoolIamMemberGenerated
  • TestAccIAMWorkforcePoolWorkforcePoolIamPolicyGenerated

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccIAMWorkforcePoolWorkforcePoolIamBindingGenerated [Error message] [Debug log]
TestAccIAMWorkforcePoolWorkforcePoolIamMemberGenerated [Error message] [Debug log]
TestAccIAMWorkforcePoolWorkforcePoolIamPolicyGenerated [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@github-actions
Copy link

@BBBmau This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@ramonvermeulen ramonvermeulen force-pushed the ramon/22589-google-iam-workfoce-pool branch from 5e6f2ff to 359522a Compare May 14, 2025 06:19
@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels May 14, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 651 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 6 files changed, 651 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 345 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_iam_workforce_pool_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_binding" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Resource: google_iam_workforce_pool_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_member" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Missing doc report (experimental)

The following resources have fields missing in documents.

  • google_iam_workforce_pool_iam_binding
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]
  • google_iam_workforce_pool_iam_member
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]
  • google_iam_workforce_pool_iam_policy
    • Expected Document Path: /website/docs/r/iam_workforce_pool_iam.html.markdown
    • Fields: [workforce_pool_id]

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 22
Passed tests: 19
Skipped tests: 0
Affected tests: 3

Click here to see the affected service packages
  • iamworkforcepool

Action taken

Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccIAMWorkforcePoolWorkforcePoolIamBindingGenerated
  • TestAccIAMWorkforcePoolWorkforcePoolIamMemberGenerated
  • TestAccIAMWorkforcePoolWorkforcePoolIamPolicyGenerated

Get to know how VCR tests work

@github-actions github-actions bot requested a review from SirGitsalot July 8, 2025 11:54
@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Jul 8, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 663 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 6 files changed, 663 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 345 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_iam_workforce_pool_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_binding" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Resource: google_iam_workforce_pool_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_member" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Multiple resources added

This PR adds multiple new resources: google_iam_workforce_pool_iam_binding, google_iam_workforce_pool_iam_member, google_iam_workforce_pool_iam_policy. This makes review significantly more difficult. Please split it into multiple PRs, one per resource.
An override-multiple-resources label can be added to allow merging.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 24
Passed tests: 21
Skipped tests: 0
Affected tests: 3

Click here to see the affected service packages
  • iamworkforcepool
#### Action taken
Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccIAMWorkforcePoolWorkforcePoolIamBindingGenerated
  • TestAccIAMWorkforcePoolWorkforcePoolIamMemberGenerated
  • TestAccIAMWorkforcePoolWorkforcePoolIamPolicyGenerated

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccIAMWorkforcePoolWorkforcePoolIamBindingGenerated [Debug log]
TestAccIAMWorkforcePoolWorkforcePoolIamMemberGenerated [Debug log]
TestAccIAMWorkforcePoolWorkforcePoolIamPolicyGenerated [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@github-actions
Copy link

@SirGitsalot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@github-actions
Copy link

@GoogleCloudPlatform/terraform-team @SirGitsalot This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

@SirGitsalot SirGitsalot added the override-multiple-resources Allow a PR to contain multiple new resources label Jul 15, 2025
@SirGitsalot
Copy link
Member

/gcbrun

@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Jul 15, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 6 files changed, 663 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 6 files changed, 663 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 345 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_iam_workforce_pool_iam_binding (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_binding" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Resource: google_iam_workforce_pool_iam_member (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_iam_workforce_pool_iam_member" "primary" {
  condition {
    description = # value needed
    expression  = # value needed
    title       = # value needed
  }
}

Multiple resources added

This PR adds multiple new resources: google_iam_workforce_pool_iam_binding, google_iam_workforce_pool_iam_member, google_iam_workforce_pool_iam_policy. This makes review significantly more difficult. Please split it into multiple PRs, one per resource.
An override-multiple-resources label can be added to allow merging.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 24
Passed tests: 24
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • iamworkforcepool
🟢 All tests passed!

View the build log

@SirGitsalot SirGitsalot enabled auto-merge July 15, 2025 23:56
@github-actions
Copy link

@SirGitsalot This PR is approved and has been waiting for merge for 1 week. Is it ready to merge? Use the label disable-review-reminders to disable these notifications.

@github-actions
Copy link

@SirGitsalot This PR is approved and has been waiting for merge for 2 weeks. Is it ready to merge? Use the label disable-review-reminders to disable these notifications.

@SirGitsalot SirGitsalot added this pull request to the merge queue Jul 29, 2025
Merged via the queue into GoogleCloudPlatform:main with commit a7baf6f Jul 29, 2025
27 checks passed
arnavdham pushed a commit to arnavdham/magic-modules-fork that referenced this pull request Jul 29, 2025
aditikumarii-google pushed a commit to aditikumarii-google/magic-modules that referenced this pull request Aug 14, 2025
NandiniAgrawal15 pushed a commit to NandiniAgrawal15/magic-modules that referenced this pull request Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

override-multiple-resources Allow a PR to contain multiple new resources service/iam-workforce

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iam resources for google_iam_workforce_pool are missing

5 participants