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.

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

Add mock identity provider for serverless (2nd attempt) #171513

Merged
merged 45 commits into from
Nov 23, 2023

Conversation

thomheymann
Copy link
Contributor

Attempting to merge #170852 again now that the release artefact step has been fixed as part of #171457

thomheymann and others added 30 commits November 8, 2023 11:58
@thomheymann thomheymann added the release_note:skip Skip the PR/issue when compiling release notes label Nov 20, 2023
@thomheymann thomheymann marked this pull request as ready for review November 20, 2023 10:50
@thomheymann thomheymann requested a review from a team as a code owner November 20, 2023 10:50
src/cli/serve/serve.js Outdated Show resolved Hide resolved
jbudz
jbudz previously approved these changes Nov 20, 2023
@jbudz jbudz self-requested a review November 21, 2023 13:09
@jbudz jbudz dismissed their stale review November 21, 2023 13:09

out of date

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM from the security side.

// for the ip which is not validated. As such we are getting the error
// Hostname/IP does not match certificate's altnames: IP: 127.0.0.1 is not in the cert's list:
// To work around that we are overriding the function checkServerIdentity too
checkServerIdentity: () => {
Copy link
Member

Choose a reason for hiding this comment

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

Did we try binding to localhost instead of 127.0.0.1? While this is a test environment..disabling identity checks for every node url regardless of endpoint seems heavy handed.

Copy link
Member

Choose a reason for hiding this comment

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

I think it was added in #165522 to adjust FTR waiting for cluster to be ready. @Ikuni17 you probably the best person to ask this question

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I missed that this was already committed and moved to a different spot. I'll table this discussion for another issue.

@dmlemeshko dmlemeshko enabled auto-merge (squash) November 21, 2023 18:44
@dmlemeshko dmlemeshko merged commit 9220e4d into elastic:main Nov 23, 2023
26 checks passed
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
mockIdpPlugin - 19 +19
Unknown metric groups

API count

id before after diff
mockIdpPlugin - 25 +25

History

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

@kibanamachine kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Nov 23, 2023
darnautov pushed a commit to darnautov/kibana that referenced this pull request Nov 23, 2023
## Summary

This PR disables TLS mode for Kibana run in serverless.
Related to elastic#170417 enabling serverless roles testing 
Blocked by elastic#171513

PR is created in cooperation with @azasypkin and intended to simplify
the automated testing process for serverless:
starting Kibana with TLS enabled adds unnecessary complexity to the
process of getting session cookie and overall Kibana APIs calling with
Dev certificate in the tests.
The selected approach is to disable TLS for Kibana and simply rely on
elastic#171513 to configure mocked idp realm for Serverless ES with TLS
enabled.

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants