Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provider/google: Add import support to google_sql_user #14457

32 changes: 32 additions & 0 deletions builtin/providers/google/import_sql_user_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package google

import (
"testing"

"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
)

func TestAccGoogleSqlUser_importBasic(t *testing.T) {
resourceName := "google_sql_user.user"
user := acctest.RandString(10)
instance := acctest.RandString(10)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccGoogleSqlUserDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testGoogleSqlUser_basic(instance, user),
},

resource.TestStep{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"password"},
},
},
})
}
40 changes: 29 additions & 11 deletions builtin/providers/google/resource_sql_user.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package google

import (
"errors"
"fmt"
"log"
"strings"

"github.com/hashicorp/terraform/helper/schema"

@@ -15,6 +17,9 @@ func resourceSqlUser() *schema.Resource {
Read: resourceSqlUserRead,
Update: resourceSqlUserUpdate,
Delete: resourceSqlUserDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"host": &schema.Schema{
@@ -36,8 +41,9 @@ func resourceSqlUser() *schema.Resource {
},

"password": &schema.Schema{
Type: schema.TypeString,
Required: true,
Type: schema.TypeString,
Required: true,
Sensitive: true,
},

"project": &schema.Schema{
@@ -77,6 +83,8 @@ func resourceSqlUserCreate(d *schema.ResourceData, meta interface{}) error {
"user %s into instance %s: %s", name, instance, err)
}

d.SetId(fmt.Sprintf("%s.%s", instance, name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I take back part of what I told you in person. We shouldn't change the format of the ID, since that will break refreshing (since the ID in state will still be the old version). At this point we have three possible solutions:

  1. Write a migration function to change all the IDs in state. I think it would work, but migration functions are kind of a pain.
  2. Do the ID parsing in the StateFn, so instead of doing a schema.ImportStatePassthrough, create a custom function that parses the ID and then sets the two properties in the state before it gets passed to the read fn.
  3. Create a custom function that reads all available instances, and then tries each one to see if the user belongs to that instance (see resource_pagerduty_service_integration for an example). This is actually less bad than I initially made it out to be. Since Terraform is currently storing the user name as the ID, it's already not allowing multiple users with the same name in different instances. And so a naive import should fail in this case anyway. However the point could be made that Terraform should allow users with the same name in different instances.

I'm not sure yet which the best solution is. I'll consult with Hashicorp people and get back to you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, the experts have spoken and recommended solution 1. I can explain migration fns to you once I'm in the office, or feel free to give it a try on your own (I actually don't think it'll be much of a pain, this will be a pretty straightforward one)


err = sqladminOperationWait(config, op, "Insert User")

if err != nil {
@@ -95,32 +103,42 @@ func resourceSqlUserRead(d *schema.ResourceData, meta interface{}) error {
return err
}

name := d.Get("name").(string)
instance := d.Get("instance").(string)
instanceAndName := strings.SplitN(d.Id(), ".", 2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly nervous about using dots as a separator. Even though this set of properties is safe, I can imagine a resource that needs to be imported in a similar way but dots are acceptable characters in names for both types of resources, and then we would have an inconsistency between resources. I think a "/" would be safer, since I'd be really surprised if it ever came up as a separator (because of weirdness in self_links). What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense - "/" is probably a safer character to use than ".".

Done.

if len(instanceAndName) != 2 {
return errors.New(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmt.Errorf :D

fmt.Sprintf(
"Wrong number of arguments when specifying imported id. Expected: 2. Saw: %d. Expected Input: $INSTANCENAME.$SQLUSERNAME Input: %s",
len(instanceAndName),
d.Id()))
}

instance := instanceAndName[0]
name := instanceAndName[1]

users, err := config.clientSqlAdmin.Users.List(project, instance).Do()

if err != nil {
return handleNotFoundError(err, d, fmt.Sprintf("SQL User %q in instance %q", name, instance))
}

found := false
for _, user := range users.Items {
if user.Name == name {
found = true
var user *sqladmin.User
for _, currentUser := range users.Items {
if currentUser.Name == name {
user = currentUser
break
}
}

if !found {
if user == nil {
log.Printf("[WARN] Removing SQL User %q because it's gone", d.Get("name").(string))
d.SetId("")

return nil
}

d.SetId(name)

d.Set("host", user.Host)
d.Set("instance", user.Instance)
d.Set("name", user.Name)
return nil
}

17 changes: 16 additions & 1 deletion website/source/docs/providers/google/r/sql_user.html.markdown
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@ description: |-
Creates a new Google SQL User on a Google SQL User Instance. For more information, see the [official documentation](https://cloud.google.com/sql/), or the [JSON API](https://cloud.google.com/sql/docs/admin-api/v1beta4/users).

~> **Note:** All arguments including the username and password will be stored in the raw state as plain-text.
[Read more about sensitive data in state](/docs/state/sensitive-data.html).
[Read more about sensitive data in state](/docs/state/sensitive-data.html). Passwords will not be retrieved when running
"terraform import".

## Example Usage

@@ -57,3 +58,17 @@ The following arguments are supported:
## Attributes Reference

Only the arguments listed above are exposed as attributes.

## Import Format

Importing an SQL user is formatted as:

```bash
terraform import google_sql_user.$RESOURCENAME $INSTANCENAME.$SQLUSERNAME
```

For example, the sample at the top of this page could be imported with:

```bash
terraform import google_sql_user.users master-instance.me
```