K8s: Dashboard spec should not have id (#98336)

This commit is contained in:
Stephanie Hingtgen 2025-01-03 08:28:45 -07:00 committed by GitHub
parent f3a553fb9b
commit b4ab55ae38
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 645 additions and 17 deletions

View File

@ -11,7 +11,6 @@ import (
"time"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/utils/ptr"
"github.com/grafana/authlib/claims"
@ -262,6 +261,7 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows) (*dashboardRow, error) {
meta.SetUpdatedTimestamp(&updated)
meta.SetCreatedBy(getUserID(createdBy, createdByID))
meta.SetUpdatedBy(getUserID(updatedBy, updatedByID))
meta.SetDeprecatedInternalID(dashboard_id) //nolint:staticcheck
if deleted.Valid {
meta.SetDeletionTimestamp(ptr.To(metav1.NewTime(deleted.Time)))
@ -306,8 +306,7 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows) (*dashboardRow, error) {
return row, err
}
}
// add it so we can get it from the body later
dash.Spec.Set("id", dashboard_id)
dash.Spec.Remove("id")
}
return row, err
}
@ -339,14 +338,9 @@ func (a *dashboardSqlAccess) DeleteDashboard(ctx context.Context, orgId int64, u
return dash, false, err
}
id := dash.Spec.GetNestedInt64("id")
if id == 0 {
return nil, false, fmt.Errorf("could not find id in saved body")
}
err = a.dashStore.DeleteDashboard(ctx, &dashboards.DeleteDashboardCommand{
OrgID: orgId,
ID: id,
UID: uid,
})
if err != nil {
return nil, false, err
@ -408,14 +402,18 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das
created = (out.Created.Unix() == out.Updated.Unix()) // and now?
}
dash, _, err = a.GetDashboard(ctx, orgId, out.UID, 0)
if err != nil {
return nil, false, err
}
// stash the raw value in context (if requested)
finalMeta, err := utils.MetaAccessor(dash)
if err != nil {
return nil, false, err
}
access := GetLegacyAccess(ctx)
if access != nil {
id, ok, _ := unstructured.NestedInt64(dash.Spec.Object, "id")
if ok {
access.DashboardID = id
}
access.DashboardID = finalMeta.GetDeprecatedInternalID() // nolint:staticcheck
}
return dash, created, err
}

View File

