K8s: Fix internal id setting in mode4 (#99720)

This commit is contained in:
Stephanie Hingtgen
2025-01-28 22:17:17 -07:00
committed by GitHub
parent b06f83670e
commit c7f83b7311
2 changed files with 145 additions and 2 deletions

View File

@@ -47,7 +47,7 @@ func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime
return nil, err
}
if obj.GetName() == "" {
return nil, storage.ErrResourceVersionSetOnCreate
return nil, storage.NewInvalidObjError("", "missing name")
}
if obj.GetResourceVersion() != "" {
return nil, storage.ErrResourceVersionSetOnCreate
@@ -60,8 +60,11 @@ func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime
// nolint:staticcheck
id := obj.GetDeprecatedInternalID()
if id < 1 {
// the ID must be smaller than 9007199254740991, otherwise we will lose prescision
// on the frontend, which uses the number type to store ids. The largest safe number in
// javascript is 9007199254740991, compared to 9223372036854775807 as the max int64
// nolint:staticcheck
obj.SetDeprecatedInternalID(s.snowflake.Generate().Int64())
obj.SetDeprecatedInternalID(s.snowflake.Generate().Int64() & ((1 << 52) - 1))
}
}

View File

@@ -0,0 +1,140 @@
package apistore
import (
"context"
"testing"
"time"
"github.com/bwmarrin/snowflake"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/require"
"golang.org/x/exp/rand"
"k8s.io/apimachinery/pkg/api/apitesting"
"k8s.io/apiserver/pkg/storage"
)
func TestPrepareObjectForStorage(t *testing.T) {
_ = v0alpha1.AddToScheme(scheme)
node, err := snowflake.NewNode(rand.Int63n(1024))
require.NoError(t, err)
s := &Storage{
codec: apitesting.TestCodec(codecs, v0alpha1.DashboardResourceInfo.GroupVersion()),
snowflake: node,
opts: StorageOptions{
LargeObjectSupport: nil,
},
}
ctx := identity.WithRequester(context.Background(), &user.SignedInUser{UserID: 1, UserUID: "user-uid"})
t.Run("Error getting requester from context", func(t *testing.T) {
_, err := s.prepareObjectForStorage(context.Background(), nil)
require.Error(t, err)
require.Contains(t, err.Error(), "a Requester was not found in the context")
})
t.Run("Error on missing name", func(t *testing.T) {
dashboard := v0alpha1.Dashboard{}
_, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.Error(t, err)
require.Contains(t, err.Error(), "missing name")
})
t.Run("Error on non-empty resource version", func(t *testing.T) {
dashboard := v0alpha1.Dashboard{}
dashboard.Name = "test-name"
dashboard.ResourceVersion = "123"
_, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.Error(t, err)
require.Equal(t, storage.ErrResourceVersionSetOnCreate, err)
})
t.Run("Generate UID and leave deprecated ID empty, if not required", func(t *testing.T) {
dashboard := v0alpha1.Dashboard{}
dashboard.Name = "test-name"
encodedData, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v0alpha1.Dashboard{})
require.NoError(t, err)
obj, err := utils.MetaAccessor(newObject)
require.NoError(t, err)
require.NotEmpty(t, obj.GetUID(), "")
require.Empty(t, obj.GetDeprecatedInternalID()) // nolint:staticcheck
require.Empty(t, obj.GetGenerateName())
require.Empty(t, obj.GetResourceVersion())
require.Empty(t, obj.GetSelfLink())
require.Empty(t, obj.GetUpdatedBy())
require.Equal(t, obj.GetCreatedBy(), "user:user-uid")
updatedTS, err := obj.GetUpdatedTimestamp()
require.NoError(t, err)
require.Empty(t, updatedTS)
})
t.Run("Should keep repo info", func(t *testing.T) {
dashboard := v0alpha1.Dashboard{}
dashboard.Name = "test-name"
obj := dashboard.DeepCopyObject()
meta, err := utils.MetaAccessor(obj)
require.NoError(t, err)
now := time.Now()
meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{
Name: "test-repo",
Path: "test/path",
Hash: "hash",
Timestamp: &now,
})
encodedData, err := s.prepareObjectForStorage(ctx, obj)
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v0alpha1.Dashboard{})
require.NoError(t, err)
meta, err = utils.MetaAccessor(newObject)
require.NoError(t, err)
require.Equal(t, meta.GetRepositoryHash(), "hash")
require.Equal(t, meta.GetRepositoryName(), "test-repo")
require.Equal(t, meta.GetRepositoryPath(), "test/path")
ts, err := meta.GetRepositoryTimestamp()
require.NoError(t, err)
parsed, err := time.Parse(time.RFC3339, now.UTC().Format(time.RFC3339))
require.NoError(t, err)
require.Equal(t, ts, &parsed)
})
s.opts.RequireDeprecatedInternalID = true
t.Run("Should generate internal id", func(t *testing.T) {
dashboard := v0alpha1.Dashboard{}
dashboard.Name = "test-name"
encodedData, err := s.prepareObjectForStorage(ctx, dashboard.DeepCopyObject())
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v0alpha1.Dashboard{})
require.NoError(t, err)
obj, err := utils.MetaAccessor(newObject)
require.NoError(t, err)
require.NotEmpty(t, obj.GetDeprecatedInternalID()) // nolint:staticcheck
// must be less than the max number value in javascript to avoid precision loss
require.LessOrEqual(t, obj.GetDeprecatedInternalID(), int64(9007199254740991)) // nolint:staticcheck
})
t.Run("Should use deprecated ID if given it", func(t *testing.T) {
dashboard := v0alpha1.Dashboard{}
dashboard.Name = "test-name"
obj := dashboard.DeepCopyObject()
meta, err := utils.MetaAccessor(obj)
require.NoError(t, err)
meta.SetDeprecatedInternalID(1) // nolint:staticcheck
encodedData, err := s.prepareObjectForStorage(ctx, obj)
require.NoError(t, err)
newObject, _, err := s.codec.Decode(encodedData, nil, &v0alpha1.Dashboard{})
require.NoError(t, err)
meta, err = utils.MetaAccessor(newObject)
require.NoError(t, err)
require.Equal(t, meta.GetDeprecatedInternalID(), int64(1)) // nolint:staticcheck
})
}