Skip to content

Commit

Permalink
More import formats and fix name validation for spanner databases
Browse files Browse the repository at this point in the history 
Fixes #2277
  • Loading branch information
@modular-magician @nat-henderson
modular-magician authored and nat-henderson committed Dec 21, 2018
1 parent aab0f61 commit e6ddd16
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 171 deletions.
93 changes: 35 additions & 58 deletions google/resource_spanner_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,48 +13,31 @@ import (
"google.golang.org/api/spanner/v1"
)

const (
spannerDatabaseNameFormat = "^[a-z][a-z0-9_-]*[a-z0-9]$"
)

func resourceSpannerDatabase() *schema.Resource {
return &schema.Resource{
Create: resourceSpannerDatabaseCreate,
Read: resourceSpannerDatabaseRead,
Delete: resourceSpannerDatabaseDelete,
Importer: &schema.ResourceImporter{
State: resourceSpannerDatabaseImportState,
State: resourceSpannerDatabaseImport,
},

Schema: map[string]*schema.Schema{

"instance": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},

"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)

if len(value) < 2 && len(value) > 30 {
errors = append(errors, fmt.Errorf(
"%q must be between 2 and 30 characters in length", k))
}
if !regexp.MustCompile("^[a-z0-9-]+$").MatchString(value) {
errors = append(errors, fmt.Errorf(
"%q can only contain lowercase letters, numbers and hyphens", k))
}
if !regexp.MustCompile("^[a-z]").MatchString(value) {
errors = append(errors, fmt.Errorf(
"%q must start with a letter", k))
}
if !regexp.MustCompile("[a-z0-9]$").MatchString(value) {
errors = append(errors, fmt.Errorf(
"%q must end with a number or a letter", k))
}
return
},
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateResourceSpannerDatabaseName,
},

"project": {
Expand Down Expand Up @@ -154,25 +137,23 @@ func resourceSpannerDatabaseDelete(d *schema.ResourceData, meta interface{}) err
return nil
}

func resourceSpannerDatabaseImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
func resourceSpannerDatabaseImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
config := meta.(*Config)
id, err := importSpannerDatabaseId(d.Id())
err := parseImportId([]string{
"projects/(?P<project>[^/]+)/instances/(?P<instance>[^/]+)/databases/(?P<name>[^/]+)",
"instances/(?P<instance>[^/]+)/databases/(?P<name>[^/]+)",
"(?P<project>[^/]+)/(?P<instance>[^/]+)/(?P<name>[^/]+)",
"(?P<instance>[^/]+)/(?P<name>[^/]+)",
}, d, config)
if err != nil {
return nil, err
return nil, fmt.Errorf("Error constructing id: %s", err)
}

