From 8b8480017b11e28c37194721c205b567c423a91a Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@redhat.com>
Date: Fri, 9 Apr 2021 14:04:38 +0200
Subject: [PATCH] logging: report issues in rbdImage.DEKStore API with stacks

It helps to get a stack trace when debugging issues. Certain things are
considered bugs in the code (like missing attributes in a struct), and
might cause a panic in certain occasions.

In this case, a missing string will not panic, but the behaviour will
also not be correct (DEKs getting encrypted, but unable to decrypt).
Clearly logging this as a BUG is probably better than calling panic().

Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 internal/rbd/encryption.go | 26 +++++++++++++++++++++-----
 internal/util/util.go      | 10 ++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/internal/rbd/encryption.go b/internal/rbd/encryption.go
index 1284ea212..b4dbe11e0 100644
--- a/internal/rbd/encryption.go
+++ b/internal/rbd/encryption.go
@@ -116,6 +116,11 @@ func (ri *rbdImage) setupEncryption(ctx context.Context) error {
 // from the original, so that both encrypted passphrases (potentially, depends
 // on the DEKStore) have different contents.
 func (ri *rbdImage) copyEncryptionConfig(cp *rbdImage) error {
+	if ri.VolID == cp.VolID {
+		return fmt.Errorf("BUG: %q and %q have the same VolID (%s) "+
+			"set!? Call stack: %s", ri, cp, ri.VolID, util.CallStack())
+	}
+
 	// get the unencrypted passphrase
 	passphrase, err := ri.encryption.GetCryptoPassphrase(ri.VolID)
 	if err != nil {
@@ -281,8 +286,12 @@ func (ri *rbdImage) configureEncryption(kmsID string, credentials map[string]str
 
 // StoreDEK saves the DEK in the metadata, overwrites any existing contents.
 func (ri *rbdImage) StoreDEK(volumeID, dek string) error {
-	if ri.VolID != volumeID {
-		return fmt.Errorf("volume %q can not store DEK for %q", ri.String(), volumeID)
+	if ri.VolID == "" {
+		return fmt.Errorf("BUG: %q does not have VolID set, call "+
+			"stack: %s", ri, util.CallStack())
+	} else if ri.VolID != volumeID {
+		return fmt.Errorf("volume %q can not store DEK for %q",
+			ri.String(), volumeID)
 	}
 
 	return ri.SetMetadata(metadataDEK, dek)
@@ -290,7 +299,10 @@ func (ri *rbdImage) StoreDEK(volumeID, dek string) error {
 
 // FetchDEK reads the DEK from the image metadata.
 func (ri *rbdImage) FetchDEK(volumeID string) (string, error) {
-	if ri.VolID != volumeID {
+	if ri.VolID == "" {
+		return "", fmt.Errorf("BUG: %q does not have VolID set, call "+
+			"stack: %s", ri, util.CallStack())
+	} else if ri.VolID != volumeID {
 		return "", fmt.Errorf("volume %q can not fetch DEK for %q", ri.String(), volumeID)
 	}
 
@@ -300,8 +312,12 @@ func (ri *rbdImage) FetchDEK(volumeID string) (string, error) {
 // RemoveDEK does not need to remove the DEK from the metadata, the image is
 // most likely getting removed.
 func (ri *rbdImage) RemoveDEK(volumeID string) error {
-	if ri.VolID != volumeID {
-		return fmt.Errorf("volume %q can not remove DEK for %q", ri.String(), volumeID)
+	if ri.VolID == "" {
+		return fmt.Errorf("BUG: %q does not have VolID set, call "+
+			"stack: %s", ri, util.CallStack())
+	} else if ri.VolID != volumeID {
+		return fmt.Errorf("volume %q can not remove DEK for %q",
+			ri.String(), volumeID)
 	}
 
 	return nil
diff --git a/internal/util/util.go b/internal/util/util.go
index c4d2b6ece..df2c8eaba 100644
--- a/internal/util/util.go
+++ b/internal/util/util.go
@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"math"
 	"os"
+	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -333,3 +334,12 @@ func getKeys(m map[string]interface{}) []string {
 
 	return keys
 }
+
+// CallStack returns the stack of the calls in the current goroutine. Useful
+// for debugging or reporting errors. This is a friendly alternative to
+// assert() or panic().
+func CallStack() string {
+	stack := make([]byte, 2048)
+	_ = runtime.Stack(stack, false)
+	return string(stack)
+}