diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index f6b45d910ee..998c9b8babe 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -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 := "a.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 diff --git a/pkg/tests/apis/dashboard/integration/api_validation_test.go b/pkg/tests/apis/dashboard/integration/api_validation_test.go index 4f37a30f0e1..a4a38542945 100644 --- a/pkg/tests/apis/dashboard/integration/api_validation_test.go +++ b/pkg/tests/apis/dashboard/integration/api_validation_test.go @@ -3,21 +3,23 @@ package integration import ( "context" "fmt" + "net/http" "strings" "testing" - "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/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" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" @@ -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, diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index fc3b67fe2ef..2c2b7f414f5 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -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()