Merge pull request #264 from terraform-providers/app-sp-secrets/new-i…

…d-format

New ID format for azuread_application_certificate, azuread_application_password, azuread_service_principal_certificate, azuread_service_principal_password

azuread/helpers/graph/credentials.go

@@ -90,15 +90,8 @@ func CertificateResourceSchema(object_type string) map[string]*schema.Schema {
 }
 
 // valid types are `application` and `service_principal`
-func PasswordResourceSchema(object_type string) map[string]*schema.Schema {
-	return map[string]*schema.Schema{
-		object_type + "_id": {
-			Type:         schema.TypeString,
-			Required:     true,
-			ForceNew:     true,
-			ValidateFunc: validate.UUID,
-		},
-
+func PasswordResourceSchema(objectType string) map[string]*schema.Schema {
+	theSchema := map[string]*schema.Schema{
 		"key_id": {
 			Type:         schema.TypeString,
 			Optional:     true,
@@ -147,40 +140,87 @@ func PasswordResourceSchema(object_type string) map[string]*schema.Schema {
 			ValidateFunc: validate.NoEmptyStrings,
 		},
 	}
+
+	switch objectType {
+	case "application":
+		theSchema["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",
+			ExactlyOneOf: []string{"application_object_id"},
+		}
+		theSchema["application_object_id"] = &schema.Schema{
+			Type:         schema.TypeString,
+			Optional:     true,
+			Computed:     true,
+			ForceNew:     true,
+			ValidateFunc: validate.UUID,
+			ExactlyOneOf: []string{"application_id"},
+		}
+	case "service_principal":
+		theSchema["service_principal_id"] = &schema.Schema{
+			Type:         schema.TypeString,
+			Required:     true,
+			ForceNew:     true,
+			ValidateFunc: validate.UUID,
+		}
+	}
+
+	return theSchema
 }
 
 type CredentialId struct {
 	ObjectId string
+	KeyType  string
 	KeyId    string
 }
 
 func (id CredentialId) String() string {
-	return id.ObjectId + "/" + id.KeyId
+	return id.ObjectId + "/" + id.KeyType + "/" + id.KeyId
 }
 
 func ParseCredentialId(id string) (CredentialId, error) {
 	parts := strings.Split(id, "/")
-	if len(parts) != 2 {
-		return CredentialId{}, fmt.Errorf("Password Credential ID should be in the format {objectId}/{keyId} - but got %q", id)
+	if len(parts) != 3 {
+		return CredentialId{}, fmt.Errorf("Credential ID should be in the format {objectId}/{keyType}/{keyId} - but got %q", id)
 	}
 
 	if _, err := uuid.ParseUUID(parts[0]); err != nil {
-		return CredentialId{}, fmt.Errorf("Object ID isn't a valid UUID (%q): %+v", id[0], err)
+		return CredentialId{}, fmt.Errorf("Object ID isn't a valid UUID (%q): %+v", parts[0], err)
+	}
+
+	if parts[1] != "certificate" && parts[1] != "password" {
+		return CredentialId{}, fmt.Errorf("Key type should be one of: certificate, password. Got: %q", parts[1])
 	}
 
-	if _, err := uuid.ParseUUID(parts[1]); err != nil {
-		return CredentialId{}, fmt.Errorf("Credential ID isn't a valid UUID (%q): %+v", id[1], err)
+	if _, err := uuid.ParseUUID(parts[2]); err != nil {
+		return CredentialId{}, fmt.Errorf("Key ID isn't a valid UUID (%q): %+v", parts[2], err)
 	}
 
 	return CredentialId{
 		ObjectId: parts[0],
-		KeyId:    parts[1],
+		KeyType:  parts[1],
+		KeyId:    parts[2],
 	}, nil
 }
 
-func CredentialIdFrom(objectId, keyId string) CredentialId {
+func ParseOldCredentialId(id, keyType string) (CredentialId, error) {
+	parts := strings.Split(id, "/")
+	if len(parts) != 2 {
+		return CredentialId{}, fmt.Errorf("Credential ID expected to be in the format {objectId}/{keyId} - but got %q", id)
+	}
+
+	newId := parts[0] + "/" + keyType + "/" + parts[1]
+	return ParseCredentialId(newId)
+}
+
+func CredentialIdFrom(objectId, keyType, keyId string) CredentialId {
 	return CredentialId{
 		ObjectId: objectId,
+		KeyType:  keyType,
 		KeyId:    keyId,
 	}
 }

azuread/resource_application.go

@@ -745,6 +745,12 @@ func expandADApplicationOptionalClaim(in []interface{}) *[]graphrbac.OptionalCla
 }
 
 func flattenADApplicationOptionalClaims(in *graphrbac.OptionalClaims) interface{} {
+	var result []map[string]interface{}
+
+	if in == nil {
+		return result
+	}
+
 	optionalClaims := make(map[string]interface{})
 	if claims := flattenADApplicationOptionalClaimsList(in.AccessToken); len(claims) > 0 {
 		optionalClaims["access_token"] = claims
@@ -756,7 +762,6 @@ func flattenADApplicationOptionalClaims(in *graphrbac.OptionalClaims) interface{
 	//if claims := flattenADApplicationOptionalClaimsList(in.SamlToken); len(claims) > 0 {
 	//	optionalClaims["saml_token"] = claims
 	//}
-	var result []map[string]interface{}
 	if len(optionalClaims) == 0 {
 		return result
 	}

azuread/resource_application_certificate.go

@@ -37,7 +37,7 @@ func resourceApplicationCertificateCreate(d *schema.ResourceData, meta interface
 	if err != nil {
 		return fmt.Errorf("generating certificate credentials for object ID %q: %+v", objectId, err)
 	}
-	id := graph.CredentialIdFrom(objectId, *cred.KeyID)
+	id := graph.CredentialIdFrom(objectId, "certificate", *cred.KeyID)
 
 	tf.LockByName(resourceApplicationName, id.ObjectId)
 	defer tf.UnlockByName(resourceApplicationName, id.ObjectId)

azuread/resource_application_password.go

@@ -7,12 +7,10 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
 
 	"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 {
@@ -25,73 +23,14 @@ func resourceApplicationPassword() *schema.Resource {
 			State: schema.ImportStatePassthrough,
 		},
 
-		// 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_object_id"},
-			},
-
-			"application_object_id": {
-				Type:          schema.TypeString,
-				Optional:      true,
-				Computed:      true,
-				ForceNew:      true,
-				ValidateFunc:  validate.UUID,
-				ConflictsWith: []string{"application_id"},
-			},
-
-			"key_id": {
-				Type:         schema.TypeString,
-				Optional:     true,
-				Computed:     true,
-				ForceNew:     true,
-				ValidateFunc: validate.UUID,
-			},
-
-			"description": {
-				Type:     schema.TypeString,
-				Optional: true,
-				Computed: true,
-				ForceNew: true,
-			},
-
-			"value": {
-				Type:         schema.TypeString,
-				Required:     true,
-				ForceNew:     true,
-				Sensitive:    true,
-				ValidateFunc: validation.StringLenBetween(1, 863), // Encrypted secret cannot be empty and can be at most 1024 bytes.
-			},
+		Schema: graph.PasswordResourceSchema("application"),
 
-			"start_date": {
-				Type:         schema.TypeString,
-				Optional:     true,
-				Computed:     true,
-				ForceNew:     true,
-				ValidateFunc: validation.IsRFC3339Time,
-			},
-
-			"end_date": {
-				Type:          schema.TypeString,
-				Optional:      true,
-				Computed:      true,
-				ForceNew:      true,
-				ConflictsWith: []string{"end_date_relative"},
-				ValidateFunc:  validation.IsRFC3339Time,
-			},
-
-			"end_date_relative": {
-				Type:          schema.TypeString,
-				Optional:      true,
-				ForceNew:      true,
-				ConflictsWith: []string{"end_date"},
-				ValidateFunc:  validate.NoEmptyStrings,
+		SchemaVersion: 1,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Type:    resourceApplicationPasswordInstanceResourceV0().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceApplicationPasswordInstanceStateUpgradeV0,
+				Version: 0,
 			},
 		},
 	}
@@ -105,15 +44,12 @@ func resourceApplicationPasswordCreate(d *schema.ResourceData, meta interface{})
 	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 specified")
-	}
 
 	cred, err := graph.PasswordCredentialForResource(d)
 	if err != nil {
 		return fmt.Errorf("Error generating Application Credentials for Object ID %q: %+v", objectId, err)
 	}
-	id := graph.CredentialIdFrom(objectId, *cred.KeyID)
+	id := graph.CredentialIdFrom(objectId, "password", *cred.KeyID)
 
 	tf.LockByName(resourceApplicationName, id.ObjectId)
 	defer tf.UnlockByName(resourceApplicationName, id.ObjectId)
@@ -231,3 +167,20 @@ func resourceApplicationPasswordDelete(d *schema.ResourceData, meta interface{})
 
 	return nil
 }
+
+func resourceApplicationPasswordInstanceResourceV0() *schema.Resource {
+	return &schema.Resource{
+		Schema: graph.PasswordResourceSchema("application"),
+	}
+}
+
+func resourceApplicationPasswordInstanceStateUpgradeV0(rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	log.Println("[DEBUG] Migrating ID from v0 to v1 format")
+	newId, err := graph.ParseOldCredentialId(rawState["id"].(string), "password")
+	if err != nil {
+		return rawState, fmt.Errorf("generating new ID: %s", err)
+	}
+
+	rawState["id"] = newId.String()
+	return rawState, nil
+}

azuread/resource_application_password_test.go

@@ -97,6 +97,12 @@ func TestAccAzureADApplicationPassword_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "end_date", "2099-01-01T01:02:03Z"),
 				),
 			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"value"},
+			},
 		},
 	})
 }
