`azurerm_sql_database` - support for importing from bacpac #972

hbuckle · 2018-03-13T15:20:37Z

Add ability to import a database from a bacpac (https://docs.microsoft.com/en-us/azure/sql-database/sql-database-import)

I can't see any way to add a test for this one because it needs a bacpac file in a storage account somewhere.

I've tested it using an AdventureWorks bacpac (https://s3-eu-west-1.amazonaws.com/resourcescrono/AdventureWorks2014FullForAzure.bacpac)

ShepardToTheStars · 2018-03-22T15:39:28Z

azurerm/resource_arm_sql_database.go

@@ -237,6 +285,27 @@ func resourceArmSqlDatabaseCreateUpdate(d *schema.ResourceData, meta interface{}
 		return err
 	}

+	if _, ok := d.GetOk("database_import"); ok {
+		if !strings.EqualFold(createMode, "default") {
+			return fmt.Errorf("database_import can only be used when create_mode is Default")


@hbuckle Should this error be a validation on the database_import property instead? Otherwise, you could be running your scripts and end up with the error during a deployment instead of when generating the plan? Or possibly use ConflictsWith: ["create_mode"] on the database_import since the default for create_mode is default (and its required to be default).

@vlouwagie ideally yes, however unfortunately the validation API's only expose a single property at this point; as such unfortunately I don't think that's feasible at this time

ShepardToTheStars · 2018-03-22T15:41:21Z

azurerm/resource_arm_sql_database.go

+								"SQL", "ADPassword",
+							}, true),
+						},
+						"operation_mode": {


@hbuckle If there is only one value, does this need to be required?

+1 - we should be able to make this Optional with a Default value instead :)

tombuildsstuff

hey @hbuckle

Thanks for this PR :) apologies for the delayed review here!

I've taken a look through and left some comments in-line, but the main thing missing here is an acceptance test covering this functionality. I believe it should be possible to spin up a DB, take a backup and them import that into a new DB all via the API to create the test rather than caching a file somewhere perminantly - what do you think?

Thanks!

tombuildsstuff · 2018-03-22T23:38:32Z

azurerm/resource_arm_sql_database.go

+								"SQL", "ADPassword",
+							}, true),
+						},
+						"operation_mode": {


+1 - we should be able to make this Optional with a Default value instead :)

tombuildsstuff · 2018-03-22T23:51:52Z

azurerm/resource_arm_sql_database.go

@@ -57,6 +58,53 @@ func resourceArmSqlDatabase() *schema.Resource {
 				DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
 			},