if id.Project != "" {
d.Set("project", id.Project)
} else {
project, err := getProject(d, config)
if err != nil {
return nil, err
}
id.Project = project
id, err := buildSpannerDatabaseId(d, config)
if err != nil {
return nil, fmt.Errorf("Error constructing id: %s", err)
}

d.Set("instance", id.Instance)
d.Set("name", id.Database)
d.SetId(id.terraformId())

return []*schema.ResourceData{d}, nil
Expand Down Expand Up @@ -215,26 +196,8 @@ func (s spannerDatabaseId) databaseUri() string {
return fmt.Sprintf("%s/databases/%s", s.parentInstanceUri(), s.Database)
}

func importSpannerDatabaseId(id string) (*spannerDatabaseId, error) {
if !regexp.MustCompile("^[a-z0-9-]+/[a-z0-9-]+$").Match([]byte(id)) &&
!regexp.MustCompile("^"+ProjectRegex+"/[a-z0-9-]+/[a-z0-9-]+$").Match([]byte(id)) {
return nil, fmt.Errorf("Invalid spanner database specifier. " +
"Expecting either {projectId}/{instanceId}/{dbId} OR " +
"{instanceId}/{dbId} (where project will be derived from the provider)")
}

parts := strings.Split(id, "/")
if len(parts) == 2 {
log.Printf("[INFO] Spanner database import format of {instanceId}/{dbId} specified: %s", id)
return &spannerDatabaseId{Instance: parts[0], Database: parts[1]}, nil
}

log.Printf("[INFO] Spanner database import format of {projectId}/{instanceId}/{dbId} specified: %s", id)
return extractSpannerDatabaseId(id)
}

func extractSpannerDatabaseId(id string) (*spannerDatabaseId, error) {
if !regexp.MustCompile("^" + ProjectRegex + "/[a-z0-9-]+/[a-z0-9-]+$").Match([]byte(id)) {
if !regexp.MustCompile(fmt.Sprintf("^%s/[a-z0-9-]+/%s$", ProjectRegex, spannerDatabaseNameFormat)).Match([]byte(id)) {
return nil, fmt.Errorf("Invalid spanner id format, expecting {projectId}/{instanceId}/{databaseId}")
}
parts := strings.Split(id, "/")
Expand All @@ -244,3 +207,17 @@ func extractSpannerDatabaseId(id string) (*spannerDatabaseId, error) {
Database: parts[2],
}, nil
}

func validateResourceSpannerDatabaseName(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)

if len(value) < 2 && len(value) > 30 {
errors = append(errors, fmt.Errorf(
"%q must be between 2 and 30 characters in length", k))
}

if !regexp.MustCompile(spannerDatabaseNameFormat).MatchString(value) {
errors = append(errors, fmt.Errorf("database name %q must match regexp %q", value, spannerDatabaseNameFormat))
}
return
}
162 changes: 49 additions & 113 deletions google/resource_spanner_database_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package google
import (
"fmt"
"net/http"
"strings"
"testing"

"github.com/hashicorp/errwrap"
Expand All @@ -14,126 +13,49 @@ import (
"google.golang.org/api/googleapi"
)

// Unit Tests

func TestDatabaseNameForApi(t *testing.T) {
id := spannerDatabaseId{
Project: "project123",
Instance: "instance456",
Database: "db789",
}
actual := id.databaseUri()
expected := "projects/project123/instances/instance456/databases/db789"
expectEquals(t, expected, actual)
}

func TestImportSpannerDatabaseId_InstanceDB(t *testing.T) {
id, e := importSpannerDatabaseId("instance456/database789")
if e != nil {
t.Errorf("Error should have been nil")
}
expectEquals(t, "", id.Project)
expectEquals(t, "instance456", id.Instance)
expectEquals(t, "database789", id.Database)
}

func TestImportSpannerDatabaseId_ProjectInstanceDB(t *testing.T) {
id, e := importSpannerDatabaseId("project123/instance456/database789")
if e != nil {
t.Errorf("Error should have been nil")
}
expectEquals(t, "project123", id.Project)
expectEquals(t, "instance456", id.Instance)
expectEquals(t, "database789", id.Database)
}

func TestImportSpannerDatabaseId_projectId(t *testing.T) {
shouldPass := []string{
"project-id/instance/database",
"123123/instance/123",
"hashicorptest.net:project-123/instance/123",
"123/456/789",
}

shouldFail := []string{
"project-id#/instance/database",
"project-id/instance#/database",
"project-id/instance/database#",
"hashicorptest.net:project-123:invalid:project/instance/123",
"hashicorptest.net:/instance/123",
}

for _, element := range shouldPass {
_, e := importSpannerDatabaseId(element)
if e != nil {
t.Error("importSpannerDatabaseId should pass on '" + element + "' but doesn't")
}
}

for _, element := range shouldFail {
_, e := importSpannerDatabaseId(element)
if e == nil {
t.Error("importSpannerDatabaseId should fail on '" + element + "' but doesn't")
}
}
}

func TestImportSpannerDatabaseId_invalidLeadingSlash(t *testing.T) {
id, e := importSpannerDatabaseId("/instance456/database789")
expectInvalidSpannerDbImportId(t, id, e)
}

func TestImportSpannerDatabaseId_invalidTrailingSlash(t *testing.T) {
id, e := importSpannerDatabaseId("instance456/database789/")
expectInvalidSpannerDbImportId(t, id, e)
}

func TestImportSpannerDatabaseId_invalidSingleSlash(t *testing.T) {
id, e := importSpannerDatabaseId("/")
expectInvalidSpannerDbImportId(t, id, e)
}

func TestImportSpannerDatabaseId_invalidMultiSlash(t *testing.T) {
id, e := importSpannerDatabaseId("project123/instance456/db789/next")
expectInvalidSpannerDbImportId(t, id, e)
}

func expectInvalidSpannerDbImportId(t *testing.T, id *spannerDatabaseId, e error) {
if id != nil {
t.Errorf("Expected spannerDatabaseId to be nil")
return
}
if e == nil {
t.Errorf("Expected an Error but did not get one")
return
}
if !strings.HasPrefix(e.Error(), "Invalid spanner database specifier") {
t.Errorf("Expecting Error starting with 'Invalid spanner database specifier'")
}
}

// Acceptance Tests

func TestAccSpannerDatabase_basic(t *testing.T) {
t.Parallel()

project := getTestProjectFromEnv()
rnd := acctest.RandString(10)
instanceName := fmt.Sprintf("my-instance-%s", rnd)
databaseName := fmt.Sprintf("mydb_%s", rnd)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckSpannerDatabaseDestroy,
Steps: []resource.TestStep{
{
Config: testAccSpannerDatabase_basic(rnd),
Config: testAccSpannerDatabase_basic(instanceName, databaseName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"),
),
},
{
// Test import with default Terraform ID
ResourceName: "google_spanner_database.basic",
ImportState: true,
ImportStateVerify: true,
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
},
{
ResourceName: "google_spanner_database.basic",
ImportStateId: fmt.Sprintf("%s/%s", instanceName, databaseName),
ImportState: true,
ImportStateVerify: true,
},
},
})
}
Expand All @@ -142,13 +64,15 @@ func TestAccSpannerDatabase_basicWithInitialDDL(t *testing.T) {
t.Parallel()

rnd := acctest.RandString(10)
instanceName := fmt.Sprintf("my-instance-%s", rnd)
databaseName := fmt.Sprintf("mydb-%s", rnd)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckSpannerDatabaseDestroy,
Steps: []resource.TestStep{
{
Config: testAccSpannerDatabase_basicWithInitialDDL(rnd),
Config: testAccSpannerDatabase_basicWithInitialDDL(instanceName, databaseName),
},
{
ResourceName: "google_spanner_database.basic",
Expand Down Expand Up @@ -201,37 +125,49 @@ func testAccCheckSpannerDatabaseDestroy(s *terraform.State) error {
return nil
}

func testAccSpannerDatabase_basic(rnd string) string {
func testAccSpannerDatabase_basic(instanceName, databaseName string) string {
return fmt.Sprintf(`
resource "google_spanner_instance" "basic" {
name = "my-instance-%s"
name = "%s"
config = "regional-us-central1"
display_name = "my-displayname-%s"
display_name = "display-%s"
num_nodes = 1
}
resource "google_spanner_database" "basic" {
instance = "${google_spanner_instance.basic.name}"
name = "my-db-%s"
name = "%s"
}
`, rnd, rnd, rnd)
`, instanceName, instanceName, databaseName)
}

func testAccSpannerDatabase_basicWithInitialDDL(rnd string) string {
func testAccSpannerDatabase_basicWithInitialDDL(instanceName, databaseName string) string {
return fmt.Sprintf(`
resource "google_spanner_instance" "basic" {
name = "my-instance-%s"
name = "%s"
config = "regional-us-central1"
display_name = "my-displayname-%s"
display_name = "display-%s"
num_nodes = 1
}
resource "google_spanner_database" "basic" {
instance = "${google_spanner_instance.basic.name}"
name = "my-db-%s"
name = "%s"
ddl = [
"CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)",
"CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)" ]
}
`, rnd, rnd, rnd)
`, instanceName, instanceName, databaseName)
}

// Unit Tests for type spannerDatabaseId
func TestDatabaseNameForApi(t *testing.T) {
id := spannerDatabaseId{
Project: "project123",
Instance: "instance456",
Database: "db789",
}
actual := id.databaseUri()
expected := "projects/project123/instances/instance456/databases/db789"
expectEquals(t, expected, actual)
}

0 comments on commit e6ddd16

Please sign in to comment.