@@ -121,6 +127,12 @@ func TestAccAzureADApplicationPassword_basicOld(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "end_date", "2099-01-01T01:02:03Z"),
 				),
 			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"value"},
+			},
 		},
 	})
 }
@@ -174,6 +186,12 @@ func TestAccAzureADApplicationPassword_customKeyId(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "end_date", "2099-01-01T01:02:03Z"),
 				),
 			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"value"},
+			},
 		},
 	})
 }
@@ -197,6 +215,12 @@ func TestAccAzureADApplicationPassword_description(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "end_date", "2099-01-01T01:02:03Z"),
 				),
 			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"value"},
+			},
 		},
 	})
 }
@@ -221,6 +245,12 @@ func TestAccAzureADApplicationPassword_relativeEndDate(t *testing.T) {
 					resource.TestCheckResourceAttrSet(resourceName, "end_date"),
 				),
 			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"end_date_relative", "value"},
+			},
 		},
 	})
 }

azuread/resource_service_principal_certificate.go

@@ -37,7 +37,7 @@ func resourceServicePrincipalCertificateCreate(d *schema.ResourceData, meta inte
 	if err != nil {
 		return fmt.Errorf("generating certificate credentials for object ID %q: %+v", objectId, err)
 	}
-	id := graph.CredentialIdFrom(objectId, *cred.KeyID)
+	id := graph.CredentialIdFrom(objectId, "certificate", *cred.KeyID)
 
 	tf.LockByName(servicePrincipalResourceName, id.ObjectId)
 	defer tf.UnlockByName(servicePrincipalResourceName, id.ObjectId)

azuread/resource_service_principal_password.go

@@ -24,6 +24,15 @@ func resourceServicePrincipalPassword() *schema.Resource {
 		},
 
 		Schema: graph.PasswordResourceSchema("service_principal"),
+
+		SchemaVersion: 1,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Type:    resourceServicePrincipalPasswordInstanceResourceV0().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceServicePrincipalPasswordInstanceStateUpgradeV0,
+				Version: 0,
+			},
+		},
 	}
 }
 
