Dashboards: Add validation tests for all dual writer modes (#103570)

* Add validation tests for mode 0 and 4

* Add mode 1, 2, 3, 5

---------

Co-authored-by: Marco de Abreu <18629099+marcoabreu@users.noreply.github.com>
This commit is contained in:
Marco de Abreu
2025-04-08 14:37:50 +02:00
committed by GitHub
parent 7366ea946a
commit 0c79a8927b
3 changed files with 93 additions and 26 deletions

View File

@ -269,6 +269,13 @@ func (b *DashboardsAPIBuilder) validateCreate(ctx context.Context, a admission.A
return fmt.Errorf("error getting internal ID: %w", err)
}
// Validate folder existence if specified
if !a.IsDryRun() && accessor.GetFolder() != "" {
if err := b.validateFolderExists(ctx, accessor.GetFolder(), id.GetOrgID()); err != nil {
return err
}
}
// Validate quota
if !a.IsDryRun() {
params := &quota.ScopeParameters{}
@ -350,10 +357,7 @@ func (b *DashboardsAPIBuilder) validateFolderExists(ctx context.Context, folderU
_, err := b.folderClient.Get(ctx, folderUID, orgID, metav1.GetOptions{})
if err != nil {
if errors.Is(err, dashboards.ErrFolderNotFound) {
return err
}
return fmt.Errorf("error checking folder existence: %w", err)
return err
}
return nil

View File

@ -3,22 +3,24 @@ package integration
import (
"context"
"fmt"
"net/http"
"strings"
"testing"
dashboardv1alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1alpha1"
folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
"github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/setting"
"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"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
dashboardv1alpha1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1alpha1"
folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/tests/apis"
"github.com/grafana/grafana/pkg/tests/testinfra"
"github.com/grafana/grafana/pkg/tests/testsuite"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/services/dashboards" // TODO: Check if we can remove this import
@ -32,6 +34,7 @@ func TestMain(m *testing.M) {
// TestContext holds common test resources
type TestContext struct {
Helper *apis.K8sTestHelper
DualWriterMode rest.DualWriterMode
AdminUser apis.User
EditorUser apis.User
ViewerUser apis.User
@ -48,20 +51,34 @@ func TestIntegrationValidation(t *testing.T) {
t.Skip("skipping integration test")
}
// Create a K8sTestHelper which will set up a real API server
helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
DisableAnonymous: true,
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesClientDashboardsFolders, // Enable dashboard feature
},
})
dualWriterModes := []rest.DualWriterMode{rest.Mode0, rest.Mode1, rest.Mode2, rest.Mode3, rest.Mode4, rest.Mode5}
for _, dualWriterMode := range dualWriterModes {
t.Run(fmt.Sprintf("DualWriterMode %d", dualWriterMode), func(t *testing.T) {
// Create a K8sTestHelper which will set up a real API server
helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
DisableAnonymous: true,
EnableFeatureToggles: []string{
featuremgmt.FlagKubernetesClientDashboardsFolders, // Enable dashboard feature
featuremgmt.FlagUnifiedStorageSearch,
},
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
"dashboards.dashboard.grafana.app": {
DualWriterMode: dualWriterMode,
},
}})
testIntegrationValidationForServer(t, helper, dualWriterMode)
})
}
}
func testIntegrationValidationForServer(t *testing.T, helper *apis.K8sTestHelper, dualWriterMode rest.DualWriterMode) {
t.Cleanup(func() {
helper.Shutdown()
})
// Create test contexts organization
org1Ctx := createTestContext(t, helper, helper.Org1)
org1Ctx := createTestContext(t, helper, helper.Org1, dualWriterMode)
t.Run("Organization 1 tests", func(t *testing.T) {
t.Run("Dashboard validation tests", func(t *testing.T) {
@ -194,7 +211,7 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) {
t.Run("reject dashboard with non-existent folder UID", func(t *testing.T) {
nonExistentFolderUID := "non-existent-folder-uid"
_, err := createDashboard(t, adminClient, "Dashboard in Non-existent Folder", &nonExistentFolderUID, nil)
require.Error(t, err)
ctx.Helper.EnsureStatusError(err, http.StatusNotFound, "folder not found")
})
})
@ -247,6 +264,9 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) {
// Test generation conflict when updating concurrently
t.Run("reject update with version conflict", func(t *testing.T) {
// Depends on https://github.com/grafana/grafana/pull/102527
ctx.skipIfMode(t, "Default permissions are not set yet in unified storage", rest.Mode3, rest.Mode4, rest.Mode5)
// Create a dashboard with admin
dash, err := createDashboard(t, adminClient, "Dashboard for Version Conflict Test", nil, nil)
require.NoError(t, err, "Failed to create dashboard for version conflict test")
@ -263,10 +283,17 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) {
require.NoError(t, err)
require.NotNil(t, updatedDash1)
// Try to update with the second copy (should fail with version conflict)
_, err = updateDashboard(t, editorClient, dash2, "Updated by second user", nil)
require.Error(t, err)
require.Contains(t, err.Error(), "the object has been modified", "Should fail with version conflict error")
// Try to update with the second copy (should fail with version conflict for mode 0, 4 and 5, but not for mode 1, 2 and 3)
updatedDash2, err := updateDashboard(t, editorClient, dash2, "Updated by second user", nil)
if ctx.DualWriterMode == rest.Mode1 || ctx.DualWriterMode == rest.Mode2 || ctx.DualWriterMode == rest.Mode3 {
require.NoError(t, err)
require.NotNil(t, updatedDash2)
meta, _ := utils.MetaAccessor(updatedDash2)
require.Equal(t, "Updated by second user", meta.FindTitle(""), "Dashboard title should be updated")
} else {
require.Error(t, err)
require.Contains(t, err.Error(), "the object has been modified", "Should fail with version conflict error")
}
// Clean up
err = adminClient.Resource.Delete(context.Background(), dashUID, v1.DeleteOptions{})
@ -508,6 +535,32 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) {
})
}
// skipIfMode skips the current test if running in any of the specified modes
// Usage: skipIfMode(t, rest.Mode1, rest.Mode4)
// or with a message: skipIfMode(t, "Known issue with conflict detection", rest.Mode1, rest.Mode4)
func (c *TestContext) skipIfMode(t *testing.T, args ...interface{}) {
t.Helper()
message := "Test not supported in this dual writer mode"
modes := []rest.DualWriterMode{}
// Parse args - first string is considered a message, all rest.DualWriterMode values are modes to skip
for _, arg := range args {
if msg, ok := arg.(string); ok {
message = msg
} else if mode, ok := arg.(rest.DualWriterMode); ok {
modes = append(modes, mode)
}
}
// Check if current mode is in the list of modes to skip
for _, mode := range modes {
if c.DualWriterMode == mode {
t.Skipf("%s (mode %d)", message, c.DualWriterMode)
}
}
}
// Run tests for quota validation
func runQuotaTests(t *testing.T, ctx TestContext) {
t.Helper()
@ -608,7 +661,7 @@ func runQuotaTests(t *testing.T, ctx TestContext) {
}
// Helper function to create test context for an organization
func createTestContext(t *testing.T, helper *apis.K8sTestHelper, orgUsers apis.OrgUsers) TestContext {
func createTestContext(t *testing.T, helper *apis.K8sTestHelper, orgUsers apis.OrgUsers, dualWriterMode rest.DualWriterMode) TestContext {
// Create test folder
folderTitle := "Test Folder " + orgUsers.Admin.Identity.GetLogin()
testFolder, err := createFolder(t, helper, orgUsers.Admin, folderTitle)
@ -617,6 +670,7 @@ func createTestContext(t *testing.T, helper *apis.K8sTestHelper, orgUsers apis.O
// Create test context
return TestContext{
Helper: helper,
DualWriterMode: dualWriterMode,
AdminUser: orgUsers.Admin,
EditorUser: orgUsers.Editor,
ViewerUser: orgUsers.Viewer,

View File

@ -231,6 +231,15 @@ func (c *K8sTestHelper) AsStatusError(err error) *errors.StatusError {
return statusError
}
func (c *K8sTestHelper) EnsureStatusError(err error, expectedHttpStatus int, expectedMessage string) {
statusError := c.AsStatusError(err)
require.NotNil(c.t, statusError)
require.Equal(c.t, int32(expectedHttpStatus), statusError.ErrStatus.Code)
if expectedMessage != "" {
require.Equal(c.t, expectedMessage, statusError.ErrStatus.Message)
}
}
func (c *K8sResourceClient) SanitizeJSONList(v *unstructured.UnstructuredList, replaceMeta ...string) string {
c.t.Helper()