Merge pull request #13384 from hashicorp/s3-state-locks

Unify s3 client code

backend/remote-state/s3/backend.go

@@ -2,15 +2,9 @@ package s3
 
 import (
 	"context"
-	"fmt"
 
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
-	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/dynamodb"
 	"github.com/aws/aws-sdk-go/service/s3"
-	cleanhttp "github.com/hashicorp/go-cleanhttp"
-	multierror "github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/backend"
 	"github.com/hashicorp/terraform/helper/schema"
 
@@ -175,48 +169,27 @@ func (b *Backend) configure(ctx context.Context) error {
 	b.kmsKeyID = data.Get("kms_key_id").(string)
 	b.lockTable = data.Get("lock_table").(string)
 
-	var errs []error
-	creds, err := terraformAWS.GetCredentials(&terraformAWS.Config{
+	cfg := &terraformAWS.Config{
 		AccessKey:             data.Get("access_key").(string),
-		SecretKey:             data.Get("secret_key").(string),
-		Token:                 data.Get("token").(string),
-		Profile:               data.Get("profile").(string),
-		CredsFilename:         data.Get("shared_credentials_file").(string),
 		AssumeRoleARN:         data.Get("role_arn").(string),
-		AssumeRoleSessionName: data.Get("session_name").(string),
 		AssumeRoleExternalID:  data.Get("external_id").(string),
 		AssumeRolePolicy:      data.Get("assume_role_policy").(string),
-	})
-	if err != nil {
-		return err
+		AssumeRoleSessionName: data.Get("session_name").(string),
+		CredsFilename:         data.Get("shared_credentials_file").(string),
+		Profile:               data.Get("profile").(string),
+		Region:                data.Get("region").(string),
+		S3Endpoint:            data.Get("endpoint").(string),
+		SecretKey:             data.Get("secret_key").(string),
+		Token:                 data.Get("token").(string),
 	}
 
-	// Call Get to check for credential provider. If nothing found, we'll get an
-	// error, and we can present it nicely to the user
-	_, err = creds.Get()
+	client, err := cfg.Client()
 	if err != nil {
-		if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "NoCredentialProviders" {
-			errs = append(errs, fmt.Errorf(`No valid credential sources found for AWS S3 remote.
-Please see https://www.terraform.io/docs/state/remote/s3.html for more information on
-providing credentials for the AWS S3 remote`))
-		} else {
-			errs = append(errs, fmt.Errorf("Error loading credentials for AWS S3 remote: %s", err))
-		}
-		return &multierror.Error{Errors: errs}
+		return err
 	}
 
-	endpoint := data.Get("endpoint").(string)
-	region := data.Get("region").(string)
-
-	awsConfig := &aws.Config{
-		Credentials: creds,
-		Endpoint:    aws.String(endpoint),
-		Region:      aws.String(region),
-		HTTPClient:  cleanhttp.DefaultClient(),
-	}
-	sess := session.New(awsConfig)
-	b.s3Client = s3.New(sess)
-	b.dynClient = dynamodb.New(sess)
+	b.s3Client = client.(*terraformAWS.AWSClient).S3()
+	b.dynClient = client.(*terraformAWS.AWSClient).DynamoDB()
 
 	return nil
 }

backend/remote-state/s3/backend_test.go

@@ -29,16 +29,12 @@ func TestBackend_impl(t *testing.T) {
 }
 
 func TestBackendConfig(t *testing.T) {
-	// This test just instantiates the client. Shouldn't make any actual
-	// requests nor incur any costs.
-
+	testACC(t)
 	config := map[string]interface{}{
 		"region":     "us-west-1",
 		"bucket":     "tf-test",
 		"key":        "state",
 		"encrypt":    true,
-		"access_key": "ACCESS_KEY",
-		"secret_key": "SECRET_KEY",
 		"lock_table": "dynamoTable",
 	}
 
@@ -58,11 +54,11 @@ func TestBackendConfig(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error when requesting credentials")
 	}
-	if credentials.AccessKeyID != "ACCESS_KEY" {
-		t.Fatalf("Incorrect Access Key Id was populated")
+	if credentials.AccessKeyID == "" {
+		t.Fatalf("No Access Key Id was populated")
 	}
-	if credentials.SecretAccessKey != "SECRET_KEY" {
-		t.Fatalf("Incorrect Secret Access Key was populated")
+	if credentials.SecretAccessKey == "" {
+		t.Fatalf("No Secret Access Key was populated")
 	}
 }
 

builtin/providers/aws/config.go

@@ -160,6 +160,14 @@ type AWSClient struct {
 	wafconn               *waf.WAF
 }
 
+func (c *AWSClient) S3() *s3.S3 {
+	return c.s3conn
+}
+
+func (c *AWSClient) DynamoDB() *dynamodb.DynamoDB {
+	return c.dynamodbconn
+}
+
 // Client configures and returns a fully initialized AWSClient
 func (c *Config) Client() (interface{}, error) {
 	// Get the auth and region. This can fail if keys/regions were not

state/state.go

@@ -94,9 +94,9 @@ func LockWithContext(ctx context.Context, s State, info *LockInfo) (string, erro
 			return "", err
 		}
 
-		if le.Info.ID == "" {
-			// the lock has no ID, something is wrong so don't keep trying
-			return "", fmt.Errorf("lock error missing ID: %s", err)
+		if le == nil || le.Info == nil || le.Info.ID == "" {
+			// If we dont' have a complete LockError, there's something wrong with the lock
+			return "", err
 		}
 
 		if postLockHook != nil {