Merge pull request #302 from dbt-labs/feature/erro-handling-split-ids

CHANGELOG.md

@@ -2,7 +2,13 @@
 
 All notable changes to this project will be documented in this file.
 
-## [Unreleased](https://github.com/dbt-labs/terraform-provider-dbtcloud/compare/v0.3.17...HEAD)
+## [Unreleased](https://github.com/dbt-labs/terraform-provider-dbtcloud/compare/v0.3.18...HEAD)
+
+# [0.3.18](https://github.com/dbt-labs/terraform-provider-dbtcloud/compare/v0.3.17...v0.3.18)
+
+### Behind the scenes
+
+- Add better error handling when importing resources like in [#299](https://github.com/dbt-labs/terraform-provider-dbtcloud/issues/299)
 
 # [0.3.17](https://github.com/dbt-labs/terraform-provider-dbtcloud/compare/v0.3.16...v0.3.17)
 

pkg/helper/split_id.go

@@ -0,0 +1,51 @@
+package helper
+
+import (
+	"fmt"
+	"strconv"
+	strings "strings"
+
+	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/dbt_cloud"
+)
+
+func SplitIDToStrings(id string, resource_type string) (string, string, error) {
+	parts := strings.Split(id, dbt_cloud.ID_DELIMITER)
+	if len(parts) != 2 {
+
+		err := fmt.Errorf(
+			"expected ID in the format 'id1%sid2' to import a %s, got: %s",
+			dbt_cloud.ID_DELIMITER,
+			resource_type,
+			id,
+		)
+		return "", "", err
+	}
+
+	return parts[0], parts[1], nil
+}
+
+func SplitIDToInts(id string, resource_type string) (int, int, error) {
+	parts := strings.Split(id, dbt_cloud.ID_DELIMITER)
+	if len(parts) != 2 {
+
+		err := fmt.Errorf(
+			"expected ID in the format 'id1%sid2' to import a %s, got: %s",
+			dbt_cloud.ID_DELIMITER,
+			resource_type,
+			id,
+		)
+		return 0, 0, err
+	}
+
+	id1, err := strconv.Atoi(parts[0])
+	if err != nil {
+		return 0, 0, fmt.Errorf("error converting %s to int when splitting the ID", parts[0])
+	}
+
+	id2, err := strconv.Atoi(parts[1])
+	if err != nil {
+		return 0, 0, fmt.Errorf("error converting %s to int when splitting the ID", parts[1])
+	}
+
+	return id1, id2, nil
+}

pkg/sdkv2/resources/bigquery_connection.go

@@ -312,8 +312,13 @@ func resourceBigQueryConnectionRead(
 
 	var diags diag.Diagnostics
 
-	projectIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0]
-	connectionIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1]
+	projectIdString, connectionIdString, err := helper.SplitIDToStrings(
+		d.Id(),
+		"dbtcloud_bigquery_connection",
+	)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	connection, err := c.GetBigQueryConnection(connectionIdString, projectIdString)
 	if err != nil {
@@ -407,8 +412,13 @@ func resourceBigQueryConnectionUpdate(
 ) diag.Diagnostics {
 	c := m.(*dbt_cloud.Client)
 
-	projectIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0]
-	connectionIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1]
+	projectIdString, connectionIdString, err := helper.SplitIDToStrings(
+		d.Id(),
+		"bigquery_connection",
+	)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	if d.HasChange("name") ||
 		d.HasChange("type") ||

pkg/sdkv2/resources/bigquery_connection_acceptance_test.go

@@ -6,8 +6,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/dbt_cloud"
 	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/framework/acctest_helper"
+	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/helper"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
@@ -253,8 +253,10 @@ func testAccCheckDbtCloudBigQueryConnectionDestroy(s *terraform.State) error {
 		if rs.Type != "dbtcloud_bigquery_connection" {
 			continue
 		}
-		projectId := strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[0]
-		connectionId := strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[1]
+		projectId, connectionId, _ := helper.SplitIDToStrings(
+			rs.Primary.ID,
+			"dbtcloud_bigquery_connection",
+		)
 
 		_, err := apiClient.GetConnection(connectionId, projectId)
 		if err == nil {

pkg/sdkv2/resources/bigquery_credential.go

@@ -3,10 +3,10 @@ package resources
 import (
 	"context"
 	"fmt"
-	"strconv"
 	"strings"
 
 	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/dbt_cloud"
+	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/helper"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
@@ -104,12 +104,10 @@ func resourceBigQueryCredentialRead(
 	// Warning or errors can be collected in a slice type
 	var diags diag.Diagnostics
 
-	projectId, err := strconv.Atoi(strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0])
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	BigQueryCredentialId, err := strconv.Atoi(strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1])
+	projectId, BigQueryCredentialId, err := helper.SplitIDToInts(
+		d.Id(),
+		"dbtcloud_bigquery_credential",
+	)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -149,12 +147,10 @@ func resourceBigQueryCredentialUpdate(
 ) diag.Diagnostics {
 	c := m.(*dbt_cloud.Client)
 
-	projectId, err := strconv.Atoi(strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0])
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	BigQueryCredentialId, err := strconv.Atoi(strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1])
+	projectId, BigQueryCredentialId, err := helper.SplitIDToInts(
+		d.Id(),
+		"dbtcloud_bigquery_credential",
+	)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -192,10 +188,15 @@ func resourceBigQueryCredentialDelete(
 
 	var diags diag.Diagnostics
 
-	projectIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0]
-	BigQueryCredentialIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1]
+	projectIdString, BigQueryCredentialIdString, err := helper.SplitIDToStrings(
+		d.Id(),
+		"dbtcloud_bigquery_credential",
+	)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
-	_, err := c.DeleteCredential(BigQueryCredentialIdString, projectIdString)
+	_, err = c.DeleteCredential(BigQueryCredentialIdString, projectIdString)
 	if err != nil {
 		return diag.FromErr(err)
 	}

