From 1163ae76286f1a11ec017ddca1e4f56468f3cdcf Mon Sep 17 00:00:00 2001 From: Stephanie Hingtgen Date: Tue, 29 Apr 2025 12:23:45 -0600 Subject: [PATCH] K8s: Fix timestamps and error codes --- .../apis/dashboard/legacy/sql_dashboards.go | 3 +- pkg/registry/apis/folders/legacy_storage.go | 2 +- .../dashboards/service/dashboard_service.go | 4 ++- pkg/services/folder/folderimpl/conversions.go | 4 +-- .../folder/folderimpl/conversions_test.go | 4 +-- .../folderimpl/folder_unifiedstorage_test.go | 28 +++++++++++++++---- 6 files changed, 33 insertions(+), 12 deletions(-) diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 3c032613b05..63048f6f7b2 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -10,6 +10,7 @@ import ( "sync" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -434,7 +435,7 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das return nil, created, err } if failOnExisting && !created { - return nil, created, dashboards.ErrDashboardWithSameUIDExists + return nil, created, apierrors.NewConflict(dashboardV1.DashboardResourceInfo.GroupResource(), dash.Name, dashboards.ErrDashboardWithSameUIDExists) } out, err := a.dashStore.SaveDashboard(ctx, *cmd) diff --git a/pkg/registry/apis/folders/legacy_storage.go b/pkg/registry/apis/folders/legacy_storage.go index b813f066a1a..f71993bfea2 100644 --- a/pkg/registry/apis/folders/legacy_storage.go +++ b/pkg/registry/apis/folders/legacy_storage.go @@ -273,7 +273,7 @@ func (s *legacyStorage) Update(ctx context.Context, NewParentUID: newParent, }) if err != nil { - return nil, created, fmt.Errorf("error changing parent folder spec") + return nil, created, err } } diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index e1cf5e22ff7..cffd14ccca0 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -2274,7 +2274,9 @@ func (dr *DashboardServiceImpl) unstructuredToLegacyDashboardWithUsers(item *uns out.Created = obj.GetCreationTimestamp().Time updated, err := obj.GetUpdatedTimestamp() if err == nil && updated != nil { - out.Updated = *updated + // old apis return in local time, created is already doing that + localTime := updated.Local() + out.Updated = localTime } else { // by default, set updated to created out.Updated = out.Created diff --git a/pkg/services/folder/folderimpl/conversions.go b/pkg/services/folder/folderimpl/conversions.go index fcb9cf79d58..03e4d560756 100644 --- a/pkg/services/folder/folderimpl/conversions.go +++ b/pkg/services/folder/folderimpl/conversions.go @@ -36,12 +36,12 @@ func parseUnstructuredToLegacyFolder(item *unstructured.Unstructured) (*folder.F url = dashboards.GetFolderURL(uid, slug) } - created := meta.GetCreationTimestamp().UTC() + created := meta.GetCreationTimestamp().Local() updated, _ := meta.GetUpdatedTimestamp() if updated == nil { updated = &created } else { - tmp := updated.UTC() + tmp := updated.Local() updated = &tmp } diff --git a/pkg/services/folder/folderimpl/conversions_test.go b/pkg/services/folder/folderimpl/conversions_test.go index a136cec785d..1ad648a1e0b 100644 --- a/pkg/services/folder/folderimpl/conversions_test.go +++ b/pkg/services/folder/folderimpl/conversions_test.go @@ -45,7 +45,7 @@ func TestFolderConversions(t *testing.T) { require.NoError(t, err) created, err := time.Parse(time.RFC3339, "2022-12-02T02:02:02Z") - created = created.UTC() + created = created.Local() require.NoError(t, err) fake := usertest.NewUserServiceFake() @@ -232,7 +232,7 @@ func TestFolderListConversions(t *testing.T) { require.NoError(t, err) created, err := time.Parse(time.RFC3339, "2022-12-02T02:02:02Z") - created = created.UTC() + created = created.Local() require.NoError(t, err) fake := usertest.NewUserServiceFake() diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index 78943c926a9..b0f9439e58e 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -54,6 +54,24 @@ func (r rcp) GetRestConfig(ctx context.Context) (*clientrest.Config, error) { }, nil } +func compareFoldersNormalizeTime(t *testing.T, expected, actual *folder.Folder) { + require.Equal(t, expected.Title, actual.Title) + require.Equal(t, expected.UID, actual.UID) + require.Equal(t, expected.OrgID, actual.OrgID) + require.Equal(t, expected.URL, actual.URL) + require.Equal(t, expected.Fullpath, actual.Fullpath) + require.Equal(t, expected.FullpathUIDs, actual.FullpathUIDs) + require.Equal(t, expected.CreatedByUID, actual.CreatedByUID) + require.Equal(t, expected.UpdatedByUID, actual.UpdatedByUID) + require.Equal(t, expected.ParentUID, actual.ParentUID) + require.Equal(t, expected.Description, actual.Description) + require.Equal(t, expected.HasACL, actual.HasACL) + require.Equal(t, expected.Version, actual.Version) + require.Equal(t, expected.ManagedBy, actual.ManagedBy) + require.Equal(t, expected.Created.Local(), actual.Created.Local()) + require.Equal(t, expected.Updated.Local(), actual.Updated.Local()) +} + func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") @@ -313,7 +331,7 @@ func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) { SignedInUser: usr, }) require.NoError(t, err) - require.Equal(t, f, actualFolder) + compareFoldersNormalizeTime(t, f, actualFolder) }) t.Run("When creating folder should return error if uid is general", func(t *testing.T) { @@ -403,8 +421,8 @@ func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) { OrgID: fooFolder.OrgID, SignedInUser: usr, }) - require.Equal(t, fooFolder, actual) require.NoError(t, err) + compareFoldersNormalizeTime(t, fooFolder, actual) }) t.Run("When get folder by uid and uid is general should return the root folder object", func(t *testing.T) { @@ -437,8 +455,8 @@ func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) { } actual, err := folderService.Get(context.Background(), query) - require.Equal(t, fooFolder, actual) require.NoError(t, err) + compareFoldersNormalizeTime(t, fooFolder, actual) }) t.Run("When get folder by non existing ID should return not found error", func(t *testing.T) { @@ -471,8 +489,8 @@ func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) { } actual, err := folderService.Get(context.Background(), query) - require.Equal(t, fooFolder, actual) require.NoError(t, err) + compareFoldersNormalizeTime(t, fooFolder, actual) }) t.Run("When get folder by non existing Title should return not found error", func(t *testing.T) { @@ -847,7 +865,7 @@ func TestGetFoldersFromApiServer(t *testing.T) { CreatedByUID: ":0", UpdatedByUID: ":0", } - require.Equal(t, expectedResult, result) + compareFoldersNormalizeTime(t, expectedResult, result) fakeK8sClient.AssertExpectations(t) }) }