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.

Sign up for GitHub

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

azuread_group: only send additional fields when absolutely necessary #1028

Merged
merged 2 commits into from
Feb 22, 2023
Merged

azuread_group: only send additional fields when absolutely necessary #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c72913d
azuread_group: only set additional fields on create, when they are fo…
manicminer Feb 21, 2023
9e899d3
azuread_group: only set additional fields on update, when the value d…
manicminer Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions internal/services/groups/group_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,12 +700,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
// See https://docs.microsoft.com/en-us/graph/known-issues#groups

// AllowExternalSenders can only be set in its own PATCH request; including other properties returns a 400
if allowExternalSenders := d.Get("external_senders_allowed").(bool); allowExternalSenders {
if allowExternalSenders, ok := d.GetOkExists("external_senders_allowed"); ok { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
},
AllowExternalSenders: utils.Bool(allowExternalSenders),
AllowExternalSenders: utils.Bool(allowExternalSenders.(bool)),
}); err != nil {
return tf.ErrorDiagF(err, "Failed to set `external_senders_allowed` for group with object ID %q", *group.ID())
}
Expand All @@ -724,12 +724,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
}

// AutoSubscribeNewMembers can only be set in its own PATCH request; including other properties returns a 400
if autoSubscribeNewMembers := d.Get("auto_subscribe_new_members").(bool); autoSubscribeNewMembers {
if autoSubscribeNewMembers, ok := d.GetOkExists("auto_subscribe_new_members"); ok { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
},
AutoSubscribeNewMembers: utils.Bool(autoSubscribeNewMembers),
AutoSubscribeNewMembers: utils.Bool(autoSubscribeNewMembers.(bool)),
}); err != nil {
return tf.ErrorDiagF(err, "Failed to set `auto_subscribe_new_members` for group with object ID %q", *group.ID())
}
Expand All @@ -748,12 +748,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
}

// HideFromAddressLists can only be set in its own PATCH request; including other properties returns a 400
if hideFromAddressList := d.Get("hide_from_address_lists").(bool); hideFromAddressList {
if hideFromAddressList, ok := d.GetOkExists("hide_from_address_lists"); ok { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
},
HideFromAddressLists: utils.Bool(hideFromAddressList),
HideFromAddressLists: utils.Bool(hideFromAddressList.(bool)),
}); err != nil {
return tf.ErrorDiagF(err, "Failed to set `hide_from_address_lists` for group with object ID %q", *group.ID())
}
Expand All @@ -772,12 +772,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
}

// HideFromOutlookClients can only be set in its own PATCH request; including other properties returns a 400
if hideFromOutlookClients := d.Get("hide_from_outlook_clients").(bool); hideFromOutlookClients {
if hideFromOutlookClients, ok := d.GetOkExists("hide_from_outlook_clients"); ok { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
},
HideFromOutlookClients: utils.Bool(hideFromOutlookClients),
HideFromOutlookClients: utils.Bool(hideFromOutlookClients.(bool)),
}); err != nil {
return tf.ErrorDiagF(err, "Failed to set `hide_from_outlook_clients` for group with object ID %q", *group.ID())
}
Expand Down Expand Up @@ -916,11 +916,15 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter
if hasGroupType(groupTypes, msgraph.GroupTypeUnified) {
// The unified group properties in this block only support delegated auth
// Application-authenticated requests will return a 4xx error, so we only
// set these when explicitly configured
// set these when explicitly configured, and when the value differs.
// See https://docs.microsoft.com/en-us/graph/known-issues#groups
extra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
return tf.ErrorDiagF(err, "Retrieving extra fields for group with ID: %q", *group.ID())
}

// AllowExternalSenders can only be set in its own PATCH request; including other properties returns a 400
if v, ok := d.GetOkExists("external_senders_allowed"); ok { //nolint:staticcheck
if v, ok := d.GetOkExists("external_senders_allowed"); ok && (extra.AllowExternalSenders == nil || *extra.AllowExternalSenders != v.(bool)) { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
Expand All @@ -944,7 +948,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter
}

// AutoSubscribeNewMembers can only be set in its own PATCH request; including other properties returns a 400
if v, ok := d.GetOkExists("auto_subscribe_new_members"); ok { //nolint:staticcheck
if v, ok := d.GetOkExists("auto_subscribe_new_members"); ok && (extra.AutoSubscribeNewMembers == nil || *extra.AutoSubscribeNewMembers != v.(bool)) { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
Expand All @@ -968,7 +972,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter
}

// HideFromAddressLists can only be set in its own PATCH request; including other properties returns a 400
if v, ok := d.GetOkExists("hide_from_address_lists"); ok { //nolint:staticcheck
if v, ok := d.GetOkExists("hide_from_address_lists"); ok && (extra.HideFromAddressLists == nil || *extra.HideFromAddressLists != v.(bool)) { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
Expand All @@ -992,7 +996,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter
}

// HideFromOutlookClients can only be set in its own PATCH request; including other properties returns a 400
if v, ok := d.GetOkExists("hide_from_outlook_clients"); ok { //nolint:staticcheck
if v, ok := d.GetOkExists("hide_from_outlook_clients"); ok && (extra.HideFromOutlookClients == nil || *extra.HideFromOutlookClients != v.(bool)) { //nolint:staticcheck
if _, err := client.Update(ctx, msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
Id: group.ID(),
Expand Down
18 changes: 9 additions & 9 deletions internal/services/groups/group_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,34 +961,34 @@ resource "azuread_group" "test" {
func (r GroupResource) administrativeUnits(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_administrative_unit" "test" {
display_name = "acctestGroup-administrative-unit-%[1]d"
display_name = "acctestGroup-administrative-unit-%[1]d"
}

resource "azuread_administrative_unit" "test2" {
display_name = "acctestGroup-administrative-unit-%[1]d"
display_name = "acctestGroup-administrative-unit-%[1]d"
}

resource "azuread_group" "test" {
display_name = "acctestGroup-%[1]d"
security_enabled = true
administrative_unit_ids = [azuread_administrative_unit.test.id, azuread_administrative_unit.test2.id]
display_name = "acctestGroup-%[1]d"
security_enabled = true
administrative_unit_ids = [azuread_administrative_unit.test.id, azuread_administrative_unit.test2.id]
}
`, data.RandomInteger, data.RandomInteger)
}

func (r GroupResource) administrativeUnitsWithoutAssociation(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_administrative_unit" "test" {
display_name = "acctestGroup-administrative-unit-%[1]d"
display_name = "acctestGroup-administrative-unit-%[1]d"
}

resource "azuread_administrative_unit" "test2" {
display_name = "acctestGroup-administrative-unit-%[1]d"
display_name = "acctestGroup-administrative-unit-%[1]d"
}

resource "azuread_group" "test" {
display_name = "acctestGroup-%[1]d"
security_enabled = true
display_name = "acctestGroup-%[1]d"
security_enabled = true
}
`, data.RandomInteger, data.RandomInteger)
}