Make generic secrets importable.

This involves turning "allow_read" into "disable_read", because we have
no way to set "allow_read" to true when importing, and we can't import
if we can't read. This requires a migration and will require users to
update their config files.

vault/import_generic_secret_test.go

@@ -0,0 +1,27 @@
+package vault
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/acctest"
+	"github.com/hashicorp/terraform/helper/resource"
+)
+
+func TestAccGenericSecret_importBasic(t *testing.T) {
+	path := acctest.RandomWithPrefix("secret/test-")
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testResourceGenericSecret_initialConfig(path),
+				Check:  testResourceGenericSecret_initialCheck(path),
+			},
+			{
+				ResourceName:      "vault_generic_secret.test",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}

vault/resource_generic_secret.go

@@ -12,10 +12,16 @@ import (
 
 func genericSecretResource() *schema.Resource {
 	return &schema.Resource{
+		SchemaVersion: 1,
+
 		Create: genericSecretResourceWrite,
 		Update: genericSecretResourceWrite,
 		Delete: genericSecretResourceDelete,
 		Read:   genericSecretResourceRead,
+		Importer: &schema.ResourceImporter{
+			State: schema.ImportStatePassthrough,
+		},
+		MigrateState: resourceGenericSecretMigrateState,
 
 		Schema: map[string]*schema.Schema{
 			"path": &schema.Schema{
@@ -34,16 +40,16 @@ func genericSecretResource() *schema.Resource {
 				// We rebuild the attached JSON string to a simple singleline
 				// string. This makes terraform not want to change when an extra
 				// space is included in the JSON string. It is also necesarry
-				// when allow_read is true for comparing values.
+				// when disable_read is false for comparing values.
 				StateFunc:    NormalizeDataJSON,
 				ValidateFunc: ValidateDataJSON,
 			},
 
-			"allow_read": &schema.Schema{
+			"disable_read": &schema.Schema{
 				Type:        schema.TypeBool,
 				Optional:    true,
 				Default:     false,
-				Description: "True if the provided token is allowed to read the secret from vault",
+				Description: "Don't attempt to read the token from Vault if true; drift won't be detected.",
 			},
 		},
 	}
@@ -99,7 +105,7 @@ func genericSecretResourceWrite(d *schema.ResourceData, meta interface{}) error
 
 	d.SetId(path)
 
-	return nil
+	return genericSecretResourceRead(d, meta)
 }
 
 func genericSecretResourceDelete(d *schema.ResourceData, meta interface{}) error {
@@ -117,10 +123,10 @@ func genericSecretResourceDelete(d *schema.ResourceData, meta interface{}) error
 }
 
 func genericSecretResourceRead(d *schema.ResourceData, meta interface{}) error {
-	allowed_to_read := d.Get("allow_read").(bool)
-	path := d.Get("path").(string)
+	shouldRead := !d.Get("disable_read").(bool)
+	path := d.Id()
 
-	if allowed_to_read {
+	if shouldRead {
 		client := meta.(*api.Client)
 
 		log.Printf("[DEBUG] Reading %s from Vault", path)
@@ -129,15 +135,17 @@ func genericSecretResourceRead(d *schema.ResourceData, meta interface{}) error {
 			return fmt.Errorf("error reading from Vault: %s", err)
 		}
 
+		log.Printf("[DEBUG] secret: %#v", secret)
+
 		jsonDataBytes, err := json.Marshal(secret.Data)
 		if err != nil {
 			return fmt.Errorf("Error marshaling JSON for %q: %s", path, err)
 		}
 		d.Set("data_json", string(jsonDataBytes))
+		d.Set("path", path)
 	} else {
-		log.Printf("[WARN] vault_generic_secret does not automatically refresh if allow_read is set to false")
+		log.Printf("[WARN] vault_generic_secret does not refresh when disable_read is set to true")
 	}
-
-	d.SetId(path)
+	d.Set("disable_read", !shouldRead)
 	return nil
 }

vault/resource_generic_secret_migrate.go

@@ -0,0 +1,37 @@
+package vault
+
+import (
+	"fmt"
+	"log"
+
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func resourceGenericSecretMigrateState(v int, s *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
+	if s.Empty() {
+		log.Println("[DEBUG] Empty InstanceState; nothing to migrate.")
+		return s, nil
+	}
+
+	switch v {
+	case 0:
+		log.Println("[INFO] Found Vault Generic Secret state v0; migrating to v1")
+		s, err := migrateGenericSecretStateV0toV1(s)
+		return s, err
+	default:
+		return s, fmt.Errorf("Unexpected schema version: %d", v)
+	}
+}
+
+func migrateGenericSecretStateV0toV1(s *terraform.InstanceState) (*terraform.InstanceState, error) {
+	log.Printf("[DEBUG] Attributes before migration: %#v", s.Attributes)
+
+	disabledRead := s.Attributes["allow_read"] != "true"
+	if disabledRead {
+		s.Attributes["disable_read"] = "true"
+	}
+	delete(s.Attributes, "allow_read")
+
+	log.Printf("[DEBUG] Attributes after migration: %#v:", s.Attributes)
+	return s, nil
+}

vault/resource_generic_secret_migrate_test.go

@@ -0,0 +1,70 @@
+package vault
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func TestGenericSecretMigrateState(t *testing.T) {
+	cases := map[string]struct {
+		StateVersion int
+		Attributes   map[string]string
+		Expected     map[string]string
+	}{
+		"unset allow_read to disable_read": {
+			StateVersion: 0,
+			Attributes: map[string]string{
+				"data_json": `{"hello": "world"}`,
+				"path":      "secret/test-123",
+			},
+			Expected: map[string]string{
+				"data_json": `{"hello": "world"}`,
+				"path":      "secret/test-123",
+			},
+		},
+		"allow_read false to disable_read": {
+			StateVersion: 0,
+			Attributes: map[string]string{
+				"data_json":  `{"hello": "world"}`,
+				"path":       "secret/test-123",
+				"allow_read": "false",
+			},
+			Expected: map[string]string{
+				"data_json":    `{"hello": "world"}`,
+				"path":         "secret/test-123",
+				"disable_read": "true",
+			},
+		},
+		"allow_read true to disable_read": {
+			StateVersion: 0,
+			Attributes: map[string]string{
+				"data_json":  `{"hello": "world"}`,
+				"path":       "secret/test-123",
+				"allow_read": "true",
+			},
+			Expected: map[string]string{
+				"data_json": `{"hello": "world"}`,
+				"path":      "secret/test-123",
+			},
+		},
+	}
+
+	for tn, tc := range cases {
+		is, err := resourceGenericSecretMigrateState(
+			tc.StateVersion, &terraform.InstanceState{
+				ID:         tc.Attributes["path"],
+				Attributes: tc.Attributes,
+			}, nil)
+
+		if err != nil {
+			t.Fatalf("Unexpected error for migration %q: %+v", tn, err)
+		}
+
+		for k, v := range tc.Expected {
+			if is.Attributes[k] != v {
+				t.Fatalf("Expected %q to be %v for %q, got %v", k, v, tn, is.Attributes[k])
+			}
+		}
+	}
+}

vault/resource_generic_secret_test.go

@@ -4,81 +4,83 @@ import (
 	"fmt"
 	"testing"
 
-	r "github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/helper/acctest"
+	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 
 	"github.com/hashicorp/vault/api"
 )
 
 func TestResourceGenericSecret(t *testing.T) {
-	r.Test(t, r.TestCase{
+	path := acctest.RandomWithPrefix("secret/test")
+	resource.Test(t, resource.TestCase{
 		Providers: testProviders,
 		PreCheck:  func() { testAccPreCheck(t) },
-		Steps: []r.TestStep{
-			r.TestStep{
-				Config: testResourceGenericSecret_initialConfig,
-				Check:  testResourceGenericSecret_initialCheck,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testResourceGenericSecret_initialConfig(path),
+				Check:  testResourceGenericSecret_initialCheck(path),
 			},
-			r.TestStep{
+			resource.TestStep{
 				Config: testResourceGenericSecret_updateConfig,
 				Check:  testResourceGenericSecret_updateCheck,
 			},
 		},
 	})
 }
 
-var testResourceGenericSecret_initialConfig = `
-
+func testResourceGenericSecret_initialConfig(path string) string {
+	return fmt.Sprintf(`
 resource "vault_generic_secret" "test" {
-    path = "secret/foo"
-    allow_read = true
+    path = "%s"
     data_json = <<EOT
 {
     "zip": "zap"
 }
 EOT
+}`, path)
 }
 
-`
-
-func testResourceGenericSecret_initialCheck(s *terraform.State) error {
-	resourceState := s.Modules[0].Resources["vault_generic_secret.test"]
-	if resourceState == nil {
-		return fmt.Errorf("resource not found in state")
-	}
-
-	instanceState := resourceState.Primary
-	if instanceState == nil {
-		return fmt.Errorf("resource has no primary instance")
-	}
-
-	path := instanceState.ID
-
-	if path != instanceState.Attributes["path"] {
-		return fmt.Errorf("id doesn't match path")
+func testResourceGenericSecret_initialCheck(expectedPath string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		resourceState := s.Modules[0].Resources["vault_generic_secret.test"]
+		if resourceState == nil {
+			return fmt.Errorf("resource not found in state")
+		}
+
+		instanceState := resourceState.Primary
+		if instanceState == nil {
+			return fmt.Errorf("resource has no primary instance")
+		}
+
+		path := instanceState.ID
+
+		if path != instanceState.Attributes["path"] {
+			return fmt.Errorf("id doesn't match path")
+		}
+		if path != expectedPath {
+			return fmt.Errorf("unexpected secret path")
+		}
+
+		client := testProvider.Meta().(*api.Client)
+		secret, err := client.Logical().Read(path)
+		if err != nil {
+			return fmt.Errorf("error reading back secret: %s", err)
+		}
+
+		if got, want := secret.Data["zip"], "zap"; got != want {
+			return fmt.Errorf("'zip' data is %q; want %q", got, want)
+		}
+
+		return nil
 	}
-	if path != "secret/foo" {
-		return fmt.Errorf("unexpected secret path")
-	}
-
-	client := testProvider.Meta().(*api.Client)
-	secret, err := client.Logical().Read(path)
-	if err != nil {
-		return fmt.Errorf("error reading back secret: %s", err)
-	}
-
-	if got, want := secret.Data["zip"], "zap"; got != want {
-		return fmt.Errorf("'zip' data is %q; want %q", got, want)
-	}
-
-	return nil
 }
 
 var testResourceGenericSecret_updateConfig = `
 
 resource "vault_generic_secret" "test" {
     path = "secret/foo"
-    allow_read = true
+    disable_read = false
     data_json = <<EOT
 {
     "zip": "zoop"