From a95cf0205350b2b37b36514abbcfb0901b0461f0 Mon Sep 17 00:00:00 2001
From: Edwin Robbins <edwin.robbins@gmail.com>
Date: Tue, 23 Apr 2019 14:04:32 -0700
Subject: [PATCH] Make Unlock key delete conditional on being old leader's

---
 physical/dynamodb/dynamodb.go | 29 +++++++++++++++++++++++++----
 vault/ha.go                   |  4 +++-
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/physical/dynamodb/dynamodb.go b/physical/dynamodb/dynamodb.go
index c827ea2595c5..0e800cafc83c 100644
--- a/physical/dynamodb/dynamodb.go
+++ b/physical/dynamodb/dynamodb.go
@@ -571,10 +571,28 @@ func (l *DynamoDBLock) Unlock() error {
 	}
 
 	l.held = false
-	if err := l.backend.Delete(context.Background(), l.key); err != nil {
-		return err
+
+	// Conditionally delete after check that the key is actually this Vault's and
+	// not been already claimed by another leader
+	condition := "#identity = :identity"
+	deleteMyLock := &dynamodb.DeleteItemInput{
+		TableName:           &l.backend.table,
+		ConditionExpression: &condition,
+		Key: map[string]*dynamodb.AttributeValue{
+			"Path": &dynamodb.AttributeValue{S: aws.String(recordPathForVaultKey(l.key))},
+			"Key":  &dynamodb.AttributeValue{S: aws.String(recordKeyForVaultKey(l.key))},
+		},
+		ExpressionAttributeNames: map[string]*string{
+			"#identity": aws.String("Identity"),
+		},
+		ExpressionAttributeValues: map[string]*dynamodb.AttributeValue{
+			":identity": &dynamodb.AttributeValue{B: []byte(l.identity)},
+		},
 	}
-	return nil
+
+	 _, err := l.backend.client.DeleteItem(deleteMyLock)
+
+	return err
 }
 
 // Value checks whether or not the lock is held by any instance of DynamoDBLock,
@@ -634,7 +652,10 @@ func (l *DynamoDBLock) periodicallyRenewLock(done chan struct{}) {
 		select {
 		case <-ticker.C:
 			// This should not renew the lock if the lock was deleted from under you.
-			l.updateItem(false)
+			err := l.updateItem(false)
+			if err != nil {
+				l.backend.logger.Error("error renewing leadership lock", err)
+			}
 		case <-done:
 			ticker.Stop()
 			return
diff --git a/vault/ha.go b/vault/ha.go
index ee6fe18a5849..386f03ad83a4 100644
--- a/vault/ha.go
+++ b/vault/ha.go
@@ -576,7 +576,9 @@ func (c *Core) waitForLeadership(newLeaderCh chan func(), manualStepDownCh, stop
 					c.logger.Error("clearing leader advertisement failed", "error", err)
 				}
 
-				c.heldHALock.Unlock()
+				if err := c.heldHALock.Unlock(); err != nil {
+					c.logger.Error("unlocking HA lock failed", "error", err)
+				}
 				c.heldHALock = nil
 			}