SPU Group's TLS secret name is hardcoded (probabaly shouldn't be)

[tothandras]

What happened

I'm using the Helm charts for deployment and noticed that my SPU Group is only able to come up when the TLS secret name (cert.tls) is defined as fluvio-tls (the default) because it seems to be hardcoded: https://github.com/infinyon/fluvio/blob/master/crates/fluvio-sc/src/k8/objects/spg_group.rs#L285

Environment (please complete the following information):

• Fluvio Version: 0.9.27
• Kubernetes version: 1.22.8

[sehz]

Hi, Let me see if I understand the issue. This is about TLS Secret name is hard coded to fluvio-tls?

[tothandras]

@sehz yes

[sehz]

Why would that be an issue? it's just id.

[tothandras]

I think it's supposed to be configurable, just like for SC: https://github.com/infinyon/fluvio/blob/master/k8-util/helm/fluvio-app/values.yaml#L16 https://github.com/infinyon/fluvio/blob/master/k8-util/helm/fluvio-app/templates/sc-deployment.yaml#L66 Preferably it should use the same value or there should be a field in the CRD.

[sehz]

I agree, that it should be configurable. Contribution Welcome!

[github-actions[bot]]

Stale issue message

[udsamani]

@sehz Do you mind if I pick this up ? Looks like a good issue to start with !

[udsamani]

Just to make sure that all of us are on same page on what is happening here.

• Currently we have hard coded fluvio-tls secret name everywhere. This is true even for SC. What I mean to say is that even if you change value of cert.tls here : https://github.com/infinyon/fluvio/blob/master/k8-util/helm/fluvio-app/values.yaml#L16, it would not work as when we upload the secret to Kubernetes it is hardcoded https://github.com/infinyon/fluvio/blob/2968b7af1e450f00e405d9527c9ad9637f4154da/crates/fluvio-cluster/src/start/k8.rs#L1257 I did a fun experiment of changing the value here, and found that SC would run into error with tls enabled.

How do we solve it ?

• Idea would be to have an option for secret-name as part of TlsConfig, and pass it accordingly. We can keep it as optional and default it to fluvio-tls.
• @sehz Do you feel the need for doing it for ca-cert too ? https://github.com/infinyon/fluvio/blob/2968b7af1e450f00e405d9527c9ad9637f4154da/crates/fluvio-cluster/src/start/k8.rs#L1250

@sehz Let me know if you are fine with making both configurable. I can move ahead in that case. Sorry for the delay, took a bit of time to get used to the code base.

[sehz]

Yes, this is good solution. I dont' think we need do it for ca-cert.

[udsamani]

PR: #2788

[ajhunyady]

@tothandras, please let us know if the fix works for you.