From d8b6ded101e61b44fc6e4a454a765802ae8508dd Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 21 Jan 2025 12:38:14 +0300 Subject: [PATCH] K8s: use deprecatedInternalID label rather than SQL annotation (#99263) --- .../apis/dashboard/legacy/sql_dashboards.go | 5 +- pkg/registry/apis/dashboard/sub_dto.go | 9 +-- pkg/registry/apis/folders/conversions.go | 6 -- pkg/registry/apis/iam/serviceaccount/store.go | 6 +- pkg/registry/apis/iam/team/store.go | 5 +- pkg/registry/apis/iam/user/store.go | 6 +- pkg/registry/apps/playlist/conversions.go | 17 +----- .../apps/playlist/conversions_test.go | 56 +++++++++---------- pkg/storage/unified/resource/document_test.go | 4 +- .../resource/testdata/playlist-resource.json | 4 +- .../search/testdata/doc/report-aaa-out.json | 8 +-- .../search/testdata/doc/report-aaa.json | 6 +- pkg/tests/apis/helper.go | 5 ++ pkg/tests/apis/iam/iam_test.go | 9 +-- pkg/tests/apis/playlist/playlist_test.go | 6 +- 15 files changed, 62 insertions(+), 90 deletions(-) diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 96d1819461a..eff5b77f259 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -532,10 +532,7 @@ func (a *dashboardSqlAccess) GetLibraryPanels(ctx context.Context, query Library meta.SetCreatedBy(p.CreatedBy) meta.SetUpdatedBy(p.UpdatedBy) meta.SetUpdatedTimestamp(&p.Updated) - meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "SQL", - Path: strconv.FormatInt(p.ID, 10), - }) + meta.SetDeprecatedInternalID(p.ID) // nolint:staticcheck res.Items = append(res.Items, item) if len(res.Items) > limit { diff --git a/pkg/registry/apis/dashboard/sub_dto.go b/pkg/registry/apis/dashboard/sub_dto.go index 53c9c3da4bb..e48970a7344 100644 --- a/pkg/registry/apis/dashboard/sub_dto.go +++ b/pkg/registry/apis/dashboard/sub_dto.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "strconv" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -120,16 +119,14 @@ func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Ob dto := &dashboards.Dashboard{ UID: name, OrgID: info.OrgID, + ID: obj.GetDeprecatedInternalID(), // nolint:staticcheck } repo, err := obj.GetRepositoryInfo() if err != nil { return nil, err } - if repo != nil && repo.Name == "SQL" { - dto.ID, err = strconv.ParseInt(repo.Path, 10, 64) - if err == nil { - return nil, err - } + if repo != nil && repo.Name == "plugin" { + dto.PluginID = repo.Path } guardian, err := guardian.NewByDashboard(ctx, dto, info.OrgID, user) diff --git a/pkg/registry/apis/folders/conversions.go b/pkg/registry/apis/folders/conversions.go index 4cc10fdbb73..474911d1ac1 100644 --- a/pkg/registry/apis/folders/conversions.go +++ b/pkg/registry/apis/folders/conversions.go @@ -158,12 +158,6 @@ func convertToK8sResource(v *folder.Folder, namespacer request.NamespaceMapper) meta.SetUpdatedTimestamp(&v.Updated) if v.ID > 0 { // nolint:staticcheck meta.SetDeprecatedInternalID(v.ID) // nolint:staticcheck - - meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "SQL", - Path: fmt.Sprintf("%d", v.ID), // nolint:staticcheck - Timestamp: &v.Created, - }) } // #TODO: turns out these get overwritten by Unified Storage (see pkg/storage/unified/apistore/prepare.go) // We're going to have to align with that. For now we do need the user ID because the folder type stores it diff --git a/pkg/registry/apis/iam/serviceaccount/store.go b/pkg/registry/apis/iam/serviceaccount/store.go index 16272972177..d8884e304e8 100644 --- a/pkg/registry/apis/iam/serviceaccount/store.go +++ b/pkg/registry/apis/iam/serviceaccount/store.go @@ -3,7 +3,6 @@ package serviceaccount import ( "context" "fmt" - "strconv" "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -109,10 +108,7 @@ func toSAItem(sa legacy.ServiceAccount, ns string) iamv0.ServiceAccount { } obj, _ := utils.MetaAccessor(&item) obj.SetUpdatedTimestamp(&sa.Updated) - obj.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "SQL", - Path: strconv.FormatInt(sa.ID, 10), - }) + obj.SetDeprecatedInternalID(sa.ID) // nolint:staticcheck return item } diff --git a/pkg/registry/apis/iam/team/store.go b/pkg/registry/apis/iam/team/store.go index 0876913477a..c56bc92ccdf 100644 --- a/pkg/registry/apis/iam/team/store.go +++ b/pkg/registry/apis/iam/team/store.go @@ -135,10 +135,7 @@ func toTeamObject(t team.Team, ns claims.NamespaceInfo) iamv0.Team { } meta, _ := utils.MetaAccessor(&obj) meta.SetUpdatedTimestamp(&t.Updated) - meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "SQL", - Path: strconv.FormatInt(t.ID, 10), - }) + meta.SetDeprecatedInternalID(t.ID) // nolint:staticcheck return obj } diff --git a/pkg/registry/apis/iam/user/store.go b/pkg/registry/apis/iam/user/store.go index 76509686590..e00c47fe891 100644 --- a/pkg/registry/apis/iam/user/store.go +++ b/pkg/registry/apis/iam/user/store.go @@ -3,7 +3,6 @@ package user import ( "context" "fmt" - "strconv" "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -136,9 +135,6 @@ func toUserItem(u *user.User, ns string) iamv0.User { } obj, _ := utils.MetaAccessor(item) obj.SetUpdatedTimestamp(&u.Updated) - obj.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "SQL", - Path: strconv.FormatInt(u.ID, 10), - }) + obj.SetDeprecatedInternalID(u.ID) // nolint:staticcheck return *item } diff --git a/pkg/registry/apps/playlist/conversions.go b/pkg/registry/apps/playlist/conversions.go index 5e7f0ece551..f14f6033d9c 100644 --- a/pkg/registry/apps/playlist/conversions.go +++ b/pkg/registry/apps/playlist/conversions.go @@ -3,7 +3,6 @@ package playlist import ( "encoding/json" "fmt" - "strconv" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -96,12 +95,7 @@ func convertToK8sResource(v *playlistsvc.PlaylistDTO, namespacer request.Namespa if err == nil { meta.SetUpdatedTimestampMillis(v.UpdatedAt) if v.Id > 0 { - createdAt := time.UnixMilli(v.CreatedAt) - meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "SQL", - Path: fmt.Sprintf("%d", v.Id), - Timestamp: &createdAt, - }) + meta.SetDeprecatedInternalID(v.Id) // nolint:staticcheck } } @@ -135,12 +129,5 @@ func getLegacyID(item *unstructured.Unstructured) int64 { if err != nil { return 0 } - info, _ := meta.GetRepositoryInfo() - if info != nil && info.Name == "SQL" { - i, err := strconv.ParseInt(info.Path, 10, 64) - if err == nil { - return i - } - } - return 0 + return meta.GetDeprecatedInternalID() // nolint:staticcheck } diff --git a/pkg/registry/apps/playlist/conversions_test.go b/pkg/registry/apps/playlist/conversions_test.go index a1e1b52a54b..b6dd5d0bc9d 100644 --- a/pkg/registry/apps/playlist/conversions_test.go +++ b/pkg/registry/apps/playlist/conversions_test.go @@ -36,36 +36,36 @@ func TestPlaylistConversion(t *testing.T) { // fmt.Printf("%s", string(out)) require.JSONEq(t, `{ "metadata": { - "name": "abc", - "namespace": "org-3", - "uid": "f0zxjm7ApxOafsn6DLQZ4Ezp78WRUsZqSc4taOSHq1gX", - "resourceVersion": "54321", - "creationTimestamp": "1970-01-01T00:00:12Z", - "annotations": { - "grafana.app/repoPath": "123", - "grafana.app/repoName": "SQL", - "grafana.app/repoTimestamp":"1970-01-01T00:00:12Z", - "grafana.app/updatedTimestamp": "1970-01-01T00:00:54Z" - } + "name": "abc", + "namespace": "org-3", + "uid": "f0zxjm7ApxOafsn6DLQZ4Ezp78WRUsZqSc4taOSHq1gX", + "resourceVersion": "54321", + "creationTimestamp": "1970-01-01T00:00:12Z", + "labels": { + "grafana.app/deprecatedInternalID": "123" + }, + "annotations": { + "grafana.app/updatedTimestamp": "1970-01-01T00:00:54Z" + } }, "spec": { - "title": "MyPlaylists", - "interval": "10s", - "items": [ - { - "type": "dashboard_by_uid", - "value": "UID0" - }, - { - "type": "dashboard_by_tag", - "value": "tagA" - }, - { - "type": "dashboard_by_id", - "value": "123" - } - ] + "title": "MyPlaylists", + "interval": "10s", + "items": [ + { + "type": "dashboard_by_uid", + "value": "UID0" + }, + { + "type": "dashboard_by_tag", + "value": "tagA" + }, + { + "type": "dashboard_by_id", + "value": "123" + } + ] }, "status": {} - }`, string(out)) + }`, string(out)) } diff --git a/pkg/storage/unified/resource/document_test.go b/pkg/storage/unified/resource/document_test.go index 44f180ce703..21ef906c6c0 100644 --- a/pkg/storage/unified/resource/document_test.go +++ b/pkg/storage/unified/resource/document_test.go @@ -41,8 +41,8 @@ func TestStandardDocumentBuilder(t *testing.T) { "updatedBy": "user:XYZ", "name": "test1", "repo": { - "name": "SQL", - "path": "15", + "name": "something", + "path": "path/in/system.json", "hash": "xyz" } }`, string(jj)) diff --git a/pkg/storage/unified/resource/testdata/playlist-resource.json b/pkg/storage/unified/resource/testdata/playlist-resource.json index bab6a01c29f..236ce893368 100644 --- a/pkg/storage/unified/resource/testdata/playlist-resource.json +++ b/pkg/storage/unified/resource/testdata/playlist-resource.json @@ -9,8 +9,8 @@ "annotations": { "grafana.app/createdBy": "user:ABC", "grafana.app/updatedBy": "user:XYZ", - "grafana.app/repoName": "SQL", - "grafana.app/repoPath": "15", + "grafana.app/repoName": "something", + "grafana.app/repoPath": "path/in/system.json", "grafana.app/repoHash": "xyz", "grafana.app/updatedTimestamp": "2024-07-01T10:11:12Z" } diff --git a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json index 26c9b3ee090..ad38a85e3c8 100644 --- a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json @@ -9,9 +9,9 @@ "rv": 1234, "title": "Test AAA", "title_sort": "test aaa", + "labels": { + "grafana.app/deprecatedInternalID": "123" + }, "created": 1706690655000, - "createdBy": "user:abc", - "repo": { - "name": "SQL" - } + "createdBy": "user:abc" } \ No newline at end of file diff --git a/pkg/storage/unified/search/testdata/doc/report-aaa.json b/pkg/storage/unified/search/testdata/doc/report-aaa.json index e6cb9bca024..647cfb8df96 100644 --- a/pkg/storage/unified/search/testdata/doc/report-aaa.json +++ b/pkg/storage/unified/search/testdata/doc/report-aaa.json @@ -8,8 +8,10 @@ "resourceVersion": "1706690655000", "creationTimestamp": "2024-01-31T08:44:15Z", "annotations": { - "grafana.app/createdBy": "user:abc", - "grafana.app/originName": "SQL" + "grafana.app/createdBy": "user:abc" + }, + "labels": { + "grafana.app/deprecatedInternalID": "123" } }, "spec": { diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index 7d1d3084e81..2403743d76a 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -223,6 +223,11 @@ func (c *K8sResourceClient) sanitizeObject(v *unstructured.Unstructured, replace replaceMeta = append(replaceMeta, "creationTimestamp", "resourceVersion", "uid") for _, key := range replaceMeta { + if key == "labels" { + delete(meta, key) + continue + } + old, ok := meta[key] if ok { require.NotEmpty(c.t, old) diff --git a/pkg/tests/apis/iam/iam_test.go b/pkg/tests/apis/iam/iam_test.go index 39deafaccb3..65e4bfdf810 100644 --- a/pkg/tests/apis/iam/iam_test.go +++ b/pkg/tests/apis/iam/iam_test.go @@ -4,13 +4,14 @@ import ( "context" "testing" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/tests/apis" "github.com/grafana/grafana/pkg/tests/testinfra" "github.com/grafana/grafana/pkg/tests/testsuite" - "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" ) var gvrTeams = schema.GroupVersionResource{ @@ -51,7 +52,7 @@ func TestIntegrationIdentity(t *testing.T) { }) rsp, err := teamClient.Resource.List(ctx, metav1.ListOptions{}) require.NoError(t, err) - found := teamClient.SanitizeJSONList(rsp, "name") + found := teamClient.SanitizeJSONList(rsp, "name", "labels") require.JSONEq(t, `{ "items": [ { diff --git a/pkg/tests/apis/playlist/playlist_test.go b/pkg/tests/apis/playlist/playlist_test.go index 903edcf2102..fa93da02e26 100644 --- a/pkg/tests/apis/playlist/playlist_test.go +++ b/pkg/tests/apis/playlist/playlist_test.go @@ -523,17 +523,17 @@ func doPlaylistTests(t *testing.T, helper *apis.K8sTestHelper) *apis.K8sTestHelp k8sList, err := client.Resource.List(context.Background(), metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, 1, len(k8sList.Items)) - require.JSONEq(t, expectedResult, client.SanitizeJSON(&k8sList.Items[0])) + require.JSONEq(t, expectedResult, client.SanitizeJSON(&k8sList.Items[0], "labels")) // Get should return the same result found, err := client.Resource.Get(context.Background(), uid, metav1.GetOptions{}) require.NoError(t, err) - require.JSONEq(t, expectedResult, client.SanitizeJSON(found)) + require.JSONEq(t, expectedResult, client.SanitizeJSON(found, "labels")) // Now modify the interval updatedInterval := `"interval": "10m"` legacyPayload = strings.Replace(legacyPayload, `"interval": "20s"`, updatedInterval, 1) - require.JSONEq(t, expectedResult, client.SanitizeJSON(&k8sList.Items[0])) + require.JSONEq(t, expectedResult, client.SanitizeJSON(&k8sList.Items[0], "labels")) dtoResponse := apis.DoRequest(helper, apis.RequestParams{ User: client.Args.User, Method: http.MethodPut,