@ -1,11 +1,14 @@
package v0alpha1
import (
"context"
"errors"
"fmt"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
@ -13,6 +16,7 @@ import (
"k8s.io/kube-openapi/pkg/spec3"
"k8s.io/kube-openapi/pkg/validation/spec"
"github.com/grafana/grafana/pkg/apimachinery/utils"
dashboardinternal "github.com/grafana/grafana/pkg/apis/dashboard"
dashboardv0alpha1 "github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1"
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
@ -230,3 +234,27 @@ func (b *DashboardsAPIBuilder) GetAPIRoutes() *builder.APIRoutes {
defs := b.GetOpenAPIDefinitions()(func(path string) spec.Ref { return spec.Ref{} })
return b.search.GetAPIRoutes(defs)
}
// Mutate removes any internal ID set in the spec & adds it as a label
func (b *DashboardsAPIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
op := a.GetOperation()
if op == admission.Create || op == admission.Update {
obj := a.GetObject()
dash, ok := obj.(*dashboardv0alpha1.Dashboard)
if !ok {
return fmt.Errorf("expected v0alpha1 dashboard")
}
if id, ok := dash.Spec.Object["id"].(float64); ok {
delete(dash.Spec.Object, "id")
if id != 0 {
meta, err := utils.MetaAccessor(obj)
if err != nil {
return err
}
meta.SetDeprecatedInternalID(int64(id)) // nolint:staticcheck
}
}
}
return nil
}

View File

@ -0,0 +1,162 @@
package v0alpha1
import (
"context"
"testing"
common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
"github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
)
func TestDashboardAPIBuilder_Mutate(t *testing.T) {
tests := []struct {
name string
verb string
input *v0alpha1.Dashboard
expected *v0alpha1.Dashboard
}{
{
name: "should remove id and add as label in create",
verb: "CREATE",
input: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should remove id and add as label in update",
verb: "UPDATE",
input: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should only remove id ",
verb: "UPDATE",
input: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
"testing": "this",
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{
"testing": "this",
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should not set label if id is 0",
verb: "CREATE",
input: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{
"id": float64(0),
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v0alpha1.Dashboard{
Spec: common.Unstructured{
Object: map[string]interface{}{},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
},
}
b := &DashboardsAPIBuilder{}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := b.Mutate(context.Background(), admission.NewAttributesRecord(
tt.input,
nil,
v0alpha1.DashboardResourceInfo.GroupVersionKind(),
"stacks-123",
tt.input.Name,
v0alpha1.DashboardResourceInfo.GroupVersionResource(),
"",
admission.Operation(tt.verb),
nil,
true,
&user.SignedInUser{},
), nil)
require.NoError(t, err)
require.Equal(t, tt.expected, tt.input)
})
}
}

View File

@ -1,15 +1,20 @@
package v1alpha1
import (
"context"
"fmt"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
"k8s.io/kube-openapi/pkg/common"
"k8s.io/kube-openapi/pkg/spec3"
"github.com/grafana/grafana/pkg/apimachinery/utils"
dashboardinternal "github.com/grafana/grafana/pkg/apis/dashboard"
dashboardv1alpha1 "github.com/grafana/grafana/pkg/apis/dashboard/v1alpha1"
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
@ -211,3 +216,27 @@ func (b *DashboardsAPIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.Op
}
return oas, nil
}
// Mutate removes any internal ID set in the spec & adds it as a label
func (b *DashboardsAPIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
op := a.GetOperation()
if op == admission.Create || op == admission.Update {
obj := a.GetObject()
dash, ok := obj.(*dashboardv1alpha1.Dashboard)
if !ok {
return fmt.Errorf("expected v1alpha1 dashboard")
}
if id, ok := dash.Spec.Object["id"].(float64); ok {
delete(dash.Spec.Object, "id")
if id != 0 {
meta, err := utils.MetaAccessor(obj)
if err != nil {
return err
}
meta.SetDeprecatedInternalID(int64(id)) // nolint:staticcheck
}
}
}
return nil
}

View File

@ -0,0 +1,186 @@
package v1alpha1
import (
"context"
"testing"
common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
"github.com/grafana/grafana/pkg/apis/dashboard/v1alpha1"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
)
func TestDashboardAPIBuilder_Mutate(t *testing.T) {
tests := []struct {
name string
verb string
input *v1alpha1.Dashboard
expected *v1alpha1.Dashboard
}{
{
name: "should remove id and add as label in create",
verb: "CREATE",
input: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should remove id and add as label in update",
verb: "UPDATE",
input: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should only remove id ",
verb: "UPDATE",
input: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
"testing": "this",
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"testing": "this",
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should not set label if id is 0",
verb: "CREATE",
input: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(0),
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v1alpha1.Dashboard{
Spec: v1alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
},
}
b := &DashboardsAPIBuilder{}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := b.Mutate(context.Background(), admission.NewAttributesRecord(
tt.input,
nil,
v1alpha1.DashboardResourceInfo.GroupVersionKind(),
"stacks-123",
tt.input.Name,
v1alpha1.DashboardResourceInfo.GroupVersionResource(),
"",
admission.Operation(tt.verb),
nil,
true,
&user.SignedInUser{},
), nil)
require.NoError(t, err)
require.Equal(t, tt.expected, tt.input)
})
}
}

View File

@ -1,15 +1,20 @@
package v2alpha1
import (
"context"
"fmt"
"github.com/prometheus/client_golang/prometheus"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authorization/authorizer"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
"k8s.io/kube-openapi/pkg/common"
"k8s.io/kube-openapi/pkg/spec3"
"github.com/grafana/grafana/pkg/apimachinery/utils"
dashboardinternal "github.com/grafana/grafana/pkg/apis/dashboard"
dashboardv2alpha1 "github.com/grafana/grafana/pkg/apis/dashboard/v2alpha1"
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
@ -211,3 +216,27 @@ func (b *DashboardsAPIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.Op
}
return oas, nil
}
// Mutate removes any internal ID set in the spec & adds it as a label
func (b *DashboardsAPIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
op := a.GetOperation()
if op == admission.Create || op == admission.Update {
obj := a.GetObject()
dash, ok := obj.(*dashboardv2alpha1.Dashboard)
if !ok {
return fmt.Errorf("expected v2alpha1 dashboard")
}
if id, ok := dash.Spec.Object["id"].(float64); ok {
delete(dash.Spec.Object, "id")
if id != 0 {
meta, err := utils.MetaAccessor(obj)
if err != nil {
return err
}
meta.SetDeprecatedInternalID(int64(id)) // nolint:staticcheck
}
}
}
return nil
}

View File

@ -0,0 +1,186 @@
package v2alpha1
import (
"context"
"testing"
common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
"github.com/grafana/grafana/pkg/apis/dashboard/v2alpha1"
"github.com/grafana/grafana/pkg/services/user"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission"
)
func TestDashboardAPIBuilder_Mutate(t *testing.T) {
tests := []struct {
name string
verb string
input *v2alpha1.Dashboard
expected *v2alpha1.Dashboard
}{
{
name: "should remove id and add as label in create",
verb: "CREATE",
input: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should remove id and add as label in update",
verb: "UPDATE",
input: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should only remove id ",
verb: "UPDATE",
input: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(1),
"testing": "this",
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"testing": "this",
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Labels: map[string]string{"grafana.app/deprecatedInternalID": "1"},
},
},
},
{
name: "should not set label if id is 0",
verb: "CREATE",
input: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{
"id": float64(0),
},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
expected: &v2alpha1.Dashboard{
Spec: v2alpha1.DashboardSpec{
Title: "test",
Unstructured: common.Unstructured{
Object: map[string]interface{}{},
},
},
TypeMeta: metav1.TypeMeta{
Kind: "Dashboard",
},
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
},
},
}
b := &DashboardsAPIBuilder{}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := b.Mutate(context.Background(), admission.NewAttributesRecord(
tt.input,
nil,
v2alpha1.DashboardResourceInfo.GroupVersionKind(),
"stacks-123",
tt.input.Name,
v2alpha1.DashboardResourceInfo.GroupVersionResource(),
"",
admission.Operation(tt.verb),
nil,
true,
&user.SignedInUser{},
), nil)
require.NoError(t, err)
require.Equal(t, tt.expected, tt.input)
})
}
}

