Environment variable name contains dash (-)

likeMyCoffee commented 6 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)

Due to a wrong environment variable name the config cannot be loaded from configmap or inline config. Dash (-) is not supported on modern bash shells. Alpine doesn't use bash so there is no issue there.

This becomes a problem when using a different container OS like Debian. I've had to move to a Debian based container because Alpine and Kubernetes don't work together (funky/flunky DNS due to musl library).

Please change

        - name: SPRING_CONFIG_ADDITIONAL-LOCATION

to

        - name: SPRING_CONFIG_ADDITIONALLOCATION

To increase OS support of the product.

Expected behavior

Config should load

Your installation details

I'm running kafka-ui helm chart 0.7.6 and container 0.7.2. I'm about to try your spin-off version and have ported this issue to here.

Steps to reproduce

Convert the Dockerfile to the zulu openjdk debian base image. Put a config in via yamlApplicationConfig(Config) Run the product Watch it ignore your config.

Screenshots

Not applicable

Logs

Not applicable

Additional context

With the mentioned solution it works and the config is processed.

Haarolean commented 6 months ago

Hi, we do not manage these kind of properties, it's a part of spring framework. Considering spring's relaxed binding, SPRING_CONFIG_ADDITIONAL_LOCATION should also work, can you try that?

likeMyCoffee commented 6 months ago

I will try this today.

My current solution is removing the dash from line 57 your deployment.yaml in the helm chart.

My solution is conform Environment Variables, Simple Types in your linked document. Using the _ you proposed is mentioned in the note as not a solution. That said I'm no expert 😊

likeMyCoffee commented 6 months ago

You are right. SPRING_CONFIG_ADDITIONAL_LOCATION works for me too :-)

Modified the helm chart Deployed kafbat Dud a kubernetes exec to verify that the variable was set

So for me this small modification would be great!

Haarolean commented 6 months ago

Glad it works. I believe we don't need the changes in the chart itself as the chart is, well, a chart, not a bash shell.

Also,

I'm running kafka-ui helm chart 0.7.6 and container 0.7.2. Please note versions below 1.0.0 are not supported and contain a lot of bugs and security vulnerabilities. I advise you to update, the latest image is ghcr.io/kafbat/kafka-ui:latest

likeMyCoffee commented 6 months ago

I’m running 1.0.0 on the 1.4.0 helm chart. I initially ported the issue as specified in the provectus repo. Within kubernetes using Alpine Linux is a known problem for which I needed this minor modification. Anyhow thanks for closing the issue.

kafbat / helm-charts