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

DRAFT: [EAPQE-2410] Add test to verify AJP Listener allowed-request-attributes-pattern #120

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,14 @@ Following is a table of supported environment properties that can be used when r

| Property name | Default value | Description |
| :--------------- | :---------------------------------------------- | :-------------------------------------------------------------------------------------------- |
| `HAL_IMAGE` | `quay.io/halconsole/hal-development:latest` | [HAL standalone image](https://hal.github.io/documentation/get-started/#container) to be used |
| `WILDFLY_IMAGE` | `quay.io/halconsole/wildfly-development:latest` | WildFly/JBoss EAP image to be used |
| `KEYCLOAK_IMAGE` | `quay.io/keycloak/keycloak:latest` | Keycloak/RH-SSO image to be used for OIDC tests |
| `POSTGRES_IMAGE` | `docker.io/library/postgres:latest` | PostgreSQL image to be used for datasource tests |
| `MYSQL_IMAGE` | `docker.io/library/mysql:latest` | MySQL image to be used for datasource tests |
| `MARIADB_IMAGE` | `docker.io/library/mariadb:latest` | MariaDB image to be used for datasource tests |
| `MSSQL_IMAGE` | `mcr.microsoft.com/mssql/server:2022-latest` | Microsoft SQL Server image to be used for datasource tests |
| `HAL_IMAGE` | `quay.io/halconsole/hal-development:latest` | [HAL standalone image](https://hal.github.io/documentation/get-started/#container) to be used |
| `WILDFLY_IMAGE` | `quay.io/halconsole/wildfly-development:latest` | WildFly/JBoss EAP image to be used |
| `KEYCLOAK_IMAGE` | `quay.io/keycloak/keycloak:latest` | Keycloak/RH-SSO image to be used for OIDC tests |
| `POSTGRES_IMAGE` | `docker.io/library/postgres:latest` | PostgreSQL image to be used for datasource tests |
| `MYSQL_IMAGE` | `docker.io/library/mysql:latest` | MySQL image to be used for datasource tests |
| `MARIADB_IMAGE` | `docker.io/library/mariadb:latest` | MariaDB image to be used for datasource tests |
| `MSSQL_IMAGE` | `mcr.microsoft.com/mssql/server:2022-latest` | Microsoft SQL Server image to be used for datasource tests |
| `WILDFLY_STABILITY_LEVEL` | NO DEFAULT VALUE | Value for `--stability=<VALUE>` in WildFly startup command, if not set, parameter is not used |

## Custom method documentation

Expand Down
15 changes: 7 additions & 8 deletions packages/testsuite/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export default defineConfig({
"start:wildfly:container": ({ name, configuration, useNetworkHostMode }) => {
return new Promise((resolve, reject) => {
let portOffset = 0;
let wildflyCmdParameters = ["-c", configuration || "standalone-insecure.xml"];
if (process.env.WILDFLY_STABILITY_LEVEL) {
wildflyCmdParameters.push("--stability=" + process.env.WILDFLY_STABILITY_LEVEL);
}
const wildfly = new GenericContainer(
process.env.WILDFLY_IMAGE || "quay.io/halconsole/wildfly-development:latest"
)
Expand All @@ -35,16 +39,11 @@ export default defineConfig({
.withStartupTimeout(333000);
if (useNetworkHostMode === true) {
console.log("host mode");
wildflyCmdParameters.push(`-Djboss.socket.binding.port-offset=${portOffset.toString()}`);
findAPortNotInUse(8080, 8180)
.then((freePort) => {
portOffset = freePort - 8080;
wildfly
.withNetworkMode("host")
.withCommand([
"-c",
configuration || "standalone-insecure.xml",
`-Djboss.socket.binding.port-offset=${portOffset.toString()}`,
] as string[]);
wildfly.withNetworkMode("host").withCommand(wildflyCmdParameters as string[]);
})
.catch((error) => {
console.log(error);
Expand All @@ -55,7 +54,7 @@ export default defineConfig({
.withNetworkMode(config.env.NETWORK_NAME as string)
.withNetworkAliases("wildfly")
.withExposedPorts(9990)
.withCommand(["-c", configuration || "standalone-insecure.xml"] as string[]);
.withCommand(wildflyCmdParameters as string[]);
}
wildfly
.start()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
describe("TESTS: Configuration => Subsystem => Undertow => Global settings", () => {
let managementEndpoint: string;

const testValues = {
serverName: "default-server",
ajpListener: "test-ajp",
ajpSocketBindings: "ajp",
allowedReqAttrsPatternExpression: "${NON-EXISTING:value.*}",
allowedReqAttrsPatternResolved: "value.*",
};
const address = ["subsystem", "undertow", "server", testValues.serverName, "ajp-listener", testValues.ajpListener];
const serverSelectors = {
serverListenersItem: "#undertow-server-listener-item",
ajpListenerItem: "#undertow-server-ajp-listener-item",
};
const ajpListenerPageSelectors = {
ajpListenersTableId: "undertow-server-ajp-listener-table",
};
const ajpListenerForm = {
id: "undertow-server-ajp-listener-form",
allowedReqAttrsPattern: "allowed-request-attributes-pattern",
};

before(() => {
cy.startWildflyContainer()
Copy link
Contributor

Choose a reason for hiding this comment

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

startWildflyContainer could be extended to accept parameters for standalone.sh, so that the test could be executed for the preview level.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you could define optional additional start parameter to: https://github.com/istraka/berg/blob/EAPQE-2410/packages/testsuite/cypress.config.ts#L19

and here just use it (additionalStartParameter="--stability=preview")

what do you think @OndrejKotek @istraka?

Copy link
Author

Choose a reason for hiding this comment

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

@OndrejKotek added. I didn't add the parameter to the task, because I assume I would have to modify tests (setting the parameter). Now I control it via environment variable.

Copy link
Collaborator

@kstekovi kstekovi Oct 10, 2024

Choose a reason for hiding this comment

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

@istraka i don't like this approach for two reasons. You have to edit job automation and you don't see the test require a preview mode. could you please introduce a new parameter which you will define when the server is stared.

You can find inspiration in hostMode implemented for OIDC. This is almost same case.

https://github.com/hal/berg/blob/main/packages/testsuite/cypress/e2e/elytron-oidc-client/test-oidc-security-configure-oidc.cy.ts#L62

Copy link
Author

Choose a reason for hiding this comment

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

IIUC this solution would always start wildfly in preview stability mode. However we want to test the feature NOT in preview mode (in the future). With the proposed solution we wouldn't check default one, which we intend to do so. Once WFLY-19808 is resolved I would have to submit yet another PR. I'd like to keep it to 1:)

Copy link
Author

Choose a reason for hiding this comment

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

My question is: do we want to have tests in the TS that runs in preview mode? And what if they are promoted to higher stability level? Should the test change?

Copy link
Collaborator

@kstekovi kstekovi Oct 10, 2024

Choose a reason for hiding this comment

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

IIUC this solution would always start wildfly in preview stability mode

I don't think. You can define the default value to false and only in your case you can set it to true. so only your test you will start server in preview mode.

Like in case hostMode. Default value is network mode so when you will not use any parameter it will started in network mode. And only OIDC test define the parameter useNetworkHostMode: true so only when the parameter is present then the server will start a server in hostMode.

However we want to test the feature NOT in preview mode (in the future)

When this will be changed there have to be some JIRA to track this change. Because the documentation have to be also update etc. So also these test have to update to remove the parameter for preview mode.

Now you still adding the required subsystem manually and when it will be promoted to regular feature you will have to remove this manual configuration.

You will have to update a tests in every case. So why not to do property and explicitly define the preview mode is active?

do we want to have tests in the TS that runs in preview mode? And what if they are promoted to higher stability level? Should the test change?

No we wont start server in preview mode only for your test.

And what if they are promoted to higher stability level? Should the test change?

Only the parameter from your test will be removed.

Copy link
Author

Choose a reason for hiding this comment

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

I am afraid of forgotten test running in preview stability mode. We have discussed this and agreed we should not do that and I'd like to wait with this PR for WFLY-19808.

We are waiting for the feature to be promoted to the default stability level WFLY-19808. EAP7-1836 is blocked by EAPDOC-2314.

IIUC we need the RFE to be in a sprint for WFLY-19808. But Bartosz has submitted the MR, and if it gets merged....well, we can merge this.

.then((result) => {
managementEndpoint = result as string;
})
.then(() => {
// create fixtures
cy.task("execute:cli", {
managementApi: managementEndpoint + "/management",
address: address,
operation: "add",
"socket-binding": testValues.ajpSocketBindings,
});
});
});

after(() => {
cy.task("stop:containers");
});

beforeEach(() => {
cy.navigateTo(managementEndpoint, "undertow-server;name=default-server");
// the form takes a brief moment to initialize
cy.wait(200);
cy.get(serverSelectors.serverListenersItem).click();
kstekovi marked this conversation as resolved.
Show resolved Hide resolved
cy.get(serverSelectors.ajpListenerItem).click();
});

it("Test AJP Listener: allowed request attributes pattern", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do test also negatived scenario? When is no possible to define AJP_ALLOWED_REQUEST_ATTRIBUTES_PATTERN

What a user see and what happened when a preview mode is not allowed?

Copy link
Author

Choose a reason for hiding this comment

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

If a preview mode is not allowed, the parameter is simply not there. However it should be visible all the time - once it is in default stability level.

I guess there is no negative scenario - it is a pattern. Only invalid value is invalid patter (i.e. (.*), but this is parsed during service startup (after you click on the reload) and the WEB UI doesn't check it. Same for the CLI op - it fails to laod after the reload.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be a negative scenarios when a user fill a wrong expression.

Copy link
Author

Choose a reason for hiding this comment

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

What would you expect in such case?

Copy link
Collaborator

@kstekovi kstekovi Oct 10, 2024

Choose a reason for hiding this comment

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

I don't know. You are working on the RFE and you should know how does it behave when it is wrongly configured.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would expect some notification which say the expression is wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As i understand. You can configure a wrong expression without any warning then restart a server and then it will not start again. So you have a dead server which you can't reconfigure back because it is offline now. So you have to manually update the standalone.xml to revert you last change.

I don't know if it is possible to check it in advance. I am just asking.

Copy link
Author

Choose a reason for hiding this comment

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

We have discussed this offline. Reload produce an error - the service won't start because of wrong expression. Console is still up and you can edit the expression (CLI or Web console).

cy.selectInTable(ajpListenerPageSelectors.ajpListenersTableId, testValues.ajpListener);

cy.editForm(ajpListenerForm.id);
cy.text(ajpListenerForm.id, ajpListenerForm.allowedReqAttrsPattern, testValues.allowedReqAttrsPatternExpression, {
parseSpecialCharSequences: false,
});
cy.saveForm(ajpListenerForm.id);
cy.verifySuccess();
cy.verifyAttributeAsExpression(
managementEndpoint,
["subsystem", "undertow", "server", testValues.serverName, "ajp-listener", testValues.ajpListener],
ajpListenerForm.allowedReqAttrsPattern,
testValues.allowedReqAttrsPatternExpression
);
});
});