Closed rkraneis closed 2 years ago
/cc @matejvasek, @patriot1burke, @pedroigor, @sberyozkin
@cescoffier, @stuartwdouglas FYI, OidcClient
is initializing its Vert.x Mutiny Client
with Vertx
obtained from CoreVertxBuildItem
- is it possible that this Vertx
instance is causing an HTTP stack initialization at a default port and thus interfering with AWS Lambda
? OidcClient
itself is not doing any port allocations.
If yes - then should OidcClient
manage its own Vertx
, similarly to how it is done in DevServices for Keycloak
:
void setup(CuratedApplicationShutdownBuildItem closeBuildItem) {
Vertx vertxInstance = Vertx.vertx();
// and make sure it is closed:
Runnable closeTask = new Runnable() {
@Override
public void run() {
if (vertxInstance != null) {
try {
vertxInstance.close();
} catch (Throwable t) {
LOG.error("Failed to close Vertx instance", t);
}
}
}
};
closeBuildItem.addCloseTask(closeTask, true);
}
?
We should prevent using another managed instance of vertx, it will duplicate the event loops.
I wonder if the problem comes from the virtual http layer used by the lambda code extension.
Sorry for long delay. @sberyozkin , The quarkus-oidc-client extension is pulling in quarkus-vertx-http-deployment which causes an HTTP server to be started which conflects with the mock event lambda server.
This would be ok if the user was using quarkus-amazon-lambda-http (or -rest), but I believe the user has a plain lambda project and wants to use the oidc client from within a regular lambda.
@sberyozkin The quarkus-oidc-client extension should not be using quarkus-vertx-http-deployment as this starts an HTTP server within the quarkus application. I believe you will need to just pull in quarkus-vertx-deployment and grab any specific vertx library directly and not use quarkus-vertx-http. I'll will fool around with this.
I agree with @patriot1burke - OIDC should use the "core" vert.x extension, not the HTTP one (handling the HTTP server).
This would be ok if the user was using quarkus-amazon-lambda-http (or -rest), but I believe the user has a plain lambda project and wants to use the oidc client from within a regular lambda.
Yes, he is :)
@sberyozkin let me know if you trust me to fix this. :)
Hi @patriot1burke I do, 100% :-), have a look please at fixing it, would be appreciated, thanks
Hi Bill @patriot1burke, if I understand it correctly we probably need to do something similar to what is suggested above, https://github.com/quarkusio/quarkus/issues/22082#issuecomment-1000271756, and then pass this Vertx
to OidcClientRecorder
instead of the one coming from vertx-http
. This is how it is created for DevServices for Keycloak
but perhaps in this case the Vertx instance should be created in the recorder instead - not quite sure about it.
Hi @patriot1burke Stuart just fixed it, so AWS Lambda and OIDC Client are friends :-), thanks for analyzing the problem
sigh....
was just about to submit a PR.
@sberyozkin I still see that oidc-common-deployment pulls in quarkus-vertx-http-deployment. The fact that oidc-client-deployment pulls in this dependency will cause an http server to be started which will screw up AWS Lambda + OIDC client testing.
https://github.com/quarkusio/quarkus/blob/main/extensions/oidc-common/deployment/pom.xml#L31
Hi Bill @patriot1burke I see, thanks for spotting it, a copy and paste issue likely, I guess it should also depend on vert-core
please open a PR :-)
@sberyozkin I brought it up because you said Stuart fixed it. I thought maybe he did something funky or something and you could elaborate.
@patriot1burke I just saw #22929 and by merging it I closed this issue, but I haven't verified that it was likely not compete, sorry.
I think we should add a test confirming OidcClient
can be used with AWS lambda. I can do it, probably adding a new integration tests module is warranted in this case, aws-lambda-oidc-client
, but if you are interested, would you like to test it ?
I've looked, we probably can copy and paste integration-tests/amazon-lambda-http-resteasy
, keep may be GreetingResource
with a single method, add quarkus-oidc
which will automatically start a Keycloak container in a test mode, add quarkus-oidc-client-filter
and configure the client like this:
# Dev Services for Keycloak will set all the referenced properties
quarkus.oidc.client.auth-server-url=${quarkus.oidc.auth-server-url}
quarkus.oidc-client.client-id=${quarkus.oidc.client-id}
quarkus.oidc-client.credentials.secret=${quarkus.oidc.credentials.secret}
quarkus.oidc-client.grant.type=password
quarkus.oidc-client.grant-options.password.username=alice
quarkus.oidc-client.grant-options.password.password=alice
then add another endpoint like ProtectedResource
(with @RolesAllowed('user')
), attach OidcClientRequestFilter
to the corresponding RestClient, https://quarkus.io/guides/security-openid-connect-client#oidc-client-filter, and then make a call from GreetingResource
to ProtectedResource
which should return alice
.
What do you think ? Let me know if you'd like to start, I'll help along the way
thanks
@sberyozkin I can do the test too. No worries. I had already started this work before, but moved on as I thought it was fixed.
@patriot1burke Sounds good :-), thanks.
Describe the bug
Adding quarkus-oidc-client extension to a working aws lambda quarkus application makes the tests fail with
java.net.BindException: Address already in use
:This can be resolved manually by
quarkus.lambda.mock-event-server.test-port
to a free portgiven().port(...).body(...)...
), as it is not picked up automatically.Non-aws-lambda projects don't seem to be affeced by this.
Expected behavior
Adding quarkus-oidc-client should not break working projects. I don't even know what is hogging the default test port as the oidc-client documentation doesn't mention anything.
Actual behavior
Adding quarkus-oidc-client breaks working AWS lambda projects as the mock-event-server cannot be started.
How to Reproduce?
No response
Output of
uname -a
orver
No response
Output of
java -version
No response
GraalVM version (if different from Java)
No response
Quarkus version or git rev
2.5.1
Build tool (ie. output of
mvnw --version
orgradlew --version
)mvn
Additional information
No response