From 13f4e67766197847797f0c3c2f1497ff4180e94a Mon Sep 17 00:00:00 2001 From: Xavier Gallardo Date: Wed, 4 Jul 2018 01:01:52 -0400 Subject: [PATCH] Address code review comments. --- azurerm/key_credentials.go | 123 +++++++++--------- azurerm/resource_arm_azuread_application.go | 25 ++-- .../resource_arm_azuread_application_test.go | 64 ++++----- .../docs/r/azuread_application.html.markdown | 14 +- 4 files changed, 107 insertions(+), 119 deletions(-) diff --git a/azurerm/key_credentials.go b/azurerm/key_credentials.go index 2826cffd1581c..aa52cd521cb15 100644 --- a/azurerm/key_credentials.go +++ b/azurerm/key_credentials.go @@ -46,7 +46,7 @@ func keyCredentialsSchema() *schema.Schema { ValidateFunc: validation.StringInSlice([]string{ "AsymmetricX509Cert", "Symmetric", - }, true), + }, false), }, "usage": { @@ -55,7 +55,7 @@ func keyCredentialsSchema() *schema.Schema { ValidateFunc: validation.StringInSlice([]string{ "Verify", "Sign", - }, true), + }, false), }, "value": { @@ -123,68 +123,38 @@ func resourceKeyCredentialHash(v interface{}) int { return hashcode.String(buf.String()) } -func flattenAzureRmKeyCredential(cred *graphrbac.KeyCredential) interface{} { - l := make(map[string]interface{}) - l["key_id"] = *cred.KeyID - l["type"] = *cred.Type - l["usage"] = *cred.Usage - l["start_date"] = string((*cred.StartDate).Format(time.RFC3339)) - l["end_date"] = string((*cred.EndDate).Format(time.RFC3339)) - - return l -} - func flattenAzureRmKeyCredentials(creds *[]graphrbac.KeyCredential) []interface{} { - result := make([]interface{}, 0, len(*creds)) - for _, cred := range *creds { - result = append(result, flattenAzureRmKeyCredential(&cred)) - } - return result -} + result := make([]interface{}, 0) -func expandAzureRmKeyCredential(d *map[string]interface{}) (*graphrbac.KeyCredential, error) { - keyId := (*d)["key_id"].(string) - startDate := (*d)["start_date"].(string) - endDate := (*d)["end_date"].(string) - keyType := (*d)["type"].(string) - usage := (*d)["usage"].(string) - value := (*d)["value"].(string) + if creds != nil { + for _, cred := range *creds { + l := make(map[string]interface{}) - if keyType == "AsymmetricX509Cert" && usage == "Sign" { - return nil, fmt.Errorf("Usage cannot be set to %s when %s is set as type for a Key Credential", usage, keyType) - } + if cred.KeyID != nil { + l["key_id"] = *cred.KeyID + } - // Match against the prefix/suffix and '\n' of certificate values. - // The API doesn't accept them so they need to be removed. - pkre := regexp.MustCompile(`(-{5}.+?-{5})|(\n)`) - value = pkre.ReplaceAllString(value, ``) + if cred.Type != nil { + l["type"] = *cred.Type + } - kc := graphrbac.KeyCredential{ - KeyID: &keyId, - Type: &keyType, - Usage: &usage, - Value: &value, - } + if cred.Usage != nil { + l["usage"] = *cred.Usage + } - if startDate != "" { - starttime, sterr := time.Parse(time.RFC3339, startDate) - if sterr != nil { - return nil, fmt.Errorf("Cannot parse start_date: %q", startDate) - } - stdt := date.Time{Time: starttime.Truncate(time.Second)} - kc.StartDate = &stdt - } + if cred.StartDate != nil { + l["start_date"] = string((*cred.StartDate).Format(time.RFC3339)) + } + + if cred.EndDate != nil { + l["end_date"] = string((*cred.EndDate).Format(time.RFC3339)) + } - if endDate != "" { - endtime, eterr := time.Parse(time.RFC3339, endDate) - if eterr != nil { - return nil, fmt.Errorf("Cannot parse end_date: %q", endDate) + result = append(result, l) } - etdt := date.Time{Time: endtime.Truncate(time.Second)} - kc.EndDate = &etdt } - return &kc, nil + return result } func expandAzureRmKeyCredentials(d *schema.ResourceData, o *schema.Set) (*[]graphrbac.KeyCredential, error) { @@ -192,10 +162,45 @@ func expandAzureRmKeyCredentials(d *schema.ResourceData, o *schema.Set) (*[]grap keyCreds := make([]graphrbac.KeyCredential, 0, len(creds)) for _, v := range creds { - cfg := v.(map[string]interface{}) - cred, err := expandAzureRmKeyCredential(&cfg) - if err != nil { - return nil, err + c := v.(map[string]interface{}) + + keyId := c["key_id"].(string) + startDate := c["start_date"].(string) + endDate := c["end_date"].(string) + keyType := c["type"].(string) + usage := c["usage"].(string) + value := c["value"].(string) + + if keyType == "AsymmetricX509Cert" && usage == "Sign" { + return nil, fmt.Errorf("Usage cannot be set to %s when %s is set as type for a Key Credential", usage, keyType) + } + + // Match against the prefix/suffix and '\n' of certificate values. + // The API doesn't accept them so they need to be removed. + pkre := regexp.MustCompile(`(-{5}.+?-{5})|(\n)`) + value = pkre.ReplaceAllString(value, ``) + + cred := graphrbac.KeyCredential{ + KeyID: &keyId, + Type: &keyType, + Usage: &usage, + Value: &value, + } + + if startDate != "" { + starttime, sterr := time.Parse(time.RFC3339, startDate) + if sterr != nil { + return nil, fmt.Errorf("Cannot parse start_date: %q", startDate) + } + cred.StartDate = &date.Time{Time: starttime.Truncate(time.Second)} + } + + if endDate != "" { + endtime, eterr := time.Parse(time.RFC3339, endDate) + if eterr != nil { + return nil, fmt.Errorf("Cannot parse end_date: %q", endDate) + } + cred.EndDate = &date.Time{Time: endtime.Truncate(time.Second)} } // Azure only allows an in-place update of the Key Credentials list. @@ -207,7 +212,7 @@ func expandAzureRmKeyCredentials(d *schema.ResourceData, o *schema.Set) (*[]grap cred.Value = nil } - keyCreds = append(keyCreds, *cred) + keyCreds = append(keyCreds, cred) } return &keyCreds, nil diff --git a/azurerm/resource_arm_azuread_application.go b/azurerm/resource_arm_azuread_application.go index b5616bd60b58b..ae02670ec4ebe 100644 --- a/azurerm/resource_arm_azuread_application.go +++ b/azurerm/resource_arm_azuread_application.go @@ -21,7 +21,7 @@ func resourceArmActiveDirectoryApplication() *schema.Resource { }, Schema: map[string]*schema.Schema{ - "display_name": { + "name": { Type: schema.TypeString, Required: true, }, @@ -68,12 +68,7 @@ func resourceArmActiveDirectoryApplication() *schema.Resource { "password_credential": passwordCredentialsSchema(), - "app_id": { - Type: schema.TypeString, - Computed: true, - }, - - "object_id": { + "application_id": { Type: schema.TypeString, Computed: true, }, @@ -98,7 +93,7 @@ func resourceArmActiveDirectoryApplicationCreate(d *schema.ResourceData, meta in client := meta.(*ArmClient).applicationsClient ctx := meta.(*ArmClient).StopContext - name := d.Get("display_name").(string) + name := d.Get("name").(string) multitenant := d.Get("available_to_other_tenants").(bool) properties := graphrbac.ApplicationCreateParameters{ @@ -154,9 +149,8 @@ func resourceArmActiveDirectoryApplicationRead(d *schema.ResourceData, meta inte return fmt.Errorf("Error loading Azure AD Application %q: %+v", d.Id(), err) } - d.Set("display_name", resp.DisplayName) - d.Set("app_id", resp.AppID) - d.Set("object_id", resp.ObjectID) + d.Set("name", resp.DisplayName) + d.Set("application_id", resp.AppID) d.Set("homepage", resp.Homepage) d.Set("identifier_uris", resp.IdentifierUris) d.Set("reply_urls", resp.ReplyUrls) @@ -188,11 +182,11 @@ func resourceArmActiveDirectoryApplicationUpdate(d *schema.ResourceData, meta in client := meta.(*ArmClient).applicationsClient ctx := meta.(*ArmClient).StopContext - name := d.Get("display_name").(string) + name := d.Get("name").(string) var properties graphrbac.ApplicationUpdateParameters - if d.HasChange("display_name") { + if d.HasChange("name") { properties.DisplayName = &name } @@ -225,14 +219,13 @@ func resourceArmActiveDirectoryApplicationUpdate(d *schema.ResourceData, meta in return err } - d.SetPartial("display_name") + d.SetPartial("name") d.SetPartial("homepage") d.SetPartial("identifier_uris") d.SetPartial("reply_urls") d.SetPartial("available_to_other_tenants") d.SetPartial("oauth2_allow_implicit_flow") - d.SetPartial("app_id") - d.SetPartial("object_id") + d.SetPartial("application_id") if d.HasChange("key_credential") { o, _ := d.GetChange("key_credential") diff --git a/azurerm/resource_arm_azuread_application_test.go b/azurerm/resource_arm_azuread_application_test.go index ced2af7710034..1daba4192d64a 100644 --- a/azurerm/resource_arm_azuread_application_test.go +++ b/azurerm/resource_arm_azuread_application_test.go @@ -26,10 +26,9 @@ func TestAccAzureRMActiveDirectoryApplication_simple(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://%s", id)), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, }, @@ -50,10 +49,9 @@ func TestAccAzureRMActiveDirectoryApplication_advanced(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://homepage-%s", id)), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, }, @@ -77,7 +75,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateAdvanced(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://%s", id)), ), }, @@ -85,7 +83,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateAdvanced(t *testing.T) { Config: updatedConfig, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", updatedId), + resource.TestCheckResourceAttr(resourceName, "name", updatedId), resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://homepage-%s", updatedId)), ), }, @@ -108,11 +106,10 @@ func TestAccAzureRMActiveDirectoryApplication_keyCredential(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://%s", id)), resource.TestCheckResourceAttr(resourceName, "key_credential.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, }, @@ -135,8 +132,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateKeyCredential_changeAttribut Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "key_credential.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, { @@ -148,8 +144,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateKeyCredential_changeAttribut Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "key_credential.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, }, @@ -173,7 +168,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateKeyCredential_changeCount(t Config: configSingle, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "key_credential.#", "1"), ), }, @@ -181,7 +176,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateKeyCredential_changeCount(t Config: configDouble, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "key_credential.#", "2"), ), }, @@ -189,7 +184,7 @@ func TestAccAzureRMActiveDirectoryApplication_updateKeyCredential_changeCount(t Config: configSingle, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "key_credential.#", "1"), ), }, @@ -214,11 +209,10 @@ func TestAccAzureRMActiveDirectoryApplication_passwordCredential(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "display_name", id), + resource.TestCheckResourceAttr(resourceName, "name", id), resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://%s", id)), resource.TestCheckResourceAttr(resourceName, "password_credential.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, }, @@ -244,8 +238,7 @@ func TestAccAzureRMActiveDirectoryApplication_updatePasswordCredential_changeAtt Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "password_credential.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, { @@ -257,8 +250,7 @@ func TestAccAzureRMActiveDirectoryApplication_updatePasswordCredential_changeAtt Check: resource.ComposeTestCheckFunc( testCheckAzureRMActiveDirectoryApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "password_credential.#", "1"), - resource.TestCheckResourceAttrSet(resourceName, "app_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), ), }, }, @@ -312,15 +304,15 @@ func testCheckAzureRMActiveDirectoryApplicationExists(name string) resource.Test return fmt.Errorf("Not found: %q", name) } - objectId := rs.Primary.Attributes["object_id"] + id := rs.Primary.Attributes["id"] client := testAccProvider.Meta().(*ArmClient).applicationsClient ctx := testAccProvider.Meta().(*ArmClient).StopContext - resp, err := client.Get(ctx, objectId) + resp, err := client.Get(ctx, id) if err != nil { if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Bad: Azure AD Application %q does not exist", objectId) + return fmt.Errorf("Bad: Azure AD Application %q does not exist", id) } return fmt.Errorf("Bad: Get on Azure AD applicationsClient: %+v", err) } @@ -335,11 +327,11 @@ func testCheckAzureRMActiveDirectoryApplicationDestroy(s *terraform.State) error continue } - objectId := rs.Primary.Attributes["object_id"] + id := rs.Primary.Attributes["id"] client := testAccProvider.Meta().(*ArmClient).applicationsClient ctx := testAccProvider.Meta().(*ArmClient).StopContext - resp, err := client.Get(ctx, objectId) + resp, err := client.Get(ctx, id) if err != nil { if utils.ResponseWasNotFound(resp.Response) { @@ -358,7 +350,7 @@ func testCheckAzureRMActiveDirectoryApplicationDestroy(s *terraform.State) error func testAccAzureRMActiveDirectoryApplication_simple(id string) string { return fmt.Sprintf(` resource "azurerm_azuread_application" "test" { - display_name = "%s" + name = "%s" } `, id) } @@ -366,7 +358,7 @@ resource "azurerm_azuread_application" "test" { func testAccAzureRMActiveDirectoryApplication_advanced(id string) string { return fmt.Sprintf(` resource "azurerm_azuread_application" "test" { - display_name = "%s" + name = "%s" homepage = "http://homepage-%s" identifier_uris = ["http://uri-%s"] reply_urls = ["http://replyurl-%s"] @@ -403,7 +395,7 @@ resource "tls_self_signed_cert" "test" { } resource "azurerm_azuread_application" "test" { - display_name = "%s" + name = "%s" key_credential { key_id = "%s" @@ -468,7 +460,7 @@ resource "tls_self_signed_cert" "test2" { } resource "azurerm_azuread_application" "test" { - display_name = "%s" + name = "%s" key_credential { key_id = "%s" @@ -496,7 +488,7 @@ func testAccAzureRMActiveDirectoryApplication_passwordCredential_single(id strin te := string(timeEnd.Format(time.RFC3339)) return fmt.Sprintf(` resource "azurerm_azuread_application" "test" { - display_name = "%s" + name = "%s" password_credential { key_id = "%s" @@ -513,7 +505,7 @@ func testAccAzureRMActiveDirectoryApplication_passwordCredential_double(id strin te := string(timeEnd.Format(time.RFC3339)) return fmt.Sprintf(` resource "azurerm_azuread_application" "test" { - display_name = "%s" + name = "%s" password_credential { key_id = "%s" diff --git a/website/docs/r/azuread_application.html.markdown b/website/docs/r/azuread_application.html.markdown index b5208e89f87b7..6d92cca6e202a 100644 --- a/website/docs/r/azuread_application.html.markdown +++ b/website/docs/r/azuread_application.html.markdown @@ -18,7 +18,7 @@ Create a new application in Azure Active Directory. If your account is not an ad ```hcl resource "azurerm_azuread_application" "example" { - display_name = "example" + name = "example" } ``` @@ -26,7 +26,7 @@ resource "azurerm_azuread_application" "example" { ```hcl resource "azurerm_azuread_application" "example" { - display_name = "example" + name = "example" homepage = "http://homepage" identifier_uris = ["http://uri"] reply_urls = ["http://replyurl"] @@ -63,7 +63,7 @@ resource "tls_self_signed_cert" "example" { } resource "azurerm_azuread_application" "example" { - display_name = "example" + name = "example" key_credential { key_id = "00000000-0000-0000-0000-000000000000" @@ -78,7 +78,7 @@ resource "azurerm_azuread_application" "example" { ```hcl resource "azurerm_azuread_application" "test" { - display_name = "example" + name = "example" password_credential { key_id = "00000000-0000-0000-0000-000000000000" @@ -93,7 +93,7 @@ resource "azurerm_azuread_application" "test" { The following arguments are supported: -* `display_name` - (Required) The display name for the application. +* `name` - (Required) The display name for the application. * `homepage` - (optional) The URL to the application's home page. @@ -137,9 +137,7 @@ The following arguments are supported: The following attributes are exported: -* `app_id` - The Application ID. - -* `object_id` - The Application Object ID. +* `application_id` - The Application ID. ## Import