Skip to content

Commit

Permalink
Merge pull request #36120 from bschaatsbergen/b/s3-object-lock-file
Browse files Browse the repository at this point in the history 
s3: fix S3 Object Lock header issue for lock file writes
  • Loading branch information
@jar-b
jar-b authored Dec 2, 2024
2 parents 6476b30 + 86ca532 commit 57f63cf
Show file tree
Hide file tree
Showing 3 changed files with 343 additions and 2 deletions.
180 changes: 180 additions & 0 deletions internal/backend/remote-state/s3/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2038,6 +2038,76 @@ func TestBackendLockedWithFile(t *testing.T) {
backend.TestBackendStateForceUnlock(t, b1, b2)
}

func TestBackendLockedWithFile_ObjectLock_Compliance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix())
keyName := "test/state"

b1 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
"region": "us-west-2",
})).(*Backend)

b2 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
"region": "us-west-2",
})).(*Backend)

createS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region,
s3BucketWithVersioning,
s3BucketWithObjectLock(s3types.ObjectLockRetentionModeCompliance),
)
defer deleteS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)

backend.TestBackendStateLocks(t, b1, b2)
backend.TestBackendStateForceUnlock(t, b1, b2)
}

func TestBackendLockedWithFile_ObjectLock_Governance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix())
keyName := "test/state"

b1 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
"region": "us-west-2",
})).(*Backend)

b2 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
"region": "us-west-2",
})).(*Backend)

createS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region,
s3BucketWithVersioning,
s3BucketWithObjectLock(s3types.ObjectLockRetentionModeGovernance),
)
defer deleteS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)

backend.TestBackendStateLocks(t, b1, b2)
backend.TestBackendStateForceUnlock(t, b1, b2)
}

func TestBackendLockedWithFileAndDynamoDB(t *testing.T) {
testACC(t)

Expand Down Expand Up @@ -2158,6 +2228,116 @@ func TestBackend_LockFileCleanupOnDynamoDBLock(t *testing.T) {
}
}

func TestBackend_LockFileCleanupOnDynamoDBLock_ObjectLock_Compliance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix())
keyName := "test/state"

b1 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": false, // Only use DynamoDB
"dynamodb_table": bucketName,
"region": "us-west-2",
})).(*Backend)

b2 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true, // Use both DynamoDB and lockfile
"dynamodb_table": bucketName,
"region": "us-west-2",
})).(*Backend)

createS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region,
s3BucketWithVersioning,
s3BucketWithObjectLock(s3types.ObjectLockRetentionModeCompliance),
)
defer deleteS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)
createDynamoDBTable(ctx, t, b1.dynClient, bucketName)
defer deleteDynamoDBTable(ctx, t, b1.dynClient, bucketName)

backend.TestBackendStateLocks(t, b1, b2)

// Attempt to retrieve the lock file from S3.
_, err := b1.s3Client.GetObject(ctx, &s3.GetObjectInput{
Bucket: aws.String(b1.bucketName),
Key: aws.String(b1.keyName + ".tflock"),
})
// We expect an error here, indicating that the lock file does not exist.
// The absence of the lock file is expected, as it should have been
// cleaned up following a failed lock acquisition due to `b1` already
// acquiring a DynamoDB lock.
if err != nil {
if !IsA[*s3types.NoSuchKey](err) {
t.Fatalf("unexpected error: %s", err)
}
} else {
t.Fatalf("expected error, got none")
}
}

func TestBackend_LockFileCleanupOnDynamoDBLock_ObjectLock_Governance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix())
keyName := "test/state"

b1 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": false, // Only use DynamoDB
"dynamodb_table": bucketName,
"region": "us-west-2",
})).(*Backend)

b2 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true, // Use both DynamoDB and lockfile
"dynamodb_table": bucketName,
"region": "us-west-2",
})).(*Backend)

createS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region,
s3BucketWithVersioning,
s3BucketWithObjectLock(s3types.ObjectLockRetentionModeGovernance),
)
defer deleteS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)
createDynamoDBTable(ctx, t, b1.dynClient, bucketName)
defer deleteDynamoDBTable(ctx, t, b1.dynClient, bucketName)

backend.TestBackendStateLocks(t, b1, b2)

// Attempt to retrieve the lock file from S3.
_, err := b1.s3Client.GetObject(ctx, &s3.GetObjectInput{
Bucket: aws.String(b1.bucketName),
Key: aws.String(b1.keyName + ".tflock"),
})
// We expect an error here, indicating that the lock file does not exist.
// The absence of the lock file is expected, as it should have been
// cleaned up following a failed lock acquisition due to `b1` already
// acquiring a DynamoDB lock.
if err != nil {
if !IsA[*s3types.NoSuchKey](err) {
t.Fatalf("unexpected error: %s", err)
}
} else {
t.Fatalf("expected error, got none")
}
}

