Cleaning up and reworking some implementation for better readability

Signed-off-by: yottta <andrei.ciobanu@opentofu.org>
This commit is contained in:
yottta 2025-02-19 11:25:14 +02:00
parent a6df475a37
commit b61a0f5ff8
4 changed files with 154 additions and 73 deletions

View File

@ -457,7 +457,7 @@ See details: https://cs.opensource.google/go/x/net/+/refs/tags/v0.17.0:http/http
"use_lockfile": {
Type: cty.Bool,
Optional: true,
Description: "Manage locking with an S3 object",
Description: "Manage locking in the same configured S3 bucket",
},
},
}

View File

@ -1810,7 +1810,7 @@ func unmarshalObject(dec cty.Value, atys map[string]cty.Type, path cty.Path) (ct
return cty.ObjectVal(vals), nil
}
func numberOfObjects(t *testing.T, ctx context.Context, s3Client *s3.Client, bucketName string) int {
func numberOfObjectsInBucket(t *testing.T, ctx context.Context, s3Client *s3.Client, bucketName string) int {
resp, err := s3Client.ListObjects(ctx, &s3.ListObjectsInput{Bucket: &bucketName})
if err != nil {
t.Fatalf("error getting objects from bucket %s: %v", bucketName, err)

View File

@ -276,38 +276,9 @@ func (c *RemoteClient) Delete() error {
}
func (c *RemoteClient) Lock(info *statemgr.LockInfo) (string, error) {
s3LockID, err := c.s3Lock(info)
if err != nil {
return "", err
}
dynamoLockID, err := c.dynamoDBLock(info)
if err != nil {
// when second lock fails to get acquired, release the initially acquired one
if uErr := c.s3Unlock(s3LockID); uErr != nil {
log.Printf("[WARN] failed to release the S3 lock on after failed to acquire the dynamoDD lock: %v", uErr)
}
return "", err
}
switch {
case c.useLockfile && c.ddbTable != "":
if s3LockID != dynamoLockID {
log.Printf("[WARN] acquiring s3 and dynamodb lock resulted in different lockIds. s3 lockId: %s; dynamodb lockId; %s", s3LockID, dynamoLockID)
}
return s3LockID, nil
case c.useLockfile:
return s3LockID, nil
case c.ddbTable != "":
return dynamoLockID, nil
}
return "", nil
}
func (c *RemoteClient) dynamoDBLock(info *statemgr.LockInfo) (string, error) {
if c.ddbTable == "" {
if !c.IsLockingEnabled() {
return "", nil
}
info.Path = c.lockPath()
if info.ID == "" {
lockID, err := uuid.GenerateUUID()
if err != nil {
@ -317,6 +288,26 @@ func (c *RemoteClient) dynamoDBLock(info *statemgr.LockInfo) (string, error) {
info.ID = lockID
}
if err := c.s3Lock(info); err != nil {
return "", err
}
if err := c.dynamoDBLock(info); err != nil {
// when the second lock fails from getting acquired, release the initially acquired one
if uErr := c.s3Unlock(info.ID); uErr != nil {
log.Printf("[WARN] failed to release the S3 lock after failed to acquire the dynamoDD lock: %v", uErr)
}
return "", err
}
return info.ID, nil
}
// dynamoDBLock expects the statemgr.LockInfo#ID to be filled already
func (c *RemoteClient) dynamoDBLock(info *statemgr.LockInfo) error {
if c.ddbTable == "" {
return nil
}
info.Path = c.lockPath()
putParams := &dynamodb.PutItemInput{
Item: map[string]dtypes.AttributeValue{
"LockID": &dtypes.AttributeValueMemberS{Value: c.lockPath()},
@ -329,7 +320,7 @@ func (c *RemoteClient) dynamoDBLock(info *statemgr.LockInfo) (string, error) {
ctx := context.TODO()
_, err := c.dynClient.PutItem(ctx, putParams)
if err != nil {
lockInfo, infoErr := c.getLockInfo(ctx)
lockInfo, infoErr := c.getLockInfoFromDynamoDB(ctx)
if infoErr != nil {
err = multierror.Append(err, infoErr)
}
@ -338,26 +329,19 @@ func (c *RemoteClient) dynamoDBLock(info *statemgr.LockInfo) (string, error) {
Err: err,
Info: lockInfo,
}
return "", lockErr
return lockErr
}
return info.ID, nil
return nil
}
func (c *RemoteClient) s3Lock(info *statemgr.LockInfo) (string, error) {
// s3Lock expects the statemgr.LockInfo#ID to be filled already
func (c *RemoteClient) s3Lock(info *statemgr.LockInfo) error {
if !c.useLockfile {
return "", nil
return nil
}
info.Path = c.lockPath()
if info.ID == "" {
lockID, err := uuid.GenerateUUID()
if err != nil {
return "", err
}
info.ID = lockID
}
lInfo := info.Marshal()
putParams := &s3.PutObjectInput{
ContentType: aws.String(contentTypeJSON),
@ -372,7 +356,7 @@ func (c *RemoteClient) s3Lock(info *statemgr.LockInfo) (string, error) {
ctx, _ = attachLoggerToContext(ctx)
_, err := c.s3Client.PutObject(ctx, putParams)
if err != nil {
lockInfo, infoErr := c.getS3LockInfo(ctx)
lockInfo, infoErr := c.getLockInfoFromS3(ctx)
if infoErr != nil {
err = multierror.Append(err, infoErr)
}
@ -381,10 +365,10 @@ func (c *RemoteClient) s3Lock(info *statemgr.LockInfo) (string, error) {
Err: err,
Info: lockInfo,
}
return "", lockErr
return lockErr
}
return info.ID, nil
return nil
}
func (c *RemoteClient) getMD5(ctx context.Context) ([]byte, error) {
@ -464,7 +448,7 @@ func (c *RemoteClient) deleteMD5(ctx context.Context) error {
return nil
}
func (c *RemoteClient) getLockInfo(ctx context.Context) (*statemgr.LockInfo, error) {
func (c *RemoteClient) getLockInfoFromDynamoDB(ctx context.Context) (*statemgr.LockInfo, error) {
getParams := &dynamodb.GetItemInput{
Key: map[string]dtypes.AttributeValue{
"LockID": &dtypes.AttributeValueMemberS{Value: c.lockPath()},
@ -499,7 +483,7 @@ func (c *RemoteClient) getLockInfo(ctx context.Context) (*statemgr.LockInfo, err
return lockInfo, nil
}
func (c *RemoteClient) getS3LockInfo(ctx context.Context) (*statemgr.LockInfo, error) {
func (c *RemoteClient) getLockInfoFromS3(ctx context.Context) (*statemgr.LockInfo, error) {
getParams := &s3.GetObjectInput{
Bucket: aws.String(c.bucketName),
Key: aws.String(c.lockFilePath()),
@ -539,25 +523,31 @@ func (c *RemoteClient) getS3LockInfo(ctx context.Context) (*statemgr.LockInfo, e
}
func (c *RemoteClient) Unlock(id string) error {
if c.useLockfile {
if err := c.s3Unlock(id); err != nil {
return err
}
}
if c.ddbTable != "" {
if err := c.dynamoDBUnlock(id); err != nil {
return err
}
// Attempt to release the lock from both sources.
// We want to do so to be sure that we are leaving no locks unhandled
s3Err := c.s3Unlock(id)
dynamoDBErr := c.dynamoDBUnlock(id)
switch {
case s3Err != nil && dynamoDBErr != nil:
s3Err.Err = multierror.Append(s3Err.Err, dynamoDBErr.Err)
return s3Err
case s3Err != nil:
return s3Err
case dynamoDBErr != nil:
return dynamoDBErr
}
return nil
}
func (c *RemoteClient) s3Unlock(id string) error {
func (c *RemoteClient) s3Unlock(id string) *statemgr.LockError {
if !c.useLockfile {
return nil
}
lockErr := &statemgr.LockError{}
ctx := context.TODO()
ctx, _ = attachLoggerToContext(ctx)
lockInfo, err := c.getS3LockInfo(ctx)
lockInfo, err := c.getLockInfoFromS3(ctx)
if err != nil {
lockErr.Err = fmt.Errorf("failed to retrieve s3 lock info: %w", err)
return lockErr
@ -582,7 +572,7 @@ func (c *RemoteClient) s3Unlock(id string) error {
return nil
}
func (c *RemoteClient) dynamoDBUnlock(id string) error {
func (c *RemoteClient) dynamoDBUnlock(id string) *statemgr.LockError {
if c.ddbTable == "" {
return nil
}
@ -590,7 +580,7 @@ func (c *RemoteClient) dynamoDBUnlock(id string) error {
lockErr := &statemgr.LockError{}
ctx := context.TODO()
lockInfo, err := c.getLockInfo(ctx)
lockInfo, err := c.getLockInfoFromDynamoDB(ctx)
if err != nil {
lockErr.Err = fmt.Errorf("failed to retrieve lock info: %w", err)
return lockErr

View File

@ -59,7 +59,6 @@ func TestRemoteClientLocks(t *testing.T) {
"key": keyName,
"encrypt": true,
"dynamodb_table": bucketName,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
b2 := backend.TestBackendConfig(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(map[string]interface{}{
@ -67,7 +66,6 @@ func TestRemoteClientLocks(t *testing.T) {
"key": keyName,
"encrypt": true,
"dynamodb_table": bucketName,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
ctx := context.TODO()
@ -99,7 +97,6 @@ func TestRemoteS3ClientLocks(t *testing.T) {
"key": keyName,
"encrypt": true,
"use_lockfile": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
b2, _ := backend.TestBackendConfig(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(map[string]interface{}{
@ -107,7 +104,6 @@ func TestRemoteS3ClientLocks(t *testing.T) {
"key": keyName,
"encrypt": true,
"use_lockfile": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
ctx := context.TODO()
@ -138,7 +134,6 @@ func TestRemoteS3AndDynamoDBClientLocks(t *testing.T) {
"key": keyName,
"dynamodb_table": bucketName,
"encrypt": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
b2, _ := backend.TestBackendConfig(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(map[string]interface{}{
@ -147,7 +142,6 @@ func TestRemoteS3AndDynamoDBClientLocks(t *testing.T) {
"dynamodb_table": bucketName,
"encrypt": true,
"use_lockfile": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
ctx := context.TODO()
@ -188,7 +182,6 @@ func TestRemoteS3AndDynamoDBClientLocksWithNoDBInstance(t *testing.T) {
"dynamodb_table": bucketName,
"encrypt": true,
"use_lockfile": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
ctx := context.TODO()
@ -209,7 +202,7 @@ func TestRemoteS3AndDynamoDBClientLocksWithNoDBInstance(t *testing.T) {
}
expected := 0
if actual := numberOfObjects(t, ctx, b1.s3Client, bucketName); actual != expected {
if actual := numberOfObjectsInBucket(t, ctx, b1.s3Client, bucketName); actual != expected {
t.Fatalf("expected to have %d objects but got %d", expected, actual)
}
}
@ -318,7 +311,6 @@ func TestForceUnlockS3Only(t *testing.T) {
"key": keyName,
"encrypt": true,
"use_lockfile": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
b2, _ := backend.TestBackendConfig(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(map[string]interface{}{
@ -326,7 +318,6 @@ func TestForceUnlockS3Only(t *testing.T) {
"key": keyName,
"encrypt": true,
"use_lockfile": true,
// "use_path_style": true, // NOTE: enable this to test against localstack (https://docs.localstack.cloud/getting-started/)
})).(*Backend)
ctx := context.TODO()
@ -400,6 +391,106 @@ func TestForceUnlockS3Only(t *testing.T) {
}
}
// verify that we can unlock a state with an existing lock
func TestForceUnlockS3AndDynamo(t *testing.T) {
testACC(t)
bucketName := fmt.Sprintf("%s-force-s3-dynamo-%x", testBucketPrefix, time.Now().Unix())
keyName := "testState"
b1, _ := backend.TestBackendConfig(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
"dynamodb_table": bucketName,
})).(*Backend)
b2, _ := backend.TestBackendConfig(t, New(encryption.StateEncryptionDisabled()), backend.TestWrapConfig(map[string]interface{}{
"bucket": bucketName,
"key": keyName,
"encrypt": true,
"use_lockfile": true,
"dynamodb_table": bucketName,
})).(*Backend)
ctx := context.TODO()
createS3Bucket(ctx, t, b1.s3Client, bucketName, b1.awsConfig.Region)
defer deleteS3Bucket(ctx, t, b1.s3Client, bucketName)
createDynamoDBTable(ctx, t, b1.dynClient, bucketName)
defer deleteDynamoDBTable(ctx, t, b1.dynClient, bucketName)
// 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")
}
// 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 := []error{
fmt.Errorf("failed to retrieve s3 lock info: operation error S3: GetObject, https response error StatusCode: 404"),
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),
}
for _, expectedErr := range expectedErrorMsg {
if !strings.Contains(err.Error(), expectedErr.Error()) {
t.Errorf("Unlock() should contain expected.\nactual = %v\nexpected = %v", err, expectedErr)
}
}
}
func TestRemoteClient_clientMD5(t *testing.T) {
testACC(t)