Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Search] [Onboarding] Search api key refactor #199790

Merged
merged 26 commits into from
Nov 27, 2024
Merged

[Search] [Onboarding] Search api key refactor #199790

merged 26 commits into from
Nov 27, 2024

Conversation

yansavitski
Copy link
Contributor

@yansavitski yansavitski commented Nov 12, 2024 •
edited
Loading

Refactor search api key

  • get rid from useReduces
  • simplify logic in provider
  • create request hooks
  • fix multiple initialization

@yansavitski yansavitski requested a review from a team as a code owner November 12, 2024 13:18
@yansavitski yansavitski added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Search labels Nov 12, 2024
@yansavitski yansavitski enabled auto-merge (squash) November 14, 2024 11:17
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7405

[✅] x-pack/test_serverless/functional/test_suites/search/config.ts: 50/50 tests passed.

see run history

@yansavitski yansavitski linked an issue Nov 14, 2024 that may be closed by this pull request
Failing test: Serverless Search Functional Tests.x-pack/test_serverless/functional/test_suites/search/search_index_detail·ts - serverless search UI Search index detail page index details page overview With data menu action item should be replaced with playground #197144
Closed
@yansavitski yansavitski force-pushed the search-api-key-refactor branch from 40a80bd to 3f5e7c5 Compare November 14, 2024 18:26
TattdCodeMonkey
TattdCodeMonkey reviewed Nov 14, 2024
View reviewed changes
Copy link
Contributor

@TattdCodeMonkey TattdCodeMonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is a good improvement.

x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts Outdated
before(async () => {
await es.index({
index: indexName,
refresh: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure this is enough of a fix, we probably want to leave this skipped until we can so a larger refactor on these tests.

Copy link
Contributor Author

@yansavitski yansavitski Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I've got the test was flaky because when we update data this appear in kibana in random moment and makes test flaky. So I hope this one will permanently fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/elastic/search-team/issues/8627#issuecomment-2476826752

x-pack/test_serverless/functional/test_suites/search/search_index_detail.ts Outdated
await pageObjects.svlApiKeys.expectAPIKeyAvailable();
const apiKey = await pageObjects.svlApiKeys.getAPIKeyFromUI();
await pageObjects.svlSearchIndexDetailPage.expectAPIKeyToBeVisibleInCodeBlock(apiKey);
describe('API key details', () => {
Copy link
Contributor

@TattdCodeMonkey TattdCodeMonkey Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ok, but I'd actually like to see us move these and the start page API key tests into a different file so that we can tag them to skip if they continue to be flakey.

@yansavitski yansavitski disabled auto-merge November 19, 2024 11:54
@yansavitski yansavitski removed a link to an issue Nov 19, 2024
Failing test: Serverless Search Functional Tests.x-pack/test_serverless/functional/test_suites/search/search_index_detail·ts - serverless search UI Search index detail page index details page overview With data menu action item should be replaced with playground #197144
Closed
@yansavitski yansavitski linked an issue Nov 19, 2024 that may be closed by this pull request
Failing test: Serverless Search Functional Tests.x-pack/test_serverless/functional/test_suites/search/search_index_detail·ts - serverless search UI Search index detail page index details page overview With data menu action item should be replaced with playground #197144
Closed
@joemcelroy
Copy link
Member

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#7467

[❌] x-pack/test_serverless/functional/test_suites/search/config.ts: 70/105 tests passed.

see run history

@joemcelroy
Copy link
Member

joemcelroy commented Nov 25, 2024 •
edited
Loading

if the user is a viewer, they should see the "cannot create API key, no permissions" message. Currently allows you to create an API Key and the validaty check also throws an exception

security_exception\n\tRoot causes:\n\t\tsecurity_exception: action [cluster:admin/xpack/security/api_key/get] is unauthorized for user [test_user] with effective roles [viewer], this action is granted by the cluster privileges [read_security,manage_api_key,manage_security,all]"

Not sure why we dont have a FTR test for this. Could you check?

Checked the other scenarios:

  • no key in cluster, should create API key
  • Key already in cluster, doesn't create an API key
  • invalidate API key will delete the current API key and set a new API key.

@yansavitski yansavitski force-pushed the search-api-key-refactor branch from 017845b to 7a47972 Compare November 27, 2024 13:00
@yansavitski yansavitski enabled auto-merge (squash) November 27, 2024 13:02
@yansavitski yansavitski merged commit 1f4bfa9 into elastic:main Nov 27, 2024
8 checks passed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
searchIndices 236 238 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
searchIndices 169.4KB 168.9KB -466.0B

History

@yansavitski yansavitski deleted the search-api-key-refactor branch November 27, 2024 16:51
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
@yansavitski @elasticmachine @CAWilson94
[Search] [Onboarding] Search api key refactor (elastic#199790)
1705075 
Refactor search api key
- get rid from useReduces
- simplify logic in provider
- create request hooks
- fix multiple initialization

---------

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joemcelroy joemcelroy joemcelroy approved these changes

@TattdCodeMonkey TattdCodeMonkey Awaiting requested review from TattdCodeMonkey

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Search v9.0.0
Projects
None yet
Milestone
No milestone
5 participants
@yansavitski @kibanamachine @joemcelroy @elasticmachine @TattdCodeMonkey