[dashboards as code] only validate id on PUT route when creating new dashboard by nreese · Pull Request #264161 · elastic/kibana

nreese · 2026-04-17T16:46:44Z

Resolves issue by moving "as code" id validation from route to handler. Id is only validated for "create" requests. Added integration tests to verify both update and create case.

…dashboard

nreese · 2026-04-17T16:46:49Z

/ci

nreese · 2026-04-17T16:48:33Z

/ci

macroscopeapp · 2026-04-17T16:49:01Z

🟢 Good practices in this PR

Nice additions to the test coverage:

The new tests for invalid "as code" id behavior (both update and create paths) validate both status code and response body — following Validate the response body.
Proper use of minimal permissions (editor for writes, viewer for the 403 path) rather than defaulting to admin — following Test with minimal permissions.
Good reuse of shared fixtures (COMMON_HEADERS, KBN_ARCHIVES, apiClient) from the existing fixtures/ directory — following Use constants for shared test values.
Archive fixture for the (my)dashboard saved object is added to the shared archive, keeping test data declarative.

Just remove the two .only calls and this is ready to go.

^{Posted via Macroscope — Scout Test Review}

macroscopeapp · 2026-04-17T16:49:23Z

Catch flakiness early (recommended)

Recommended before merge: run the flaky test runner against this PR to catch flakiness early.

Trigger a run with the Flaky Test Runner UI or post this comment on the PR:

/flaky scoutConfig:src/platform/plugins/shared/dashboard/test/scout/api/playwright.config.ts:30 ftrConfig:src/platform/test/api_integration/config.js:30

This check is experimental. Share your feedback in the #appex-qa channel.

^{Posted via Macroscope — Flaky Test Runner nudge}

nreese · 2026-04-17T16:53:10Z

/ci

macroscopeapp · 2026-04-17T16:53:38Z

🟡 Dropped test coverage — validation - returns 400 if panels is not an array

The previous version of this file (update_dashboard.spec.ts) included a test verifying that panels: {} (an object instead of an array) returns 400. This test was removed in the rename/restructure but not replaced.

Old test (removed):

apiTest('validation - returns 400 if panels is not an array', async ({ apiClient }) => {
  const response = await apiClient.put(`${DASHBOARD_API_PATH}/${TEST_DASHBOARD_ID}`, {
    headers: { ...COMMON_HEADERS, ...editorCredentials.apiKeyHeader },
    body: { title: 'foo', panels: {} },
    responseType: 'json',
  });
  expect(response).toHaveStatusCode(400);
});

This validated the API's schema enforcement for panels. If the removal is intentional (e.g. schema validation changed), please confirm. Otherwise, consider adding it back to upsert_dashboard.spec.ts to preserve coverage.

Ref: Validate the response body

^{Posted via Macroscope — Scout Test Review}

macroscopeapp · 2026-04-17T16:55:18Z

🔵 Minor — Dropped validation test reduces coverage

The previous version of upsert_dashboard.spec.ts included validation - returns 400 if panels is not an array (testing panels: {} against the update endpoint). This test was removed during the reorganization but doesn't appear to be replaced by any of the new tests.

If the removal was intentional (e.g. panels validation is covered elsewhere or is no longer relevant), no action needed — but please confirm. Otherwise, consider re-adding it:

apiTest('validation - returns 400 if panels is not an array', async ({ apiClient }) => {
  const response = await apiClient.put(`${DASHBOARD_API_PATH}/${TEST_DASHBOARD_ID}`, {
    headers: {
      ...COMMON_HEADERS,
      ...editorCredentials.apiKeyHeader,
    },
    body: {
      title: 'foo',
      panels: {},
    },
    responseType: 'json',
  });

  expect(response).toHaveStatusCode(400);
});

See Validate the response body — while the original test only checked the status code, the assertion still provided valuable input-validation coverage.

^{Posted via Macroscope — Scout Test Review}

nreese · 2026-04-17T16:58:03Z

If the removal was intentional (e.g. panels validation is covered elsewhere or is no longer relevant), no action needed — but please confirm. Otherwise, consider re-adding it:

Dropped "panels validation is covered elsewhere or is no longer relevant" test because it was the same as "'validation - returns 400 when body is not valid dashboard shape". We do not need a test for everything key in the dashboard schema. We need just one test that dashboard schema validates request body shape.

nreese · 2026-04-17T16:59:09Z

/ci

nreese · 2026-04-17T19:23:59Z

/ci

nreese · 2026-04-17T19:26:18Z

/ci

nreese · 2026-04-17T20:15:40Z

/ci

elasticmachine · 2026-04-17T21:25:56Z

Pinging @elastic/kibana-presentation (Team:Presentation)

elasticmachine · 2026-04-17T21:27:13Z

💚 Build Succeeded

Buildkite Build
Commit: 97fd318

Metrics [docs]

✅ unchanged

History

💔 Build #429991 failed ac91ff9
💔 Build #429919 failed 5670bb0

Heenawter

LGTM 👍 Changes make sense

TinaHeiligers

left a comment about moving validation from the path to the handler that you may want to keep in mind for debugging sluggish requests.

