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.

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

Workaround for corrupted or missing @odata.id for directory objects #616

Merged
merged 2 commits into from
Oct 7, 2021

Conversation

manicminer
Copy link
Contributor

@manicminer manicminer commented Oct 6, 2021

We use the @odata.id field (DirectoryObject.ODataId) to reference objects for @odata.bind fields, e.g. when creating applications, groups, service principals, or posting new members for groups or directory roles. Two breaking changes were recently introduced in the API:

  1. @odata.id stopped being returned for requests with Accept: odata.metadata=minimal. Hamilton has a fix for this in v0.32.0.
  2. The format of the @odata.id value has changed for some tenants. Previously it was always the URI of the object, now in many cases despite the odata-json spec suggesting this should be a URI, it takes the form directoryObjects('0000...'). This form isn't recognised by other API endpoints like applications, groups etc.

To work around (2), we're manually constructing the OData ID to look like a URI. This is intended to be a temporary fix until the API stabilizes.

Fixes: #588

We use the `@odata.id` field (`DirectoryObject.ODataId`) to reference
objects for `@odata.bind` fields, e.g. when creating applications,
groups, service principals, or posting new members for groups or
directory roles. Two breaking changes were recently introduced in the
API:

1. `@odata.id` stopped being returned for requests with
   `Accept: odata.metadata=minimal`. Hamilton has a fix for this in
   v0.32.0.
2. The format of the `@odata.id` value has changed for some tenants.
   Previously it was always the URI of the object, now in many cases
   despite the odata.json spec suggesting this should be a URI, it
   takes the form `directoryObject('0000...')`. This form isn't
   recognised by other API endpoints like applications, groups etc.

To work around (2), we're manually constructing the OData ID to look
like a URI. This is intended to be a temporary fix until the API
stabilizes.

See #588
Comment on lines +925 to +926
callerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, callerId)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this intentionally hardcodes the API version into the URI as that's what the API accepts, regardless of the actual version

@brnwn4
Copy link

brnwn4 commented Oct 6, 2021

@manicminer - Does this mean we can still use the 2.5 azurerm provider and expect it to work? Or are we still pending the bug fix release tomorrow?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

LGTM 🏗️

@manicminer
Copy link
Contributor Author

Test Results

App Role Assignments
Screenshot 2021-10-07 at 12 38 23

Applications
Screenshot 2021-10-07 at 12 38 32

Directory Roles
Screenshot 2021-10-07 at 12 38 39

Groups (failures unrelated)
Screenshot 2021-10-07 at 12 38 45

Service Principals
Screenshot 2021-10-07 at 12 38 52

@manicminer manicminer merged commit f049c74 into main Oct 7, 2021
@manicminer manicminer deleted the bugfix/odata-id branch October 7, 2021 11:40
manicminer added a commit that referenced this pull request Oct 7, 2021
@smnmtzgr
Copy link

smnmtzgr commented Oct 7, 2021

@manicminer thanks for working on this! Can you give us feedback in which version this will be available? Will this also be a backport-fix? Or only in 2.6.0? If only in 2.6.0, is there already a release date?

@mariussm
Copy link

mariussm commented Oct 7, 2021

We're also having this issue. Could this be considered for a 2.5.1 type release?

@bardholthe
Copy link

We're also having this issue.

@RafaelM1994
Copy link

We are also impacted by this issue. I hope we can get this new version today.

@github-actions
Copy link

github-actions bot commented Oct 7, 2021

This functionality has been released in v2.6.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

github-actions bot commented Nov 7, 2021

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: ODataId was nil when creating an azuread_group resource
7 participants