pkg/sdkv2/resources/bigquery_credential_acceptance_test.go

@@ -3,12 +3,11 @@ package resources_test
 import (
 	"fmt"
 	"regexp"
-	"strconv"
 	"strings"
 	"testing"
 
-	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/dbt_cloud"
 	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/framework/acctest_helper"
+	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/helper"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
@@ -73,13 +72,13 @@ func testAccCheckDbtCloudBigQueryCredentialExists(resource string) resource.Test
 		if rs.Primary.ID == "" {
 			return fmt.Errorf("No Record ID is set")
 		}
-		projectId, err := strconv.Atoi(strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[0])
-		if err != nil {
-			return fmt.Errorf("Can't get projectId")
-		}
-		credentialId, err := strconv.Atoi(strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[1])
+
+		projectId, credentialId, err := helper.SplitIDToInts(
+			rs.Primary.ID,
+			"dbtcloud_bigquery_credential",
+		)
 		if err != nil {
-			return fmt.Errorf("Can't get credentialId")
+			return err
 		}
 
 		apiClient, err := acctest_helper.SharedClient()
@@ -104,13 +103,12 @@ func testAccCheckDbtCloudBigQueryCredentialDestroy(s *terraform.State) error {
 		if rs.Type != "dbtcloud_bigquery_credential" {
 			continue
 		}
-		projectId, err := strconv.Atoi(strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[0])
-		if err != nil {
-			return fmt.Errorf("Can't get projectId")
-		}
-		credentialId, err := strconv.Atoi(strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[1])
+		projectId, credentialId, err := helper.SplitIDToInts(
+			rs.Primary.ID,
+			"dbtcloud_bigquery_credential",
+		)
 		if err != nil {
-			return fmt.Errorf("Can't get credentialId")
+			return err
 		}
 
 		_, err = apiClient.GetBigQueryCredential(projectId, credentialId)

pkg/sdkv2/resources/connection.go

@@ -228,8 +228,13 @@ func resourceConnectionRead(
 
 	var diags diag.Diagnostics
 
-	projectIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0]
-	connectionIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1]
+	projectIdString, connectionIdString, err := helper.SplitIDToStrings(
+		d.Id(),
+		"dbtcloud_connection",
+	)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	connection, err := c.GetConnection(connectionIdString, projectIdString)
 	if err != nil {
@@ -240,7 +245,6 @@ func resourceConnectionRead(
 		return diag.FromErr(err)
 	}
 
-	// TODO: Remove when done better
 	connection.Details.OAuthClientID = d.Get("oauth_client_id").(string)
 	connection.Details.OAuthClientSecret = d.Get("oauth_client_secret").(string)
 
@@ -347,8 +351,13 @@ func resourceConnectionUpdate(
 ) diag.Diagnostics {
 	c := m.(*dbt_cloud.Client)
 
-	projectIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0]
-	connectionIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1]
+	projectIdString, connectionIdString, err := helper.SplitIDToStrings(
+		d.Id(),
+		"dbtcloud_connection",
+	)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
 	if d.HasChange("name") ||
 		d.HasChange("type") ||
@@ -467,10 +476,15 @@ func resourceConnectionDelete(
 
 	var diags diag.Diagnostics
 
-	projectIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[0]
-	connectionIdString := strings.Split(d.Id(), dbt_cloud.ID_DELIMITER)[1]
+	projectIdString, connectionIdString, err := helper.SplitIDToStrings(
+		d.Id(),
+		"dbtcloud_connection",
+	)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
-	_, err := c.DeleteConnection(connectionIdString, projectIdString)
+	_, err = c.DeleteConnection(connectionIdString, projectIdString)
 	if err != nil {
 		return diag.FromErr(err)
 	}

pkg/sdkv2/resources/connection_acceptance_test.go

@@ -7,8 +7,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/dbt_cloud"
 	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/framework/acctest_helper"
+	"github.com/dbt-labs/terraform-provider-dbtcloud/pkg/helper"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
@@ -587,8 +587,13 @@ func testAccCheckDbtCloudConnectionExists(resource string) resource.TestCheckFun
 		if err != nil {
 			return fmt.Errorf("Issue getting the client")
 		}
-		projectId := strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[0]
-		connectionId := strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[1]
+		projectId, connectionId, err := helper.SplitIDToStrings(
+			rs.Primary.ID,
+			"dbtcloud_connection",
+		)
+		if err != nil {
+			return err
+		}
 
 		_, err = apiClient.GetConnection(connectionId, projectId)
 		if err != nil {
@@ -608,10 +613,15 @@ func testAccCheckDbtCloudConnectionDestroy(s *terraform.State) error {
 		if rs.Type != "dbtcloud_connection" {
 			continue
 		}
-		projectId := strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[0]
-		connectionId := strings.Split(rs.Primary.ID, dbt_cloud.ID_DELIMITER)[1]
+		projectId, connectionId, err := helper.SplitIDToStrings(
+			rs.Primary.ID,
+			"dbtcloud_connection",
+		)
+		if err != nil {
+			return err
+		}
 
-		_, err := apiClient.GetConnection(connectionId, projectId)
+		_, err = apiClient.GetConnection(connectionId, projectId)
 		if err == nil {
 			return fmt.Errorf("Connection still exists")
 		}