@@ -37,7 +46,7 @@ func resourceServicePrincipalPasswordCreate(d *schema.ResourceData, meta interfa
 	if err != nil {
 		return fmt.Errorf("Error generating Service Principal Credentials for Object ID %q: %+v", objectId, err)
 	}
-	id := graph.CredentialIdFrom(objectId, *cred.KeyID)
+	id := graph.CredentialIdFrom(objectId, "password", *cred.KeyID)
 
 	tf.LockByName(servicePrincipalResourceName, id.ObjectId)
 	defer tf.UnlockByName(servicePrincipalResourceName, id.ObjectId)
@@ -155,3 +164,20 @@ func resourceServicePrincipalPasswordDelete(d *schema.ResourceData, meta interfa
 
 	return nil
 }
+
+func resourceServicePrincipalPasswordInstanceResourceV0() *schema.Resource {
+	return &schema.Resource{
+		Schema: graph.PasswordResourceSchema("service_principal"),
+	}
+}
+
+func resourceServicePrincipalPasswordInstanceStateUpgradeV0(rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	log.Println("[DEBUG] Migrating ID from v0 to v1 format")
+	newId, err := graph.ParseOldCredentialId(rawState["id"].(string), "password")
+	if err != nil {
+		return rawState, fmt.Errorf("generating new ID: %s", err)
+	}
+
+	rawState["id"] = newId.String()
+	return rawState, nil
+}