Skip to content

Commit

Permalink
Merge pull request #616 from hashicorp/bugfix/odata-id
Browse files Browse the repository at this point in the history
Workaround for corrupted or missing `@odata.id` for directory objects
  • Loading branch information
manicminer authored Oct 7, 2021
2 parents 2fca067 + 933b4a6 commit f049c74
Show file tree
Hide file tree
Showing 36 changed files with 506 additions and 249 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ require (
github.com/hashicorp/terraform-plugin-sdk/v2 v2.7.0
github.com/hashicorp/yamux v0.0.0-20210316155119-a95892c5f864 // indirect
github.com/klauspost/compress v1.12.2 // indirect
github.com/manicminer/hamilton v0.31.1
github.com/manicminer/hamilton v0.32.0
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
github.com/mitchellh/mapstructure v1.4.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/manicminer/hamilton v0.31.1 h1:uDM9q4bE2mJGieH42M17A6Q1avSxZP9UKwpD6ctX+LI=
github.com/manicminer/hamilton v0.31.1/go.mod h1:QryxpD/4+cdKuXNi0UjLDvgxYdP0LLmYz7dYU7DAX4U=
github.com/manicminer/hamilton v0.32.0 h1:awVHVXOLp9P5QbWaOFqneuWjAvzAk3BYtTEX6AeWB5k=
github.com/manicminer/hamilton v0.32.0/go.mod h1:QryxpD/4+cdKuXNi0UjLDvgxYdP0LLmYz7dYU7DAX4U=
github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA=
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
Expand Down
42 changes: 30 additions & 12 deletions internal/services/applications/application_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,13 @@ func applicationResourceCreate(ctx context.Context, d *schema.ResourceData, meta
if callerObject == nil {
return tf.ErrorDiagF(errors.New("returned callerObject was nil"), "Could not retrieve calling principal object %q", callerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if callerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve calling principal object %q", callerId)
//}
callerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, callerId)))

ownersFirst20 := msgraph.Owners{*callerObject}
var ownersExtra msgraph.Owners

Expand All @@ -927,21 +934,25 @@ func applicationResourceCreate(ctx context.Context, d *schema.ResourceData, meta
// Retrieve and set the initial owners, which can be up to 20 in total when creating the application
if v, ok := d.GetOk("owners"); ok {
ownerCount := 0
for _, id := range v.(*schema.Set).List() {
if strings.EqualFold(id.(string), callerId) {
for _, ownerId := range v.(*schema.Set).List() {
if strings.EqualFold(ownerId.(string), callerId) {
removeCallerOwner = false
continue
}
ownerObject, _, err := directoryObjectsClient.Get(ctx, id.(string), odata.Query{})
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId.(string), odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", id)
}
if ownerObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

if ownerCount < 19 {
ownersFirst20 = append(ownersFirst20, *ownerObject)
} else {
Expand Down Expand Up @@ -1077,14 +1088,21 @@ func applicationResourceUpdate(ctx context.Context, d *schema.ResourceData, meta

if len(ownersToAdd) > 0 {
newOwners := make(msgraph.Owners, 0)
for _, m := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, ownerId := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

newOwners = append(newOwners, *ownerObject)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/hashicorp/terraform-provider-azuread/internal/clients"
"github.com/hashicorp/terraform-provider-azuread/internal/services/directoryroles/parse"
"github.com/hashicorp/terraform-provider-azuread/internal/tf"
"github.com/hashicorp/terraform-provider-azuread/internal/utils"
"github.com/hashicorp/terraform-provider-azuread/internal/validate"
)

Expand Down Expand Up @@ -90,9 +91,13 @@ func directoryRoleMemberResourceCreate(ctx context.Context, d *schema.ResourceDa
if memberObject == nil {
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", id.MemberId)
}
if memberObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", id.MemberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if memberObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", id.MemberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, id.MemberId)))

role.Members = &msgraph.Members{*memberObject}

if _, err := client.AddMembers(ctx, role); err != nil {
Expand Down
12 changes: 9 additions & 3 deletions internal/services/groups/group_member_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package groups
import (
"context"
"errors"
"fmt"
"log"
"net/http"
"strings"
Expand All @@ -16,6 +17,7 @@ import (
"github.com/hashicorp/terraform-provider-azuread/internal/clients"
"github.com/hashicorp/terraform-provider-azuread/internal/services/groups/parse"
"github.com/hashicorp/terraform-provider-azuread/internal/tf"
"github.com/hashicorp/terraform-provider-azuread/internal/utils"
"github.com/hashicorp/terraform-provider-azuread/internal/validate"
)

Expand Down Expand Up @@ -95,9 +97,13 @@ func groupMemberResourceCreate(ctx context.Context, d *schema.ResourceData, meta
if memberObject == nil {
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", memberId)
}
if memberObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", memberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if memberObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", memberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, memberId)))

group.Members = &msgraph.Members{*memberObject}

if _, err := client.AddMembers(ctx, group); err != nil {
Expand Down
73 changes: 46 additions & 27 deletions internal/services/groups/group_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,13 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
if ownerObject.ID == nil {
return nil, errors.New("ownerObject ID was nil")
}
if ownerObject.ODataId == nil {
return nil, errors.New("ODataId was nil")
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return nil, errors.New("ODataId was nil")
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, id)))

if ownerObject.ODataType == nil {
return nil, errors.New("ownerObject ODataType was nil")
}
Expand All @@ -436,15 +440,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// First look for the calling principal in the specified owners; it should always be included in the initial
// owners to avoid orphaning a group when the caller doesn't have the Groups.ReadWrite.All scope.
for _, id := range owners {
ownerObject, err := getOwnerObject(ctx, id.(string))
for _, ownerId := range owners {
ownerObject, err := getOwnerObject(ctx, ownerId.(string))
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if strings.EqualFold(*ownerObject.ID, callerId) {
if ownerObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", id)
}
if ownerCount < 20 {
ownersFirst20 = append(ownersFirst20, *ownerObject)
} else {
Expand All @@ -456,10 +457,10 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Then look for users, and finally service principals
for _, t := range []odata.Type{odata.TypeUser, odata.TypeServicePrincipal} {
for _, id := range owners {
ownerObject, err := getOwnerObject(ctx, id.(string))
for _, ownerId := range owners {
ownerObject, err := getOwnerObject(ctx, ownerId.(string))
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if *ownerObject.ODataType == t && !strings.EqualFold(*ownerObject.ID, callerId) {
if ownerCount < 20 {
Expand Down Expand Up @@ -508,17 +509,21 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
// Add members after the group is created
members := make(msgraph.Members, 0)
if v, ok := d.GetOk("members"); ok {
for _, id := range v.(*schema.Set).List() {
memberObject, _, err := directoryObjectsClient.Get(ctx, id.(string), odata.Query{})
for _, memberId := range v.(*schema.Set).List() {
memberObject, _, err := directoryObjectsClient.Get(ctx, memberId.(string), odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve member principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve member principal object %q", memberId)
}
if memberObject == nil {
return tf.ErrorDiagF(errors.New("memberObject was nil"), "Could not retrieve member principal object %q", id)
}
if memberObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", id)
return tf.ErrorDiagF(errors.New("memberObject was nil"), "Could not retrieve member principal object %q", memberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if memberObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve member principal object %q", memberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, memberId)))

members = append(members, *memberObject)
}
}
Expand Down Expand Up @@ -603,14 +608,21 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

if len(membersToAdd) > 0 {
newMembers := make(msgraph.Members, 0)
for _, m := range membersToAdd {
memberObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, memberId := range membersToAdd {
memberObject, _, err := directoryObjectsClient.Get(ctx, memberId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve principal object %q", memberId)
}
if memberObject == nil {
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", m)
return tf.ErrorDiagF(errors.New("returned memberObject was nil"), "Could not retrieve member principal object %q", memberId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", memberId)
//}
memberObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, memberId)))

newMembers = append(newMembers, *memberObject)
}

Expand Down Expand Up @@ -641,14 +653,21 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

if len(ownersToAdd) > 0 {
newOwners := make(msgraph.Owners, 0)
for _, m := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, ownerId := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

newOwners = append(newOwners, *ownerObject)
}

Expand Down
41 changes: 29 additions & 12 deletions internal/services/serviceprincipals/service_principal_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,13 @@ func servicePrincipalResourceCreate(ctx context.Context, d *schema.ResourceData,
if callerObject == nil {
return tf.ErrorDiagF(errors.New("returned callerObject was nil"), "Could not retrieve calling principal object %q", callerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if callerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve calling principal object %q", callerId)
//}
callerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, callerId)))

ownersFirst20 := msgraph.Owners{*callerObject}
var ownersExtra msgraph.Owners

Expand All @@ -385,21 +392,25 @@ func servicePrincipalResourceCreate(ctx context.Context, d *schema.ResourceData,
// Retrieve and set the initial owners, which can be up to 20 in total when creating the service principal
if v, ok := d.GetOk("owners"); ok {
ownerCount := 0
for _, id := range v.(*schema.Set).List() {
if strings.EqualFold(id.(string), callerId) {
for _, ownerId := range v.(*schema.Set).List() {
if strings.EqualFold(ownerId.(string), callerId) {
removeCallerOwner = false
continue
}
ownerObject, _, err := directoryObjectsClient.Get(ctx, id.(string), odata.Query{})
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId.(string), odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", id)
}
if ownerObject.ODataId == nil {
return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", id)
return tf.ErrorDiagF(errors.New("ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))

if ownerCount < 19 {
ownersFirst20 = append(ownersFirst20, *ownerObject)
} else {
Expand Down Expand Up @@ -485,14 +496,20 @@ func servicePrincipalResourceUpdate(ctx context.Context, d *schema.ResourceData,

if len(ownersToAdd) > 0 {
newOwners := make(msgraph.Owners, 0)
for _, m := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, m, odata.Query{})
for _, ownerId := range ownersToAdd {
ownerObject, _, err := directoryObjectsClient.Get(ctx, ownerId, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(err, "Could not retrieve owner principal object %q", ownerId)
}
if ownerObject == nil {
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", m)
return tf.ErrorDiagF(errors.New("returned ownerObject was nil"), "Could not retrieve owner principal object %q", ownerId)
}
// TODO: remove this workaround for https://github.com/hashicorp/terraform-provider-azuread/issues/588
//if ownerObject.ODataId == nil {
// return tf.ErrorDiagF(errors.New("ODataId was nil"), "Could not retrieve owner principal object %q", ownerId)
//}
ownerObject.ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, ownerId)))
newOwners = append(newOwners, *ownerObject)
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit f049c74

Please sign in to comment.