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

Temporal Cloud: Support SSL #96

Closed
melloware opened this issue Aug 30, 2024 · 29 comments · Fixed by #99
Closed

Temporal Cloud: Support SSL #96

melloware opened this issue Aug 30, 2024 · 29 comments · Fixed by #99
Assignees
Labels
enhancement New feature or request

Comments

@melloware
Copy link
Contributor

From the docs here: https://docs.temporal.io/develop/java/temporal-clients#connect-to-temporal-cloud

/ Generate an SSL context
            InputStream clientCertInputStream = new FileInputStream(clientCertPath);
            InputStream clientKeyInputStream = new FileInputStream(clientKeyPath);
            SslContext sslContext = SimpleSslContextBuilder.forPKCS8(clientCertInputStream, clientKeyInputStream).build();

            // Set the Service Stub options (SSL context and gRPC endpoint)
            WorkflowServiceStubsOptions stubsOptions = WorkflowServiceStubsOptions
                .newBuilder()
                .setSslContext(sslContext)
                .setTarget(gRPCEndpoint)
                .build();
@melloware melloware added the enhancement New feature or request label Aug 30, 2024
@rmanibus
Copy link
Contributor

is the idea to expose the cert and key as config ?

@rmanibus
Copy link
Contributor

should we allow the use of a keystore ?

@rmanibus
Copy link
Contributor

Also we can probably leverage the Quarkus TLS Centralized Registry

I will have a look into this if I have some time over the weekend !

@melloware
Copy link
Contributor Author

Yep my thought was the tls resigtry. I haven't thought it all the way through but thought we should support it for people using their cloud product.

@melloware
Copy link
Contributor Author

I assigned it to you.

@melloware
Copy link
Contributor Author

I really like this statement and it makes using the TlsRegistry a no brainer i think for this

The TlsConfiguration object contains the key stores, trust stores, cipher suites, protocols, and other properties. It also provides a way to create an SSLContext from the configuration.

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

The TLS registry has been introduced in quarkus 3.12.0, we will need to make this the lowest required version if we want to use this

@melloware
Copy link
Contributor Author

Ok I think I will cut 1.0.0 after we merge your last change then we can start on 2.0.x on Quarkus 3.12. Sound good?

@melloware
Copy link
Contributor Author

That way we have at least a version for people staying on LTS

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

another callout:
Temporal Service Stub option is taking a io.grpc.netty.shaded.io.netty.handler.ssl.SslContext, when the TLS registry is providing a javax.net.ssl.SSLContext.

I still think it is valuable to pursue this route, but this will require some more digging

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

about the version, I think that quarkus 3.15 is around the corner and will be the next LTS. maybe we should wait for this one for our 2.0.x ?

@melloware
Copy link
Contributor Author

ok great idea. for now we can just add two config params for the certificates i have seen a lot of other extensions do this in the codebase. Then maybe we switch to TLS Registry in 3.15?

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

I have been exploring options, and I am starting to wonder if implementing our own SSL config is the right thing to do.

Instead, we could inject a managed channel into the temporal client. This way, the TLS config would be done within the quarkus grpc extension directly.

But I would love some feedback from the folks contributing in the quarkus GRPC space

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

@mkouba Any though about using a quarkus-managed channel in the temporal client instead of creating our own (or more exactly to let the client create his own) ?

The setChannel method documentation of the temporal client mention:

Sets fully custom user-configured gRPC channel to use.
Before supplying a fully custom channel using this method, it's recommended to first consider using setTarget(String) + other options of WorkflowServiceStubsOptions. Builder + setChannelInitializer(Consumer) for some rarely used configuration. This option is not intended for the majority of users as it disables some Temporal connection management features and can lead to outages if the channel is configured or managed improperly.
Mutually exclusive with setTarget(String), setChannelInitializer(Consumer), setSslContext(SslContext), setGrpcReconnectFrequency(Duration) and setConnectionBackoffResetFrequency(Duration). These options are ignored if the custom channel is supplied.

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

it is mostly working (see the attached PR), the only things that stand out is that we would need to map the config key
quarkus.temporal.target to quarkus.grpc.clients.temporal-client.host and quarkus.grpc.clients.temporal-client.port

@rmanibus
Copy link
Contributor

rmanibus commented Sep 2, 2024

Is there a build item that can be used to set a value of a config property ?

@melloware
Copy link
Contributor Author

Nice. As far as your config item question I am pretty sure they are meant to be read only and not changed by anything but I could be wrong. I will let the experts weigh in.

@melloware
Copy link
Contributor Author

Actually let me look i think i did see somewhere recently they were updating a config item.

@melloware
Copy link
Contributor Author

@rmanibus should we add a documentation section on how to connect to Temporal Cloud using the GRCP certificate?

@shrikanthkr
Copy link

We are trying to use temporal cloud from quarkus via this extension. I have the latest version 0.0.9. Anyways I could use client cert path and client key to be used?
I have they secrets stored in aws secrets, also finding a challenge on how to bring it in. Any help or workarounds would be of great use.

@rmanibus
Copy link
Contributor

rmanibus commented Sep 25, 2024

hey @shrikanthkr, I will break a new release today.
You have two ways of doing this:

  • use a quarkus managed channel, and configure TLS via quarkus grpc (quarkus.grpc.clients.temporal-client ...) : https://quarkus.io/guides/grpc-reference#configuring-mtls
  • using the build in channel, the new version expose quarkus.temporal.client-cert-path and quarkus.temporal.client-key-path

@rmanibus
Copy link
Contributor

note that this is still an early release, I would love to get your feedback on using it in real life !

@melloware
Copy link
Contributor Author

We also need to add this to our docs because others will ask the same thing

@melloware
Copy link
Contributor Author

@shrikanthkr also you can use Amazon Secrets Manager to bring in your secrets: https://docs.quarkiverse.io/quarkus-amazon-services/dev/amazon-secretsmanager.html

@melloware
Copy link
Contributor Author

@all-contributors add @shrikanthkr for testing

Copy link
Contributor

@melloware

I've put up a pull request to add @shrikanthkr! 🎉

@shrikanthkr
Copy link

shrikanthkr commented Sep 25, 2024

@rmanibus @melloware Thank you. I will try it out and let you know if I come across any challenges. Thank you so much for the quick response.

@shrikanthkr
Copy link

@melloware I tried the 0.0.10 version today. Having the mlts certpath and certkey locally on resources folder works. Am trying to figure out a way to get it from aws secrets directly.
Overall connections to temporal cloud and worker processing it via quarkus works.

@melloware
Copy link
Contributor Author

@shrikanthkr awesome news!!! As for AWS secret this post has example code on how to get those vlaues as AWS Secrets: quarkiverse/quarkus-amazon-services#510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants