Merge pull request #14937 from hashicorp/jbardin/GH-14927

check for nil state in s3 client Get

backend/remote-state/s3/client.go

@@ -58,11 +58,19 @@ func (c *RemoteClient) Get() (payload *remote.Payload, err error) {
 			return nil, err
 		}
 
+		// If the remote state was manually removed the payload will be nil,
+		// but if there's still a digest entry for that state we will still try
+		// to compare the MD5 below.
+		var digest []byte
+		if payload != nil {
+			digest = payload.MD5
+		}
+
 		// verify that this state is what we expect
 		if expected, err := c.getMD5(); err != nil {
 			log.Printf("[WARNING] failed to fetch state md5: %s", err)
-		} else if len(expected) > 0 && !bytes.Equal(expected, payload.MD5) {
+		} else if len(expected) > 0 && !bytes.Equal(expected, digest) {
-			log.Printf("[WARNING] state md5 mismatch: expected '%x', got '%x'", expected, payload.MD5)
+			log.Printf("[WARNING] state md5 mismatch: expected '%x', got '%x'", expected, digest)
 
 			if testChecksumHook != nil {
 				testChecksumHook()
@@ -74,7 +82,7 @@ func (c *RemoteClient) Get() (payload *remote.Payload, err error) {
 				continue
 			}
 
-			return nil, fmt.Errorf(errBadChecksumFmt, payload.MD5)
+			return nil, fmt.Errorf(errBadChecksumFmt, digest)
 		}
 
 		break

backend/remote-state/s3/client_test.go

@@ -167,6 +167,8 @@ func TestRemoteClient_clientMD5(t *testing.T) {
 		"lock_table": bucketName,
 	}).(*Backend)
 
+	createS3Bucket(t, b.s3Client, bucketName)
+	defer deleteS3Bucket(t, b.s3Client, bucketName)
 	createDynamoDBTable(t, b.dynClient, bucketName)
 	defer deleteDynamoDBTable(t, b.dynClient, bucketName)
 
@@ -263,8 +265,8 @@ func TestRemoteClient_stateChecksum(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// put the old state in place of the new, without updating the checksum
+	// put an empty state in place to check for panics during get
-	if err := client2.Put(oldState.Bytes()); err != nil {
+	if err := client2.Put([]byte{}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -278,7 +280,18 @@ func TestRemoteClient_stateChecksum(t *testing.T) {
 	consistencyRetryTimeout = 0
 	consistencyRetryPollInterval = 0
 
-	// fetching the state through client1 should now error out due to a
+	// fetching an empty state through client1 should now error out due to a
+	// mismatched checksum.
+	if _, err := client1.Get(); !strings.HasPrefix(err.Error(), errBadChecksumFmt[:80]) {
+		t.Fatalf("expected state checksum error: got %s", err)
+	}
+
+	// put the old state in place of the new, without updating the checksum
+	if err := client2.Put(oldState.Bytes()); err != nil {
+		t.Fatal(err)
+	}
+
+	// fetching the wrong state through client1 should now error out due to a
 	// mismatched checksum.
 	if _, err := client1.Get(); !strings.HasPrefix(err.Error(), errBadChecksumFmt[:80]) {
 		t.Fatalf("expected state checksum error: got %s", err)