spring-cloud / spring-cloud-vault

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

Vault isn't considering profile-specific files #571

Closed nkvaratskhelia closed 3 years ago

nkvaratskhelia commented 3 years ago

I'm using spring boot 2.4.2 and spring-cloud 2020.0.1 (spring-cloud-starter-vault-config).

I have two properties files: application.properties and application-test.properties. application.properties content:

spring.application.name=app-name
spring.config.import=vault://
spring.cloud.vault.scheme=http
spring.cloud.vault.host=${VAULT_HOST}
spring.cloud.vault.port=${VAULT_PORT}
spring.cloud.vault.token=${VAULT_TOKEN}
spring.cloud.vault.kv.backend=kv-backend

VAULT_HOST, VAULT_PORT and VAULT_TOKEN are environment variables.

application-test.properties content:

spring.cloud.vault.enabled=false

Everything works fine when running with default profile. However, when running tests with the test profile (using the @ActiveProfiles("test") annotation) in a ci/cd environment, vault ignores the spring.cloud.vault.enabled=false property and still tries to create a connection.

Log:

...
[main] DEBUG org.springframework.core.env.StandardEnvironment - Activating profiles [test]
...
[main] DEBUG org.springframework.boot.diagnostics.LoggingFailureAnalysisReporter - Application failed to start due to an exception
org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'spring.cloud.vault.port' to int
...
Caused by: java.lang.NumberFormatException: For input string: "${VAULT_PORT}"

Even if I add the spring.cloud.vault.port=8200 property to the application-test.properties file I get the same exception. So it seems that probably vault isn't considering the application-test.properties file at all.

nkvaratskhelia commented 3 years ago

Any update on this?

mp911de commented 3 years ago

Can you help us understand the issue better by attaching the full stack trace?

nkvaratskhelia commented 3 years ago

stacktrace.txt

mp911de commented 3 years ago

Thanks. This looks as if Spring Boot isn't resolving placeholders during at the time we initialize the Vault configuration.

nkvaratskhelia commented 3 years ago

Let me know if you need any more input from me.

mp911de commented 3 years ago

I created a reproducer that is able to demonstrate the issue. When disabling Vault support make sure that it's prefixed with optional (as in optional:vault://), otherwise Spring Boot will report: Config data resource 'file [vault:]' via location 'vault://' cannot be found.

The reproducer is at https://github.com/sampledump/spring-cloud-vault-reproducer-571. Running VaultApplicationTests will demonstrate the issue unless running the test with -Dspring.cloud.vault.enabled.

mp911de commented 3 years ago

Waiting for further guidance from the Spring Boot team.

philwebb commented 3 years ago

We discussed this on our team call and think that the current behavior is expected. The ConfigDataEnvironmentPostProcessor intentionally handles property loading in distinct phases. When the application.properties file is processed, no profile specific files are active. This means that spring.config.import=vault:// is processed regardless of the spring.cloud.vault.enabled setting in application-test.properties.

We want to do things this way so that imports can contribute spring.profile.* properties if they want to.

For your example, I would suggest using @TestPropertySource(properties="spring.cloud.vault.enabled:false") instead of using @ActiveProfiles. This will add the property before the spring.config.import value is processed and should allow it to be disabled.

philwebb commented 3 years ago

Just discussing a similar issue with @spencergibb related to https://stackoverflow.com/questions/66550595/disable-consul-for-integration-tests-with-sb-2-4. It might be worth changing VaultConfigDataLocationResolver.isResolvable(...) to return true based on the location string alone. The vaultEnabled logic can be moved to resolve. This would mean the optional: prefix isn't needed.

philwebb commented 3 years ago

Actually, put that on hold. We might need a boot update to support optional resolvers.

nkvaratskhelia commented 3 years ago
  1. added @TestPropertySource(properties="spring.cloud.vault.enabled=false") to test class
  2. removed spring.cloud.vault.enabled=false from application-test.properties
  3. also had to add optional: to the spring.config.import=vault:// property in application.properties

This now works, but I don't think step 3 is correct. This property isn't intended to be optional.

philwebb commented 3 years ago

I've added a new constructor to ConfigDataResource that can be used to signal if a resource is optional.

DemianTinkiel commented 3 years ago

I'm having the same issue with spring boot 2.4.2 and spring-cloud 2020.0.1 (spring-cloud-starter-vault-config) for properties spring.cloud.vault.ssl.trust-store and spring.cloud.vault.ssl.key-store.

Is there a workaround you could suggest?

nkvaratskhelia commented 3 years ago

@DemianTinkiel You can see a possible workaround in philwebb's and my comments above. Not sure if it's gonna work in your case, but you can try. As for the issue itself, I believe there were some changes done in spring boot 2.4.6 that would help spring-cloud-vault's team fix this, so we're still waiting for the fix in spring-cloud-vault.

mp911de commented 3 years ago

As per Phil's recommendation, we've moved checks around to let the VaultConfigDataLoader check whether Vault is enabled. If Vault is disabled, the loader returns null to indicate that it didn't load a resource, and Spring Boot can skip the location if it was marked previously as optional.

nkvaratskhelia commented 2 years ago

@mp911de I might be wrong but I think this still doesn't work. You can update the reproducer (https://github.com/sampledump/spring-cloud-vault-reproducer-571) to spring boot 2.5.5 and spring cloud 2020.0.4. Vault is still trying to establish connection when running VaultApplicationTests.

mp911de commented 2 years ago

Have you tested against Vault 3.0.4 snapshots? 3.0.4 was released a few days ago hence my question.

nkvaratskhelia commented 2 years ago

Oh, I was under the impression that it was already released as part of spring cloud 2020.0.4: image

mp911de commented 2 years ago

Thanks for the details. Let me investigate what's going on and come back to you.

dragneelfps commented 2 years ago

I am using 3.0.4 and it still is not picking up application-{env}.yml files to load spring.cloud.vault.* configurations