Skip to content

Commit

Permalink
Address s3 compatible remote state issues + logging (opentofu#840)
Browse files Browse the repository at this point in the history
Signed-off-by: Christian Mesh <christianmesh1@gmail.com>
  • Loading branch information
cam72cam authored and thumperward committed Jan 10, 2024
1 parent 2278212 commit 38f20a0
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 5 deletions.
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
}

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

0 comments on commit 38f20a0

Please sign in to comment.