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

Feature/groups_datasource show_all_flag #520

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 10 additions & 1 deletion docs/data-sources/groups.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,29 @@ When authenticated with a user principal, this data source does not require any

## Example Usage

*Look up by group name*
```terraform
data "azuread_groups" "groups" {
display_names = ["group-a", "group-b"]
}
```

*Look up all groups*
```terraform
data "azuread_groups" "allGroups" {
return_all = true
}
```

## Argument Reference

The following arguments are supported:

* `display_names` - (Optional) The display names of the groups.
* `object_ids` - (Optional) The object IDs of the groups.
* `return_all` - (Optional) A flag to denote if all groups should be fetched and returned.

~> One of `display_names` or `object_ids` should be specified. Either of these _may_ be specified as an empty list, in which case no results will be returned.
~> One of `display_names`, `object_ids` or `return_all` should be specified. Either of the first two _may_ be specified as an empty list, in which case no results will be returned.

## Attributes Reference

Expand Down
29 changes: 25 additions & 4 deletions internal/services/groups/groups_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func groupsDataSource() *schema.Resource {
Type: schema.TypeList,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"display_names", "object_ids"},
ExactlyOneOf: []string{"display_names", "object_ids", "return_all"},
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateDiagFunc: validate.UUID,
Expand All @@ -46,12 +46,19 @@ func groupsDataSource() *schema.Resource {
Type: schema.TypeList,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"display_names", "object_ids"},
ExactlyOneOf: []string{"display_names", "object_ids", "return_all"},
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateDiagFunc: validate.NoEmptyStrings,
},
},

"return_all": {
Description: "Retrieve all groups with no filter",
Type: schema.TypeBool,
Optional: true,
ExactlyOneOf: []string{"display_names", "object_ids", "return_all"},
},
},
}
}
Expand All @@ -62,13 +69,27 @@ func groupsDataSourceRead(ctx context.Context, d *schema.ResourceData, meta inte

var groups []msgraph.Group
var expectedCount int
var returnAll = d.Get("return_all").(bool)

var displayNames []interface{}
if v, ok := d.GetOk("display_names"); ok {
displayNames = v.([]interface{})
}

if len(displayNames) > 0 {
if returnAll {
result, _, err := client.List(ctx, odata.Query{})
if err != nil {
return tf.ErrorDiagF(err, "Could not retrieve groups")
}
Comment on lines +81 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

We try to check errors separately so it's more obvious when they occur

if result == nil {
return tf.ErrorDiagF(errors.New("API returned nil result"), "Bad API Response")
}
Comment on lines +84 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check for nil pointers before dereferencing as doing *result when result is nil would cause a crash

if len(*result) == 0 {
return tf.ErrorDiagPathF(err, "return_all", "No groups found")
}

groups = append(groups, *result...)
Copy link
Contributor

Choose a reason for hiding this comment

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

This notation is a useful shortcut to iterating with a for loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know this one - Thank you for that - I will read through all these comments :)

} else if len(displayNames) > 0 {
expectedCount = len(displayNames)
for _, v := range displayNames {
displayName := v.(string)
Expand Down Expand Up @@ -105,7 +126,7 @@ func groupsDataSourceRead(ctx context.Context, d *schema.ResourceData, meta inte
}
}

if len(groups) != expectedCount {
if !returnAll && len(groups) != expectedCount {
return tf.ErrorDiagF(fmt.Errorf("Expected: %d, Actual: %d", expectedCount, len(groups)), "Unexpected number of groups returned")
}

Expand Down
22 changes: 22 additions & 0 deletions internal/services/groups/groups_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,20 @@ func TestAccGroupsDataSource_noNames(t *testing.T) {
})
}

func TestAccGroupsDataSource_returnAll(t *testing.T) {
data := acceptance.BuildTestData(t, "data.azuread_groups", "test")

data.DataSourceTest(t, []resource.TestStep{
{
Config: GroupsDataSource{}.returnAll(),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).Key("display_names.#").Exists(),
check.That(data.ResourceName).Key("object_ids.#").Exists(),
),
},
})
}

func (GroupsDataSource) template(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_group" "testA" {
Expand Down Expand Up @@ -99,3 +113,11 @@ data "azuread_groups" "test" {
}
`
}

func (GroupsDataSource) returnAll() string {
return `
data "azuread_groups" "test" {
return_all = true
}
`
}