[SLO] Check for unique SLO ids across spaces #214496

baileycash-elastic · 2025-03-13T22:26:06Z

Summary

Resolves #212784
Ensure that when an SLO is created, the id is verified across all spaces.

Release Notes

Ensure that when an SLO is created, the id is verified across all spaces.

Testing

Create an SLO and save the id returned in the response in a space "A"
Create a second SLO with the id saved from the first SLO in the request in a different space "B"
User should receive a 409 error from the SLO API.

shahzad31 · 2025-03-14T14:05:00Z

x-pack/solutions/observability/plugins/slo/server/services/slo_repository.ts

      type: SO_SLO_TYPE,
      perPage: 0,
      filter: `slo.attributes.id:(${id})`,
+      namespaces: [...namespaces],


this will only work to spaces which user have access to.
you need to use kibana internal user for this kinda use-case.

Ah yes good catch - Sorry Bailey I forgot the soClient was user bound in this service

…fix'

…-fix'

…fix'

dmlemeshko

x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts changes LGTM

kdelemme · 2025-03-15T14:23:02Z

x-pack/solutions/observability/plugins/slo/server/services/slo_validator.ts

+      type: SO_SLO_TYPE,
+      perPage: 0,
+      filter: `slo.attributes.id:(${id})`,
+      namespaces: [...namespaces],


You can always specify namespaces: ["*"] because there is no other usage now, and remove the params from the function.

kdelemme · 2025-03-15T14:25:45Z

x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts

  }

  private async assertSLOInexistant(slo: SLODefinition) {
-    const exists = await this.repository.exists(slo.id);


I know I'm the one who suggested a service for that but thinking it more thoroughly we could avoid the extra wiring, and just provide the internalSOClient to the CreateSLO application service, and this assertSLOInexistant would query the internalSOClient directly.

The service was actually easier to implement because passing in the new internal client directly would have required updating the repository instances in all routes. Adding a new service was less of a breaking change.

kdelemme · 2025-03-15T14:27:39Z

x-pack/test/api_integration/deployment_agnostic/apis/observability/slo/create_slo.ts

      });
    });