View File

@ -1432,6 +1432,7 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex
out := dashboards.Dashboard{
OrgID: orgID,
ID: obj.GetDeprecatedInternalID(), // nolint:staticcheck
UID: uid,
Slug: obj.GetSlug(),
FolderUID: obj.GetFolder(),
@ -1471,8 +1472,12 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex
out.UpdatedBy = updator.ID
}
// any dashboards that have already been synced to unified storage will have the id in the spec
// and not as a label. We will need to support this conversion until they have all been updated
// to labels
if id, ok := spec["id"].(int64); ok {
out.ID = id
out.Data.Del("id")
}
if gnetID, ok := spec["gnet_id"].(int64); ok {

View File

@ -929,7 +929,6 @@ func TestUnstructuredToLegacyDashboard(t *testing.T) {
"spec": map[string]interface{}{
"title": title,
"version": int64(1),
"id": int64(1),
},
},
}
@ -940,7 +939,7 @@ func TestUnstructuredToLegacyDashboard(t *testing.T) {
obj.SetName(uid)
obj.SetCreatedBy("user:useruid")
obj.SetUpdatedBy("user:useruid")
obj.SetDeprecatedInternalID(1) // nolint:staticcheck
result, err := dr.UnstructuredToLegacyDashboard(context.Background(), item, orgID)
assert.NoError(t, err)
assert.NotNil(t, result)

View File

@ -123,8 +123,14 @@ func (s *Storage) prepareObjectForUpdate(ctx context.Context, updateObject runti
obj.SetCreatedBy(previous.GetCreatedBy())
obj.SetCreationTimestamp(previous.GetCreationTimestamp())
obj.SetResourceVersion("") // removed from saved JSON because the RV is not yet calculated
obj.SetDeprecatedInternalID(previous.GetDeprecatedInternalID()) // nolint:staticcheck
obj.SetResourceVersion("") // removed from saved JSON because the RV is not yet calculated
// for dashboards, a mutation hook will set it if it didn't exist on the previous obj
// avoid setting it back to 0
previousInternalID := previous.GetDeprecatedInternalID() // nolint:staticcheck
if previousInternalID != 0 {
obj.SetDeprecatedInternalID(previousInternalID) // nolint:staticcheck
}
// Read+write will verify that origin format is accurate
repo, err := obj.GetRepositoryInfo()