From 921041478289be2a8a4e998aa128468796e74f3f Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 10 Sep 2024 13:32:18 +0300 Subject: [PATCH] K8s: support unstructured spec+status mutation with GrafanaMetaAccessor (#92970) --- pkg/apimachinery/utils/meta.go | 68 ++++++++++- pkg/apimachinery/utils/meta_test.go | 169 +++++++++++++++++++++++++++- 2 files changed, 229 insertions(+), 8 deletions(-) diff --git a/pkg/apimachinery/utils/meta.go b/pkg/apimachinery/utils/meta.go index b0e1626a678..3c16efc81ca 100644 --- a/pkg/apimachinery/utils/meta.go +++ b/pkg/apimachinery/utils/meta.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -95,6 +96,9 @@ type GrafanaMetaAccessor interface { SetSpec(any) error GetStatus() (any, error) + + // Used by the generic strategy to keep the status value unchanged on an update + // NOTE the type must match the existing value, or an error will be thrown SetStatus(any) error // Find a title in the object @@ -504,7 +508,22 @@ func (m *grafanaMetaAccessor) GetSpec() (spec any, err error) { err = fmt.Errorf("error reading spec") } }() - spec = m.r.FieldByName("Spec").Interface() + + f := m.r.FieldByName("Spec") + if f.IsValid() { + spec = f.Interface() + return + } + + // Unstructured + u, ok := m.raw.(*unstructured.Unstructured) + if ok { + spec, ok = u.Object["spec"] + if ok { + return // no error + } + } + err = fmt.Errorf("unable to read spec") return } @@ -514,7 +533,20 @@ func (m *grafanaMetaAccessor) SetSpec(s any) (err error) { err = fmt.Errorf("error setting spec") } }() - m.r.FieldByName("Spec").Set(reflect.ValueOf(s)) + + f := m.r.FieldByName("Spec") + if f.IsValid() { + f.Set(reflect.ValueOf(s)) + return + } + + // Unstructured + u, ok := m.raw.(*unstructured.Unstructured) + if ok { + u.Object["spec"] = s + } else { + err = fmt.Errorf("unable to set spec") + } return } @@ -524,7 +556,22 @@ func (m *grafanaMetaAccessor) GetStatus() (status any, err error) { err = fmt.Errorf("error reading status") } }() - status = m.r.FieldByName("Status").Interface() + + f := m.r.FieldByName("Status") + if f.IsValid() { + status = f.Interface() + return + } + + // Unstructured + u, ok := m.raw.(*unstructured.Unstructured) + if ok { + status, ok = u.Object["status"] + if ok { + return // no error + } + } + err = fmt.Errorf("unable to read status") return } @@ -534,7 +581,20 @@ func (m *grafanaMetaAccessor) SetStatus(s any) (err error) { err = fmt.Errorf("error setting status") } }() - m.r.FieldByName("Status").Set(reflect.ValueOf(s)) + + f := m.r.FieldByName("Status") + if f.IsValid() { + f.Set(reflect.ValueOf(s)) + return + } + + // Unstructured + u, ok := m.raw.(*unstructured.Unstructured) + if ok { + u.Object["status"] = s + } else { + err = fmt.Errorf("unable to read status") + } return } diff --git a/pkg/apimachinery/utils/meta_test.go b/pkg/apimachinery/utils/meta_test.go index b769c2a2866..76ec22ed4fb 100644 --- a/pkg/apimachinery/utils/meta_test.go +++ b/pkg/apimachinery/utils/meta_test.go @@ -1,14 +1,14 @@ package utils_test import ( + "encoding/json" "testing" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - - "github.com/grafana/grafana/pkg/apimachinery/utils" ) type TestResource struct { @@ -19,6 +19,9 @@ type TestResource struct { metav1.ObjectMeta `json:"metadata,omitempty"` Spec Spec `json:"spec,omitempty"` + + // Read/write raw status + Status Spec `json:"status,omitempty"` } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -76,6 +79,9 @@ type TestResource2 struct { metav1.ObjectMeta `json:"metadata,omitempty"` Spec Spec2 `json:"spec,omitempty"` + + // Exercise read/write pointer status + Status *Spec `json:"status,omitempty"` } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -147,8 +153,79 @@ func TestMetaAccessor(t *testing.T) { require.NoError(t, err) // Must be a pointer }) - t.Run("get and set grafana metadata", func(t *testing.T) { - res := &unstructured.Unstructured{} + t.Run("get and set grafana metadata (unstructured)", func(t *testing.T) { + // Error reading spec+status when missing + res := &unstructured.Unstructured{ + Object: map[string]any{}, + } + meta, err := utils.MetaAccessor(res) + require.NoError(t, err) + spec, err := meta.GetSpec() + require.Error(t, err) + require.Nil(t, spec) + status, err := meta.GetStatus() + require.Error(t, err) + require.Nil(t, status) + + // Now set a spec and status + res.Object = map[string]any{ + "spec": map[string]any{ + "hello": "world", + }, + "status": map[string]any{ + "sloth": "🦥", + }, + } + + meta.SetOriginInfo(originInfo) + meta.SetFolder("folderUID") + + require.Equal(t, map[string]string{ + "grafana.app/originName": "test", + "grafana.app/originPath": "a/b/c", + "grafana.app/originHash": "kkk", + "grafana.app/folder": "folderUID", + }, res.GetAnnotations()) + + meta.SetNamespace("aaa") + meta.SetResourceVersionInt64(12345) + require.Equal(t, "aaa", res.GetNamespace()) + require.Equal(t, "aaa", meta.GetNamespace()) + + rv, err := meta.GetResourceVersionInt64() + require.NoError(t, err) + require.Equal(t, int64(12345), rv) + + // Make sure access to spec works for Unstructured + spec, err = meta.GetSpec() + require.NoError(t, err) + require.Equal(t, res.Object["spec"], spec) + spec = &map[string]string{"a": "b"} + err = meta.SetSpec(spec) + require.NoError(t, err) + spec, err = meta.GetSpec() + require.NoError(t, err) + require.Equal(t, res.Object["spec"], spec) + + // Make sure access to spec works for Unstructured + status, err = meta.GetStatus() + require.NoError(t, err) + require.Equal(t, res.Object["status"], status) + status = &map[string]string{"a": "b"} + err = meta.SetStatus(status) + require.NoError(t, err) + status, err = meta.GetStatus() + require.NoError(t, err) + require.Equal(t, res.Object["status"], status) + }) + + t.Run("get and set grafana metadata (TestResource)", func(t *testing.T) { + res := &TestResource{ + Spec: Spec{ + Title: "test", + }, + // Status is empty, but not nil! + } meta, err := utils.MetaAccessor(res) require.NoError(t, err) @@ -170,6 +247,78 @@ func TestMetaAccessor(t *testing.T) { rv, err := meta.GetResourceVersionInt64() require.NoError(t, err) require.Equal(t, int64(12345), rv) + + // Make sure access to spec works for Unstructured + spec, err := meta.GetSpec() + require.NoError(t, err) + require.Equal(t, res.Spec, spec) + err = meta.SetSpec(Spec{Title: "t2"}) + require.NoError(t, err) + spec, err = meta.GetSpec() + require.NoError(t, err) + require.Equal(t, res.Spec, spec) + require.Equal(t, `{"title":"t2"}`, asJSON(spec, false)) + + // Check read/write status + status, err := meta.GetStatus() + require.NoError(t, err) + require.NotNil(t, status) + err = meta.SetStatus(Spec{Title: "111"}) + require.NoError(t, err) + status, err = meta.GetStatus() + require.NoError(t, err) + require.Equal(t, res.Status, status) + require.Equal(t, "111", res.Status.Title) + require.Equal(t, `{"title":"111"}`, asJSON(status, false)) + }) + + t.Run("get and set grafana metadata (TestResource2)", func(t *testing.T) { + res := &TestResource2{ + Spec: Spec2{}, + Status: &Spec{Title: "X"}, + } + meta, err := utils.MetaAccessor(res) + require.NoError(t, err) + + meta.SetOriginInfo(originInfo) + meta.SetFolder("folderUID") + + require.Equal(t, map[string]string{ + "grafana.app/originName": "test", + "grafana.app/originPath": "a/b/c", + "grafana.app/originHash": "kkk", + "grafana.app/folder": "folderUID", + }, res.GetAnnotations()) + + meta.SetNamespace("aaa") + meta.SetResourceVersionInt64(12345) + require.Equal(t, "aaa", res.GetNamespace()) + require.Equal(t, "aaa", meta.GetNamespace()) + + rv, err := meta.GetResourceVersionInt64() + require.NoError(t, err) + require.Equal(t, int64(12345), rv) + + // Make sure access to spec works for TestResource2 + spec, err := meta.GetSpec() + require.NoError(t, err) + require.Equal(t, res.Spec, spec) + err = meta.SetSpec(Spec2{}) + require.NoError(t, err) + spec, err = meta.GetSpec() + require.NoError(t, err) + require.Equal(t, res.Spec, spec) + + // Make sure access to spec works for TestResource2 + status, err := meta.GetStatus() + require.NoError(t, err) + require.Equal(t, res.Status, status) + err = meta.SetStatus(&Spec{Title: "ZZ"}) + require.NoError(t, err) + status, err = meta.GetStatus() + require.NoError(t, err) + require.Equal(t, res.Status, status) + require.Equal(t, "ZZ", res.Status.Title) }) t.Run("blob info", func(t *testing.T) { @@ -238,3 +387,15 @@ func TestMetaAccessor(t *testing.T) { require.NoError(t, err) }) } + +func asJSON(v any, pretty bool) string { + if v == nil { + return "" + } + if pretty { + bytes, _ := json.MarshalIndent(v, "", " ") + return string(bytes) + } + bytes, _ := json.Marshal(v) + return string(bytes) +}