+    it('creates two SLOs with matching ids across different spaces', async () => {


👍🏻 Nice!

baileycash-elastic · 2025-03-17T14:33:20Z

@elasticmachine merge upstream

elasticmachine · 2025-03-17T18:21:26Z

💚 Build Succeeded

Buildkite Build
Commit: 0e08115

Metrics [docs]

✅ unchanged

History

💔 Build #284623 failed a05554f
💔 Build #284328 failed b445717
💔 Build #284310 failed ccf84c2
💔 Build #284269 failed 03f2be8
💔 Build #284152 failed 7d865cb
💔 Build #284148 failed 61745bb

kdelemme

LGTM! Thanks for making the changes

shahzad31 · 2025-03-17T21:34:07Z

x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts

+    const [coreStart] = await corePlugins.getStartServices();
+    const internalSoClient = new SavedObjectsClient(
+      coreStart.savedObjects.createInternalRepository()
+    );


one nice refactor you could do is move these common declarations like internalSOClinet to https://github.com/elastic/kibana/blob/main/x-pack/solutions/observability/plugins/slo/server/routes/register_routes.ts#L30

this it would become available in const sloContext = await context.slo;
and you could pass that context to each service, that way you won't have to declare stuff like this in each route where required.

kibanamachine · 2025-03-18T14:45:23Z

Starting backport for target branches: 9.0

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

kibanamachine · 2025-03-18T14:50:54Z

💔 All backports failed

Status	Branch	Result
❌	9.0	Backport failed because of merge conflicts You might need to backport the following PRs to 9.0: - chore(slo): factorize error handler (#209671)

Manual backport

To create the backport manually run:

node scripts/backport --pr 214496

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2025-03-20T14:48:43Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 214496 locally

baileycash-elastic · 2025-03-20T17:03:09Z

💚 All backports created successfully

Status	Branch	Result
✅	9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

## Summary Resolves elastic#212784 Ensure that when an SLO is created, the id is verified across all spaces. ## Release Notes Ensure that when an SLO is created, the id is verified across all spaces. ## Testing 1. Create an SLO and save the id returned in the response in a space "A" 2. Create a second SLO with the id saved from the first SLO in the request in a different space "B" 3. User should receive a 409 error from the SLO API. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 56f1ebf) # Conflicts: # x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts # x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts # x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts

## Summary Resolves elastic#212784 Ensure that when an SLO is created, the id is verified across all spaces. ## Release Notes Ensure that when an SLO is created, the id is verified across all spaces. ## Testing 1. Create an SLO and save the id returned in the response in a space "A" 2. Create a second SLO with the id saved from the first SLO in the request in a different space "B" 3. User should receive a 409 error from the SLO API. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

## Summary Resolves elastic#212784 Ensure that when an SLO is created, the id is verified across all spaces. ## Release Notes Ensure that when an SLO is created, the id is verified across all spaces. ## Testing 1. Create an SLO and save the id returned in the response in a space "A" 2. Create a second SLO with the id saved from the first SLO in the request in a different space "B" 3. User should receive a 409 error from the SLO API. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 56f1ebf) # Conflicts: # x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts # x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts # x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts

baileycash-elastic · 2025-03-21T13:42:32Z

💚 All backports created successfully

Status	Branch	Result
✅	9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2025-03-21T14:48:51Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

# Backport This will backport the following commits from `main` to `9.0`: - [[SLO] Check for unique SLO ids across spaces (#214496)](#214496)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <[email protected]>

## Summary Resolves elastic#212784 Ensure that when an SLO is created, the id is verified across all spaces. ## Release Notes Ensure that when an SLO is created, the id is verified across all spaces. ## Testing 1. Create an SLO and save the id returned in the response in a space "A" 2. Create a second SLO with the id saved from the first SLO in the request in a different space "B" 3. User should receive a 409 error from the SLO API. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

…lastic#215379) # Backport This will backport the following commits from `main` to `9.0`: - [[SLO] Check for unique SLO ids across spaces (elastic#214496)](elastic#214496)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 6fe8399) # Conflicts: # x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts # x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts

## Summary Resolves elastic#212784 Ensure that when an SLO is created, the id is verified across all spaces. ## Release Notes Ensure that when an SLO is created, the id is verified across all spaces. ## Testing 1. Create an SLO and save the id returned in the response in a space "A" 2. Create a second SLO with the id saved from the first SLO in the request in a different space "B" 3. User should receive a 409 error from the SLO API. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 56f1ebf) # Conflicts: # x-pack/solutions/observability/plugins/slo/server/routes/slo/create_slo.ts # x-pack/solutions/observability/plugins/slo/server/routes/slo/inspect_slo.ts # x-pack/solutions/observability/plugins/slo/server/services/create_slo.test.ts # x-pack/solutions/observability/plugins/slo/server/services/create_slo.ts

baileycash-elastic · 2025-05-06T15:27:53Z

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.19`: - [[SLO] Check for unique SLO ids across spaces (#214496)](#214496)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kevin Delemme <[email protected]>

baileycash-elastic added 4 commits March 13, 2025 09:20

consider namespace for slo existence validation

da55fe0

add test for space check

8400f3f

improve syntax

bb9a1c3

Merge branch 'main' into slo-212784

48dfbf4

baileycash-elastic marked this pull request as ready for review March 14, 2025 13:56

baileycash-elastic requested review from a team as code owners March 14, 2025 13:56

baileycash-elastic added backport:prev-minor Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// release_note:fix labels Mar 14, 2025

baileycash-elastic changed the title ~~Slo 212784~~ [SLO] Check for unique SLO ids across spaces Mar 14, 2025

baileycash-elastic added the v9.1.0 label Mar 14, 2025

shahzad31 reviewed Mar 14, 2025

View reviewed changes

kibanamachine added 3 commits March 14, 2025 14:06

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

0c8df52

…fix'

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

61745bb

…-fix'

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

7d865cb

…fix'

dmlemeshko reviewed Mar 14, 2025

View reviewed changes

dmlemeshko self-requested a review March 14, 2025 15:27

dmlemeshko approved these changes Mar 14, 2025

View reviewed changes

kc13greiner approved these changes Mar 14, 2025

View reviewed changes

baileycash-elastic added 2 commits March 14, 2025 14:20

introduce internal so client for slo validation

03f2be8

rework solution

ccf84c2

baileycash-elastic force-pushed the slo-212784 branch from f8a3c61 to ccf84c2 Compare March 14, 2025 20:14

fix tests

b445717

kdelemme reviewed Mar 15, 2025

View reviewed changes

always validate against all spaces

fde79d6

Merge branch 'main' into slo-212784

a05554f

baileycash-elastic added 4 commits March 17, 2025 12:36

simplify solution, remove incompaitble fields for serverless test

34d37f0

remove empty line

1446124

remove export no longer used

b665609

use spaces constant

0e08115

kdelemme approved these changes Mar 17, 2025

View reviewed changes

shahzad31 reviewed Mar 17, 2025

View reviewed changes

baileycash-elastic merged commit 56f1ebf into elastic:main Mar 18, 2025
9 checks passed

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 20, 2025

baileycash-elastic mentioned this pull request Mar 20, 2025

[9.0] [SLO] Check for unique SLO ids across spaces (#214496) #215379

Merged

kibanamachine added v9.0.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 21, 2025

baileycash-elastic mentioned this pull request May 6, 2025

[8.19] [SLO] Check for unique SLO ids across spaces (#214496) #220267

Merged

kibanamachine added the v8.19.0 label May 7, 2025

[SLO] Check for unique SLO ids across spaces #214496

[SLO] Check for unique SLO ids across spaces #214496

Uh oh!

Conversation

baileycash-elastic commented Mar 13, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Release Notes

Testing

Uh oh!

shahzad31 Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme Mar 14, 2025

Choose a reason for hiding this comment

Uh oh!

dmlemeshko left a comment

Choose a reason for hiding this comment

Uh oh!

kdelemme Mar 15, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme Mar 15, 2025

Choose a reason for hiding this comment

Uh oh!

baileycash-elastic Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

kdelemme Mar 15, 2025

Choose a reason for hiding this comment

Uh oh!

baileycash-elastic commented Mar 17, 2025

Uh oh!

elasticmachine commented Mar 17, 2025

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

kdelemme left a comment

Choose a reason for hiding this comment

Uh oh!

shahzad31 Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Mar 18, 2025

Uh oh!

kibanamachine commented Mar 18, 2025

💔 All backports failed

Manual backport

Questions ?

Uh oh!

kibanamachine commented Mar 20, 2025

Uh oh!

baileycash-elastic commented Mar 20, 2025

💚 All backports created successfully

Questions ?

Uh oh!

baileycash-elastic commented Mar 21, 2025

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Mar 21, 2025

Uh oh!

baileycash-elastic commented May 6, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

baileycash-elastic commented Mar 13, 2025 •

edited by kibanamachine

Loading