spring-cloud / spring-cloud-kubernetes

Kubernetes integration with Spring Cloud Discovery Client, Configuration, etc...
Apache License 2.0
3.47k stars 1.03k forks source link

Lack of alternative for spring-cloud-kubernetes-config without (presumably deprecated) bootstrap.yml and lack of documentation for migrating it #1491

Closed nightswimmings closed 1 year ago

nightswimmings commented 1 year ago

I am a good citizen developer, and I realised spring-cloud-starter-kubernetes-fabric8-config does not include spring-cloud-bootstrap anymore in latest migration. I could have added it again, but I wanted to check the reason and it seems bootstrap.yml mechanism is now deprecated and Spring Cloud ConfigData API is now expected as the way to do things, as you know.

I spent quite a lot of time reading, and then I see that most people translates ConfigData API to a conventional Vault Hashicorp third-party app that must be installed in the cluster. Too much of an arch change for just making the code of my app compliant with latest expectations. Then I read that you can have a ConfigData API server embedded in the app itself if you use @EnableConfigServer, but I don't see any info on how to bind it to Kubernetes. Looks like it implies git as a source?

Instead I find out spring-cloud-kubernetes-configserver project but it is not clear if it can be used as an autoembedded server.

Finally I go back to https://docs.spring.io/spring-cloud-kubernetes/docs/current/reference/html/, in hopes of finding an alternative usage that does not rely on bootstrap.yml but here there's no references to our use case. Boostrap.yml is referred all the time.

So, in risk of you considering this ticket as a veiled support request, I really do believe there should be a clear centralized guidance on the simple way to migrate the old simple spring-cloud-kubernetes-config usage in the most simple compliant non-bootastrap.yaml way, without third-party servers (if that is possible). This business case is absolutely a must especially for Spring Cloud Dataflow Tasks, since we don't manage the deployment and sometimes we need them to "see" configmaps/servers. On a regular app we can manage their deployment.ymls to control the mounts. Deploying a configmap/secret manager component for a few tasks seems like an overkill, we really had no problem with them so far

ryanjbaxter commented 1 year ago

the reason and it seems bootstrap.yml mechanism is now deprecated

That is not true, did you read that somewhere?

You can use spring-cloud-starter-kubernetes-fabric8-config without using bootstrap by setting spring.config.import=kubernetes: as stated in this part of the documentation https://docs.spring.io/spring-cloud-kubernetes/docs/current/reference/html/#kubernetes-propertysource-implementations

nightswimmings commented 1 year ago

Hi Ryan! I seem to understand the deprecation here

So, I understand by your words this is all a big misunderstanding from my side, and despite new ConfigData API and server and the like, using bootstrap mechanism for kubernetes-config is still perfectly legit as first-option. I had already read before that part of the doc you sent me but it says:

To enable this functionality you need to set spring.config.import=kubernetes: in your application’s configuration properties. Currently you can not specify a ConfigMap or Secret to load using spring.config.import, by default Spring Cloud Kubernetes will load a ConfigMap and/or Secret based on the spring.application.name property. If spring.application.name is not set it will load a ConfigMap and/or Secret with the name application.

So unless docs are outdated, this means I cannot specifythe name of my configmaps secrets, or list them in precedence order. Besides,

If you would like to load Kubernetes PropertySources during the bootstrap phase like it worked prior to the 3.0.x release you can either add spring-cloud-starter-bootstrap to your application’s classpath or set spring.cloud.bootstrap.enabled=true as an environment variable.

seems to imply that spring.config.import=kubernetes: in order to read configmaps and secrets can only be used after context initialization, and therefore, its the same as placing the properties in application.yml and not bootstrap.yml, which fails for me because some cm/secret properties are needed as external config with precedence

Either way, I really appreciate your previous response.

nightswimmings commented 1 year ago

