From ecc8e4992d7987578c74852305200bacdc0a052e Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 21 Aug 2023 11:34:00 -0700 Subject: [PATCH 1/5] Adds standard S3 endpoint envvar and deprecates existing envvar --- internal/backend/remote-state/s3/backend.go | 20 +++++- .../backend/remote-state/s3/backend_test.go | 69 ++++++++++++++++--- internal/backend/remote-state/s3/validate.go | 4 ++ .../docs/language/settings/backends/s3.mdx | 2 +- 4 files changed, 83 insertions(+), 12 deletions(-) diff --git a/internal/backend/remote-state/s3/backend.go b/internal/backend/remote-state/s3/backend.go index f43f1e758d..eecf3eff67 100644 --- a/internal/backend/remote-state/s3/backend.go +++ b/internal/backend/remote-state/s3/backend.go @@ -329,8 +329,7 @@ func (b *Backend) PrepareConfig(obj cty.Value) (cty.Value, tfdiags.Diagnostics) } } else { if defined := findDeprecatedFields(obj, assumeRoleDeprecatedFields); len(defined) != 0 { - diags = diags.Append(tfdiags.WholeContainingBody( - tfdiags.Warning, + diags = diags.Append(wholeBodyWarningDiag( "Deprecated Parameters", `The following parameters have been deprecated. Replace them as follows:`+"\n"+ formatDeprecations(defined), @@ -455,6 +454,14 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { } } + for envvar, replacement := range map[string]string{ + "AWS_S3_ENDPOINT": "AWS_ENDPOINT_URL_S3", + } { + if val := os.Getenv(envvar); val != "" { + diags = diags.Append(deprecatedEnvVarDiag(envvar, replacement)) + } + } + cfg := &awsbase.Config{ AccessKey: stringAttr(obj, "access_key"), CallerDocumentationURL: "https://www.terraform.io/docs/language/settings/backends/s3.html", @@ -541,7 +548,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { b.dynClient = dynamodb.New(sess.Copy(&dynamoConfig)) var s3Config aws.Config - if v, ok := stringAttrDefaultEnvVarOk(obj, "endpoint", "AWS_S3_ENDPOINT"); ok { + if v, ok := stringAttrDefaultEnvVarOk(obj, "endpoint", "AWS_ENDPOINT_URL_S3", "AWS_S3_ENDPOINT"); ok { s3Config.Endpoint = aws.String(v) } if v, ok := boolAttrOk(obj, "force_path_style"); ok { @@ -966,3 +973,10 @@ func assumeRoleFullSchema() objectSchema { }, } } + +func deprecatedEnvVarDiag(envvar, replacement string) tfdiags.Diagnostic { + return wholeBodyWarningDiag( + "Deprecated Environment Variable", + fmt.Sprintf(`The environment variable "%s" is deprecated. Use environment variable "%s" instead.`, envvar, replacement), + ) +} diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go index 97ca43fc7f..2510352a95 100644 --- a/internal/backend/remote-state/s3/backend_test.go +++ b/internal/backend/remote-state/s3/backend_test.go @@ -18,6 +18,8 @@ import ( "github.com/aws/aws-sdk-go/service/s3" "github.com/google/go-cmp/cmp" awsbase "github.com/hashicorp/aws-sdk-go-base" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/configs/hcl2shim" @@ -257,24 +259,34 @@ func TestBackendConfig_S3Endpoint(t *testing.T) { testACC(t) cases := map[string]struct { - config map[string]any - vars map[string]string - expected string + config map[string]any + vars map[string]string + expectedEndpoint string + expectedDiags tfdiags.Diagnostics }{ "none": { - expected: "", + expectedEndpoint: "", }, "config": { config: map[string]any{ "endpoint": "s3.test", }, - expected: "s3.test", + expectedEndpoint: "s3.test", }, "envvar": { + vars: map[string]string{ + "AWS_ENDPOINT_URL_S3": "s3.test", + }, + expectedEndpoint: "s3.test", + }, + "deprecated envvar": { vars: map[string]string{ "AWS_S3_ENDPOINT": "s3.test", }, - expected: "s3.test", + expectedEndpoint: "s3.test", + expectedDiags: tfdiags.Diagnostics{ + deprecatedEnvVarDiag("AWS_S3_ENDPOINT", "AWS_ENDPOINT_URL_S3"), + }, }, } @@ -303,9 +315,14 @@ func TestBackendConfig_S3Endpoint(t *testing.T) { } } - b := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(config)).(*Backend) + raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config)) + b := raw.(*Backend) + + checkClientEndpoint(t, b.s3Client.Config, tc.expectedEndpoint) - checkClientEndpoint(t, b.s3Client.Config, tc.expected) + if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" { + t.Errorf("unexpected diagnostics difference: %s", diff) + } }) } } @@ -1653,3 +1670,39 @@ func must[T any](v T, err error) T { return v } } + +// testBackendConfigDiags is an equivalent to `backend.TestBackendConfig` which returns the diags to the caller +// instead of failing the test +func testBackendConfigDiags(t *testing.T, b backend.Backend, c hcl.Body) (backend.Backend, tfdiags.Diagnostics) { + t.Helper() + + t.Logf("TestBackendConfig on %T with %#v", b, c) + + var diags tfdiags.Diagnostics + + // To make things easier for test authors, we'll allow a nil body here + // (even though that's not normally valid) and just treat it as an empty + // body. + if c == nil { + c = hcl.EmptyBody() + } + + schema := b.ConfigSchema() + spec := schema.DecoderSpec() + obj, decDiags := hcldec.Decode(c, spec, nil) + diags = diags.Append(decDiags) + + newObj, valDiags := b.PrepareConfig(obj) + diags = diags.Append(valDiags.InConfigBody(c, "")) + + // it's valid for a Backend to have warnings (e.g. a Deprecation) as such we should only raise on errors + if diags.HasErrors() { + return b, diags + } + + obj = newObj + + confDiags := b.Configure(obj) + + return b, diags.Append(confDiags) +} diff --git a/internal/backend/remote-state/s3/validate.go b/internal/backend/remote-state/s3/validate.go index 610b6c2e7d..03bac21225 100644 --- a/internal/backend/remote-state/s3/validate.go +++ b/internal/backend/remote-state/s3/validate.go @@ -352,3 +352,7 @@ func attributeErrDiag(summary, detail string, attrPath cty.Path) tfdiags.Diagnos func wholeBodyErrDiag(summary, detail string) tfdiags.Diagnostic { return tfdiags.WholeContainingBody(tfdiags.Error, summary, detail) } + +func wholeBodyWarningDiag(summary, detail string) tfdiags.Diagnostic { + return tfdiags.WholeContainingBody(tfdiags.Warning, summary, detail) +} diff --git a/website/docs/language/settings/backends/s3.mdx b/website/docs/language/settings/backends/s3.mdx index ef85f68c0a..96db74dba2 100644 --- a/website/docs/language/settings/backends/s3.mdx +++ b/website/docs/language/settings/backends/s3.mdx @@ -214,7 +214,7 @@ The following configuration is optional: * `acl` - (Optional) [Canned ACL](https://docs.aws.amazon.com/AmazonS3/latest/userguide/acl-overview.html#canned-acl) to be applied to the state file. * `encrypt` - (Optional) Enable [server side encryption](https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingServerSideEncryption.html) of the state file. -* `endpoint` - (Optional) Custom endpoint for the AWS S3 API. This can also be sourced from the `AWS_S3_ENDPOINT` environment variable. +* `endpoint` - (Optional) Custom endpoint for the AWS S3 API. This can also be sourced from the environment variable `AWS_ENDPOINT_URL_S3` or the deprecated environment variable `AWS_S3_ENDPOINT`. * `force_path_style` - (Optional) Enable path-style S3 URLs (`https:///` instead of `https://.`). * `kms_key_id` - (Optional) Amazon Resource Name (ARN) of a Key Management Service (KMS) Key to use for encrypting the state. Note that if this value is specified, Terraform will need `kms:Encrypt`, `kms:Decrypt` and `kms:GenerateDataKey` permissions on this KMS key. * `sse_customer_key` - (Optional) The key to use for encrypting state with [Server-Side Encryption with Customer-Provided Keys (SSE-C)](https://docs.aws.amazon.com/AmazonS3/latest/userguide/ServerSideEncryptionCustomerKeys.html). This is the base64-encoded value of the key, which must decode to 256 bits. This can also be sourced from the `AWS_SSE_CUSTOMER_KEY` environment variable, which is recommended due to the sensitivity of the value. Setting it inside a terraform file will cause it to be persisted to disk in `terraform.tfstate`. From 1961ef51b6968ca88c64894828a3708f0028387c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 21 Aug 2023 11:40:36 -0700 Subject: [PATCH 2/5] Adds standard DynamoDB endpoint envvar and deprecates existing envvar --- internal/backend/remote-state/s3/backend.go | 5 +-- .../backend/remote-state/s3/backend_test.go | 31 ++++++++++++++----- .../docs/language/settings/backends/s3.mdx | 2 +- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/internal/backend/remote-state/s3/backend.go b/internal/backend/remote-state/s3/backend.go index eecf3eff67..faeb58e25f 100644 --- a/internal/backend/remote-state/s3/backend.go +++ b/internal/backend/remote-state/s3/backend.go @@ -455,7 +455,8 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { } for envvar, replacement := range map[string]string{ - "AWS_S3_ENDPOINT": "AWS_ENDPOINT_URL_S3", + "AWS_DYNAMODB_ENDPOINT": "AWS_ENDPOINT_URL_DYNAMODB", + "AWS_S3_ENDPOINT": "AWS_ENDPOINT_URL_S3", } { if val := os.Getenv(envvar); val != "" { diags = diags.Append(deprecatedEnvVarDiag(envvar, replacement)) @@ -542,7 +543,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { } var dynamoConfig aws.Config - if v, ok := stringAttrDefaultEnvVarOk(obj, "dynamodb_endpoint", "AWS_DYNAMODB_ENDPOINT"); ok { + if v, ok := stringAttrDefaultEnvVarOk(obj, "dynamodb_endpoint", "AWS_ENDPOINT_URL_DYNAMODB", "AWS_DYNAMODB_ENDPOINT"); ok { dynamoConfig.Endpoint = aws.String(v) } b.dynClient = dynamodb.New(sess.Copy(&dynamoConfig)) diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go index 2510352a95..bee5383a17 100644 --- a/internal/backend/remote-state/s3/backend_test.go +++ b/internal/backend/remote-state/s3/backend_test.go @@ -202,24 +202,34 @@ func TestBackendConfig_DynamoDBEndpoint(t *testing.T) { testACC(t) cases := map[string]struct { - config map[string]any - vars map[string]string - expected string + config map[string]any + vars map[string]string + expectedEndpoint string + expectedDiags tfdiags.Diagnostics }{ "none": { - expected: "", + expectedEndpoint: "", }, "config": { config: map[string]any{ "dynamodb_endpoint": "dynamo.test", }, - expected: "dynamo.test", + expectedEndpoint: "dynamo.test", }, "envvar": { + vars: map[string]string{ + "AWS_ENDPOINT_URL_DYNAMODB": "dynamo.test", + }, + expectedEndpoint: "dynamo.test", + }, + "deprecated envvar": { vars: map[string]string{ "AWS_DYNAMODB_ENDPOINT": "dynamo.test", }, - expected: "dynamo.test", + expectedEndpoint: "dynamo.test", + expectedDiags: tfdiags.Diagnostics{ + deprecatedEnvVarDiag("AWS_DYNAMODB_ENDPOINT", "AWS_ENDPOINT_URL_DYNAMODB"), + }, }, } @@ -248,9 +258,14 @@ func TestBackendConfig_DynamoDBEndpoint(t *testing.T) { } } - b := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(config)).(*Backend) + raw, diags := testBackendConfigDiags(t, New(), backend.TestWrapConfig(config)) + b := raw.(*Backend) - checkClientEndpoint(t, b.dynClient.Config, tc.expected) + checkClientEndpoint(t, b.dynClient.Config, tc.expectedEndpoint) + + if diff := cmp.Diff(diags, tc.expectedDiags, cmp.Comparer(diagnosticComparer)); diff != "" { + t.Errorf("unexpected diagnostics difference: %s", diff) + } }) } } diff --git a/website/docs/language/settings/backends/s3.mdx b/website/docs/language/settings/backends/s3.mdx index 96db74dba2..ebbed3725e 100644 --- a/website/docs/language/settings/backends/s3.mdx +++ b/website/docs/language/settings/backends/s3.mdx @@ -224,7 +224,7 @@ The following configuration is optional: The following configuration is optional: -* `dynamodb_endpoint` - (Optional) Custom endpoint for the AWS DynamoDB API. This can also be sourced from the `AWS_DYNAMODB_ENDPOINT` environment variable. +* `dynamodb_endpoint` - (Optional) Custom endpoint for the AWS DynamoDB API. This can also be sourced from the environment variable `AWS_ENDPOINT_URL_DYNAMODB` or the deprecated environment variable `AWS_DYNAMODB_ENDPOINT`. * `dynamodb_table` - (Optional) Name of DynamoDB Table to use for state locking and consistency. The table must have a partition key named `LockID` with type of `String`. If not configured, state locking will be disabled. ## Multi-account AWS Architecture From dc98a94b801386fa5376699c63f0678142ae78f7 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 21 Aug 2023 11:43:37 -0700 Subject: [PATCH 3/5] Adds standard IAM endpoint envvar and deprecates existing envvar --- internal/backend/remote-state/s3/backend.go | 3 ++- website/docs/language/settings/backends/s3.mdx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/backend/remote-state/s3/backend.go b/internal/backend/remote-state/s3/backend.go index faeb58e25f..8c05aab35d 100644 --- a/internal/backend/remote-state/s3/backend.go +++ b/internal/backend/remote-state/s3/backend.go @@ -456,6 +456,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { for envvar, replacement := range map[string]string{ "AWS_DYNAMODB_ENDPOINT": "AWS_ENDPOINT_URL_DYNAMODB", + "AWS_IAM_ENDPOINT": "AWS_ENDPOINT_URL_IAM", "AWS_S3_ENDPOINT": "AWS_ENDPOINT_URL_S3", } { if val := os.Getenv(envvar); val != "" { @@ -469,7 +470,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { CallerName: "S3 Backend", CredsFilename: stringAttr(obj, "shared_credentials_file"), DebugLogging: logging.IsDebugOrHigher(), - IamEndpoint: stringAttrDefaultEnvVar(obj, "iam_endpoint", "AWS_IAM_ENDPOINT"), + IamEndpoint: stringAttrDefaultEnvVar(obj, "iam_endpoint", "AWS_ENDPOINT_URL_IAM", "AWS_IAM_ENDPOINT"), MaxRetries: intAttrDefault(obj, "max_retries", 5), Profile: stringAttr(obj, "profile"), Region: stringAttr(obj, "region"), diff --git a/website/docs/language/settings/backends/s3.mdx b/website/docs/language/settings/backends/s3.mdx index ebbed3725e..3a992e86db 100644 --- a/website/docs/language/settings/backends/s3.mdx +++ b/website/docs/language/settings/backends/s3.mdx @@ -154,7 +154,7 @@ The following configuration is optional: * `access_key` - (Optional) AWS access key. If configured, must also configure `secret_key`. This can also be sourced from the `AWS_ACCESS_KEY_ID` environment variable, AWS shared credentials file (e.g. `~/.aws/credentials`), or AWS shared configuration file (e.g. `~/.aws/config`). * `secret_key` - (Optional) AWS access key. If configured, must also configure `access_key`. This can also be sourced from the `AWS_SECRET_ACCESS_KEY` environment variable, AWS shared credentials file (e.g. `~/.aws/credentials`), or AWS shared configuration file (e.g. `~/.aws/config`). -* `iam_endpoint` - (Optional) Custom endpoint for the AWS Identity and Access Management (IAM) API. This can also be sourced from the `AWS_IAM_ENDPOINT` environment variable. +* `iam_endpoint` - (Optional) Custom endpoint for the AWS Identity and Access Management (IAM) API. This can also be sourced from the environment variable `AWS_ENDPOINT_URL_IAM` or the deprecated environment variable `AWS_IAM_ENDPOINT`. * `max_retries` - (Optional) The maximum number of times an AWS API request is retried on retryable failure. Defaults to 5. * `profile` - (Optional) Name of AWS profile in AWS shared credentials file (e.g. `~/.aws/credentials`) or AWS shared configuration file (e.g. `~/.aws/config`) to use for credentials and/or configuration. This can also be sourced from the `AWS_PROFILE` environment variable. * `shared_credentials_file` - (Optional) Path to the AWS shared credentials file. Defaults to `~/.aws/credentials`. From 3253b684935b0c9202916ffe39dc0997b76c090a Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 21 Aug 2023 11:50:40 -0700 Subject: [PATCH 4/5] Adds standard STS endpoint envvar and deprecates existing envvar --- internal/backend/remote-state/s3/backend.go | 3 ++- website/docs/language/settings/backends/s3.mdx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/backend/remote-state/s3/backend.go b/internal/backend/remote-state/s3/backend.go index 8c05aab35d..37ec9bb97a 100644 --- a/internal/backend/remote-state/s3/backend.go +++ b/internal/backend/remote-state/s3/backend.go @@ -458,6 +458,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { "AWS_DYNAMODB_ENDPOINT": "AWS_ENDPOINT_URL_DYNAMODB", "AWS_IAM_ENDPOINT": "AWS_ENDPOINT_URL_IAM", "AWS_S3_ENDPOINT": "AWS_ENDPOINT_URL_S3", + "AWS_STS_ENDPOINT": "AWS_ENDPOINT_URL_STS", } { if val := os.Getenv(envvar); val != "" { diags = diags.Append(deprecatedEnvVarDiag(envvar, replacement)) @@ -477,7 +478,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics { SecretKey: stringAttr(obj, "secret_key"), SkipCredsValidation: boolAttr(obj, "skip_credentials_validation"), SkipMetadataApiCheck: boolAttr(obj, "skip_metadata_api_check"), - StsEndpoint: stringAttrDefaultEnvVar(obj, "sts_endpoint", "AWS_STS_ENDPOINT"), + StsEndpoint: stringAttrDefaultEnvVar(obj, "sts_endpoint", "AWS_ENDPOINT_URL_STS", "AWS_STS_ENDPOINT"), Token: stringAttr(obj, "token"), UserAgentProducts: []*awsbase.UserAgentProduct{ {Name: "APN", Version: "1.0"}, diff --git a/website/docs/language/settings/backends/s3.mdx b/website/docs/language/settings/backends/s3.mdx index 3a992e86db..5a6e1b7190 100644 --- a/website/docs/language/settings/backends/s3.mdx +++ b/website/docs/language/settings/backends/s3.mdx @@ -161,7 +161,7 @@ The following configuration is optional: * `skip_credentials_validation` - (Optional) Skip credentials validation via the STS API. * `skip_region_validation` - (Optional) Skip validation of provided region name. * `skip_metadata_api_check` - (Optional) Skip usage of EC2 Metadata API. -* `sts_endpoint` - (Optional) Custom endpoint for the AWS Security Token Service (STS) API. This can also be sourced from the `AWS_STS_ENDPOINT` environment variable. +* `sts_endpoint` - (Optional) Custom endpoint for the AWS Security Token Service (STS) API. This can also be sourced from the environment variable `AWS_ENDPOINT_URL_STS` or the deprecated environment variable `AWS_STS_ENDPOINT`. * `token` - (Optional) Multi-Factor Authentication (MFA) token. This can also be sourced from the `AWS_SESSION_TOKEN` environment variable. #### Assume Role Configuration From 62cc16a48eefbce868baa62e65d793e2ce5339b5 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 21 Aug 2023 15:47:09 -0700 Subject: [PATCH 5/5] Updates CHANGELOG --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1340d3f19..82b49e2e10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,10 +20,12 @@ BUG FIXES: S3 BACKEND -Enhancements +Update Notes * Moves arguments associated with assuming an IAM role into nested block `assume_role`. This deprecates the arguments `role_arn`, `session_name`, `external_id`, `assume_role_duration_seconds`, `assume_role_policy`, `assume_role_policy_arns`, `assume_role_tags`, and `assume_role_transitive_tag_keys`. [GH-33630] +* Supports the default AWS environment variables for overrding API endpoints: `AWS_ENDPOINT_URL_DYNAMODB`, `AWS_ENDPOINT_URL_IAM`, `AWS_ENDPOINT_URL_S3`, and `AWS_ENDPOINT_URL_STS`. + This deprecates the environment variables `AWS_DYNAMODB_ENDPOINT`, `AWS_IAM_ENDPOINT`, `AWS_S3_ENDPOINT`, and `AWS_STS_ENDPOINT`. [GH-33715] ## Previous Releases