Skip to content

Setup E2E against Serverless ES, Kibana, Fleet server standalone and Elastic endpoint agent in VM#167720

Merged
patrykkopycinski merged 57 commits intoelastic:mainfrom
patrykkopycinski:feat/dw-serverless-endpoint-cypress
Oct 4, 2023
Merged

Setup E2E against Serverless ES, Kibana, Fleet server standalone and Elastic endpoint agent in VM#167720
patrykkopycinski merged 57 commits intoelastic:mainfrom
patrykkopycinski:feat/dw-serverless-endpoint-cypress

Conversation

@patrykkopycinski
Copy link
Copy Markdown
Contributor

@patrykkopycinski patrykkopycinski commented Sep 30, 2023

Summary

Run Defend Workflows Cypress E2E against Serverless stack, similar to #165415

patrykkopycinski and others added 30 commits July 22, 2023 15:10
…cypress

# Conflicts:
#	x-pack/test/security_solution_cypress/cypress/support/old_headless.ts
#	x-pack/test_serverless/functional/test_suites/security/cypress/cypress.config.ts
#	yarn.lock
…cypress

# Conflicts:
#	package.json
#	packages/kbn-management/settings/components/field_row/__stories__/common.tsx
#	packages/kbn-management/settings/components/field_row/description/deprecation.test.tsx
#	x-pack/plugins/fleet/package.json
#	x-pack/plugins/threat_intelligence/package.json
#	x-pack/test_serverless/functional/test_suites/observability/cypress/package.json
#	x-pack/test_serverless/functional/test_suites/security/cypress/package.json
#	yarn.lock
@patrykkopycinski patrykkopycinski requested review from a team as code owners October 2, 2023 07:20
@patrykkopycinski patrykkopycinski self-assigned this Oct 2, 2023
@patrykkopycinski patrykkopycinski added v8.11.0 release_note:skip Skip the PR/issue when compiling release notes labels Oct 2, 2023
Copy link
Copy Markdown
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

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

:elasticheart:

Copy link
Copy Markdown
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

Left some comments, but ok to merge 👍

return u.toString();
})();

export const waitForKibana = async (kbnClient: KbnClient): Promise<void> => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you edit the @param above in the jsdocs to match the new param name

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI: I seem to being able to use kbnClient for some reason when I initially added this method, but don't recall what that was now. Did you test this code here to ensure it works? (ex. ensuring that the pRetry() below is executes the callback at least twice)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, I did, the previous solution was failing due to the nodeFetch missing cert for SSL endpoints

Copy link
Copy Markdown
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

@patrykkopycinski thank you for making it possible to run Fleet server standalone in Servelress 🙏

I'm good with the changes, left some minor comments.


export const getLocalhostRealIp = (): string | undefined => {
for (const netInterfaceList of Object.values(networkInterfaces())) {
if (netInterfaceList) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: this could be simplified for improved readability

Suggested change
if (netInterfaceList) {
const address = netInterfaceList?.find(
(networkInterface) =>
networkInterface.family === 'IPv4' &&
networkInterface.internal === false &&
networkInterface.address
)?.address;
if (address) {
return address;
}

const realLocalhostIp = getLocalhostRealIp();

if (realLocalhostIp) {
value.push('-p', `${realLocalhostIp}:${port}:${port}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm curious why it was necessary to add real address while it wasn't necessary in #165415?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because in previous case all the containers are running on the Docker and in this case we need to be able to reach the ES docker container from the VM running via Vagrant on the host machine


const getEsPort = <T>(): T | number => {
if (isOpen) {
return 9220;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Avoid hardcoded values. Is there a chance the port changes and this file must be updated as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nope, 9220 is the default port we are using in our FTR tests

};

describe('Artifacts pages', { tags: '@ess' }, () => {
describe('Artifacts pages', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the test really broken in Serverless or you wanna just skip it for now? @skipInServerless tag could be a better choice, see here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

to me, it seems both tags are doing the same, maybe we could simplify and just have one tag for both use cases?

import { login, ROLE } from '../../tasks/login';

describe('Form', { tags: '@ess' }, () => {
describe('Form', { tags: ['@ess', '@serverless', '@brokenInServerless'] }, () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the test really broken in Serverless or you wanna just skip it for now? @skipInServerless tag could be a better choice, see here

@patrykkopycinski patrykkopycinski requested a review from a team as a code owner October 3, 2023 23:51
Copy link
Copy Markdown
Contributor

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

Small comment, otherwise kbn-test, kbn-es, and ftr_configs.yml LGTM


export interface ServerlessOptions extends EsClusterExecOptions, BaseOptions {
/** Publish ES docker container on additional host IP */
host?: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment on lines +317 to +319
if ((options as ServerlessOptions).host) {
value.push('-p', `${(options as ServerlessOptions).host}:${port}:${port}`);
}
Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko Oct 4, 2023

Choose a reason for hiding this comment

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

Just curious if this change needed for Cypress tests or as a general capability to improve developer UX?
If we are keeping the function name resolvePort as is, could you update the description to have few words about host usage and return value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this change is needed to improve dev UX, mostly in case we want to set elastic-agent on a dedicated VM outside of the Docker network

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dmlemeshko updated description, please let me know if that works

@dmlemeshko dmlemeshko self-requested a review October 4, 2023 08:54
Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

kbn-test changes LGTM

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @patrykkopycinski

@patrykkopycinski patrykkopycinski merged commit 5fb9a38 into elastic:main Oct 4, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Oct 4, 2023
@patrykkopycinski patrykkopycinski deleted the feat/dw-serverless-endpoint-cypress branch October 4, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants