From fd3650e9c0252efba33bf4f638ef1f44408cbbd0 Mon Sep 17 00:00:00 2001
From: Qing Hao <qhao@redhat.com>
Date: Wed, 11 Sep 2024 17:08:29 +0800
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20only=20read=20the=20first=20item?=
 =?UTF-8?q?=20when=20RawFeedbackJsonString=20is=20disabled=20(#613)=20(#12?=
 =?UTF-8?q?4)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* only read the first item when RawFeedbackJsonString is disabled

This is to ensure the backward compatible when the feature gate
is disabled



* Add a test for backward compatible



---------

Signed-off-by: Jian Qiu <jqiu@redhat.com>
Co-authored-by: Jian Qiu <jqiu@redhat.com>
---
 pkg/work/spoke/statusfeedback/reader.go      |  9 +++---
 pkg/work/spoke/statusfeedback/reader_test.go | 31 +++++++++++++++++++-
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/pkg/work/spoke/statusfeedback/reader.go b/pkg/work/spoke/statusfeedback/reader.go
index d2eac1e51..f2a6be884 100644
--- a/pkg/work/spoke/statusfeedback/reader.go
+++ b/pkg/work/spoke/statusfeedback/reader.go
@@ -100,12 +100,11 @@ func (s *StatusReader) getValueByJsonPath(name, path string, obj *unstructured.U
 	}
 
 	var value any
-	switch {
-	case len(results) == 0 || len(results[0]) == 0:
-		return nil, nil
-	case len(results) == 1 && len(results[0]) == 1:
+	// if the RawFeedbackJsonString is disabled, we always get the first item
+	if (len(results) == 1 && len(results[0]) == 1) ||
+		!features.SpokeMutableFeatureGate.Enabled(ocmfeature.RawFeedbackJsonString) {
 		value = results[0][0].Interface()
-	default:
+	} else {
 		var resultList []any
 		// only take care the first item in the results list.
 		for _, r := range results[0] {
diff --git a/pkg/work/spoke/statusfeedback/reader_test.go b/pkg/work/spoke/statusfeedback/reader_test.go
index 73243c430..834242f57 100644
--- a/pkg/work/spoke/statusfeedback/reader_test.go
+++ b/pkg/work/spoke/statusfeedback/reader_test.go
@@ -76,6 +76,10 @@ const (
 				{
 					"type":"Available",
 					"status":"true"
+				},
+                {
+					"type":"Ready",
+					"status":"true"
 				}
 			]
 		}
@@ -216,7 +220,7 @@ func TestStatusReader(t *testing.T) {
 			expectedValue: []workapiv1.FeedbackValue{},
 		},
 		{
-			name:   "wrog version set for jsonpaths",
+			name:   "wrong version set for jsonpaths",
 			object: unstrctureObject(deploymentJson),
 			rule: workapiv1.FeedbackRule{
 				Type: workapiv1.JSONPathsType,
@@ -287,6 +291,31 @@ func TestStatusReader(t *testing.T) {
 				},
 			},
 		},
+		{
+			// this is for a backward compatible test, when rawjson is disabled, and there are multiple match on
+			// json path, it should return the first item.
+			name:   "Return 1st item with multiple patch",
+			object: unstrctureObject(deploymentJson),
+			rule: workapiv1.FeedbackRule{
+				Type: workapiv1.JSONPathsType,
+				JsonPaths: []workapiv1.JsonPath{
+					{
+						Name: "type",
+						Path: ".status.conditions[?(@.status==\"true\")].type ",
+					},
+				},
+			},
+			expectError: false,
+			expectedValue: []workapiv1.FeedbackValue{
+				{
+					Name: "type",
+					Value: workapiv1.FieldValue{
+						Type:   workapiv1.String,
+						String: pointer.String("Available"),
+					},
+				},
+			},
+		},
 		{
 			name:      "rawjson value format",
 			object:    unstrctureObject(podJson),