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

azuread_application - allow approle value property to be nil #157

Merged
merged 6 commits into from
Oct 11, 2019
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
2 changes: 1 addition & 1 deletion azuread/helpers/graph/replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar"
)

func WaitForReplication(f func() (interface{}, error)) (interface{}, error) {
func WaitForCreationReplication(f func() (interface{}, error)) (interface{}, error) {
return (&resource.StateChangeConf{
Pending: []string{"404", "BadCast"},
Target: []string{"Found"},
Expand Down
31 changes: 23 additions & 8 deletions azuread/resource_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func resourceApplication() *schema.Resource {
"name": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.NoZeroValues,
ValidateFunc: validate.NoEmptyStrings,
},

"available_to_other_tenants": {
Expand Down Expand Up @@ -102,6 +102,7 @@ func resourceApplication() *schema.Resource {
Schema: map[string]*schema.Schema{
"id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
},

Expand Down Expand Up @@ -137,9 +138,8 @@ func resourceApplication() *schema.Resource {
},

"value": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validate.NoEmptyStrings,
Type: schema.TypeString,
Optional: true,
},
},
},
Expand Down Expand Up @@ -215,7 +215,6 @@ func resourceApplicationCreate(d *schema.ResourceData, meta interface{}) error {
ReplyUrls: tf.ExpandStringSlicePtr(d.Get("reply_urls").(*schema.Set).List()),
AvailableToOtherTenants: p.Bool(d.Get("available_to_other_tenants").(bool)),
RequiredResourceAccess: expandADApplicationRequiredResourceAccess(d),
AppRoles: expandADApplicationAppRoles(d.Get("app_role")),
}

if v, ok := d.GetOk("homepage"); ok {
Expand Down Expand Up @@ -248,7 +247,7 @@ func resourceApplicationCreate(d *schema.ResourceData, meta interface{}) error {
}
d.SetId(*app.ObjectID)

_, err = graph.WaitForReplication(func() (interface{}, error) {
_, err = graph.WaitForCreationReplication(func() (interface{}, error) {
return client.Get(ctx, *app.ObjectID)
})
if err != nil {
Expand All @@ -269,6 +268,18 @@ func resourceApplicationCreate(d *schema.ResourceData, meta interface{}) error {
}
}

// to use an empty value we need to patch the resource
appRoles := expandADApplicationAppRoles(d.Get("app_role"))
if appRoles != nil {
properties2 := graphrbac.ApplicationUpdateParameters{
AppRoles: appRoles,
}

if _, err := client.Patch(ctx, *app.ObjectID, properties2); err != nil {
return err
}
}

return resourceApplicationRead(d, meta)
}

Expand Down Expand Up @@ -547,7 +558,11 @@ func expandADApplicationAppRoles(i interface{}) *[]graphrbac.AppRole {
appRoleDescription := appRole["description"].(string)
appRoleDisplayName := appRole["display_name"].(string)
appRoleIsEnabled := appRole["is_enabled"].(bool)
appRoleValue := appRole["value"].(string)

var appRoleValue *string
if v, ok := appRole["value"].(string); ok {
appRoleValue = &v
}

output = append(output,
graphrbac.AppRole{
Expand All @@ -556,7 +571,7 @@ func expandADApplicationAppRoles(i interface{}) *[]graphrbac.AppRole {
Description: &appRoleDescription,
DisplayName: &appRoleDisplayName,
IsEnabled: &appRoleIsEnabled,
Value: &appRoleValue,
Value: appRoleValue,
},
)
}
Expand Down
71 changes: 51 additions & 20 deletions azuread/resource_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,31 @@ func TestAccAzureADApplication_appRoles(t *testing.T) {
})
}

func TestAccAzureADApplication_appRolesBlankValue(t *testing.T) {
resourceName := "azuread_application.test"
id := uuid.New().String()

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckADApplicationDestroy,
Steps: []resource.TestStep{
{
Config: testAccADApplication_appRolesNoValue(id),
Check: resource.ComposeTestCheckFunc(
testCheckADApplicationExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "app_role.#", "1"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAzureADApplication_appRolesUpdate(t *testing.T) {
resourceName := "azuread_application.test"
id := uuid.New().String()
Expand All @@ -243,34 +268,25 @@ func TestAccAzureADApplication_appRolesUpdate(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testCheckADApplicationExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "app_role.#", "1"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.allowed_member_types.#", "2"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.allowed_member_types.2550101162", "Application"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.allowed_member_types.2906997583", "User"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.description", "Admins can manage roles and perform all task actions"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.display_name", "Admin"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.is_enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "app_role.3282540397.value", "Admin"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccADApplication_appRolesUpdate(id),
Check: resource.ComposeTestCheckFunc(
testCheckADApplicationExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "app_role.#", "2"),
resource.TestCheckResourceAttr(resourceName, "app_role.1786747921.allowed_member_types.#", "1"),
resource.TestCheckResourceAttr(resourceName, "app_role.1786747921.allowed_member_types.2906997583", "User"),
resource.TestCheckResourceAttr(resourceName, "app_role.1786747921.description", "ReadOnly roles have limited query access"),
resource.TestCheckResourceAttr(resourceName, "app_role.1786747921.display_name", "ReadOnly"),
resource.TestCheckResourceAttr(resourceName, "app_role.1786747921.is_enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "app_role.1786747921.value", "User"),
resource.TestCheckResourceAttr(resourceName, "app_role.2608972077.allowed_member_types.#", "1"),
resource.TestCheckResourceAttr(resourceName, "app_role.2608972077.allowed_member_types.2906997583", "User"),
resource.TestCheckResourceAttr(resourceName, "app_role.2608972077.description", "Admins can manage roles and perform all task actions"),
resource.TestCheckResourceAttr(resourceName, "app_role.2608972077.display_name", "Admin"),
resource.TestCheckResourceAttr(resourceName, "app_role.2608972077.is_enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "app_role.2608972077.value", "Admin"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand Down Expand Up @@ -639,6 +655,21 @@ resource "azuread_application" "test" {
`, id)
}

func testAccADApplication_appRolesNoValue(id string) string {
return fmt.Sprintf(`
resource "azuread_application" "test" {
name = "acctestApp-%s"

app_role {
allowed_member_types = ["User"]
description = "Admins can manage roles and perform all task actions"
display_name = "Admin"
is_enabled = true
}
}
`, id)
}

func testAccADApplication_appRolesUpdate(id string) string {
return fmt.Sprintf(`
resource "azuread_application" "test" {
Expand All @@ -649,7 +680,7 @@ resource "azuread_application" "test" {
description = "Admins can manage roles and perform all task actions"
display_name = "Admin"
is_enabled = true
value = "Admin"
value = ""
}

app_role {
Expand Down
2 changes: 1 addition & 1 deletion azuread/resource_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func resourceGroupCreate(d *schema.ResourceData, meta interface{}) error {
}
}

_, err = graph.WaitForReplication(func() (interface{}, error) {
_, err = graph.WaitForCreationReplication(func() (interface{}, error) {
return client.Get(ctx, *group.ObjectID)
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion azuread/resource_service_principal.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func resourceServicePrincipalCreate(d *schema.ResourceData, meta interface{}) er
}
d.SetId(*sp.ObjectID)

_, err = graph.WaitForReplication(func() (interface{}, error) {
_, err = graph.WaitForCreationReplication(func() (interface{}, error) {
return client.Get(ctx, *sp.ObjectID)
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion azuread/resource_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func resourceUserCreate(d *schema.ResourceData, meta interface{}) error {
}
d.SetId(*user.ObjectID)

_, err = graph.WaitForReplication(func() (interface{}, error) {
_, err = graph.WaitForCreationReplication(func() (interface{}, error) {
return client.Get(ctx, *user.ObjectID)
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/application.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ The following arguments are supported:

* `is_enabled` - (Optional) Determines if the app role is enabled: Defaults to `true`.

* `value` - (Required) Specifies the value of the roles claim that the application should expect in the authentication and access tokens.
* `value` - (Optional) Specifies the value of the roles claim that the application should expect in the authentication and access tokens.

## Attributes Reference

Expand Down