TinaHeiligers · 2026-04-17T21:36:40Z

          params: schema.object({
-            id: asCodeIdSchema,
+            // Can not validate id at route level
+            // existing dashboards may have invalid "as code" ids 


FMI, what makes an "as code" id invalid? Is it not pure string type?

src/platform/packages/shared/as-code/shared-schemas/src/schemas/id/id.ts

ID must contain only lowercase letters, numbers, hyphens, and underscores.

TinaHeiligers · 2026-04-17T21:51:23Z

+
+  // Validate id at handler level for create requests
+  if (isCreateRequest) {
+    asCodeIdSchema.validate(id);


Moving the object ID validation from the route into the handler shifts this from “fail fast at the edge” to application logic. That means invalid IDs will be rejected slightly later (a bit more server work/latency per bad request), while HTTP traffic/payload size stays the same—though high volumes of invalid/malicious requests could cost a bit more CPU. The upside is the handler can support richer, context-aware checks (e.g., authorization/ownership), not just format validation.

Really no other way. At the route level, there is no way to know if the PUT request is an update or a create.

We can not have the route failing for update requests since this would make it impossible to update old dashboard with ids that are not valid for new schema. Saved object id schema allowed any character that did is allowed in a URL. We want to limit characters to only letters, numbers, hyphens, and underscores

kibanamachine · 2026-04-18T01:50:00Z

Starting backport for target branches: 9.4

https://github.com/elastic/kibana/actions/runs/24594190205

…dashboard (elastic#264161) Closes elastic#264135 Resolves issue by moving "as code" id validation from route to handler. Id is only validated for "create" requests. Added integration tests to verify both update and create case. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 504f0ba)

…g new dashboard (#264161) (#264243) # Backport This will backport the following commits from `main` to `9.4`: - [dashboards as code] only validate id on PUT route when creating new dashboard (#264161) (504f0ba0)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Nathan Reese <reese.nathan@elastic.co>

* main: (114 commits) Fix observability_ai_assistant_tool_call EBT error when connector is an inference endpoint (elastic#263334) init on install (elastic#263732) [One Workflow] fail-fast TaskRecovery for interrupted runs (elastic#261275) [Entity Store] Reset state error after successful task run (elastic#263087) [api-docs] 2026-04-19 Daily api_docs build (elastic#264280) [UII] Fix integration card row height calculation (elastic#264212) [scout] migrate FTR logstash api tests (elastic#262953) [StorageIndexAdapter] Set auto_expand_replicas to fix yellow health on single-node ES clusters (elastic#263096) [api-docs] 2026-04-18 Daily api_docs build (elastic#264260) [Scout] Update test config manifests (elastic#264257) [Security Solution][Detection Engine] enables AI rule creation feature flag (elastic#264036) [dashboards as code] only validate id on PUT route when creating new dashboard (elastic#264161) chore(NA): bump version to 9.5.0 (elastic#262165) skip failing test suite (elastic#263649) skip failing test suite (elastic#264236) [Discover] Convert remaining Enzyme tests to RTL (elastic#259676) auto-implement: Labels in model endpoints table of the model details flyout look misaligned (elastic#263770) [ci] Promote ES docker image after verification (elastic#263890) [Observability:Onboarding] Remove suppress global announcements that was breaking ensemble tests (elastic#264169) [Cases][AttachmentV2] Migrate persistable state part 2 - ML and AIOps charts (elastic#262597) ...

[dashboards as code] only validate id on PUT route when creating new …

bd34df2

…dashboard

macroscopeapp Bot reviewed Apr 17, 2026

View reviewed changes

Comment thread src/platform/plugins/shared/dashboard/test/scout/api/tests/upsert_dashboard.spec.ts Outdated

macroscopeapp Bot reviewed Apr 17, 2026

View reviewed changes

Comment thread src/platform/plugins/shared/dashboard/test/scout/api/tests/upsert_dashboard.spec.ts Outdated

nreese added 3 commits April 17, 2026 10:49

clean up

61281f6

remove .only

9ed2c5e

eslint

ee92d6a

macroscopeapp Bot reviewed Apr 17, 2026

View reviewed changes

Comment thread src/platform/plugins/shared/dashboard/server/api/update/update.ts

review feedback

5670bb0

kibanamachine and others added 2 commits April 17, 2026 18:01

Changes from node scripts/eslint_all_files --no-cache --fix

a2a7cb7

put test artifact in its own file to avoid breaking other tests

610f5fb

macroscopeapp Bot reviewed Apr 17, 2026

View reviewed changes

Comment thread src/platform/plugins/shared/dashboard/test/scout/api/tests/upsert_dashboard.spec.ts Outdated

cleanup

ac91ff9

Changes from node scripts/eslint_all_files --no-cache --fix

97fd318

nreese marked this pull request as ready for review April 17, 2026 21:25

nreese requested review from a team as code owners April 17, 2026 21:25

nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes labels Apr 17, 2026

nreese added backport:version Backport to applied version labels v9.4.0 v9.5.0 labels Apr 17, 2026

nreese added the Project:Dashboards API label Apr 17, 2026

Heenawter approved these changes Apr 17, 2026

View reviewed changes

TinaHeiligers approved these changes Apr 17, 2026

View reviewed changes

nreese merged commit 504f0ba into elastic:main Apr 18, 2026
27 checks passed

kibanamachine mentioned this pull request Apr 18, 2026

[9.4] [dashboards as code] only validate id on PUT route when creating new dashboard (#264161) #264243

Merged

Conversation

nreese commented Apr 17, 2026 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

Uh oh!

Uh oh!

macroscopeapp Bot commented Apr 17, 2026

Uh oh!

macroscopeapp Bot commented Apr 17, 2026

Catch flakiness early (recommended)

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

Uh oh!

macroscopeapp Bot commented Apr 17, 2026

Uh oh!

macroscopeapp Bot commented Apr 17, 2026

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

nreese commented Apr 17, 2026

Uh oh!

elasticmachine commented Apr 17, 2026

Uh oh!

elasticmachine commented Apr 17, 2026

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

Heenawter left a comment

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers left a comment

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

nreese Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

nreese Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nreese commented Apr 17, 2026 •

edited by kibanamachine

Loading