From 53208307819fb3af10f71fe5c861109a51076a3f Mon Sep 17 00:00:00 2001
From: Blaine Gardner <blaine.gardner@redhat.com>
Date: Wed, 19 Oct 2022 11:59:19 -0600
Subject: [PATCH] object: fix logic for rgw period commit detection

The `periodWillChange()` implementation to detect map keys to ignore.
Previously, we relied on the `GoString()` method to output the same
format always to determine the current path to the
map[string]interface{} node, but that may change depending on the underlying
implementation of go-cmp.

Instead, parse the go-cmp `Path` step by step and construct a JSON path
to the node in a stable format.

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
---
 pkg/operator/ceph/object/admin.go | 36 +++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/pkg/operator/ceph/object/admin.go b/pkg/operator/ceph/object/admin.go
index b6db1e743..30f705680 100644
--- a/pkg/operator/ceph/object/admin.go
+++ b/pkg/operator/ceph/object/admin.go
@@ -380,20 +380,18 @@ func periodWillChange(current, staged string) (bool, error) {
 	// different in the staged period, even when no updates are needed. Sometimes, the values are
 	// reported as different in the staging output but aren't actually changed upon commit.
 	ignorePaths := cmp.FilterPath(func(path cmp.Path) bool {
-		// path.String() outputs nothing for the crude map[string]interface{} JSON struct
-		// Example of path.GoString() output for a long path in the period JSON:
-		// root["period_map"].(map[string]interface {})["short_zone_ids"].([]interface {})[0].(map[string]interface {})["val"].(float64)
-		switch path.GoString() {
-		case `root["id"]`:
-			// "id" is always changed in the staged period, but it doesn't always update.
+		j := toJsonPath(path)
+		switch j {
+		case ".id": // {"epoch"}
+			// "id" always changes in staged period, but not always when committed
 			return true
-		case `root["predecessor_uuid"]`:
-			// "predecessor_uuid" is always changed in the staged period, but it doesn't always update.
+		case ".predecessor_uuid":
+			// "predecessor_uuid" always changes in staged period, but not always when committed
 			return true
-		case `root["realm_epoch"]`:
-			// "realm_epoch" is always incremented in the staged period, but it doesn't always increment.
+		case ".realm_epoch":
+			// "realm_epoch" always increments in staged period, but not always when committed
 			return true
-		case `root["epoch"]`:
+		case ".epoch":
 			// Strangely, "epoch" is not incremented in the staged period even though it is always
 			// incremented upon an actual commit. It could be argued that this behavior is a bug.
 			// Ignore this value to handle the possibility that the behavior changes in the future.
@@ -410,6 +408,22 @@ func periodWillChange(current, staged string) (bool, error) {
 	return (diff != ""), nil
 }
 
+// convert a cmp.Path to a path in json format like might be used with jq to query the item
+// e.g., {"a": {"b": "c"}} returns .a.b
+func toJsonPath(path cmp.Path) string {
+	out := ""
+	for _, step := range path {
+		mi, ok := step.(cmp.MapIndex)
+		if !ok {
+			// because we use a dumb map[string]interface{}, we only need to process map indexes,
+			// but the path has other node types because of how the json gets processed
+			continue
+		}
+		out = out + "." + mi.Key().String()
+	}
+	return out
+}
+
 func GetAdminOPSUserCredentials(objContext *Context, spec *cephv1.ObjectStoreSpec) (string, string, error) {
 	ns := objContext.clusterInfo.Namespace