Managed Active Directory IAM Resources

[DrFaust92]

Closes hashicorp/terraform-provider-google#12801

If this PR is for Terraform, I acknowledge that I have:

• Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
• Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
• Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
• Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
• Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

google_active_directory_domain_iam_binding

google_active_directory_domain_iam_member

google_active_directory_domain_iam_policy

[DrFaust92]

did best effort as i cant test this

[modular-magician]

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

I've detected that you're a community contributor. @melinath, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

---

Your assigned reviewer will help review your code by:

• Ensuring it's backwards compatible, covers common error cases, etc.
• Summarizing the change into a user-facing changelog note.
• Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

---

[modular-magician]

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.

Terraform GA: Diff ( 3 files changed, 314 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 314 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 293 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 2196
Passed tests 1957
Skipped tests: 239
Failed tests: 0

All tests passed in REPLAYING mode
View the build log

[modular-magician]

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.

Terraform GA: Diff ( 7 files changed, 620 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 7 files changed, 620 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 293 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

[modular-magician]

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.

Terraform GA: Diff ( 8 files changed, 651 insertions(+), 37 deletions(-))
Terraform Beta: Diff ( 8 files changed, 651 insertions(+), 37 deletions(-))
TF Validator: Diff ( 5 files changed, 325 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 2384
Passed tests 2131
Skipped tests: 248
Failed tests: 5

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccActiveDirectoryDomainIamPolicyGenerated|TestAccActiveDirectoryDomainIamMemberGenerated|TestAccActiveDirectoryDomainIamBindingGenerated|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccContainerCluster_withInvalidGatewayApiConfigChannel

[DrFaust92]

test is now passing

--- PASS: TestAccActiveDirectoryDomainIamBindingGenerated (2465.31s)

[DrFaust92]

making an effort to make tests runnable via CI

[modular-magician]

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.

Terraform GA: Diff ( 8 files changed, 689 insertions(+), 40 deletions(-))
Terraform Beta: Diff ( 8 files changed, 689 insertions(+), 40 deletions(-))
TF Validator: Diff ( 5 files changed, 325 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 1 file changed, 8 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 2384
Passed tests 2130
Skipped tests: 248
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccActiveDirectoryDomainIamBindingGenerated|TestAccActiveDirectoryDomainIamPolicyGenerated|TestAccActiveDirectoryDomainIamMemberGenerated

[DrFaust92]

not sure whats the issue with tests in CI, passes locally

[modular-magician]

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccContainerCluster_withInvalidGatewayApiConfigChannel[Debug log]

Tests failed during RECORDING mode:
TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample[Error message] [Debug log]
TestAccActiveDirectoryDomainIamBindingGenerated[Error message] [Debug log]
TestAccActiveDirectoryDomainIamPolicyGenerated[Error message] [Debug log]
TestAccActiveDirectoryDomainIamMemberGenerated[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

[melinath]

Error message is:

Error: Error creating Domain: Post "https://managedidentities.googleapis.com/v1beta1/projects/ci-test-project/locations/global/domains?alt=json&domainName=tfgendui6fitnj4.org.com": context deadline exceeded (Client.Timeout exceeded while awaiting headers)

Possibly it just needs a longer timeout? But I'm not sure; I know we have also had other issues with active directory in the past - see hashicorp/terraform-provider-google#9238

[modular-magician]

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.

Terraform GA: Diff ( 8 files changed, 691 insertions(+), 42 deletions(-))
Terraform Beta: Diff ( 8 files changed, 691 insertions(+), 42 deletions(-))
TF Validator: Diff ( 5 files changed, 325 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 1 file changed, 8 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 2421
Passed tests 2164
Skipped tests: 251
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccActiveDirectoryDomainIamPolicyGenerated|TestAccActiveDirectoryDomainIamMemberGenerated|TestAccActiveDirectoryDomainIamBindingGenerated|TestAccRegionInstanceGroupManager_stateful|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample

[modular-magician]

The provider crashed while running the VCR tests in RECORDING mode
Please fix it to complete your PR
View the build log

[melinath]

The provider crashed while running the VCR tests in RECORDING mode

This is due to the 1h30m timeout on VCR test runs.

[melinath]

Ah - it looks like I didn't look closely enough at the logs. It looks like the resource is trying repeatedly to create the active directory domain and is getting an HTTP 429 Too Many Requests response with the message "Quota 'GlobalDomainLocations' exhausted. Limit 8 in global". The resource is interpreting that as a retryable error, which is causing it to retry indefinitely until it times out.

We'll need to fix the quota issue, but also we'll need to fix the retrying issue. But neither represents a blocking issue with this ticket.

[melinath]

The timeout was probably fine, sorry for the confusion. I think that this might be easiest to resolve by making the IAM tests handwritten and using BootstrapSharedTestADDomain to ensure that they don't compete for resource quota.

[DrFaust92]

melinath i have a concern with this, if we use a shared resource here, 1. how would we manage its state? 2. some of these resources mutate the iam while some additive and it might clash. i think specifically in IAM case its an issue

[modular-magician]

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.

Terraform GA: Diff ( 8 files changed, 689 insertions(+), 40 deletions(-))
Terraform Beta: Diff ( 8 files changed, 689 insertions(+), 40 deletions(-))
TF Validator: Diff ( 5 files changed, 325 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 1 file changed, 8 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 2423
Passed tests 2165
Skipped tests: 252
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeForwardingRule_update|TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample|TestAccActiveDirectoryDomainIamPolicyGenerated|TestAccActiveDirectoryDomainIamBindingGenerated|TestAccActiveDirectoryDomainIamMemberGenerated

[modular-magician]

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccComputeForwardingRule_update[Debug log]

Tests failed during RECORDING mode:
TestAccActiveDirectoryDomain_activeDirectoryDomainBasicExample[Error message] [Debug log]
TestAccActiveDirectoryDomainIamPolicyGenerated[Error message] [Debug log]
TestAccActiveDirectoryDomainIamBindingGenerated[Error message] [Debug log]
TestAccActiveDirectoryDomainIamMemberGenerated[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

[melinath]

melinath i have a concern with this, if we use a shared resource here, 1. how would we manage its state? 2. some of these resources mutate the iam while some additive and it might clash. i think specifically in IAM case its an issue

1. for this test we don't need to manage the state of the parent resource, because the IAM resources are under test.

2. yeah that's a good point. If the tests were handwritten, you could force them to run sequentially, so they wouldn't conflict with each other. For now, it looks like we don't rely on any IAM permissions for other tests using the bootstrapped AD domain; it could cause problems in the future, but it would probably be fine for now?

The other option would be creating separate projects for each test that would each have its own quota.

[melinath]

b/265804681

[DrFaust92]

Thanks for the response, let me see what I can do. Probably the handwritten route (I.e I’ll generate and serialize tests)

…

On Tue, 17 Jan 2023 at 19:51 Stephen Lewis (Burrows) < ***@***.***> wrote: b/265804681 — Reply to this email directly, view it on GitHub <#6717 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIA2W7QCFR6KAXSX2MYNGGLWS3L3TANCNFSM6AAAAAARJBXT5M> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[melinath]

marking as "change requested" per above conversation

[DrFaust92]

melinath I dont see myself pushing this PR further. closing this maybe ill revisit in the future