func TestBackend_LockDeletedOutOfBand(t *testing.T) {
testACC(t)

Expand Down
6 changes: 5 additions & 1 deletion internal/backend/remote-state/s3/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,9 @@ func (c *RemoteClient) lockWithFile(ctx context.Context, info *statemgr.LockInfo
Key: aws.String(c.lockFilePath),
IfNoneMatch: aws.String("*"),
}
if !c.skipS3Checksum {
input.ChecksumAlgorithm = s3types.ChecksumAlgorithmSha256
}

if c.serverSideEncryption {
if c.kmsKeyID != "" {
Expand All @@ -378,7 +381,8 @@ func (c *RemoteClient) lockWithFile(ctx context.Context, info *statemgr.LockInfo

log.Debug("Uploading lock file")

_, err = c.s3Client.PutObject(ctx, input)
uploader := manager.NewUploader(c.s3Client)
_, err = uploader.Upload(ctx, input)
if err != nil {
// Attempt to retrieve lock info from the file, and merge errors if it fails.
lockInfo, infoErr := c.getLockInfoWithFile(ctx)
Expand Down
159 changes: 158 additions & 1 deletion internal/backend/remote-state/s3/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,83 @@ func TestForceUnlock(t *testing.T) {
}
}

func TestForceUnlock_withLockfile(t *testing.T) {
testACC(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-force-%x", time.Now().Unix())
keyName := "testState"

b1 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
})).(*Backend)

b2 := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
})).(*Backend)

createS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)
defer deleteS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)

// first test with default
s1, err := b1.StateMgr(backend.DefaultStateName)
if err != nil {
t.Fatal(err)
}

info := statemgr.NewLockInfo()
info.Operation = "test"
info.Who = "clientA"

lockID, err := s1.Lock(info)
if err != nil {
t.Fatal("unable to get initial lock:", err)
}

// s1 is now locked, get the same state through s2 and unlock it
s2, err := b2.StateMgr(backend.DefaultStateName)
if err != nil {
t.Fatal("failed to get default state to force unlock:", err)
}

if err := s2.Unlock(lockID); err != nil {
t.Fatal("failed to force-unlock default state")
}

// now try the same thing with a named state
// first test with default
s1, err = b1.StateMgr("test")
if err != nil {
t.Fatal(err)
}

info = statemgr.NewLockInfo()
info.Operation = "test"
info.Who = "clientA"

lockID, err = s1.Lock(info)
if err != nil {
t.Fatal("unable to get initial lock:", err)
}

// s1 is now locked, get the same state through s2 and unlock it
s2, err = b2.StateMgr("test")
if err != nil {
t.Fatal("failed to get named state to force unlock:", err)
}

if err = s2.Unlock(lockID); err != nil {
t.Fatal("failed to force-unlock named state")
}
}

func TestRemoteClient_clientMD5(t *testing.T) {
testACC(t)

Expand Down Expand Up @@ -343,7 +420,7 @@ func TestRemoteClient_stateChecksum(t *testing.T) {
}
}

func TestRemoteClientPutLargeUploadWithObjectLock(t *testing.T) {
func TestRemoteClientPutLargeUploadWithObjectLock_Compliance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

Expand Down Expand Up @@ -382,6 +459,86 @@ func TestRemoteClientPutLargeUploadWithObjectLock(t *testing.T) {
}
}

func TestRemoteClientLockFileWithObjectLock_Compliance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix())
keyName := "testState"

b := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"use_lockfile": true,
})).(*Backend)

createS3Bucket(ctx, t, b.s3Client, bucketName, b.awsConfig.Region,
s3BucketWithVersioning,
s3BucketWithObjectLock(s3types.ObjectLockRetentionModeCompliance),
)
defer deleteS3Bucket(ctx, t, b.s3Client, bucketName, b.awsConfig.Region)

s1, err := b.StateMgr(backend.DefaultStateName)
if err != nil {
t.Fatal(err)
}
client := s1.(*remote.State).Client

var state bytes.Buffer
dataW := io.LimitReader(neverEnding('x'), manager.DefaultUploadPartSize)
_, err = state.ReadFrom(dataW)
if err != nil {
t.Fatalf("writing dummy data: %s", err)
}

err = client.Put(state.Bytes())
if err != nil {
t.Fatalf("putting data: %s", err)
}
}

func TestRemoteClientLockFileWithObjectLock_Governance(t *testing.T) {
testACC(t)
objectLockPreCheck(t)

ctx := context.TODO()

bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix())
keyName := "testState"

b := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"use_lockfile": true,
})).(*Backend)

createS3Bucket(ctx, t, b.s3Client, bucketName, b.awsConfig.Region,
s3BucketWithVersioning,
s3BucketWithObjectLock(s3types.ObjectLockRetentionModeGovernance),
)
defer deleteS3Bucket(ctx, t, b.s3Client, bucketName, b.awsConfig.Region)

s1, err := b.StateMgr(backend.DefaultStateName)
if err != nil {
t.Fatal(err)
}
client := s1.(*remote.State).Client

var state bytes.Buffer
dataW := io.LimitReader(neverEnding('x'), manager.DefaultUploadPartSize)
_, err = state.ReadFrom(dataW)
if err != nil {
t.Fatalf("writing dummy data: %s", err)
}

err = client.Put(state.Bytes())
if err != nil {
t.Fatalf("putting data: %s", err)
}
}

type neverEnding byte

func (b neverEnding) Read(p []byte) (n int, err error) {
Expand Down

0 comments on commit 57f63cf

Please sign in to comment.