azuread_group_member - add eventual consistency check for Create

[ic-smiller]

Community Note

• Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
• Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

groupMemberResourceCreate calls AddMemberRef and then reads the resource back immediately. Because Microsoft Graph is eventually consistent, that read can come back with the member absent, which groupMemberResourceRead interprets as the membership having been removed and so clears the ID from state. Terraform then reports Provider produced inconsistent result after apply. The membership does exist in Azure but was never recorded in state, so the next run fails with A resource with the ID already exists, and the user has to terraform import the orphaned membership to recover.

This adds an eventual consistency check between the write and the read, so the membership link is visible before Create returns.

The approach is the same one already used for this class of problem elsewhere in the provider:

• azuread_invitation - azuread_invitation - add eventual consistency check for Create #1844
• azuread_directory_role_member and azuread_administrative_unit_member already perform this exact wait after adding a member ref
• azuread_group_member already has the mirrored consistency.WaitForDeletion in its Delete function; Create was simply missing the counterpart

One deliberate detail: the ID is set before the wait rather than after it. If the wait times out, that leaves a tainted resource in state rather than a membership that exists in Azure with nothing in state, which is the condition that produces the A resource with the ID already exists failure on the following run. Happy to reorder it to match the two resources listed above if you would rather keep them identical.

Reported in #793 (open since 2022, reproduces with for_each over a set of members, where the concurrent writes make the race much more likely). #1810 looks like a duplicate of the same issue.

Changes to existing Resource / Data Source

• I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
• I have written new tests for my resource or datasource changes & updated any relevant documentation.
• I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
• (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

• My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

No new test is included, and I want to be upfront about why rather than imply coverage that is not there.

The failure is a non-deterministic race against Graph's replication delay, so an acceptance test cannot reliably demonstrate either the bug or the fix. A high-concurrency variant of TestAccGroupMember_multipleUser (the reproduction in #793 uses ten or more members) would raise the odds of tripping it, but it would be flaky by construction and I did not think that was a fair thing to add to the suite. This matches #1844, which shipped the equivalent check for azuread_invitation without new test coverage. The existing TestAccGroupMember_* tests do exercise the modified Create path, including TestAccGroupMember_requiresImport.

I do not have an isolated directory available to run the acceptance tests in. TestAccGroupMember_* provisions users, groups, applications and service principals, and I was not willing to point that at a directory that is not disposable. If a maintainer can run TEST=./internal/services/groups TESTARGS='-run TestAccGroupMember' I would appreciate it, and I am glad to act on anything it turns up.

The full set of checks that CI runs on a PR pass locally against this change:

make quick-checks      # gofmt, gofumpt, goimports, whitespace - 0 issues
make lint              # golangci-lint v2.12.2 - 0 issues
make tfproviderlint    # pass
make docs-lint         # pass
make validate-examples # pass
make depscheck         # no diff in go.mod / go.sum / vendor
make generate          # no change to generated code
make test              # ok
go build ./... && go vet ./internal/services/groups/

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

• azuread_group_member - add eventual consistency check for Create

This is a (please select all that apply):

• Bug Fix
• New Feature (ie adding a service, resource, or data source)
• Enhancement
• Breaking Change

Related Issue(s)

Fixes #793

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the provider.

Changes to Security Controls

No changes to security controls. The change adds a read-back poll of an existing membership using the same client and permissions already required by the resource; it does not alter what is created, what permissions are needed, or what is logged.

Note

If this PR changes meaningfully during the course of review please update the title and description as required.