owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.27k stars 170 forks source link

Configure TLS for connection to NATS KV #9036

Open benfuu opened 2 months ago

benfuu commented 2 months ago

Is your feature request related to a problem? Please describe.

Currently there are options to connect to the events bus (NATS) with certificates provided in env vars:

However, there aren't any similar env vars to connect to the registry when using nats-js-kv.

Describe the solution you'd like

Add similar env vars to connect to the app registry:

The NATS options can be configured in the file: https://github.com/owncloud/ocis/blob/master/ocis-pkg/natsjsregistry/registry.go#L170-L182

Describe alternatives you've considered

Currently we'd have to disable TLS completely in our NATS instance, or have a separate NATS instance just for the registry.

Additional context

micbar commented 2 months ago

@kobergj Can you take a look please?

kobergj commented 2 months ago

Yes. Absolutely valid point. This was not taken into account on initial implementation, but should be fairly easy to implement.

We should also add an option to add TLS config for the nats-js-kv stores, not only for the registry: https://github.com/cs3org/reva/blob/edge/pkg/store/store.go#L139-L151

But maybe we should adjust envvar names so we can reuse the same on all nats related features (evenst/stores/caches/registry)? I mean this will probably always be the same anyways.

micbar commented 2 months ago

But maybe we should adjust envvar names so we can reuse the same on all nats related features (evenst/stores/caches/registry)? I mean this will probably always be the same anyways.

I strongly vote for sticking to KISS

kobergj commented 2 months ago

I strongly vote for sticking to KISS

So that means we add new envvar OCIS_NATS_ENABLE_TLS and OCIS_NATS_TLS_ROOT_CA_CERTIFICATE and use them everywhere? We can deprecate exiting events - tls envars.

benfuu commented 2 weeks ago

Any updates on this? I'd be happy to provide a PR, but not sure what direction the team wants to go in regarding the env var naming conventions.

kobergj commented 2 weeks ago

Any updates on this? I'd be happy to provide a PR, but not sure what direction the team wants to go in regarding the env var naming conventions.

That would be very much appreciated, thanks!

Let's stick with simple solution for now. OCIS_NATS_ENABLE_TLS and OCIS_NATS_TLS_ROOT_CA_CERTIFICATE