Keycloak fixes and Quarkus FW bump to 1.4.0.Beta5 #1581
Conversation
|
run tests |
michalvavrik
force-pushed
the
feature/fix-rh-sso-ocp-failures
branch
from
9bf6b00 to
c9287da
Compare
michalvavrik
changed the title
michalvavrik
changed the title
|
run tests |
|
Remaining OCP / baremetal failures are failing also without this PR, therefore they are not result of this PR. |
| @@ -10,8 +14,8 @@ | |||
| public class KeycloakAuthzSecurityReactiveIT extends BaseAuthzSecurityReactiveIT { | |||
|
|
|||
| //TODO Remove workaround after Keycloak is fixed https://github.com/keycloak/keycloak/issues/9916 | |||
| @KeycloakContainer(command = { "start-dev --import-realm --hostname-strict=false" }) | |||
| static KeycloakService keycloak = new KeycloakService("/keycloak-realm.json", REALM_DEFAULT, "/realms") | |||
| @KeycloakContainer(command = { "start-dev", "--import-realm" }) | |||
There was a problem hiding this comment.
Is there reason why --hostname-strict=false is removed when in other cases it stay.
There was a problem hiding this comment.
yes, I wrote about this (vaguely) in the PR description, I changed it accidentally and it made no difference, so I suppose it doesn't really matter. If it doesn't cause test failure I don't think it needs to be there. Good catch, it was mistake. If it is somehow important for you, I'll change it in next PR when fixing mTLS.
There was a problem hiding this comment.
If it's work I'm fine with it. Hope that this not cause the problem in future.
There was a problem hiding this comment.
If it's work I'm fine with it. Hope that this not cause the problem in future.
IMO tests need to be idempotent, otherwise our job will become very dangerous
There was a problem hiding this comment.
we will see other integration runs like Windows and Podman... I don't expect there should be difference.
| @Container(image = "${rhsso.image}", expectedLog = "Http management interface listening", port = KEYCLOAK_PORT) | ||
| static KeycloakService keycloak = new KeycloakService(REALM_DEFAULT) | ||
| .withProperty("SSO_IMPORT_FILE", "resource::/keycloak-realm.json"); | ||
| @KeycloakContainer(command = { "start-dev", "--import-realm" }, image = "${rhbk.image}") |
There was a problem hiding this comment.
Why some of the command KeycloakContainers contains contain more commands that these two. When they were the same with using Container.
There was a problem hiding this comment.
I removed .withProperty("SSO_IMPORT_FILE", "resource::/keycloak-realm.json"); so I want to import it from command. I can't answer it in general, if you deem particular change wrong, please comment on the very change and I'll check.
There was a problem hiding this comment.
Just that somewhere @Conatiner change to this
@KeycloakContainer(command = { "start-dev", "--import-realm", "--hostname-strict=false",
"--features=token-exchange" }, image = "${rhbk.image}")
and somewhere to this
@KeycloakContainer(command = { "start-dev", "--import-realm" }, image = "${rhbk.image}")
But this was answered in other comment so now there is probably no need for some of these commands.
But as it was written in PR description let's solve these problem as the start-dev is solved.
There was a problem hiding this comment.
just note - I won't pretend I couldn't make mistake, but this is how I worked:
- if I remove import env var, I need to add
--import-realm - If I change RHSSO for RHBK it is change from Wildfly to Quarkus based KC, so I followed what is done in that very project for non-OCP tests. so typically if there were
--features=token-exchangefor baremetal tests I just added it as well. I didn't try to think whether it needs to be in baremetal tests at the first place.
Summary
Main goal of this PR is to fix OCP failures related to the fact that Keycloak auth server URL has changed in our FW: quarkus-qe/quarkus-test-framework#960
OpenShiftRhSsoOidcMtlsITwill require much more thinkering, at the very least I hit that config map name is created with illegal chars when resource is in nested directory. But I think it will be even more difficult, I'll address it later this week.start-devbecause it all seems like a one big workaround for proper solution to meOidcRestClientITfailure (both baremetal and OCP) goes down to the Fix primitive class handling in Serialisers quarkusio/quarkus#37788, I hope it's going to be merged today as that reviewer is active in other Quarkus PRsPlease select the relevant options.
run testsphrase in comment)Checklist: