Merge pull request #16865 from hashicorp/jbardin/gcs

gcs provider fixes

backend/remote-state/gcs/backend.go

@@ -3,14 +3,17 @@ package gcs
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"strings"
 
 	"cloud.google.com/go/storage"
 	"github.com/hashicorp/terraform/backend"
+	"github.com/hashicorp/terraform/helper/pathorcontents"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/terraform"
+	"golang.org/x/oauth2/jwt"
 	"google.golang.org/api/option"
 )
 
@@ -111,16 +114,39 @@ func (b *gcsBackend) configure(ctx context.Context) error {
 		b.region = r
 	}
 
-	opts := []option.ClientOption{
+	var opts []option.ClientOption
-		option.WithScopes(storage.ScopeReadWrite),
+
-		option.WithUserAgent(terraform.UserAgentString()),
+	creds := data.Get("credentials").(string)
-	}
+	if creds == "" {
-	if credentialsFile := data.Get("credentials").(string); credentialsFile != "" {
+		creds = os.Getenv("GOOGLE_CREDENTIALS")
-		opts = append(opts, option.WithCredentialsFile(credentialsFile))
-	} else if credentialsFile := os.Getenv("GOOGLE_CREDENTIALS"); credentialsFile != "" {
-		opts = append(opts, option.WithCredentialsFile(credentialsFile))
 	}
 
+	if creds != "" {
+		var account accountFile
+
+		// to mirror how the provider works, we accept the file path or the contents
+		contents, _, err := pathorcontents.Read(creds)
+		if err != nil {
+			return fmt.Errorf("Error loading credentials: %s", err)
+		}
+
+		if err := json.Unmarshal([]byte(contents), &account); err != nil {
+			return fmt.Errorf("Error parsing credentials '%s': %s", contents, err)
+		}
+
+		conf := jwt.Config{
+			Email:      account.ClientEmail,
+			PrivateKey: []byte(account.PrivateKey),
+			Scopes:     []string{storage.ScopeReadWrite},
+			TokenURL:   "https://accounts.google.com/o/oauth2/token",
+		}
+
+		opts = append(opts, option.WithHTTPClient(conf.Client(ctx)))
+	} else {
+		opts = append(opts, option.WithScopes(storage.ScopeReadWrite))
+	}
+
+	opts = append(opts, option.WithUserAgent(terraform.UserAgentString()))
 	client, err := storage.NewClient(b.storageContext, opts...)
 	if err != nil {
 		return fmt.Errorf("storage.NewClient() failed: %v", err)
@@ -128,22 +154,13 @@ func (b *gcsBackend) configure(ctx context.Context) error {
 
 	b.storageClient = client
 
-	return b.ensureBucketExists()
+	return nil
 }
 
-func (b *gcsBackend) ensureBucketExists() error {
+// accountFile represents the structure of the account file JSON file.
-	_, err := b.storageClient.Bucket(b.bucketName).Attrs(b.storageContext)
+type accountFile struct {
-	if err != storage.ErrBucketNotExist {
+	PrivateKeyId string `json:"private_key_id"`
-		return err
+	PrivateKey   string `json:"private_key"`
-	}
+	ClientEmail  string `json:"client_email"`
-
+	ClientId     string `json:"client_id"`
-	if b.projectID == "" {
-		return fmt.Errorf("bucket %q does not exist; specify the \"project\" option or create the bucket manually using `gsutil mb gs://%s`", b.bucketName, b.bucketName)
-	}
-
-	attrs := &storage.BucketAttrs{
-		Location: b.region,
-	}
-
-	return b.storageClient.Bucket(b.bucketName).Create(b.storageContext, b.projectID, attrs)
 }

backend/remote-state/gcs/backend_test.go

@@ -2,10 +2,13 @@ package gcs
 
 import (
 	"fmt"
+	"log"
 	"os"
 	"strings"
 	"testing"
+	"time"
 
+	"cloud.google.com/go/storage"
 	"github.com/hashicorp/terraform/backend"
 	"github.com/hashicorp/terraform/state/remote"
 )
@@ -48,7 +51,8 @@ func TestStateFile(t *testing.T) {
 func TestRemoteClient(t *testing.T) {
 	t.Parallel()
 
-	be := setupBackend(t, noPrefix)
+	bucket := bucketName(t)
+	be := setupBackend(t, bucket, noPrefix)
 	defer teardownBackend(t, be, noPrefix)
 
 	ss, err := be.State(backend.DefaultStateName)
@@ -67,7 +71,8 @@ func TestRemoteClient(t *testing.T) {
 func TestRemoteLocks(t *testing.T) {
 	t.Parallel()
 
-	be := setupBackend(t, noPrefix)
+	bucket := bucketName(t)
+	be := setupBackend(t, bucket, noPrefix)
 	defer teardownBackend(t, be, noPrefix)
 
 	remoteClient := func() (remote.Client, error) {
@@ -99,10 +104,12 @@ func TestRemoteLocks(t *testing.T) {
 func TestBackend(t *testing.T) {
 	t.Parallel()
 
-	be0 := setupBackend(t, noPrefix)
+	bucket := bucketName(t)
+
+	be0 := setupBackend(t, bucket, noPrefix)
 	defer teardownBackend(t, be0, noPrefix)
 
-	be1 := setupBackend(t, noPrefix)
+	be1 := setupBackend(t, bucket, noPrefix)
 
 	backend.TestBackend(t, be0, be1)
 }
@@ -110,17 +117,18 @@ func TestBackendWithPrefix(t *testing.T) {
 	t.Parallel()
 
 	prefix := "test/prefix"
+	bucket := bucketName(t)
 
-	be0 := setupBackend(t, prefix)
+	be0 := setupBackend(t, bucket, prefix)
 	defer teardownBackend(t, be0, prefix)
 
-	be1 := setupBackend(t, prefix+"/")
+	be1 := setupBackend(t, bucket, prefix+"/")
 
 	backend.TestBackend(t, be0, be1)
 }
 
 // setupBackend returns a new GCS backend.
-func setupBackend(t *testing.T, prefix string) backend.Backend {
+func setupBackend(t *testing.T, bucket, prefix string) backend.Backend {
 	t.Helper()
 
 	projectID := os.Getenv("GOOGLE_PROJECT")
@@ -132,92 +140,67 @@ func setupBackend(t *testing.T, prefix string) backend.Backend {
 
 	config := map[string]interface{}{
 		"project": projectID,
-		"bucket":  toBucketName(projectID + "-" + t.Name()),
+		"bucket":  bucket,
 		"prefix":  prefix,
 	}
 
-	if creds := os.Getenv("GOOGLE_CREDENTIALS"); creds != "" {
+	b := backend.TestBackendConfig(t, New(), config)
-		config["credentials"] = creds
+	be := b.(*gcsBackend)
-		t.Logf("using credentials from %q", creds)
+
-	} else {
+	// create the bucket if it doesn't exist
-		t.Log("using default credentials; set GOOGLE_CREDENTIALS for custom credentials")
+	bkt := be.storageClient.Bucket(bucket)
+	_, err := bkt.Attrs(be.storageContext)
+	if err != nil {
+		if err != storage.ErrBucketNotExist {
+			t.Fatal(err)
+		}
+
+		attrs := &storage.BucketAttrs{
+			Location: be.region,
+		}
+		err := bkt.Create(be.storageContext, be.projectID, attrs)
+		if err != nil {
+			t.Fatal(err)
+		}
 	}
 
-	return backend.TestBackendConfig(t, New(), config)
+	return b
 }
 
 // teardownBackend deletes all states from be except the default state.
 func teardownBackend(t *testing.T, be backend.Backend, prefix string) {
 	t.Helper()
-
-	// Delete all states. The bucket must be empty before it can be deleted.
-	states, err := be.States()
-	if err != nil {
-		t.Fatalf("be.States() = %v; manual clean-up may be required", err)
-	}
-	for _, st := range states {
-		if st == backend.DefaultStateName {
-			continue
-		}
-		if err := be.DeleteState(st); err != nil {
-			t.Fatalf("be.DeleteState(%q) = %v; manual clean-up may be required", st, err)
-		}
-	}
-
 	gcsBE, ok := be.(*gcsBackend)
 	if !ok {
 		t.Fatalf("be is a %T, want a *gcsBackend", be)
 	}
 	ctx := gcsBE.storageContext
 
-	// Delete the default state, which DeleteState() will refuse to do.
+	bucket := gcsBE.storageClient.Bucket(gcsBE.bucketName)
-	// It's okay if this fails, not all tests create a default state.
+	objs := bucket.Objects(ctx, nil)
-	ds := "default.tfstate"
+
-	if prefix != "" {
+	for o, err := objs.Next(); err == nil; o, err = objs.Next() {
-		ds = fmt.Sprintf("%s/%s", prefix, ds)
+		if err := bucket.Object(o.Name).Delete(ctx); err != nil {
-	}
+			log.Printf("Error trying to delete object: %s %s\n\n", o.Name, err)
-	if err := gcsBE.storageClient.Bucket(gcsBE.bucketName).Object(ds).Delete(ctx); err != nil {
+		} else {
-		t.Logf("deleting \"%s\": %v; manual clean-up may be required", ds, err)
+			log.Printf("Object deleted: %s", o.Name)
+		}
 	}
 
 	// Delete the bucket itself.
-	if err := gcsBE.storageClient.Bucket(gcsBE.bucketName).Delete(ctx); err != nil {
+	if err := bucket.Delete(ctx); err != nil {
-		t.Fatalf("deleting bucket failed: %v; manual cleanup may be required, though later test runs will happily reuse an existing bucket", err)
+		t.Errorf("deleting bucket %q failed, manual cleanup may be required: %v", gcsBE.bucketName, err)
 	}
 }
 
-// toBucketName returns a copy of in that is suitable for use as a bucket name.
+// bucketName returns a valid bucket name for this test.
-// All upper case characters are converted to lower case, other invalid
+func bucketName(t *testing.T) string {
-// characters are replaced by '_'.
+	name := fmt.Sprintf("tf-%x-%s", time.Now().UnixNano(), t.Name())
-func toBucketName(in string) string {
-	// Bucket names must contain only lowercase letters, numbers, dashes
-	// (-), and underscores (_).
-	isValid := func(r rune) bool {
-		switch {
-		case r >= 'a' && r <= 'z':
-			return true
-		case r >= '0' && r <= '9':
-			return true
-		case r == '-' || r == '_':
-			return true
-		default:
-			return false
-
-		}
-	}
-
-	out := make([]rune, 0, len(in))
-	for _, r := range strings.ToLower(in) {
-		if !isValid(r) {
-			r = '_'
-		}
-		out = append(out, r)
-	}
 
 	// Bucket names must contain 3 to 63 characters.
-	if len(out) > 63 {
+	if len(name) > 63 {
-		out = out[:63]
+		name = name[:63]
 	}
 
-	return string(out)
+	return strings.ToLower(name)
 }

backend/testing.go

@@ -281,6 +281,14 @@ func testBackendStateLock(t *testing.T, b1, b2 Backend) {
 		t.Fatal("unable to get initial lock:", err)
 	}
 
+	// Make sure we can still get the state.State from another instance even
+	// when locked.  This should only happen when a state is loaded via the
+	// backend, and as a remote state.
+	_, err = b2.State(DefaultStateName)
+	if err != nil {
+		t.Fatalf("failed to read locked state from another backend instance: %s", err)
+	}
+
 	// If the lock ID is blank, assume locking is disabled
 	if lockIDA == "" {
 		t.Logf("TestBackend: %T: empty string returned for lock, assuming disabled", b1)