From 19cd7ae881a42d97e580885363e52960ab31959b Mon Sep 17 00:00:00 2001 From: kt Date: Mon, 10 Jun 2019 11:47:33 -0700 Subject: [PATCH 1/6] azuread_application_password: deprecate application_id in favour of application_object_id --- azuread/resource_application.go | 1 - azuread/resource_application_password.go | 34 ++++++++- azuread/resource_application_password_test.go | 70 ++++++++++++++----- ...esource_service_principal_password_test.go | 6 +- azuread/resource_service_principal_test.go | 4 +- azuread/resource_user_test.go | 1 - .../docs/r/application_password.html.markdown | 2 +- 7 files changed, 90 insertions(+), 28 deletions(-) diff --git a/azuread/resource_application.go b/azuread/resource_application.go index d0fd8c3d2a..af81f2a6ff 100644 --- a/azuread/resource_application.go +++ b/azuread/resource_application.go @@ -5,7 +5,6 @@ import ( "log" "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" - "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" diff --git a/azuread/resource_application_password.go b/azuread/resource_application_password.go index 3bf0d9338f..f0c97a598d 100644 --- a/azuread/resource_application_password.go +++ b/azuread/resource_application_password.go @@ -11,9 +11,29 @@ import ( "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/tf" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate" ) func resourceApplicationPassword() *schema.Resource { + + // temporary terrible hack/code to allow deprecation of `application_id` + // todo remove in 1.0 + s := graph.PasswordResourceSchema("application_object") + s["application_id"] = &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + ValidateFunc: validate.UUID, + Deprecated: "Deprecated in favour of `application_object_id` to prevent confusion", + ConflictsWith: []string{"application_id"}, + } + // this is bad, i am aware of it, and I feel awful about it + s["application_object_id"].Required = false + s["application_object_id"].Optional = true + s["application_object_id"].Computed = true + s["application_object_id"].ConflictsWith = []string{"application_object_id"} + return &schema.Resource{ Create: resourceApplicationPasswordCreate, Read: resourceApplicationPasswordRead, @@ -23,7 +43,8 @@ func resourceApplicationPassword() *schema.Resource { State: schema.ImportStatePassthrough, }, - Schema: graph.PasswordResourceSchema("application"), + //Schema: graph.PasswordResourceSchema("application_object"), + Schema: s, } } @@ -31,7 +52,13 @@ func resourceApplicationPasswordCreate(d *schema.ResourceData, meta interface{}) client := meta.(*ArmClient).applicationsClient ctx := meta.(*ArmClient).StopContext - objectId := d.Get("application_id").(string) + objectId := d.Get("application_object_id").(string) + if objectId == "" { // todo remove in 1.0 + objectId = d.Get("application_id").(string) + } + if objectId == "" { + return fmt.Errorf("one of `application_object_id` or `application_id` must be specifed") + } cred, err := graph.PasswordCredentialForResource(d) if err != nil { @@ -95,7 +122,8 @@ func resourceApplicationPasswordRead(d *schema.ResourceData, meta interface{}) e } // todo, move this into a graph helper function? - d.Set("application_id", id.ObjectId) + d.Set("application_object_id", id.ObjectId) + d.Set("application_id", id.ObjectId) //todo remove in 2.0 d.Set("key_id", id.KeyId) if endDate := credential.EndDate; endDate != nil { diff --git a/azuread/resource_application_password_test.go b/azuread/resource_application_password_test.go index e034681532..feb6391b87 100644 --- a/azuread/resource_application_password_test.go +++ b/azuread/resource_application_password_test.go @@ -4,12 +4,12 @@ import ( "fmt" "testing" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" - "github.com/google/uuid" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" ) func testCheckADApplicationPasswordExists(name string) resource.TestCheckFunc { //nolint unparam @@ -101,6 +101,30 @@ func TestAccAzureADApplicationPassword_basic(t *testing.T) { }) } +func TestAccAzureADApplicationPassword_basicOld(t *testing.T) { + resourceName := "azuread_application_password.test" + applicationId := uuid.New().String() + value := uuid.New().String() + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckADApplicationPasswordCheckDestroy, + Steps: []resource.TestStep{ + { + Config: testAccADObjectPasswordApplication_basicOld(applicationId, value), + Check: resource.ComposeTestCheckFunc( + // can't assert on Value since it's not returned + testCheckADApplicationPasswordExists(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "start_date"), + resource.TestCheckResourceAttrSet(resourceName, "key_id"), + resource.TestCheckResourceAttr(resourceName, "end_date", "2020-01-01T01:02:03Z"), + ), + }, + }, + }) +} + func TestAccAzureADApplicationPassword_requiresImport(t *testing.T) { if !requireResourcesToBeImported { t.Skip("Skipping since resources aren't required to be imported") @@ -191,9 +215,21 @@ func testAccADObjectPasswordApplication_basic(applicationId, value string) strin %s resource "azuread_application_password" "test" { - application_id = "${azuread_application.test.id}" - value = "%s" - end_date = "2020-01-01T01:02:03Z" + application_object_id = "${azuread_application.test.id}" + value = "%s" + end_date = "2020-01-01T01:02:03Z" +} +`, testAccADApplicationPassword_template(applicationId), value) +} + +func testAccADObjectPasswordApplication_basicOld(applicationId, value string) string { + return fmt.Sprintf(` +%s + +resource "azuread_application_password" "test" { + application_id = "${azuread_application.test.id}" + value = "%s" + end_date = "2020-01-01T01:02:03Z" } `, testAccADApplicationPassword_template(applicationId), value) } @@ -204,10 +240,10 @@ func testAccADApplicationPassword_requiresImport(applicationId, value string) st %s resource "azuread_application_password" "import" { - application_id = "${azuread_application_password.test.application_id}" - key_id = "${azuread_application_password.test.key_id}" - value = "${azuread_application_password.test.value}" - end_date = "${azuread_application_password.test.end_date}" + application_object_id = "${azuread_application_password.test.application_id}" + key_id = "${azuread_application_password.test.key_id}" + value = "${azuread_application_password.test.value}" + end_date = "${azuread_application_password.test.end_date}" } `, template) } @@ -217,10 +253,10 @@ func testAccADApplicationPassword_customKeyId(applicationId, keyId, value string %s resource "azuread_application_password" "test" { - application_id = "${azuread_application.test.id}" - key_id = "%s" - value = "%s" - end_date = "2020-01-01T01:02:03Z" + application_object_id = "${azuread_application.test.id}" + key_id = "%s" + value = "%s" + end_date = "2020-01-01T01:02:03Z" } `, testAccADApplicationPassword_template(applicationId), keyId, value) } @@ -230,9 +266,9 @@ func testAccADApplicationPassword_relativeEndDate(applicationId, value string) s %s resource "azuread_application_password" "test" { - application_id = "${azuread_application.test.id}" - value = "%s" - end_date_relative = "8760h" + application_object_id = "${azuread_application.test.id}" + value = "%s" + end_date_relative = "8760h" } `, testAccADApplicationPassword_template(applicationId), value) } diff --git a/azuread/resource_service_principal_password_test.go b/azuread/resource_service_principal_password_test.go index ccdfad41d5..6a18e2182e 100644 --- a/azuread/resource_service_principal_password_test.go +++ b/azuread/resource_service_principal_password_test.go @@ -4,12 +4,12 @@ import ( "fmt" "testing" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" - "github.com/google/uuid" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" ) func testCheckADServicePrincipalPasswordExists(name string) resource.TestCheckFunc { //nolint unparam diff --git a/azuread/resource_service_principal_test.go b/azuread/resource_service_principal_test.go index 32cbba8381..d1af283d1b 100644 --- a/azuread/resource_service_principal_test.go +++ b/azuread/resource_service_principal_test.go @@ -4,11 +4,11 @@ import ( "fmt" "testing" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" - "github.com/google/uuid" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" ) func TestAccAzureADServicePrincipal_basic(t *testing.T) { diff --git a/azuread/resource_user_test.go b/azuread/resource_user_test.go index 6f520a2a29..3776a08347 100644 --- a/azuread/resource_user_test.go +++ b/azuread/resource_user_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/hashicorp/terraform/helper/acctest" - "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" diff --git a/website/docs/r/application_password.html.markdown b/website/docs/r/application_password.html.markdown index bcc383d894..36fe1a4c9c 100644 --- a/website/docs/r/application_password.html.markdown +++ b/website/docs/r/application_password.html.markdown @@ -36,7 +36,7 @@ resource "azuread_application_password" "example" { The following arguments are supported: -* `object_id` - (Required) The Object ID of the Application for which this password should be created. Changing this field forces a new resource to be created. +* `application_object_id` - (Required) The Object ID of the Application for which this password should be created. Changing this field forces a new resource to be created. * `value` - (Required) The Password for this Application . From 4a3d9cb47a0a9be45966e22d29e502af44ca16d4 Mon Sep 17 00:00:00 2001 From: kt Date: Mon, 10 Jun 2019 11:52:03 -0700 Subject: [PATCH 2/6] make goimports --- azuread/data_service_principal.go | 6 +++--- azuread/data_user_test.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/azuread/data_service_principal.go b/azuread/data_service_principal.go index e9b8afa860..22e009dd99 100644 --- a/azuread/data_service_principal.go +++ b/azuread/data_service_principal.go @@ -3,11 +3,11 @@ package azuread import ( "fmt" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate" - "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/hashicorp/terraform/helper/schema" + + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate" ) func dataServicePrincipal() *schema.Resource { diff --git a/azuread/data_user_test.go b/azuread/data_user_test.go index 88d7bb523c..b48bf3a247 100644 --- a/azuread/data_user_test.go +++ b/azuread/data_user_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/hashicorp/terraform/helper/acctest" - "github.com/hashicorp/terraform/helper/resource" ) From 3f13606cb94e4ba1ba832450790eb0212a06a5d3 Mon Sep 17 00:00:00 2001 From: kt Date: Mon, 10 Jun 2019 11:56:23 -0700 Subject: [PATCH 3/6] regain sanity --- azuread/resource_application_password.go | 84 ++++++++++++++++++------ 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/azuread/resource_application_password.go b/azuread/resource_application_password.go index f0c97a598d..383f921bfb 100644 --- a/azuread/resource_application_password.go +++ b/azuread/resource_application_password.go @@ -7,6 +7,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/hashicorp/terraform/helper/schema" + "github.com/hashicorp/terraform/helper/validation" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" @@ -15,25 +16,6 @@ import ( ) func resourceApplicationPassword() *schema.Resource { - - // temporary terrible hack/code to allow deprecation of `application_id` - // todo remove in 1.0 - s := graph.PasswordResourceSchema("application_object") - s["application_id"] = &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Computed: true, - ValidateFunc: validate.UUID, - Deprecated: "Deprecated in favour of `application_object_id` to prevent confusion", - ConflictsWith: []string{"application_id"}, - } - // this is bad, i am aware of it, and I feel awful about it - s["application_object_id"].Required = false - s["application_object_id"].Optional = true - s["application_object_id"].Computed = true - s["application_object_id"].ConflictsWith = []string{"application_object_id"} - return &schema.Resource{ Create: resourceApplicationPasswordCreate, Read: resourceApplicationPasswordRead, @@ -43,8 +25,68 @@ func resourceApplicationPassword() *schema.Resource { State: schema.ImportStatePassthrough, }, - //Schema: graph.PasswordResourceSchema("application_object"), - Schema: s, + // Schema: graph.PasswordResourceSchema("application_object"), //todo switch back to this in 1.0 + Schema: map[string]*schema.Schema{ + "application_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Computed: true, + ValidateFunc: validate.UUID, + Deprecated: "Deprecated in favour of `application_object_id` to prevent confusion", + ConflictsWith: []string{"application_id"}, + }, + + "application_object_id": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.UUID, + ConflictsWith: []string{"application_object_id"}, + }, + + "key_id": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.UUID, + }, + + "value": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Sensitive: true, + ValidateFunc: validate.NoEmptyStrings, + }, + + "start_date": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validation.ValidateRFC3339TimeString, + }, + + "end_date": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"end_date_relative"}, + ValidateFunc: validation.ValidateRFC3339TimeString, + }, + + "end_date_relative": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ConflictsWith: []string{"end_date"}, + ValidateFunc: validate.NoEmptyStrings, + }, + }, } } From 2a2664a550e452dc6af68131d3d135dbbedf0839 Mon Sep 17 00:00:00 2001 From: kt Date: Mon, 10 Jun 2019 12:02:24 -0700 Subject: [PATCH 4/6] fix spelling mistake --- azuread/resource_application_password.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azuread/resource_application_password.go b/azuread/resource_application_password.go index 383f921bfb..1387cf735d 100644 --- a/azuread/resource_application_password.go +++ b/azuread/resource_application_password.go @@ -99,7 +99,7 @@ func resourceApplicationPasswordCreate(d *schema.ResourceData, meta interface{}) objectId = d.Get("application_id").(string) } if objectId == "" { - return fmt.Errorf("one of `application_object_id` or `application_id` must be specifed") + return fmt.Errorf("one of `application_object_id` or `application_id` must be specified") } cred, err := graph.PasswordCredentialForResource(d) From a721bb2351d2dc600e5fb6a678a10dfe832a701f Mon Sep 17 00:00:00 2001 From: kt Date: Wed, 12 Jun 2019 12:16:55 -0700 Subject: [PATCH 5/6] whistpace and ordering --- azuread/resource_service_principal.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/azuread/resource_service_principal.go b/azuread/resource_service_principal.go index 44b50a381d..4de4c03bf8 100644 --- a/azuread/resource_service_principal.go +++ b/azuread/resource_service_principal.go @@ -39,13 +39,13 @@ func resourceServicePrincipal() *schema.Resource { Computed: true, }, - "oauth2_permissions": graph.SchemaOauth2Permissions(), - "object_id": { Type: schema.TypeString, Computed: true, }, + "oauth2_permissions": graph.SchemaOauth2Permissions(), + "tags": { Type: schema.TypeSet, Optional: true, @@ -113,6 +113,7 @@ func resourceServicePrincipalRead(d *schema.ResourceData, meta interface{}) erro d.Set("application_id", app.AppID) d.Set("display_name", app.DisplayName) d.Set("object_id", app.ObjectID) + // tags doesn't exist as a property, so extract it if err := d.Set("tags", app.Tags); err != nil { return fmt.Errorf("Error setting `tags`: %+v", err) From 40ac0b92eb94359ddf86e15f4d800e58cbf3f589 Mon Sep 17 00:00:00 2001 From: kt Date: Wed, 12 Jun 2019 12:20:31 -0700 Subject: [PATCH 6/6] make goimports --- azuread/data_service_principal.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/azuread/data_service_principal.go b/azuread/data_service_principal.go index 8a039e3fb4..3c1261c806 100644 --- a/azuread/data_service_principal.go +++ b/azuread/data_service_principal.go @@ -3,14 +3,11 @@ package azuread import ( "fmt" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate" - "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/hashicorp/terraform/helper/schema" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate" )