Added implementation of TLS-registry for GraphQL Client #41
Conversation
mskacelik
force-pushed
the
tls-registry-smallrye-graphql
branch
5 times, most recently
from
7b5ef19 to
69a70f5
Compare
|
//cc @jmartisk |
|
@cescoffier could you give this a look, please? It's for my branch for now, because this branch captures all the stuff that we will apply together with the SmallRye GraphQL 2.11 update. The related smallrye-side changes are in smallrye/smallrye-graphql#2194 |
| import io.vertx.core.net.JksOptions; | ||
|
|
||
| public class SSLTestingTools { | ||
| static Vertx vertx = Vertx.vertx(); |
| options.setHost("localhost"); | ||
|
|
||
| if (keystorePath != null) { | ||
| JksOptions keystoreOptions = new JksOptions(); |
There was a problem hiding this comment.
If you pass p12 better use the associated options.
|
|
||
| private static final String CONFIGURATION = """ | ||
| quarkus.smallrye-graphql-client.my-client.tls-configuration-name=my-tls-client | ||
| quarkus.tls.my-tls-client.key-store.p12.path=target/certs/wrong-graphql-client-keystore.p12 |
There was a problem hiding this comment.
Do you know what is supported by the client? P12, JKS seems to be supported. What about pem?
There was a problem hiding this comment.
As far as I know, SRGQL by itself supports only P12 and JKS since it works with path, password, and type fields.
see:
https://github.com/smallrye/smallrye-graphql/blob/5ff2b59a0e16386ba03a51e47f908426667cd1c3/client/implementation/src/main/java/io/smallrye/graphql/client/impl/GraphQLClientConfiguration.java#L65-L78
However, this new integration should support all three types
Should I add separate tests for them (JKS/P12/PEM)?
But I talked with @jmartisk about possibly adding a feature to adding integration of PEM format in SRGQL (raw).
There was a problem hiding this comment.
It should support just whatever the TLS registry extension passes to us, AFAIU the GraphQL client shouldn't need to care about the underlying format, no? As long as we receive a io.vertx.core.net.KeyCertOptions or TrustOptions. I don't think we need to test each format separately on the GraphQL client side.
As for the original (very crude) support for keystores and truststores, I wouldn't care too much about updating it with extra formats, we clearly mark is as deprecated.
There was a problem hiding this comment.
Ah, if it's Vertx based, no problem. It's for everything else :-)
| graphQLClientConfiguration | ||
| .setTlsKeyStoreOptions(updatedTlsConfiguration.getKeyStoreOptions()); | ||
| graphQLClientConfiguration | ||
| .setTlsTrustStoreOptions(updatedTlsConfiguration.getTrustStoreOptions()); |
There was a problem hiding this comment.
We might need to reload a bit more, typically CRL. But it's already great!
mskacelik
force-pushed
the
tls-registry-smallrye-graphql
branch
from
69a70f5 to
3fcee54
Compare
|
🙈 The PR is closed and the preview is expired. |
No description provided.