So summing up, if the typical scenario where an app attempts to read configmaps/secrets directly from k8s in precedence for @Conditionals and the like (as if it was an env var), has a more preferred/state-of-the-art/simpler/ConvOverConfig way to do things, I'll appreciate hearing it!

wind57 commented 1 year ago

using bootstrap mechanism for kubernetes-config is still perfectly legit as first-option

yes. until we figure out a way to have any configmap/secret as a source, bootstrap will not go anywhere.

seems to imply that spring.config.import=kubernetes: in order to read configmaps and secrets can only be used after context initialization

not really. Logically, for any app, there will no difference in using ConfigData API (spring.config.import=kubernetes) vs plain bootstrap (bootstrap.yaml)

It looks to me that you are confusing ConfigData API (which is a new way to read configmaps/secrets) and ConfigServer. These are totally different things, I guess.

So unless docs are outdated, this means I cannot specifythe name of my configmaps secrets, or list them in precedence order

You are correct and that is why bootstrap.yaml is still a valid option in this project.

So summing up, if the typical scenario where an app attempts to read configmaps/secrets directly from k8s in precedence for @conditionals

I am not sure I understand your point here. May be some pseudo-code would make things simpler.


Sorry for jumping on this thread, thought I might be helpful a bit.

nightswimmings commented 1 year ago

Thanks wind57! What I meant is that my business scenario is the following: I have a SCDF task project. This SCDF Task reads configmap/secret to retrieve vars in time of autoconfiguration as if they were regular envvars. So by your explanation I can conclude:

1.I can get rid of bootrap.yml and spring-cloud-bootstrap dependency/property if I use spring.config.import=kubernetes:, and my configmap properties will be read at initialization time with precedence as external documentation.

  1. This is a better approach looking for the future
  2. It is not feasible yet if I am reading from configmap/secrets that have custom names unrelated to my app

Is it right?

wind57 commented 1 year ago

yes, that is correct.

nightswimmings commented 1 year ago

Thanks now it all fits!

wind57 commented 1 year ago

@ryanjbaxter safe to close :)

ryanjbaxter commented 1 year ago
  1. It is not feasible yet if I am reading from configmap/secrets that have custom names unrelated to my app

You should still be able to specify names of configmaps/secrets as well as which namespace they reside in that you want to include as a property source

https://docs.spring.io/spring-cloud-kubernetes/docs/3.0.4/reference/html/#configmap-propertysource

nightswimmings commented 1 year ago

@ryanjbaxter But if I don't use bootstrap.yml those old spring.cloud.kubernetes.config props are not lodaded in autconfiguration time no (or not read a all by spring.import.config:kubernetes:)? At least in my tests.

Currently you can not specify a ConfigMap or Secret to load using spring.config.import, by default Spring Cloud Kubernetes will load a ConfigMap and/or Secret based on the spring.application.name property. If spring.application.name is not set it will load a ConfigMap and/or Secret with the name application.

ryanjbaxter commented 1 year ago

Nope if you use spring.config.import and specify the names of the confit maps and secrets you want to load via the properties in the documentation those property value should be available via auto configuration

nightswimmings commented 1 year ago

@ryanjbaxter I did a test with a SCDF task setting application.yml as

spring:
  config:
    import: kubernetes:
  main:
    cloud-platform: kubernetes
  cloud:
    kubernetes:
      config:
        sources:
          - name: environment
          - name: infrastructure
      secrets:
        enable-api: true
        sources:
          - name: environment-secret
          - name: infrastructure-secret

..and the result is that as soon as my task initialize it fails when trying to locate the properties Caused by: java.lang.IllegalArgumentException: Could not resolve placeholder 'X' in value \"${X}

That does not happen if I place all spring.cloud.* properties in bootstrap.yml and enable spring.cloud.bootstrap.enabled=true

ryanjbaxter commented 1 year ago

Can you provide a sample that reproduces the problem so I can take a look?

nightswimmings commented 1 year ago

