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

[Obs AI Assistant] Use cookie auth for internal APIs in serverless tests #203275

Merged
merged 7 commits into from
Dec 9, 2024
Merged

[Obs AI Assistant] Use cookie auth for internal APIs in serverless tests #203275

merged 7 commits into from
Dec 9, 2024

Conversation

viduni94
Copy link
Contributor

@viduni94 viduni94 commented Dec 6, 2024 •
edited
Loading

Summary

Problem

Cookie authentication was introduced in Kibana for serverless internal API tests via #192727.
The serverless tests for Obs AI Assistant still uses API key based auth.

Solution

Change authentication to cookie based auth for internal APIs in serverless tests.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • Unit or functional tests were updated or added to match the most common scenarios
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

@viduni94 viduni94 self-assigned this Dec 6, 2024
@viduni94 viduni94 added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Obs AI Assistant Observability AI Assistant backport:version Backport to applied version labels labels Dec 6, 2024
@viduni94 viduni94 marked this pull request as ready for review December 6, 2024 21:55
@viduni94 viduni94 requested a review from a team as a code owner December 6, 2024 21:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

@viduni94 viduni94 force-pushed the use-cookie-auth-for-internal-apis-serverless branch from 940a9b9 to f8b1bc7 Compare December 6, 2024 21:55
@viduni94 viduni94 linked an issue Dec 6, 2024 that may be closed by this pull request
[Obs AI Assistant] Serverless tests should use cookie based auth instead of api keys for internal routes #192711
Closed
@viduni94 viduni94 force-pushed the use-cookie-auth-for-internal-apis-serverless branch from 24bd659 to 1981617 Compare December 7, 2024 14:15
@viduni94 viduni94 mentioned this pull request Dec 9, 2024
Open
5 tasks
sorenlouv
sorenlouv reviewed Dec 9, 2024
View reviewed changes
...on/test_suites/observability/ai_assistant/tests/knowledge_base/knowledge_base_status.spec.ts Outdated
@@ -70,10 +58,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {
await deleteInferenceEndpoint({ es, name: AI_ASSISTANT_KB_INFERENCE_ID });

const res = await observabilityAIAssistantAPIClient
.slsUser({
.slsAdmin({
Copy link
Member

Choose a reason for hiding this comment

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

If admin privileges are required to access the status endpoint, what happens when users with the editor role call it? The status endpoint is called when starting a conversation to ensure the KB has been setup. Do we have a problem there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @sorenlouv
Thanks for reviewing.

admin privileges are not required to check the KB status (I've verified this locally).
And I updated the test to check the status with the editor role.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

@viduni94 viduni94 requested a review from sorenlouv December 9, 2024 13:29
@viduni94 viduni94 force-pushed the use-cookie-auth-for-internal-apis-serverless branch from 2bda15d to b7ba179 Compare December 9, 2024 13:31
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @viduni94

@neptunian
Copy link
Contributor

Looks great! Thanks!

@viduni94 viduni94 merged commit 7eb0052 into elastic:main Dec 9, 2024
8 checks passed
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
@viduni94 @CAWilson94
[Obs AI Assistant] Use cookie auth for internal APIs in serverless te…
47a43c0 
…sts (elastic#203275)

## Summary

### Problem
Cookie authentication was introduced in Kibana for serverless internal
API tests via elastic#192727.
The serverless tests for Obs AI Assistant still uses API key based auth.

### Solution
Change authentication to cookie based auth for internal APIs in
serverless tests.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/kibana that referenced this pull request Dec 10, 2024
@viduni94 @Samiul-TheSoccerFan
[Obs AI Assistant] Use cookie auth for internal APIs in serverless te…
aa7df6b 
…sts (elastic#203275)

## Summary

### Problem
Cookie authentication was introduced in Kibana for serverless internal
API tests via elastic#192727.
The serverless tests for Obs AI Assistant still uses API key based auth.

### Solution
Change authentication to cookie based auth for internal APIs in
serverless tests.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 11, 2024
@kibanamachine
Copy link
Contributor

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 203275 locally

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
@viduni94 @CAWilson94
[Obs AI Assistant] Use cookie auth for internal APIs in serverless te…
4eb3937 
…sts (elastic#203275)

## Summary

### Problem
Cookie authentication was introduced in Kibana for serverless internal
API tests via elastic#192727.
The serverless tests for Obs AI Assistant still uses API key based auth.

### Solution
Change authentication to cookie based auth for internal APIs in
serverless tests.

### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@kibanamachine
Copy link
Contributor

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 203275 locally

1 similar comment
@kibanamachine
Copy link
Contributor

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 203275 locally

@viduni94 viduni94 added backport:skip This commit does not require backporting and removed backport:version Backport to applied version labels labels Dec 13, 2024
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neptunian neptunian neptunian approved these changes

@sorenlouv sorenlouv sorenlouv approved these changes

Assignees

@viduni94 viduni94

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Obs AI Assistant Observability AI Assistant v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Obs AI Assistant] Serverless tests should use cookie based auth instead of api keys for internal routes
5 participants
@viduni94 @elasticmachine @neptunian @kibanamachine @sorenlouv