spring-cloud / spring-cloud-vault

Configuration Integration with HashiCorp Vault
http://cloud.spring.io/spring-cloud-vault/
Apache License 2.0
276 stars 150 forks source link

Spring Cloud Vault/Zookeeper do not add property sources if context paths are the same #736

Open latyshas opened 1 week ago

latyshas commented 1 week ago

Hello,

It seems that there is a bug or limitation in the Spring Boot + Spring Cloud project.

Problem description:

Precondition:

  1. There is a Zookeeper instance where configuration parameters are stored in the following paths:

    • /config/application
    • /config/${spring.application.name}
  2. There is a HashiCorp Vault instance where configuration parameters ("credentials") are stored in the following paths:

    • /config/application
    • /config/${spring.application.name}

Yes, you noticed correctly — the issue is that Zookeeper and Vault have the same paths.

How to reproduce: Create a Spring Boot application (in my case, I used Spring Boot 2.7.18, but looking at the source code, the problem will occur in the latest version as well) and connect spring-cloud-starter-vault-config (in my case, version 3.1.4, but the same issue exists in the master branch) and spring-cloud-starter-zookeeper-config (in my case, version 3.1.5, it looks the same issue exists in the master branch).

When the application starts, only one configuration source will work, specifically the one listed first in spring.config.import. For example, with the following configuration:

spring.config.import: 
  - "optional:zookeeper:"
  - "optional:vault://"

The application will not have a Vault property source with the required paths.

In this case:

spring.config.import: 
  - "optional:vault://"
  - "optional:zookeeper:"

The application will not have a Zookeeper property source with the required paths.

The issue relates to the *ConfigDataLoader classes. For example, in spring-cloud-vault-config: the VaultConfigDataLoader (see here) it creates a LeaseAwareVaultPropertySource with name() == path value. A similar approach is used for Zookeeper. Then, when starting Spring Boot, in the class org.springframework.boot.context.config.ConfigDataEnvironment, all property sources are added to the property sources list, and here we encounter an issue: the method propertySources.addLast(propertySource); is used (see here). However, the propertySources.addLast method removes the already added property source from the list based on the equals method, and the equals method in PropertySource is primitive and validates only by name. So, for both the Vault property source and the Zookeeper property source, we end up with the same names, as they are equal to the path. Adding one removes the other.

Here are links to the source code: https://github.com/spring-projects/spring-boot/blob/v3.3.4/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/config/ConfigDataEnvironment.java#L357 https://github.com/spring-projects/spring-framework/blob/v6.1.13/spring-core/src/main/java/org/springframework/core/env/MutablePropertySources.java#L116 https://github.com/spring-projects/spring-framework/blob/v6.1.13/spring-core/src/main/java/org/springframework/core/env/PropertySource.java#L143

It seems that the simplest way to fix this would be to add a prefix when creating the Vault/Zookeeper property source so that not only the path is considered, but also some indicator of which configuration source it belongs to. For example, for Vault, it could look like this:

String propertySourceName = "vault:" + accessor.getName();
LeaseAwareVaultPropertySource propertySource = new LeaseAwareVaultPropertySource(propertySourceName, secretLeaseContainer, secret, accessor.getPropertyTransformer());

Thanks, have a good day!

mp911de commented 1 week ago

I am not quite sure I follow. Paths in Vault aren't related to Zookeeper paths at all. These are two different systems.

In Spring's Vault support, key names from a contextual path are added as top-level entries. A key at /config/${spring.application.name} (username=foo) is made available under username without any prefixes or such. The only unwrapping that happens is when using Vault kv2 backends, then data.username gets unwrapped to username.

I think you're up to the fact that property source names should be unique. Looking at Config Server, these property sources get prefixed.

Can you file a similar ticket in the Zookeper repo?

latyshas commented 2 days ago

Hi @mp911de,

Yes, I understand that ZK and Vault have different sources, but let's imagine a scenario where your system has the same paths in two different configuration sources: ZK and Vault. In ZK: /config/{YOUR_APP_NAME}/property1 In Vault: /config/{YOUR_APP_NAME}/property2

Now, you want to read property1 from ZK and property2 from Vault, but currently, you can't do that because the property source must be unique by path name: /config/{YOUR_APP_NAME}, and only the first property source will be used — in this case, ZK.

An issue has also been created for spring-cloud-config-zookeeper: https://github.com/spring-cloud/spring-cloud-zookeeper/issues/336

mp911de commented 2 days ago

Alright, thanks for the detail.