I hope I can find some time, because if I am not wrong, the only way to test this at home is installing a whole Kubernetes cluster locally and configure it which I never did and I guess it takes some preparation

ryanjbaxter commented 1 year ago

You can us a lightweight cluster like minikube or kind

nightswimmings commented 10 months ago

@ryanjbaxter I found time to test this a bit and it is working with both spring.cloud.kubernetes.config.name and spring.cloud.kubernetes.config.sources. At least for configmaps

I used an script like:

mkdir configissue && cd configissue
curl 'https://start.spring.io/starter.zip?type=maven-project&dependencies=cloud-config-client' | tar xvf -
sed -i '' 's|spring-cloud-starter-config|spring-cloud-starter-kubernetes-fabric8-config|' pom.xml
sed -i '' 's|^}|\t@Bean InitializingBean test(@Value("${test-property:none}") String testProperty) {\n\t\treturn () -> System.out.println("CONFIGMAP PROPERTY VALUE:: "+testProperty); \n\t}\n}|' src/main/java/com/example/demo/DemoApplication.java
echo 'spring.main.cloud-platform=kubernetes' >> src/main/resources/application.properties
echo 'spring.config.import=kubernetes:' >> src/main/resources/application.properties
echo 'spring.cloud.kubernetes.config.name=test-configmap' >> src/main/resources/application.properties
chmod +x ./mvnw && ./mvnw spring-boot:build-image

brew install minikube && minikube start && kubectl config use-context minikube
kubectl create role configmap-reader --verb=get,list --resource=configmaps
kubectl create rolebinding configmap-reader-binding --role=configmap-reader --serviceaccount=default:default
kubectl create configmap test-configmap --from-literal=test-property=success
docker save demo:0.0.1-SNAPSHOT | (eval $(minikube docker-env) && docker load)
kubectl delete pod configissue && kubectl run -n default configissue --image=demo:0.0.1-SNAPSHOT --restart=Never
kubectl logs -f configissue

So either:

a) I had a different setup or something intercepting b) The bug was real but it has been fixed in recent versions c) I have to test the secrets as well when I got time, perhaps the missing properties were from those

Just as a side-note I can report a side-bug. Playing with previous test snippet I realised supporting bootstrap.yml only works if you add the marker dependency, but not when you use spring.cloud.bootstrap.enabled=true in your application.properties

ryanjbaxter commented 10 months ago

Thanks for the update. spring.cloud.bootstrap.enabled=true must be set as either an environment variable or system property, it needs to be set before we load configuration properties.

nightswimmings commented 10 months ago

@ryanjbaxter Ok, by adding

kubectl create secret generic test-secret --from-literal=test-secret-property=success
echo 'spring.cloud.kubernetes.secrets.enable-api=true' >> src/main/resources/application.properties
echo 'spring.cloud.kubernetes.secrets.name=test-secret' >> src/main/resources/application.properties
and printing @Value("${test-secret-property:none}") String testSecretProperty

looks like it cannot find the secret, so all those errors I saw on unexpandable @Values were probably due to secrets, not configmaps

nightswimmings commented 10 months ago

Nope sorry, it was my fault, I was missing the update of the role to support secrets kubectl create role configmap-reader --verb=get,list --resource=configmaps,services

nightswimmings commented 9 months ago

Hi @ryanjbaxter Found a new cornercase on the following application.yml scenario:

spring:
  config:
    activate:
      on-cloud-platform: kubernetes
    import: "kubernetes:"
  cloud:
    kubernetes:
      config:
        sources:
          - name: myconfigmap     # Contains "mydomain.custom.property: baz"
---

spring:
  main:
    cloud-platform: kubernetes
 mydomain:
   custom:
     property: "foo"  # This needs to be commented for the value from configmap to be retrieved

In this case, if mydomain.custom.property is defined in the application.yml as default, it takes precedence over the configmap one. Tested with Boot 3.2.2 and Cloud 2023.0.0

ryanjbaxter commented 9 months ago

