From 132edd67f709ffaf799833558689d7d50d27479d Mon Sep 17 00:00:00 2001
From: Jussi Maki <jussi.maki@isovalent.com>
Date: Thu, 31 Oct 2024 13:59:43 +0100
Subject: [PATCH] reconciler: Fix Status JSON marshalling

43587d4f2fbe broke JSON marshalling of Status by introducing a separate statusJSON
type and Unmarshal{JSON,YAML} methods, but didn't introduce the Marshal{JSON,YAML}
methods, which caused dictionary key mismatches as Status struct was used to marshal
instead of statusJSON.

Fix this by removing the whole statusJSON construct and add json and yaml tags to Status.
There is no need for filling in the `id` field when unmarshalling Status as this is completely
internal to the reconciler.

Fixes: 43587d4f2fbe ("reconciler: Implement tests using scripttest")

Signed-off-by: Jussi Maki <jussi.maki@isovalent.com>
---
 reconciler/status_test.go | 51 +++++++++++++++++++++++++++++++++++++++
 reconciler/types.go       | 40 +++---------------------------
 2 files changed, 54 insertions(+), 37 deletions(-)

diff --git a/reconciler/status_test.go b/reconciler/status_test.go
index 936f927..07783aa 100644
--- a/reconciler/status_test.go
+++ b/reconciler/status_test.go
@@ -40,6 +40,57 @@ func TestStatusString(t *testing.T) {
 	assert.Regexp(t, `Error: hey I'm an error \([0-9]+\.[0-9]+.+s ago\)`, s.String())
 }
 
+func TestStatusJSON(t *testing.T) {
+	testCases := []struct {
+		s        Status
+		expected string
+	}{
+		{
+			Status{
+				Kind:      StatusKindDone,
+				UpdatedAt: time.Unix(1, 0).UTC(),
+				Error:     "",
+			},
+			`{"kind":"Done","updated-at":"1970-01-01T00:00:01Z"}`,
+		},
+		{
+			Status{
+				Kind:      StatusKindPending,
+				UpdatedAt: time.Unix(2, 0).UTC(),
+				Error:     "",
+			},
+			`{"kind":"Pending","updated-at":"1970-01-01T00:00:02Z"}`,
+		},
+		{
+			Status{
+				Kind:      StatusKindError,
+				UpdatedAt: time.Unix(3, 0).UTC(),
+				Error:     "some-error",
+			},
+			`{"kind":"Error","updated-at":"1970-01-01T00:00:03Z","error":"some-error"}`,
+		},
+		{
+			Status{
+				Kind:      StatusKindRefreshing,
+				UpdatedAt: time.Unix(4, 0).UTC(),
+				Error:     "",
+			},
+			`{"kind":"Refreshing","updated-at":"1970-01-01T00:00:04Z"}`,
+		},
+	}
+
+	for _, tc := range testCases {
+		b, err := json.Marshal(tc.s)
+		assert.NoError(t, err, "Marshal")
+		assert.Equal(t, tc.expected, string(b))
+
+		var s Status
+		assert.NoError(t, json.Unmarshal(b, &s), "Unmarshal")
+		assert.Equal(t, tc.s, s)
+	}
+
+}
+
 func sanitizeAgo(s string) string {
 	r := regexp.MustCompile(`\(.* ago\)`)
 	return string(r.ReplaceAll([]byte(s), []byte("(??? ago)")))
diff --git a/reconciler/types.go b/reconciler/types.go
index 01b9fa8..e436fd4 100644
--- a/reconciler/types.go
+++ b/reconciler/types.go
@@ -20,7 +20,6 @@ import (
 	"github.com/cilium/statedb"
 	"github.com/cilium/statedb/index"
 	"github.com/cilium/statedb/internal"
-	"gopkg.in/yaml.v3"
 )
 
 type Reconciler[Obj any] interface {
@@ -132,9 +131,9 @@ func (s StatusKind) Key() index.Key {
 // the reconciler. Object may have multiple reconcilers and
 // multiple reconciliation statuses.
 type Status struct {
-	Kind      StatusKind
-	UpdatedAt time.Time
-	Error     string
+	Kind      StatusKind `json:"kind" yaml:"kind"`
+	UpdatedAt time.Time  `json:"updated-at" yaml:"updated-at"`
+	Error     string     `json:"error,omitempty" yaml:"error,omitempty"`
 
 	// id is a unique identifier for a pending object.
 	// The reconciler uses this to compare whether the object
@@ -144,39 +143,6 @@ type Status struct {
 	id uint64
 }
 
-// statusJSON defines the JSON/YAML format for [Status]. Separate to
-// [Status] to allow custom unmarshalling that fills in [id].
-type statusJSON struct {
-	Kind      string    `json:"kind" yaml:"kind"`
-	UpdatedAt time.Time `json:"updated-at" yaml:"updated-at"`
-	Error     string    `json:"error,omitempty" yaml:"error,omitempty"`
-}
-
-func (sj *statusJSON) fill(s *Status) {
-	s.Kind = StatusKind(sj.Kind)
-	s.UpdatedAt = sj.UpdatedAt
-	s.Error = sj.Error
-	s.id = nextID()
-}
-
-func (s *Status) UnmarshalYAML(value *yaml.Node) error {
-	var sj statusJSON
-	if err := value.Decode(&sj); err != nil {
-		return err
-	}
-	sj.fill(s)
-	return nil
-}
-
-func (s *Status) UnmarshalJSON(data []byte) error {
-	var sj statusJSON
-	if err := json.Unmarshal(data, &sj); err != nil {
-		return err
-	}
-	sj.fill(s)
-	return nil
-}
-
 func (s Status) IsPendingOrRefreshing() bool {
 	return s.Kind == StatusKindPending || s.Kind == StatusKindRefreshing
 }