spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.48k stars 40.53k forks source link

Support loading config yaml files embedded in env vars via spring.config.import #41609

Open chicobento opened 1 month ago

chicobento commented 1 month ago

Use case

Providing a Helm Chart for a Spring Boot application, typically requires translating input from values.yaml to application properties. There are many ways to achieve this, one of the most straightforward ways being using environment variables. However, there are a few drawbacks with such strategy:

Converting yaml snippet to json and injecting it in the SPRING_APPLICATION_JSON almost fits the purpose, but helm's fromYaml function has limitations such as support for multi-documents (see https://github.com/helm/helm/issues/11647) and SPRING_APPLICATION_JSON doesnt support much flexibility in terms of document order priority.

The proposal

To improve developer experience when creating helm charts for Spring Boot apps, the proposal is to allow loading via spring.config.import a full config yaml file encoded in a single environment variable.

Given the following application.yaml (burned in the container image):

logging:
  level:
    acme.MyClass: ERROR

spring:
  config:
    import:       
    - optional:env:HELM_APPLICATION_YAML[.yaml]

When the multi-line environment variable HELM_APPLICATION_YAML is defined for the container (rendered by helm):

logging:
  level:
    acme.MyClass: DEBUG
---
acme:
  property1: 1

Then, the final configured level for acme.MyClass should be DEBUG.

Optionally we can support base64-encoded env vars for simplifying creation of multi-line env vars, via something like optional:env:base64:HELM_APPLICATION_YAML[.yaml]

Ps: In the examples I mentioned yaml, but it would support properties files as well via the [.properties] hint

I already have a PoC working code for supporting this and Im willing to contribute if the feature is acceptable.

philwebb commented 1 month ago

This is an interesting proposal. I've flagged it for team discussion (hopefully sometime next week)

scottfrederick commented 1 month ago

@chicobento Environment variables are often used to map input from values.yaml into properties in the application container, but if there are a lot of properties then using ConfigMaps can be simpler.

For example, if you have something like this in values.yaml:

config:
  spring:
    application:
      name: helm-demo
  app:
    greeting: "Hello"
    name: "World!"

and create a new template in templates/configMap.yaml like this:

apiVersion: v1
kind: ConfigMap
metadata:
  name: app-configmap
data:
  helm-values.yaml: |-
    {{ .Values.config | toYaml | nindent 4 }}

then you can mount the ConfigMap into the container in templates/deployment.yaml:

spec:
  template:
    spec:
      containers:
          volumeMounts:
            - name: app-config-volume
              mountPath: /config
      volumes:
        - name: app-config-volume
          configMap:
            name: app-configmap

and directly import the contents of this configuration from the mounted helm-values.yaml file in the application's main application.yaml file:

spring:
  application:
    name: demo
  config:
    import: optional:/config/helm-values.yaml

This will work now, without any changes to Spring Boot, and without mapping from YAML to the environment. Is this approach something you have explored, and if so is there a reason this won't work for your use cases? If there is a reason why this won't work, can you give us a more complete example of what you are doing?

chicobento commented 1 month ago

Hi @scottfrederick , thank you for the very detailed feedback.

We have being doing exactly this since spring.config.import was added to SB and we want to move away from it due to several reasons, the main one being the problem with upgrades and backward compatibility of the spring properties. Sorry for the very long blog post 😆, but follows some reasoning.

Upgrade problem

Lets say that the v1 of your app has the following config:

config:
  app:
    should-say-hello: true # boolean property

Now, in the v2 version you changed the property to an enum:

config:
  app:
    should-say-hello: mornings-only # always, never or mornings-only

When you run the helm upgrade command, one of the first resources to be upgraded will be the configmap. Thats fine, the new v2 pods will take the new configmap and assuming that the old v1 ones doesnt have any hot-reload facility (which in fact we have), they will not react to the change and will still keep serving the traffic until they are removed.

What happens if something goes wrong during the upgrade ?

If the v2 pods doesn't become ready or take too much time - for whatever reason, and in this timeframe the v1 pods needs to be restarted (either because of a crash or due to scaling up), kubernetes will spawn new v1 pods for replacing the crashed ones using the deployment created from the v1 chart. At this point, the configmap has the v2 chart contents but kubernetes is trying to spawn new v1 pods which will obviously crash because they cannot recognize the new config format and we at the risk of not being able to serve any traffic.

We have reproduced this by creating a simple app and adding a delay in the startup of the v2 and manually deleting the old pods. The delay was added by placing a Thread.sleep(600000) in Spring Boot main class.

Possible solutions

We have tried to version the configmap by applying a suffix to its name, i.e: name: app-configmap-v1 and name: app-configmap-v2 but then we figured out that helm will uninstall the app-configmap-v1 and install the app-configmap-v2 in the very beginning of the upgrade, so if the v1 pods needs to be restarted they will get stuck because of the absence of the app-configmap-v1 resource. This could be solved by using helm helm.sh/resource-policy: keep but then the application would have to take care of the housekeeping of the resources - or leave it to the ops team.

Im afraid that almost every single spring boot application out there using this type of solution may be exposed to this type of bug, since it is tricky to setup this scenario for testing.

Other issues

Besides the upgrade problem, I personally dislike exposing this internal type of config as a config map. Users may be tempted to update the config via configmap rather than via the helm chart values and then the config map would become one additional interface to be honored and maintained. I'd rather use configmaps for cases where I'm fully commited to maintain as an exposed interface, or cases where for example multiple services needs to share/use the same configuration snippet that would be provisioned by another system or process, i.e operators or integration charts.

While I'd love to hear about solutions and alternatives around those problems, the bottom line is that they should not be there in first place if env variables are used instead of configmaps and for this reason Im requesting this feature to simplify config loading via env variables.

scottfrederick commented 1 month ago

@chicobento Can you show an example of how the Helm chart converts values.yaml properties to the YAML environment variable, and an example of the environment variable contents?

chicobento commented 1 month ago

Sure. I created this example https://github.com/chicobento/spring-boot-config-env-example/ based on https://github.com/helm/examples/tree/main/charts/hello-world:

Added config to values.yaml

config:
  spring:
    application:
      name: helm-demo
  app:
    greeting: "Hello"
    name: "World!"

Added envs to deployment.yaml

apiVersion: apps/v1
kind: Deployment
spec:
  template:
    spec:
      containers:
        - name: {{ .Chart.Name }}
          env:
          - name: SPRING_APPLICATION_YAML
            value: |-
            {{- .Values.config | toYaml | nindent 14 }}
          - name: SPRING_APPLICATION_YAML_BASE64
            value: "{{- .Values.config | toYaml | b64enc }}"

Now if you install the chart (I did with kind), shell into the pod and echo "$SPRING_APPLICATION_YAML" or echo $SPRING_APPLICATION_YAML_BASE64 | base64 -d you'll get the following:

app:
  greeting: Hello
  name: World!
spring:
  application:
    name: helm-demo

Then it should be able to be loaded in boot app by doing something like:

spring:
  config:
    import:       
    - optional:env:SPRING_APPLICATION_YAML[.yaml]
    - optional:env:base64:SPRING_APPLICATION_YAML_BASE64[.yaml]