+			"database_import": {


I think it might be worth renaming this to import - what do you think?

tombuildsstuff · 2018-03-22T23:52:03Z

azurerm/resource_arm_sql_database.go

+						},
+						"administrator_login_password": {
+							Type:     schema.TypeString,
+							Required: true,


can we add Sensitive: true here?

tombuildsstuff · 2018-03-22T23:52:17Z

azurerm/resource_arm_sql_database.go

+						},
+						"storage_key": {
+							Type:     schema.TypeString,
+							Required: true,


can we add Sensitive: true here?

tombuildsstuff · 2018-03-23T00:05:21Z

azurerm/resource_arm_sql_database.go

@@ -237,6 +285,27 @@ func resourceArmSqlDatabaseCreateUpdate(d *schema.ResourceData, meta interface{}
 		return err
 	}

+	if _, ok := d.GetOk("database_import"); ok {
+		if !strings.EqualFold(createMode, "default") {
+			return fmt.Errorf("database_import can only be used when create_mode is Default")


@vlouwagie ideally yes, however unfortunately the validation API's only expose a single property at this point; as such unfortunately I don't think that's feasible at this time

tombuildsstuff · 2018-03-23T00:13:45Z

azurerm/resource_arm_sql_database.go

+		// this is set in config.go, but something sets
+		// it back to 15 minutes, which isn't long enough
+		// for most imports
+		client.Client.PollingDuration = 60 * time.Minute


from what I recall the 15 minute reset is pulled from the API's retry interval recommendation * the number of retries allowed - I've been meaning to investigate this further

hbuckle · 2018-03-27T14:40:17Z

@tombuildsstuff RE the acceptance test, using the Azure portal I created a new blank database, exported it to a bacpac in a storage account and then imported it into a second new database. However the import fails with the error "Archive file cannot be size 0" so I think we need a way to get some data into the database. Do you know a way to do this using the API ?

tombuildsstuff · 2018-03-28T14:50:59Z

@hbuckle sorry I'd missed your comment about the AdventureWorks bacpac, I guess we can place that somewhere and restore it? Let me reach out internally to see where's best to do that, but we should be able to upload it to blob storage from wherever it's hosted and then restore it as needed :)

hbuckle · 2018-04-10T10:42:26Z

@tombuildsstuff Yes I think we basically just need a long lived storage account to do the restore from. Let me know if you have one available.

hbuckle · 2018-04-11T09:51:38Z

Actually I have just seen the azurerm_storage_blob resource can upload a local file to storage, so I can include a bacpac in the testdata folder. I'll go down that route

tombuildsstuff · 2018-04-12T11:19:13Z

@hbuckle rather than storing a large binary file in the repository, it may be better to use a SQL Server Driver such as this one to create a table and insert some random test data (so we have this in code) - what do you think?

hbuckle · 2018-04-17T08:29:39Z

@tombuildsstuff my only concern is that requires connecting directly to the DB, which probably won't work if you are behind a corporate firewall. I can make a bacpac that is much smaller than adventureworks, only a few kb and include that?

tombuildsstuff · 2018-04-19T18:55:48Z

@hbuckle after chatting internally as long as it's a small file (e.g. a couple hundred KB) this should be fine - we were mostly concerned about large binary files :)

hbuckle · 2018-05-05T09:43:31Z

@tombuildsstuff Acceptance test added

go test github.com/terraform-providers/terraform-provider-azurerm/azurerm -v -timeout 600s -run TestAccAzureRMSqlDatabase_bacpac
=== RUN   TestAccAzureRMSqlDatabase_bacpac
--- PASS: TestAccAzureRMSqlDatabase_bacpac (312.25s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm       312.288s

hbuckle · 2018-05-24T11:05:34Z

@tombuildsstuff anything more you need from me on this one?

tombuildsstuff · 2018-06-19T08:19:32Z

@hbuckle apologies for the delay, I've been out for a few weeks - I'm taking a look at this now :)

tombuildsstuff

hey @hbuckle

Thanks for pushing those updates - apologies on the delayed re-review of this. I've taken a look through and left a couple of minor comments in-line for which I'll push a commit to fix, but this otherwise LGTM - thanks for this!

Thanks!

tombuildsstuff · 2018-06-19T08:20:26Z

azurerm/resource_arm_sql_database.go

+							Required: true,
+							ValidateFunc: validation.StringInSlice([]string{
+								"StorageAccessKey", "SharedAccessKey",
+							}, true),


this true represents the strings being case-insensitive; as such can we also add DiffSuppressFunc: ignoreCaseDiffSuppressFunc here to handle the casing being different?

tombuildsstuff · 2018-06-19T08:21:02Z

azurerm/resource_arm_sql_database.go

+							Default:  "Import",
+							ValidateFunc: validation.StringInSlice([]string{
+								"Import",
+							}, true),


(same here)

tombuildsstuff · 2018-06-19T10:58:59Z

Tests pass:

ghost · 2019-03-06T14:11:31Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

hbuckle added 3 commits March 12, 2018 21:48

Merge remote-tracking branch 'upstream/master'

9d878b2

add database_import property

e552a59

add option to import database from bacpac

e18d160

hbuckle mentioned this pull request Mar 13, 2018

Feature request: Azure SQL Database Extensions Type for Import Database #899

Closed

bflad added enhancement service/mssql Microsoft SQL Server labels Mar 13, 2018

tombuildsstuff added this to the 1.3.1 milestone Mar 14, 2018

ShepardToTheStars reviewed Mar 22, 2018

View reviewed changes

tombuildsstuff suggested changes Mar 23, 2018

View reviewed changes

tombuildsstuff modified the milestones: 1.3.1, 1.3.2 Mar 28, 2018

tombuildsstuff added the upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc) label Mar 30, 2018

tombuildsstuff modified the milestones: 1.3.2, 1.3.3 Apr 4, 2018

Henry Buckle and others added 3 commits April 10, 2018 16:28

Merge remote-tracking branch 'upstream/master' into sqldbimport

3c6cdc2

PR tweaks

1ba3169

rename database_import to import

e3b9a4e

tombuildsstuff added waiting-response and removed upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc) labels Apr 12, 2018

tombuildsstuff modified the milestones: 1.3.3, 1.4.0 Apr 17, 2018

tombuildsstuff removed this from the 1.4.0 milestone Apr 25, 2018

tombuildsstuff added this to the 1.5.0 milestone Apr 25, 2018

hbuckle added 3 commits May 3, 2018 07:39

Merge remote-tracking branch 'upstream/master' into sqldbimport

a933a98

add acc test

a4159d0

Merge remote-tracking branch 'upstream/master' into sqldbimport

06301da

tombuildsstuff modified the milestones: 1.5.0, 1.6.0 May 8, 2018

tombuildsstuff modified the milestones: 1.6.0, 1.7.0 May 24, 2018

katbyte modified the milestones: 1.7.0, Soon Jun 12, 2018

tombuildsstuff removed the waiting-response label Jun 19, 2018

tombuildsstuff approved these changes Jun 19, 2018

View reviewed changes

Handling case-insensitivity

4c7ae1b

tombuildsstuff changed the title ~~Import SQL database from bacpac~~ azurerm_sql_database - support for importing from bacpac Jun 19, 2018

tombuildsstuff merged commit a454b5c into hashicorp:master Jun 19, 2018

tombuildsstuff added a commit that referenced this pull request Jun 19, 2018

Updating to include #972

28a538c

tombuildsstuff added a commit that referenced this pull request Jun 19, 2018

Updating to include #972

83e0562

tombuildsstuff added a commit that referenced this pull request Jun 19, 2018

Updating to include #972

802cac4

tombuildsstuff modified the milestones: Soon, Being Sorted Oct 25, 2018

ghost locked and limited conversation to collaborators Mar 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_sql_database` - support for importing from bacpac #972

`azurerm_sql_database` - support for importing from bacpac #972

hbuckle commented Mar 13, 2018

ShepardToTheStars Mar 22, 2018

tombuildsstuff Mar 23, 2018

ShepardToTheStars Mar 22, 2018

tombuildsstuff Mar 22, 2018

tombuildsstuff left a comment

tombuildsstuff Mar 22, 2018

tombuildsstuff Mar 22, 2018

tombuildsstuff Mar 22, 2018

tombuildsstuff Mar 22, 2018

tombuildsstuff Mar 23, 2018

tombuildsstuff Mar 23, 2018

hbuckle commented Mar 27, 2018

tombuildsstuff commented Mar 28, 2018

hbuckle commented Apr 10, 2018

hbuckle commented Apr 11, 2018

tombuildsstuff commented Apr 12, 2018

hbuckle commented Apr 17, 2018

tombuildsstuff commented Apr 19, 2018

hbuckle commented May 5, 2018

hbuckle commented May 24, 2018

tombuildsstuff commented Jun 19, 2018

tombuildsstuff left a comment

tombuildsstuff Jun 19, 2018

tombuildsstuff Jun 19, 2018

tombuildsstuff commented Jun 19, 2018

ghost commented Mar 6, 2019

azurerm_sql_database - support for importing from bacpac #972

azurerm_sql_database - support for importing from bacpac #972

Conversation

hbuckle commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hbuckle commented Mar 27, 2018

tombuildsstuff commented Mar 28, 2018

hbuckle commented Apr 10, 2018

hbuckle commented Apr 11, 2018

tombuildsstuff commented Apr 12, 2018

hbuckle commented Apr 17, 2018

tombuildsstuff commented Apr 19, 2018

hbuckle commented May 5, 2018

hbuckle commented May 24, 2018

tombuildsstuff commented Jun 19, 2018

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff commented Jun 19, 2018

ghost commented Mar 6, 2019

`azurerm_sql_database` - support for importing from bacpac #972

`azurerm_sql_database` - support for importing from bacpac #972