Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address s3 compatible remote state issues + logging #840

Merged
merged 11 commits into from
Nov 9, 2023
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ BUG FIXES:
* cloud: fixed a bug that would prevent nested symlinks from being dereferenced into the config sent to Cloud ([#686](https://github.com/opentofu/opentofu/issues/686))
* cloud: state snapshots could not be disabled when header x-terraform-snapshot-interval is absent ([#687](https://github.com/opentofu/opentofu/issues/687))
* Fixed crash during tofu destroy inside module with variable validation ([#817](https://github.com/opentofu/opentofu/issues/817))
* S3 backend endpoints without proto:// now default to https:// instead of failing ([#821](https://github.com/opentofu/opentofu/issues/821))
* Most S3 compatible remote state backends should now work without checksum errors / 400s by default ([#821](https://github.com/opentofu/opentofu/issues/821))
* Logging has been re-added to S3 remote state calls ([#821](https://github.com/opentofu/opentofu/issues/821))

S3 BACKEND:

Expand Down
27 changes: 23 additions & 4 deletions internal/backend/remote-state/s3/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"context"
"encoding/base64"
"fmt"
"log"
"os"
"regexp"
"sort"
"strings"
"time"
Expand All @@ -17,6 +19,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/dynamodb"
"github.com/aws/aws-sdk-go-v2/service/s3"
awsbase "github.com/hashicorp/aws-sdk-go-base/v2"
baselogging "github.com/hashicorp/aws-sdk-go-base/v2/logging"
"github.com/opentofu/opentofu/internal/backend"
"github.com/opentofu/opentofu/internal/configs/configschema"
"github.com/opentofu/opentofu/internal/httpclient"
Expand Down Expand Up @@ -693,11 +696,13 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics {
}
}

ctx := context.TODO()
ctx, baselog := attachLoggerToContext(ctx)

cfg := &awsbase.Config{
AccessKey: stringAttr(obj, "access_key"),
CallerDocumentationURL: "https://opentofu.org/docs/language/settings/backends/s3",
CallerName: "S3 Backend",
SuppressDebugLog: logging.IsDebugOrHigher(),
IamEndpoint: customEndpoints["iam"].String(obj),
MaxRetries: intAttrDefault(obj, "max_retries", 5),
Profile: stringAttr(obj, "profile"),
Expand All @@ -719,6 +724,7 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics {
CustomCABundle: stringAttrDefaultEnvVar(obj, "custom_ca_bundle", "AWS_CA_BUNDLE"),
EC2MetadataServiceEndpoint: stringAttrDefaultEnvVar(obj, "ec2_metadata_service_endpoint", "AWS_EC2_METADATA_SERVICE_ENDPOINT"),
EC2MetadataServiceEndpointMode: stringAttrDefaultEnvVar(obj, "ec2_metadata_service_endpoint_mode", "AWS_EC2_METADATA_SERVICE_ENDPOINT_MODE"),
Logger: baselog,
}

if val, ok := boolAttrOk(obj, "use_legacy_workflow"); ok {
Expand Down Expand Up @@ -772,7 +778,6 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics {
cfg.RetryMode = mode
}

ctx := context.TODO()
_, awsConfig, awsDiags := awsbase.GetAwsConfig(ctx, cfg)

for _, d := range awsDiags {
Expand Down Expand Up @@ -800,6 +805,12 @@ func (b *Backend) Configure(obj cty.Value) tfdiags.Diagnostics {
return diags
}

func attachLoggerToContext(ctx context.Context) (context.Context, baselogging.HcLogger) {
ctx, baselog := baselogging.NewHcLogger(ctx, logging.HCLogger().Named("backend-s3"))
ctx = baselogging.RegisterLogger(ctx, baselog)
return ctx, baselog
}

func verifyAllowedAccountID(ctx context.Context, awsConfig aws.Config, cfg *awsbase.Config) tfdiags.Diagnostics {
if len(cfg.ForbiddenAccountIds) == 0 && len(cfg.AllowedAccountIds) == 0 {
return nil
Expand Down Expand Up @@ -1171,19 +1182,27 @@ func (e customEndpoint) String(obj cty.Value) string {
return v
}

func includeProtoIfNessesary(endpoint string) string {
if matched, _ := regexp.MatchString("[a-z]*://.*", endpoint); !matched {
log.Printf("[DEBUG] Adding https:// prefix to endpoint '%s'", endpoint)
endpoint = fmt.Sprintf("https://%s", endpoint)
}
return endpoint
}

func (e customEndpoint) StringOk(obj cty.Value) (string, bool) {
for _, path := range e.Paths {
val, err := path.Apply(obj)
if err != nil {
continue
}
if s, ok := stringValueOk(val); ok {
return s, true
return includeProtoIfNessesary(s), true
}
}
for _, envVar := range e.EnvVars {
if v := os.Getenv(envVar); v != "" {
return v, true
return includeProtoIfNessesary(v), true
}
}
return "", false
Expand Down
5 changes: 4 additions & 1 deletion internal/backend/remote-state/s3/backend_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@ func (b *Backend) Workspaces() ([]string, error) {
MaxKeys: maxKeys,
}

ctx := context.TODO()

ctx, _ = attachLoggerToContext(ctx)

wss := []string{backend.DefaultStateName}
pg := s3.NewListObjectsV2Paginator(b.s3Client, params)

ctx := context.TODO()
for pg.HasMorePages() {
page, err := pg.NextPage(ctx)
if err != nil {
Expand Down
43 changes: 43 additions & 0 deletions internal/backend/remote-state/s3/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1375,6 +1375,49 @@ func Test_pathString(t *testing.T) {
}
}

func TestBackend_includeProtoIfNessesary(t *testing.T) {
tests := []struct {
name string
provided string
expected string
}{
{
name: "Unmodified S3",
provided: "https://s3.us-east-1.amazonaws.com",
expected: "https://s3.us-east-1.amazonaws.com",
},
{
name: "Modified S3",
provided: "s3.us-east-1.amazonaws.com",
expected: "https://s3.us-east-1.amazonaws.com",
},
{
name: "Unmodified With Port",
provided: "http://localhost:9000/",
expected: "http://localhost:9000/",
},
{
name: "Modified With Port",
provided: "localhost:9000/",
expected: "https://localhost:9000/",
},
{
name: "Umodified with strange proto",
provided: "ftp://localhost:9000/",
expected: "ftp://localhost:9000/",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := includeProtoIfNessesary(test.provided)
if result != test.expected {
t.Errorf("Expected: %s, Got: %s", test.expected, result)
}
})
}
}

func testGetWorkspaceForKey(b *Backend, key string, expected string) error {
if actual := b.keyEnv(key); actual != expected {
return fmt.Errorf("incorrect workspace for key[%q]. Expected[%q]: Actual[%q]", key, expected, actual)
Expand Down
36 changes: 36 additions & 0 deletions internal/backend/remote-state/s3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"bytes"
"context"
"crypto/md5"
"crypto/sha256"
"encoding/base64"
"encoding/hex"
"encoding/json"
Expand Down Expand Up @@ -109,6 +110,29 @@ func (c *RemoteClient) get(ctx context.Context) (*remote.Payload, error) {
var output *s3.GetObjectOutput
var err error

ctx, _ = attachLoggerToContext(ctx)

inputHead := &s3.HeadObjectInput{
Bucket: &c.bucketName,
Key: &c.path,
}

// Head works around some s3 compatible backends not handling missing GetObject requests correctly (ex: minio Get returns Missing Bucket)
_, err = c.s3Client.HeadObject(ctx, inputHead)
if err != nil {
var nb *types.NoSuchBucket
if errors.As(err, &nb) {
return nil, fmt.Errorf(errS3NoSuchBucket, err)
}

var nk *types.NotFound
if errors.As(err, &nk) {
return nil, nil
}

return nil, err
}
kislerdm marked this conversation as resolved.
Show resolved Hide resolved
cam72cam marked this conversation as resolved.
Show resolved Hide resolved

input := &s3.GetObjectInput{
Bucket: &c.bucketName,
Key: &c.path,
Expand Down Expand Up @@ -170,6 +194,14 @@ func (c *RemoteClient) Put(data []byte) error {

if !c.skipS3Checksum {
i.ChecksumAlgorithm = types.ChecksumAlgorithmSha256

// There is a conflict in the aws-go-sdk-v2 that prevents it from working with many s3 compatible services
// Since we can pre-compute the hash here, we can work around it.
// ref: https://github.com/aws/aws-sdk-go-v2/issues/1689
algo := sha256.New()
algo.Write(data)
sum64str := base64.StdEncoding.EncodeToString(algo.Sum(nil))
i.ChecksumSHA256 = &sum64str
}

if c.serverSideEncryption {
Expand All @@ -192,6 +224,8 @@ func (c *RemoteClient) Put(data []byte) error {
log.Printf("[DEBUG] Uploading remote state to S3: %#v", i)

ctx := context.TODO()
ctx, _ = attachLoggerToContext(ctx)

_, err := c.s3Client.PutObject(ctx, i)
if err != nil {
return fmt.Errorf("failed to upload state: %w", err)
Expand All @@ -210,6 +244,8 @@ func (c *RemoteClient) Put(data []byte) error {

func (c *RemoteClient) Delete() error {
ctx := context.TODO()
ctx, _ = attachLoggerToContext(ctx)

_, err := c.s3Client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &c.bucketName,
Key: &c.path,
Expand Down