Merge pull request #8666 from hashicorp/f-postgres-cleanup

provider/postgres: Clean up definitions and errors

builtin/providers/postgresql/config.go

@@ -22,7 +22,7 @@ type Client struct {
 	connStr  string
 }
 
-//NewClient returns new client config
+// NewClient returns new client config
 func (c *Config) NewClient() (*Client, error) {
 	connStr := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=postgres sslmode=%s", c.Host, c.Port, c.Username, c.Password, c.SslMode)
 
@@ -34,11 +34,12 @@ func (c *Config) NewClient() (*Client, error) {
 	return &client, nil
 }
 
-//Connect will manually connect/diconnect to prevent a large number or db connections being made
+// Connect will manually connect/disconnect to prevent a large
+// number or db connections being made
 func (c *Client) Connect() (*sql.DB, error) {
 	db, err := sql.Open("postgres", c.connStr)
 	if err != nil {
-		return nil, fmt.Errorf("Error connecting to postgresql server: %s", err)
+		return nil, fmt.Errorf("Error connecting to PostgreSQL server: %s", err)
 	}
 
 	return db, nil

builtin/providers/postgresql/provider.go

@@ -1,8 +1,7 @@
 package postgresql
 
 import (
-	"fmt"
-
+	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -11,41 +10,41 @@ import (
 func Provider() terraform.ResourceProvider {
 	return &schema.Provider{
 		Schema: map[string]*schema.Schema{
-			"host": &schema.Schema{
+			"host": {
 				Type:        schema.TypeString,
 				Required:    true,
-				DefaultFunc: schema.EnvDefaultFunc("POSTGRESQL_HOST", nil),
-				Description: "The postgresql server address",
+				DefaultFunc: schema.MultiEnvDefaultFunc([]string{"PGHOST", "POSTGRESQL_HOST"}, nil),
+				Description: "The PostgreSQL server address",
 			},
-			"port": &schema.Schema{
+			"port": {
 				Type:        schema.TypeInt,
 				Optional:    true,
 				Default:     5432,
-				Description: "The postgresql server port",
+				Description: "The PostgreSQL server port",
 			},
-			"username": &schema.Schema{
+			"username": {
 				Type:        schema.TypeString,
 				Required:    true,
-				DefaultFunc: schema.EnvDefaultFunc("POSTGRESQL_USERNAME", nil),
-				Description: "Username for postgresql server connection",
+				DefaultFunc: schema.MultiEnvDefaultFunc([]string{"PGUSER", "POSTGRESQL_USER"}, nil),
+				Description: "Username for PostgreSQL server connection",
 			},
-			"password": &schema.Schema{
+			"password": {
 				Type:        schema.TypeString,
-				Required:    true,
-				DefaultFunc: schema.EnvDefaultFunc("POSTGRESQL_PASSWORD", nil),
-				Description: "Password for postgresql server connection",
+				Optional:    true,
+				DefaultFunc: schema.MultiEnvDefaultFunc([]string{"PGPASSWORD", "POSTGRESQL_PASSWORD"}, nil),
+				Description: "Password for PostgreSQL server connection",
 			},
-			"ssl_mode": &schema.Schema{
+			"ssl_mode": {
 				Type:        schema.TypeString,
 				Optional:    true,
-				Default:     "prefer",
-				Description: "Connection mode for postgresql server",
+				DefaultFunc: schema.EnvDefaultFunc("PGSSLMODE", "require"),
+				Description: "Connection mode for PostgreSQL server",
 			},
 		},
 
 		ResourcesMap: map[string]*schema.Resource{
-			"postgresql_database": resourcePostgresqlDatabase(),
-			"postgresql_role":     resourcePostgresqlRole(),
+			"postgresql_database": resourcePostgreSQLDatabase(),
+			"postgresql_role":     resourcePostgreSQLRole(),
 		},
 
 		ConfigureFunc: providerConfigure,
@@ -63,7 +62,7 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 
 	client, err := config.NewClient()
 	if err != nil {
-		return nil, fmt.Errorf("Error initializing Postgresql client: %s", err)
+		return nil, errwrap.Wrapf("Error initializing PostgreSQL client: %s", err)
 	}
 
 	return client, nil

builtin/providers/postgresql/provider_test.go

@@ -29,13 +29,14 @@ func TestProvider_impl(t *testing.T) {
 }
 
 func testAccPreCheck(t *testing.T) {
-	if v := os.Getenv("POSTGRESQL_HOST"); v == "" {
-		t.Fatal("POSTGRESQL_HOST must be set for acceptance tests")
+	var host string
+	if host = os.Getenv("PGHOST"); host == "" {
+		t.Fatal("PGHOST must be set for acceptance tests")
 	}
-	if v := os.Getenv("POSTGRESQL_USERNAME"); v == "" {
-		t.Fatal("POSTGRESQL_USERNAME must be set for acceptance tests")
+	if v := os.Getenv("PGUSER"); v == "" {
+		t.Fatal("PGUSER must be set for acceptance tests")
 	}
-	if v := os.Getenv("POSTGRESQL_PASSWORD"); v == "" {
-		t.Fatal("POSTGRESQL_PASSWORD must be set for acceptance tests")
+	if v := os.Getenv("PGPASSWORD"); v == "" && host != "localhost" {
+		t.Fatal("PGPASSWORD must be set for acceptance tests if PGHOST is not localhost")
 	}
 }

builtin/providers/postgresql/resource_postgresql_database.go

@@ -5,38 +5,38 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/lib/pq"
 )
 
-func resourcePostgresqlDatabase() *schema.Resource {
+func resourcePostgreSQLDatabase() *schema.Resource {
 	return &schema.Resource{
-		Create: resourcePostgresqlDatabaseCreate,
-		Read:   resourcePostgresqlDatabaseRead,
-		Update: resourcePostgresqlDatabaseUpdate,
-		Delete: resourcePostgresqlDatabaseDelete,
+		Create: resourcePostgreSQLDatabaseCreate,
+		Read:   resourcePostgreSQLDatabaseRead,
+		Update: resourcePostgreSQLDatabaseUpdate,
+		Delete: resourcePostgreSQLDatabaseDelete,
 
 		Schema: map[string]*schema.Schema{
-			"name": &schema.Schema{
+			"name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
-			"owner": &schema.Schema{
+			"owner": {
 				Type:     schema.TypeString,
 				Optional: true,
-				ForceNew: false,
 				Computed: true,
 			},
 		},
 	}
 }
 
-func resourcePostgresqlDatabaseCreate(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLDatabaseCreate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
-		return err
+		return errwrap.Wrapf("Error connecting to PostgreSQL: {{err}}", err)
 	}
 	defer conn.Close()
 
@@ -60,19 +60,19 @@ func resourcePostgresqlDatabaseCreate(d *schema.ResourceData, meta interface{})
 	query := fmt.Sprintf("CREATE DATABASE %s %s", pq.QuoteIdentifier(dbName), dbOwnerCfg)
 	_, err = conn.Query(query)
 	if err != nil {
-		return fmt.Errorf("Error creating postgresql database %s: %s", dbName, err)
+		return errwrap.Wrapf(fmt.Sprintf("Error creating database %s: {{err}}", dbName), err)
 	}
 
 	d.SetId(dbName)
 
-	return nil
+	return resourcePostgreSQLDatabaseRead(d, meta)
 }
 
-func resourcePostgresqlDatabaseDelete(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLDatabaseDelete(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
-		return err
+		return errwrap.Wrapf("Error connecting to PostgreSQL: {{err}}", err)
 	}
 	defer conn.Close()
 
@@ -88,7 +88,7 @@ func resourcePostgresqlDatabaseDelete(d *schema.ResourceData, meta interface{})
 	query := fmt.Sprintf("DROP DATABASE %s", pq.QuoteIdentifier(dbName))
 	_, err = conn.Query(query)
 	if err != nil {
-		return err
+		return errwrap.Wrapf("Error dropping database: {{err}}", err)
 	}
 
 	d.SetId("")
@@ -96,7 +96,7 @@ func resourcePostgresqlDatabaseDelete(d *schema.ResourceData, meta interface{})
 	return nil
 }
 
-func resourcePostgresqlDatabaseRead(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLDatabaseRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
@@ -113,14 +113,14 @@ func resourcePostgresqlDatabaseRead(d *schema.ResourceData, meta interface{}) er
 		d.SetId("")
 		return nil
 	case err != nil:
-		return fmt.Errorf("Error reading info about database: %s", err)
+		return errwrap.Wrapf("Error reading database: {{err}}", err)
 	default:
 		d.Set("owner", owner)
 		return nil
 	}
 }
 
-func resourcePostgresqlDatabaseUpdate(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLDatabaseUpdate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
@@ -136,12 +136,12 @@ func resourcePostgresqlDatabaseUpdate(d *schema.ResourceData, meta interface{})
 			query := fmt.Sprintf("ALTER DATABASE %s OWNER TO %s", pq.QuoteIdentifier(dbName), pq.QuoteIdentifier(owner))
 			_, err := conn.Query(query)
 			if err != nil {
-				return fmt.Errorf("Error updating owner for database: %s", err)
+				return errwrap.Wrapf("Error updating owner: {{err}}", err)
 			}
 		}
 	}
 
-	return resourcePostgresqlDatabaseRead(d, meta)
+	return resourcePostgreSQLDatabaseRead(d, meta)
 }
 
 func grantRoleMembership(conn *sql.DB, dbOwner string, connUsername string) error {
@@ -153,7 +153,7 @@ func grantRoleMembership(conn *sql.DB, dbOwner string, connUsername string) erro
 			if strings.Contains(err.Error(), "duplicate key value violates unique constraint") {
 				return nil
 			}
-			return fmt.Errorf("Error granting membership: %s", err)
+			return errwrap.Wrapf("Error granting membership: {{err}}", err)
 		}
 	}
 	return nil

builtin/providers/postgresql/resource_postgresql_database_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"errors"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -16,10 +17,10 @@ func TestAccPostgresqlDatabase_Basic(t *testing.T) {
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckPostgresqlDatabaseDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccPostgresqlDatabaseConfig,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckPostgresqlDatabaseExists("postgresql_database.mydb", "myrole"),
+					testAccCheckPostgresqlDatabaseExists("postgresql_database.mydb"),
 					resource.TestCheckResourceAttr(
 						"postgresql_database.mydb", "name", "mydb"),
 					resource.TestCheckResourceAttr(
@@ -37,12 +38,14 @@ func TestAccPostgresqlDatabase_DefaultOwner(t *testing.T) {
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckPostgresqlDatabaseDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccPostgresqlDatabaseConfig,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckPostgresqlDatabaseExists("postgresql_database.mydb_default_owner", ""),
+					testAccCheckPostgresqlDatabaseExists("postgresql_database.mydb_default_owner"),
 					resource.TestCheckResourceAttr(
 						"postgresql_database.mydb_default_owner", "name", "mydb_default_owner"),
+					resource.TestCheckResourceAttrSet(
+						"postgresql_database.mydb_default_owner", "owner"),
 				),
 			},
 		},
@@ -64,14 +67,14 @@ func testAccCheckPostgresqlDatabaseDestroy(s *terraform.State) error {
 		}
 
 		if exists {
-			return fmt.Errorf("Db still exists after destroy")
+			return errors.New("Db still exists after destroy")
 		}
 	}
 
 	return nil
 }
 
-func testAccCheckPostgresqlDatabaseExists(n string, owner string) resource.TestCheckFunc {
+func testAccCheckPostgresqlDatabaseExists(n string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
 		if !ok {
@@ -79,12 +82,7 @@ func testAccCheckPostgresqlDatabaseExists(n string, owner string) resource.TestC
 		}
 
 		if rs.Primary.ID == "" {
-			return fmt.Errorf("No ID is set")
-		}
-
-		actualOwner := rs.Primary.Attributes["owner"]
-		if actualOwner != owner {
-			return fmt.Errorf("Wrong owner for db expected %s got %s", owner, actualOwner)
+			return errors.New("No ID is set")
 		}
 
 		client := testAccProvider.Meta().(*Client)
@@ -95,7 +93,7 @@ func testAccCheckPostgresqlDatabaseExists(n string, owner string) resource.TestC
 		}
 
 		if !exists {
-			return fmt.Errorf("Db not found")
+			return errors.New("Db not found")
 		}
 
 		return nil

builtin/providers/postgresql/resource_postgresql_role.go

@@ -4,35 +4,36 @@ import (
 	"database/sql"
 	"fmt"
 
+	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/lib/pq"
 )
 
-func resourcePostgresqlRole() *schema.Resource {
+func resourcePostgreSQLRole() *schema.Resource {
 	return &schema.Resource{
-		Create: resourcePostgresqlRoleCreate,
-		Read:   resourcePostgresqlRoleRead,
-		Update: resourcePostgresqlRoleUpdate,
-		Delete: resourcePostgresqlRoleDelete,
+		Create: resourcePostgreSQLRoleCreate,
+		Read:   resourcePostgreSQLRoleRead,
+		Update: resourcePostgreSQLRoleUpdate,
+		Delete: resourcePostgreSQLRoleDelete,
 
 		Schema: map[string]*schema.Schema{
-			"name": &schema.Schema{
+			"name": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 			},
-			"login": &schema.Schema{
+			"login": {
 				Type:     schema.TypeBool,
 				Optional: true,
 				ForceNew: false,
 				Default:  false,
 			},
-			"password": &schema.Schema{
+			"password": {
 				Type:     schema.TypeString,
 				Optional: true,
 				ForceNew: false,
 			},
-			"encrypted": &schema.Schema{
+			"encrypted": {
 				Type:     schema.TypeBool,
 				Optional: true,
 				ForceNew: false,
@@ -42,7 +43,7 @@ func resourcePostgresqlRole() *schema.Resource {
 	}
 }
 
-func resourcePostgresqlRoleCreate(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLRoleCreate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
@@ -59,15 +60,15 @@ func resourcePostgresqlRoleCreate(d *schema.ResourceData, meta interface{}) erro
 	query := fmt.Sprintf("CREATE ROLE %s %s %s PASSWORD '%s'", pq.QuoteIdentifier(roleName), loginAttr, encryptedCfg, password)
 	_, err = conn.Query(query)
 	if err != nil {
-		return fmt.Errorf("Error creating role: %s", err)
+		return errwrap.Wrapf("Error creating role: {{err}}", err)
 	}
 
 	d.SetId(roleName)
 
-	return nil
+	return resourcePostgreSQLRoleRead(d, meta)
 }
 
-func resourcePostgresqlRoleDelete(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLRoleDelete(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
@@ -80,7 +81,7 @@ func resourcePostgresqlRoleDelete(d *schema.ResourceData, meta interface{}) erro
 	query := fmt.Sprintf("DROP ROLE %s", pq.QuoteIdentifier(roleName))
 	_, err = conn.Query(query)
 	if err != nil {
-		return err
+		return errwrap.Wrapf("Error deleting role: {{err}}", err)
 	}
 
 	d.SetId("")
@@ -88,7 +89,7 @@ func resourcePostgresqlRoleDelete(d *schema.ResourceData, meta interface{}) erro
 	return nil
 }
 
-func resourcePostgresqlRoleRead(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLRoleRead(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
@@ -99,20 +100,20 @@ func resourcePostgresqlRoleRead(d *schema.ResourceData, meta interface{}) error
 	roleName := d.Get("name").(string)
 
 	var canLogin bool
-	err = conn.QueryRow("select rolcanlogin from pg_roles where rolname=$1", roleName).Scan(&canLogin)
+	err = conn.QueryRow("SELECT rolcanlogin FROM pg_roles WHERE rolname=$1", roleName).Scan(&canLogin)
 	switch {
 	case err == sql.ErrNoRows:
 		d.SetId("")
 		return nil
 	case err != nil:
-		return fmt.Errorf("Error reading info about role: %s", err)
+		return errwrap.Wrapf("Error reading role: {{err}}", err)
 	default:
 		d.Set("login", canLogin)
 		return nil
 	}
 }
 
-func resourcePostgresqlRoleUpdate(d *schema.ResourceData, meta interface{}) error {
+func resourcePostgreSQLRoleUpdate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
 	conn, err := client.Connect()
 	if err != nil {
@@ -129,7 +130,7 @@ func resourcePostgresqlRoleUpdate(d *schema.ResourceData, meta interface{}) erro
 		query := fmt.Sprintf("ALTER ROLE %s %s", pq.QuoteIdentifier(roleName), pq.QuoteIdentifier(loginAttr))
 		_, err := conn.Query(query)
 		if err != nil {
-			return fmt.Errorf("Error updating login attribute for role: %s", err)
+			return errwrap.Wrapf("Error updating login attribute for role: {{err}}", err)
 		}
 
 		d.SetPartial("login")
@@ -142,7 +143,7 @@ func resourcePostgresqlRoleUpdate(d *schema.ResourceData, meta interface{}) erro
 		query := fmt.Sprintf("ALTER ROLE %s %s PASSWORD '%s'", pq.QuoteIdentifier(roleName), encryptedCfg, password)
 		_, err := conn.Query(query)
 		if err != nil {
-			return fmt.Errorf("Error updating password attribute for role: %s", err)
+			return errwrap.Wrapf("Error updating password attribute for role: {{err}}", err)
 		}
 
 		d.SetPartial("password")
@@ -154,14 +155,14 @@ func resourcePostgresqlRoleUpdate(d *schema.ResourceData, meta interface{}) erro
 		query := fmt.Sprintf("ALTER ROLE %s %s PASSWORD '%s'", pq.QuoteIdentifier(roleName), encryptedCfg, password)
 		_, err := conn.Query(query)
 		if err != nil {
-			return fmt.Errorf("Error updating encrypted attribute for role: %s", err)
+			return errwrap.Wrapf("Error updating encrypted attribute for role: {{err}}", err)
 		}
 
 		d.SetPartial("encrypted")
 	}
 
 	d.Partial(false)
-	return resourcePostgresqlRoleRead(d, meta)
+	return resourcePostgreSQLRoleRead(d, meta)
 }
 
 func getLoginStr(canLogin bool) string {

builtin/providers/postgresql/resource_postgresql_role_test.go

@@ -16,7 +16,7 @@ func TestAccPostgresqlRole_Basic(t *testing.T) {
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckPostgresqlRoleDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccPostgresqlRoleConfig,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckPostgresqlRoleExists("postgresql_role.myrole2", "true"),