From eeddc3f8ea6a22778785551bee2d598c93915087 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 5 Apr 2017 09:06:47 -0400 Subject: [PATCH 1/3] add some nil checks for unexpected lock failures --- state/state.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/state/state.go b/state/state.go index f6c4f16d4e..45163852b2 100644 --- a/state/state.go +++ b/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 { From 0ec2a5cfd3cc0cfc5cfe8a754faadb15e966a49e Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 5 Apr 2017 12:29:25 -0400 Subject: [PATCH 2/3] add AWSClient methods to get s3 and dyndb conns Add getters for the AWSClient s3.S3 and dynamodb.DynamoDB clients so the s3 remote-state backend can use all the same initialization code as the aws provider. --- builtin/providers/aws/config.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/builtin/providers/aws/config.go b/builtin/providers/aws/config.go index 1cfda12b74..a65bf93e5b 100644 --- a/builtin/providers/aws/config.go +++ b/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 From 6e136c848aab07522b3bcd048ecf79c51bf74418 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 5 Apr 2017 12:37:42 -0400 Subject: [PATCH 3/3] use the aws provider client initialization Use the aws provider code to create the clients for the s3 backend, so that all the behavior matches that of the provider. Remove the fake creds from the test, as the aws provider will attempt to validate them. --- backend/remote-state/s3/backend.go | 51 ++++++------------------- backend/remote-state/s3/backend_test.go | 14 +++---- 2 files changed, 17 insertions(+), 48 deletions(-) diff --git a/backend/remote-state/s3/backend.go b/backend/remote-state/s3/backend.go index a1d9c1f9ae..846746b981 100644 --- a/backend/remote-state/s3/backend.go +++ b/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 } diff --git a/backend/remote-state/s3/backend_test.go b/backend/remote-state/s3/backend_test.go index f8b664b801..44987683ff 100644 --- a/backend/remote-state/s3/backend_test.go +++ b/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") } }