If you switch the order the of the documents does that make a difference?

nightswimmings commented 9 months ago

Right, then it works as expected. (I tested with and without the property, and with and without the configmap property, all combinations have the expected logical precedence) Still, is it by design or a bug (if so, is there a ticker yet in either Spring Boot or Spring Cloud Kubernetes? I can open it otherwise, it's just for me following it and refer to it Thanks a lot @ryanjbaxter !

ryanjbaxter commented 9 months ago

I tested with and without the property, and with and without the configmap property, all combinations have the expected logical precedence

Can you describe exactly what you tested?

nightswimmings commented 9 months ago

I double checked the following truth table:

YAML_PROP    CONFIGMAP_PROP         App Runtime Value
                                       None
     x                                Yaml
                   x                  Configmap
     x             x                 Configmap

And that when cloud-platform is none, then the Yaml property is taken

ryanjbaxter commented 9 months ago

I am unsure how to interpret that, can you just provide the sample configurations?

nightswimmings commented 9 months ago

I used my biz project as test but I provided a sample in https://github.com/spring-cloud/spring-cloud-kubernetes/issues/1491#issuecomment-1870987147

I'll try to explain the test above. Using the following application.yml in that particular order (as you pointed out, which fixed my issue),

spring:
  main:
    cloud-platform: kubernetes
mydomain:
  custom:
    property: "foo"  

---

spring:
  config:
    activate:
      on-cloud-platform: kubernetes
    import: "kubernetes:"
  cloud:
    kubernetes:
      config:
        sources:
          - name: myconfigmap     # Contains "mydomain.custom.property: baz"

1.If I dont place mydomain.custom.property in either configmap nor application.yml, then @Value("${mydomain.custom.property}:none") shows "none" in runtime 2.If I dont place mydomain.custom.property in configmap but I do in application.yml, then @Value("${mydomain.custom.property}:none") shows the value from application.yml in runtime 3.If I place mydomain.custom.property in configmap but not in application.yml, then @Value("${mydomain.custom.property}:none") shows the value from the configmap in runtime 4.If I place mydomain.custom.property in configmap AND in application.yml, with different values, then @Value("${mydomain.custom.property}:none") shows the value from the configmap in runtime 5.When the yaml contains spring.main.cloud-platform: none, then the value from application.yml is taken, regardless whether the property is defined in the configmap or not

Thats the truth table explained, and I guess it's the expected, at least for me

ryanjbaxter commented 9 months ago

And you believe case 4 above is a bug?

ryanjbaxter commented 9 months ago

I believe whatever configuration is in the last document in your YAML will take precedence. So if you have the configmap configuration last and it contains the property mydomain.custom.property it will win. If you have the configmap first and specify the property mdomain.custom.property last it will take precedence over what is in the configmap.

nightswimmings commented 9 months ago

And you believe case 4 above is a bug?

no, no, after I switched order then everything is as expected for me, as per the explanation of cases I did The only problem for me was that it feels a bit counterintuitive to require to put that config at the end of the file to make it work. It should not matter whether it is before or after the main config, in my opinion. As soon as something is defined from a configmap, it should always smartly take precedence over the local properties, as in externalised configuration rules on spring-boot (https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#features.external-config), from most generic to most specific: local props < external props < env vars < CLI arguments, etc. The rationale is: changing embedded config on multiple projects is costly, but overriding from an external source is centralised and easy

ryanjbaxter commented 8 months ago

This situation is a bit different though since the PropertySources that get created are part of the same file. Spring Boot will read the documents in the file in the order they appear. So if

  main:
    cloud-platform: kubernetes
mydomain:
  custom:
    property: "foo"  

is the first document it will create a PropertySource from this document. If there is a document that comes after this that specifies confifuration from a Config Map or secret we will create a PropertySource from that and it will be placed after the first PropertySource in the list and will therefor have precedence. While these are "external configurations" they are treaded more as internal configurations that get loaded from an external source.