-
Notifications
You must be signed in to change notification settings - Fork 19
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
RHPAM-2762 & RHPAM-2777 add tests to verify issues fixes #629
base: 7.33.x
Are you sure you want to change the base?
RHPAM-2762 & RHPAM-2777 add tests to verify issues fixes #629
Conversation
@@ -145,6 +146,92 @@ public void testControllerOperations(WorkbenchDeployment workbenchDeployment, bo | |||
} | |||
} | |||
|
|||
// Verifies https://issues.redhat.com/browse/RHPAM-2762 | |||
public void testAdminUserPasswordChange(WorkbenchDeployment workbenchDeployment) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also check that the Kie Server can connect after the username or password have changed.
For templates, the Kie Server uses the secrets to get the credentials to connect with Workbench. If we change the username or password, the Kie Server might still have the old credentials and something very wrong could be happening.
I guess this is not an issue in Operator as it's using the common config, but checking the Kie Server was restarted with the correct data seems still to be needed.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this is needed. If credential secret is changed, then all deployments using this secret needs to be redeploy manually to have up to date credentials. This redeploy is not triggered automatically as change of env variables for deployment config. If the deployments are not redeploy they are still using previous credentials.
On the other hand this adjustments and extension of tests shouldn't effect running time (if we do redeploy at the same time) and can bring extra checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean that if we allow users to change the credentials, it might cause a misconfiguration of other components? I don't think we should allow that, right?
From my point of view, the restart must occur if we change the credentials either in env vars (operator) or config map (templates).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There two types of change for templates - change of data in credential secret (no redeployments) and change of the credential secret (replace) in deployment config (this will trigger redeploy).
Templates will deploy application and we cannot manage all components at once, so if user change the credential, they needs to do it for all components of the application.
In Operator we are using kieApp that is update on change, this is additional value of Operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm only thinking in what would happen if users change the credentials using the supported approaches (not directly Openshift or CLI as we do in tests).
So, in templates, what would happen if real users change the credentials via Business Central? If no redeployments occur, it will deal in a misconfiguration and I guess we can watch this kind of changes to make redeployments happen automatically.
What about SSO or LDAP? Here we can't watch the changes, so maybe it should document what to do next?
} | ||
|
||
// Verifies https://issues.redhat.com/browse/RHPAM-2777 | ||
public void testAdminUserNameAndPasswordChange(WorkbenchDeployment workbenchDeployment) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above.
...e/src/test/java/org/kie/cloud/integrationtests/testproviders/HttpsWorkbenchTestProvider.java
Outdated
Show resolved
Hide resolved
...work-openshift/src/main/java/org/kie/cloud/openshift/deployment/WorkbenchDeploymentImpl.java
Outdated
Show resolved
Hide resolved
f07ea85
to
1ab252d
Compare
.../src/main/java/org/kie/cloud/openshift/operator/scenario/WorkbenchKieServerScenarioImpl.java
Outdated
Show resolved
Hide resolved
...templates/src/main/java/org/kie/cloud/openshift/scenario/WorkbenchKieServerScenarioImpl.java
Outdated
Show resolved
Hide resolved
...framework-cloud-api/src/main/java/org/kie/cloud/api/scenario/WorkbenchKieServerScenario.java
Show resolved
Hide resolved
...java/org/kie/cloud/openshift/operator/scenario/WorkbenchKieServerPersistentScenarioImpl.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/kie/cloud/openshift/scenario/WorkbenchKieServerPersistentScenarioImpl.java
Show resolved
Hide resolved
...e/src/test/java/org/kie/cloud/integrationtests/testproviders/HttpsWorkbenchTestProvider.java
Outdated
Show resolved
Hide resolved
...loud/framework-cloud-api/src/main/java/org/kie/cloud/api/deployment/WorkbenchDeployment.java
Outdated
Show resolved
Hide resolved
SoftAssertions.assertSoftly(softly -> { | ||
softly.assertAlso(isUserAuthorizedInWorkbench(workbenchUrl, oldUsername, oldPassword)); | ||
softly.assertAlso(isUserAuthorizedInKieServer(kieServerUrl, oldUsername, oldPassword)); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to assert old users can connect as if not, it would have failed before (to kie server when connecting to business central or when deploying the container).
SoftAssertions.assertSoftly(softly -> { | ||
softly.assertAlso(isUserAuthorizedInWorkbench(workbenchUrl, username, oldPassword)); | ||
softly.assertAlso(isUserAuthorizedInKieServer(kieServerUrl, username, oldPassword)); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to assert old users can connect as if not, it would have failed before (to kie server when connecting to business central or when deploying the container).
...mote/src/test/java/org/kie/cloud/integrationtests/testproviders/PersistenceTestProvider.java
Outdated
Show resolved
Hide resolved
...mote/src/test/java/org/kie/cloud/integrationtests/testproviders/PersistenceTestProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/kie/cloud/openshift/scenario/WorkbenchKieServerPersistentScenarioImpl.java
Outdated
Show resolved
Hide resolved
73305b5
to
79b2deb
Compare
…der and change user impl will be removed in following commit
b5e4ade
to
f1725ea
Compare
f1725ea
to
1d0f3db
Compare
No description provided.