diff --git a/internal/backend/remote-state/s3/client.go b/internal/backend/remote-state/s3/client.go index 4c95a48efa..8da2f0de0c 100644 --- a/internal/backend/remote-state/s3/client.go +++ b/internal/backend/remote-state/s3/client.go @@ -406,6 +406,10 @@ func (c *RemoteClient) getLockInfo(ctx context.Context) (*statemgr.LockInfo, err return nil, err } + if len(resp.Item) == 0 { + return nil, fmt.Errorf("no lock info found for: %q within the DynamoDB table: %s", c.lockPath(), c.ddbTable) + } + var infoData string if v, ok := resp.Item["Info"]; ok { if v, ok := v.(*dtypes.AttributeValueMemberS); ok { @@ -430,9 +434,6 @@ func (c *RemoteClient) Unlock(id string) error { lockErr := &statemgr.LockError{} ctx := context.TODO() - // TODO: store the path and lock ID in separate fields, and have proper - // projection expression only delete the lock if both match, rather than - // checking the ID from the info field first. lockInfo, err := c.getLockInfo(ctx) if err != nil { lockErr.Err = fmt.Errorf("failed to retrieve lock info: %w", err) @@ -445,11 +446,16 @@ func (c *RemoteClient) Unlock(id string) error { return lockErr } + // Use a condition expression to ensure both the lock info and lock ID match params := &dynamodb.DeleteItemInput{ Key: map[string]dtypes.AttributeValue{ "LockID": &dtypes.AttributeValueMemberS{Value: c.lockPath()}, }, - TableName: aws.String(c.ddbTable), + TableName: aws.String(c.ddbTable), + ConditionExpression: aws.String("Info = :info"), + ExpressionAttributeValues: map[string]dtypes.AttributeValue{ + ":info": &dtypes.AttributeValueMemberS{Value: string(lockInfo.Marshal())}, + }, } _, err = c.dynClient.DeleteItem(ctx, params) diff --git a/internal/backend/remote-state/s3/client_test.go b/internal/backend/remote-state/s3/client_test.go index 90708a77e2..40bc563f5b 100644 --- a/internal/backend/remote-state/s3/client_test.go +++ b/internal/backend/remote-state/s3/client_test.go @@ -163,6 +163,21 @@ func TestForceUnlock(t *testing.T) { if err = s2.Unlock(lockID); err != nil { t.Fatal("failed to force-unlock named state") } + + // No State lock information found for the new workspace. The client should throw the appropriate error message. + secondWorkspace := "new-workspace" + s2, err = b2.StateMgr(secondWorkspace) + if err != nil { + t.Fatal(err) + } + err = s2.Unlock(lockID) + if err == nil { + t.Fatal("expected an error to occur:", err) + } + expectedErrorMsg := fmt.Errorf("failed to retrieve lock info: no lock info found for: \"%s/env:/%s/%s\" within the DynamoDB table: %s", bucketName, secondWorkspace, keyName, bucketName) + if err.Error() != expectedErrorMsg.Error() { + t.Errorf("Unlock() error = %v, want: %v", err, expectedErrorMsg) + } } func TestRemoteClient_clientMD5(t *testing.T) {