[FTR] Merge Alerting Apis from `x-pack/test_serverless/shared` into `x-pack/test/api_integration/deployment_agnostic` #193975

wayneseymour · 2024-09-25T11:22:26Z

Summary

Follow up of this pr, per this discussion.

Also, switch from svlCommonApi to samlAuth for internal headers.

wayneseymour · 2024-09-25T13:15:10Z

/ci

wayneseymour · 2024-09-25T15:21:37Z

@dmlemeshko
It just hit me to run the cypress tests once more...after I put in review. Apologies

Drop duplicate

I'll let ci figure it out.

dmlemeshko · 2024-09-25T15:46:57Z

x-pack/test/api_integration/deployment_agnostic/services/alerting_api.ts

-    async findRule(ruleId: string, roleAuthc: RoleCredentials) {
+    async findRuleInRules(roleAuthc: RoleCredentials, ruleId: string) {
      const response = await supertestWithoutAuth
        .get('/api/alerting/rules/_find')
        .set(roleAuthc.apiKeyHeader)


I'm a bit confused by the naming, but I saw there is another findRule function and I think it was confusing initially.

Since you anyways work on the service, what do you think about the following:

findRule -> findRuleById(ruleId: string, roleAuthc: RoleCredentials)
when /api/alerting/rule/${ruleId} is used in API call

findRule -> findInRules(ruleId: string, roleAuthc: RoleCredentials) (or findInAllRules)
when /api/alerting/rules/_find is used in API call and later we filter response

Yeah there were two originally, due to merging the helpers
It's like the helpers were quickly made, instead of doing what we are doing now: discussing where they fit best, not just fit...good enough to compile and run

dmlemeshko · 2024-09-25T15:50:15Z

x-pack/test_serverless/shared/services/deployment_agnostic_services.ts

@@ -8,12 +8,12 @@
 import _ from 'lodash';

 import { services as apiIntegrationServices } from '@kbn/test-suites-xpack/api_integration/services';
-import { AlertingApiProvider } from './alerting_api';
+import { services as apiIntegrationDeploymentAgnosticServices } from '@kbn/test-suites-xpack/api_integration/deployment_agnostic/services';


Do we want load all the deployment-agnostic services or only AlertingApiProvider ?

Glad you caught this.

I took the risk when I did to see if it would pass all the tests.
That said, I do not mind merging it with less risk.

What do you think?

Well, I can quickly see a conflict with the same service sloApi already existing in serverless.

It is really hard to find which service file was loaded, so I suggest explicitly picking only the ones that we need for particular PR. Ideally we can revisit it when more tests are migrated to deployment-agnostic format and "remove" duplicated services, but I would anyway keep importing explicitly 1 by 1 to speedup debug process

Ahh, I like that. Will do

plus folks might update sloApi in test_serverless, but FTR will still pickup deployment-agnostic version of it. Or the opposite, it depends on lines order :)

yeah, that sounds like smth we should fix.
This is not the PR for it, but constantly re-exporting * from abc is prolly not the best for visibility.
If it's not great for us, it's gonna be worse for others

dominiqueclarke

obs-ux-management changes LGTM

wayneseymour · 2024-09-26T08:43:51Z

@elasticmachine merge upstream

findRule -> findRuleById && findRule -> findInRules

…apis

wayneseymour · 2024-09-26T12:40:41Z

@dmlemeshko ok it's looking good with all the cypress tests passing as well.

Anything else you can think of here before merging?

dmlemeshko

Code changes LGTM

kibana-ci · 2024-09-30T09:40:20Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 2c314b1

Failed CI Steps

Jest Tests #9

Test Failures

[job] [logs] Jest Tests #9 / CaseDetailsLink renders

Metrics [docs]

✅ unchanged

History

💚 Build #237298 succeeded cf43583
💚 Build #237256 succeeded dc98297
💚 Build #237115 succeeded 3f01402
💛 Build #237009 was flaky 3e51500cd1803089e504bfd88c16b9085c61ed5f
💔 Build #236958 failed d0dab8d3571d39df570217650106adec6e2a7849

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @wayneseymour

kibanamachine · 2024-10-25T11:58:07Z

Starting backport for target branches: 8.x

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

kibanamachine · 2024-10-25T11:58:08Z

Starting backport for target branches: 8.x

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

kibanamachine · 2024-10-25T12:03:04Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts You might need to backport the following PRs to 8.x: - fix: [Obs Infrastructure > Hosts][KEYBOARD]: Host flyout has buggy scrolling when I expand the State dropdown menu (#192372) - Shutdown Kibana on usages of PKCS12 truststore/keystore config (#192627)

Manual backport

To create the backport manually run:

node scripts/backport --pr 193975

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2024-10-25T12:03:19Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 193975

Questions ?

Please refer to the Backport tool documentation

wayneseymour · 2024-10-25T12:45:18Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

…x-pack/test/api_integration/deployment_agnostic` (elastic#193975) ## Summary Follow up of [this pr](elastic#192216), per [this discussion](elastic#192216 (comment)). Also, switch from `svlCommonApi` to `samlAuth` for internal headers. --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Dzmitry Lemechko <[email protected]> (cherry picked from commit 10c3373) # Conflicts: # x-pack/test_serverless/shared/services/alerting_api.ts

…into `x-pack/test/api_integration/deployment_agnostic` (#193975) (#197822) # Backport This will backport the following commits from `main` to `8.x`: - [[FTR] Merge Alerting Apis from `x-pack/test_serverless/shared` into `x-pack/test/api_integration/deployment_agnostic` (#193975)](#193975)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)

…x-pack/test/api_integration/deployment_agnostic` (elastic#193975) Follow up of [this pr](elastic#192216), per [this discussion](elastic#192216 (comment)). Also, switch from `svlCommonApi` to `samlAuth` for internal headers. --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Dzmitry Lemechko <[email protected]> (cherry picked from commit 10c3373)

wayneseymour added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting FTR labels Sep 25, 2024

wayneseymour self-assigned this Sep 25, 2024

wayneseymour marked this pull request as ready for review September 25, 2024 15:10

wayneseymour requested review from a team as code owners September 25, 2024 15:10

wayneseymour requested a review from dmlemeshko September 25, 2024 15:10

wayneseymour added the ci:all-cypress-suites label Sep 25, 2024

wayneseymour added 4 commits September 25, 2024 16:22

[FTR] Merge Alerting Apis into Deployment Agnostic Services

f081981

Drop duplicate

Bring all da services into svl shared svcs

745de84

Collapse symbols from dropped duplicate into this file.

fa32c58

Update symbols to ref with new name...could be problematic.

6848c6d

I'll let ci figure it out.

wayneseymour force-pushed the merge-alerting-apis branch from 3e51500 to 6848c6d Compare September 25, 2024 15:22

dmlemeshko reviewed Sep 25, 2024

View reviewed changes

Only alerting api.

3f01402

dominiqueclarke approved these changes Sep 26, 2024

View reviewed changes

elasticmachine and others added 3 commits September 26, 2024 10:43

Merge branch 'main' into merge-alerting-apis

dc98297

Refactor...

38dfca1

findRule -> findRuleById && findRule -> findInRules

Merge branch 'main' of github.com:elastic/kibana into merge-alerting-…

cf43583

…apis

dmlemeshko self-requested a review September 30, 2024 07:51

dmlemeshko approved these changes Sep 30, 2024

View reviewed changes

Merge branch 'main' into merge-alerting-apis

2c314b1

wayneseymour merged commit 10c3373 into elastic:main Sep 30, 2024
52 checks passed

kibanamachine added the v9.0.0 label Sep 30, 2024

wayneseymour removed the backport:skip This commit does not require backporting label Oct 25, 2024

kibanamachine added the backport:skip This commit does not require backporting label Oct 25, 2024

wayneseymour added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Oct 25, 2024

wayneseymour mentioned this pull request Oct 25, 2024

[8.x] [FTR] Merge Alerting Apis from x-pack/test_serverless/shared into x-pack/test/api_integration/deployment_agnostic (#193975) #197822

Merged

kibanamachine added the v8.17.0 label Oct 25, 2024

wayneseymour mentioned this pull request Nov 11, 2024

[FTR][Tech Debt] Too many alerting api helpers, can they be reduced? #199619

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FTR] Merge Alerting Apis from `x-pack/test_serverless/shared` into `x-pack/test/api_integration/deployment_agnostic` #193975

[FTR] Merge Alerting Apis from `x-pack/test_serverless/shared` into `x-pack/test/api_integration/deployment_agnostic` #193975

wayneseymour commented Sep 25, 2024 •

edited by kibanamachine

Loading

wayneseymour commented Sep 25, 2024

wayneseymour commented Sep 25, 2024

dmlemeshko Sep 25, 2024

wayneseymour Sep 25, 2024

wayneseymour Sep 26, 2024

dmlemeshko Sep 25, 2024

wayneseymour Sep 25, 2024

dmlemeshko Sep 25, 2024

wayneseymour Sep 25, 2024

dmlemeshko Sep 25, 2024

wayneseymour Sep 25, 2024 •

edited

Loading

wayneseymour Sep 25, 2024

dominiqueclarke left a comment

wayneseymour commented Sep 26, 2024

wayneseymour commented Sep 26, 2024

dmlemeshko left a comment

kibana-ci commented Sep 30, 2024

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

wayneseymour commented Oct 25, 2024

[FTR] Merge Alerting Apis from x-pack/test_serverless/shared into x-pack/test/api_integration/deployment_agnostic #193975

[FTR] Merge Alerting Apis from x-pack/test_serverless/shared into x-pack/test/api_integration/deployment_agnostic #193975

Conversation

wayneseymour commented Sep 25, 2024 • edited by kibanamachine Loading

Summary

wayneseymour commented Sep 25, 2024

wayneseymour commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wayneseymour Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dominiqueclarke left a comment

Choose a reason for hiding this comment

wayneseymour commented Sep 26, 2024

wayneseymour commented Sep 26, 2024

dmlemeshko left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 30, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

History

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

kibanamachine commented Oct 25, 2024

💔 All backports failed

Manual backport

Questions ?

kibanamachine commented Oct 25, 2024

💔 All backports failed

Manual backport

Questions ?

wayneseymour commented Oct 25, 2024

💚 All backports created successfully

Questions ?

[FTR] Merge Alerting Apis from `x-pack/test_serverless/shared` into `x-pack/test/api_integration/deployment_agnostic` #193975

[FTR] Merge Alerting Apis from `x-pack/test_serverless/shared` into `x-pack/test/api_integration/deployment_agnostic` #193975

wayneseymour commented Sep 25, 2024 •

edited by kibanamachine

Loading

wayneseymour Sep 25, 2024 •

edited

Loading