Add ability to pass config via secret

timchenko-a commented 2 months ago

Issue submitter TODO list

[X] I've looked up my issue in FAQ
[X] I've searched for an already existing issues here (legacy) and here
[X] I've tried installing latest charts and the issue still persists there
[X] I'm running a supported version of the application & chart which is listed here

Describe the bug (actual behavior)

Right now it's possible to pass pre-created config file from ConfigMap using yamlApplicationConfigConfigMap value. It's a feature request to add similar value, but getting config from secret using new yamlApplicationConfigSecret value.

This will be beneficial for GitOps based deployments. E.g. in our case we have quite a long config file (300+ lines), so having everything defined as env variables is quite tricky. Mainly since we have to store kafka credentials encrypted via SealedSecret, so in case we add/remove clusters we have to change quite a lot of variables. With such approach it will be possible to store the config completely encrypted and handled from a single place.

I can work on this if you think this might be useful for others and have chances to be merged.

Expected behavior

No response

Your installation details

It's rather a feature request

Steps to reproduce

It's rather a feature request

Screenshots

No response

Logs

No response

Additional context

No response

Haarolean commented 2 months ago

Hi, ideally this could be both -- yaml config and encrypted secrets passed separately via env variables. There's an open issue regarding the fact that config is not currently overridable by external env vars, but we plan on fixing that for sure. Would this be an optimal solution for you as well, or do you prefer the one you described, the whole config being a secret?

timchenko-a commented 2 months ago

There's an open issue regarding the fact that config is not currently overridable by external env vars Oh, most probably this is what I've faced, wasn't sure if this is a bug or I messed with env variables format somehow. Could I ask you to share a link to this bug? Unfortunately wasn't able to find it neither in the new nor in the old repos.

Could I ask if you have a link to the issue somewhere? Most probably I've faced it, but didn't understand if it's a bug or I was doing something wrong. Unfortunately I wasn't able to find it neither in the new repo, nor in the old one.

From my point of view being able to pass config via secrets is good since you won't be forced to change variable naming if you delete a cluster. Let me share some example: If we have 3 clusters:

---
kafka:
  clusters:
    - name: a
      ...
    - name: b
      ...
    - name: c
      ...

we should define credentials using the next variables:

KAFKA_CLUSTERS_0_PROPERTIES_SASL_JAAS_CONFIG: ...
KAFKA_CLUSTERS_1_PROPERTIES_SASL_JAAS_CONFIG: ...
KAFKA_CLUSTERS_2_PROPERTIES_SASL_JAAS_CONFIG: ...

Now, if we delete cluster a, we'll have to rename all env variables, since indentation there is based on list index. Being able to encrypt the config completely could potentially solve this issue. Does this make any sense? Or maybe I'm try to define them initially wrongly?

Haarolean commented 2 months ago

Aight, I can agree on that. Perhaps, raising an issue in the main repo for considering other options for configuring cluster related properties would be cool as well.

Haarolean commented 2 months ago

I can work on this if you think this might be useful for others and have chances to be merged.

@timchenko-a please do!

kafbat / helm-charts