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

[Serverless][Testing] Improve serverless force logout and enable some related tests #184202

Merged
merged 10 commits into from
May 31, 2024
Merged

[Serverless][Testing] Improve serverless force logout and enable some related tests #184202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9f78892
add retry (i.e. page reload) to serverless forceLogout
gergoabraham May 24, 2024
cb51073
enable endpoint FTRs
gergoabraham May 24, 2024
1d1efb6
enable landing_page tests (#168037)
gergoabraham May 24, 2024
7b5cf4c
Merge branch 'main' into improve-serverless-force-logout-and-enable-t…
kibanamachine May 24, 2024
c561d2b
Revert "enable landing_page tests (#168037)"
gergoabraham May 24, 2024
ae6bd39
Merge branch 'main' into improve-serverless-force-logout-and-enable-t…
kibanamachine May 27, 2024
cb954eb
Merge branch 'main' into improve-serverless-force-logout-and-enable-t…
kibanamachine May 30, 2024
2bfce62
enable endpoint integration tests (#184319)
gergoabraham May 31, 2024
fb9cae3
review findings: improve comments
gergoabraham May 31, 2024
04d415e
Merge branch 'main' into improve-serverless-force-logout-and-enable-t…
gergoabraham May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 42 additions & 36 deletions x-pack/test/functional/page_objects/security_page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,44 +306,50 @@ export class SecurityPageObject extends FtrService {
return;
}

this.log.debug(`Redirecting to ${this.deployment.getHostPort()}/logout to force the logout`);
const url = this.deployment.getHostPort() + '/logout';
await this.browser.get(url);

// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
if (waitForLoginPage) {
this.log.debug('Waiting on the login form to appear');
await this.waitForLoginPage();
} else {
this.log.debug('Waiting for logout to complete');
await this.retry.waitFor('logout to complete', async () => {
// There are cases when browser/Kibana would like users to confirm that they want to navigate away from the
// current page and lose the state (e.g. unsaved changes) via native alert dialog.
const alert = await this.browser.getAlert();
if (alert?.accept) {
await alert.accept();
}

// Timeout has been doubled here in attempt to quiet the flakiness
await this.retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await this.browser.getCurrentUrl();
await this.delay(1000);
const urlAfter = await this.browser.getCurrentUrl();
this.log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
const performForceLogout = async () => {
this.log.debug(`Redirecting to ${this.deployment.getHostPort()}/logout to force the logout`);
const url = this.deployment.getHostPort() + '/logout';
await this.browser.get(url);

// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
if (waitForLoginPage) {
this.log.debug('Waiting on the login form to appear');
await this.waitForLoginPage();
} else {
this.log.debug('Waiting for logout to complete');
await this.retry.waitFor('logout to complete', async () => {
// There are cases when browser/Kibana would like users to confirm that they want to navigate away from the
// current page and lose the state (e.g. unsaved changes) via native alert dialog.
const alert = await this.browser.getAlert();
if (alert?.accept) {
await alert.accept();
}

// Timeout has been doubled to 40s here in attempt to quiet the flakiness
await this.retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await this.browser.getCurrentUrl();
await this.delay(1000);
const urlAfter = await this.browser.getCurrentUrl();
this.log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
});

const currentUrl = await this.browser.getCurrentUrl();
if (this.config.get('serverless')) {
// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
} else {
return !currentUrl.includes('/logout');
}
});
}
};

const currentUrl = await this.browser.getCurrentUrl();
if (this.config.get('serverless')) {
// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
} else {
return !currentUrl.includes('/logout');
}
});
}
await this.retry.tryWithRetries('force logout with retries', performForceLogout, {
retryCount: 2,
});
}

async clickRolesSection() {
Expand Down
3 changes: 1 addition & 2 deletions x-pack/test/security_solution_endpoint/apps/endpoint/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import {
export default function (providerContext: FtrProviderContext) {
const { loadTestFile, getService, getPageObjects } = providerContext;

// FLAKY: https://github.com/elastic/kibana/issues/180401
describe.skip('endpoint', function () {
Copy link
Member

Choose a reason for hiding this comment

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

Hey @gergoabraham We saw similar flakiness before and from appex-qa perspective it is related to basic authentication in UI tests (line 42 calls svlCommonPage.login()) for serverless env when serverless env is setup with mock-idp to do SAML authentication. I tried retries before myself and flaky-test-runner proved it won't help as redirects issue still persist.

The right solution is to migrate the FTR tests to role-based SAML authentication, it is mostly 1 line change and we didn't see similar flakiness:
https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/functional/page_objects/svl_common_page.ts#L73-L130
More details with examples are here

You probably saw recent email about new preferred login method for a local serverless Kibana, with that we consider deprecating svlCommonPage.login() as all e2e UI tests should be using SAML auth.

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 @dmlemeshko, a big thank you for letting me know! 🙌

addressing the change here:

describe('endpoint', function () {
const ingestManager = getService('ingestManager');
const log = getService('log');
const endpointTestResources = getService('endpointTestResources');
Expand Down
3 changes: 1 addition & 2 deletions x-pack/test/security_solution_endpoint/apps/integrations_feature_flag/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import {
export default function (providerContext: FtrProviderContext) {
const { loadTestFile, getService, getPageObjects } = providerContext;

// FLAKY: https://github.com/elastic/kibana/issues/184319
describe.skip('endpoint', function () {
describe('endpoint', function () {
const ingestManager = getService('ingestManager');
const log = getService('log');
const endpointTestResources = getService('endpointTestResources');
Expand Down
40 changes: 23 additions & 17 deletions x-pack/test_serverless/functional/page_objects/svl_common_page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,25 +147,31 @@ export function SvlCommonPageProvider({ getService, getPageObjects }: FtrProvide

await cleanBrowserState();

log.debug(`Navigating to ${deployment.getHostPort()}/logout to force the logout`);
await browser.get(deployment.getHostPort() + '/logout');

// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
// Timeout has been doubled here in attempt to quiet the flakiness
await retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await browser.getCurrentUrl();
delay(1000);
const urlAfter = await browser.getCurrentUrl();
log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
});
const performForceLogout = async () => {
log.debug(`Navigating to ${deployment.getHostPort()}/logout to force the logout`);
await browser.get(deployment.getHostPort() + '/logout');

const currentUrl = await browser.getCurrentUrl();
// After logging out, the user can be redirected to various locations depending on the context. By default, we
// expect the user to be redirected to the login page. However, if the login page is not available for some reason,
// we should simply wait until the user is redirected *elsewhere*.
// Timeout has been doubled to 40s here in attempt to quiet the flakiness
await retry.waitForWithTimeout('URL redirects to finish', 40000, async () => {
const urlBefore = await browser.getCurrentUrl();
delay(1000);
const urlAfter = await browser.getCurrentUrl();
log.debug(`Expecting before URL '${urlBefore}' to equal after URL '${urlAfter}'`);
return urlAfter === urlBefore;
});

const currentUrl = await browser.getCurrentUrl();

// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
// Logout might trigger multiple redirects, but in the end we expect the Cloud login page
return currentUrl.includes('/login') || currentUrl.includes('/projects');
};

await retry.tryWithRetries('force logout with retries', performForceLogout, {
retryCount: 2,
});
},

async login() {
Expand Down