spring-cloud / spring-cloud-vault

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

Importing Vault Config Data with trailing slash fails to load context path #601

Closed pavankjadda closed 3 years ago

pavankjadda commented 3 years ago

Describe the bug In my spring boot project, I use this library to get and replace profile specific properties from vault with App Role authentication. After changes in Spring Boot 2.4, I started testing the new format. Here is the issue I see when using with Spring Boot 2.4 and 2.5. Properties file(dev.yml or prod.yml) place holders are not being replaced with values from Vault.

Here is an example based on 2.4 config. I have application.yml, dev.yml or prod.yml files. When the application starts, properties in dev.yml or prod.yml (based on active profile) should be replaced with actual values from Vault. But they are not

application.yml

server:
  port: 8081
## Select profile
spring:
  application:
    name: "customers"
  profiles:
    group:
      "dev": "dev"
      "prod": "prod"
  cloud:
    vault:
      authentication: APPROLE
      app-role:
        role-id: ${role-id}
        secret-id: ${secret-id}
        role: customers-read
        app-role-path: approle
      uri: https://VAULT_URL:8200
      connection-timeout: 5000
      read-timeout: 15000
      kv:
        enabled: true
        backend: secret
        application-name: app/customers
  config:
    import: vault://secret/app/customers/

---
spring:
  config:
    activate:
      on-profile: "dev"
    import: dev.yml
---
spring:
  config:
    activate:
      on-profile: "prod"
    import: prod.yml

dev.yml

spring:
  datasource:
    url: "jdbc:mysql://localhost/dev"
    username: "dev"
    password: "dev"

#### ELK Logging
elk:
  logging:
    rabbitmq:
      hostname: ${customers.elk.logging.rabbitmq.hostname}
      port: 5672
      username: ${customers.elk.logging.rabbitmq.username}
      password: ${customers.elk.logging.rabbitmq.password}
      queueName: ${customers.elk.logging.rabbitmq.queueName}

prod.yml

spring:
  datasource:
    url: "jdbc:mysql://localhost/prod"
    username: "prod"
    password: "prod"

#### ELK Logging
elk:
  logging:
    rabbitmq:
      hostname: ${customers.elk.logging.rabbitmq.hostname}
      port: 5672
      username: ${customers.elk.logging.rabbitmq.username}
      password: ${customers.elk.logging.rabbitmq.password}
      queueName: ${customers.elk.logging.rabbitmq.queueName}

Sample See this Github repo https://github.com/pavankjadda/ProfileTest for sample app

mp911de commented 3 years ago

Spring Cloud Vault isn't the one that performs property placeholder expansion. That's rather something related to Spring Boot. Can you file the ticket there?

pavankjadda commented 3 years ago

@mp911de I did ask this in Spring Boot ticket. See Phils response here, see the second from the last comment(I couldn't get a link for the comment, strange). He suggested to raise an issue here.

mp911de commented 3 years ago

Thanks for letting us know. Reopening this ticket for further investigation.

mp911de commented 3 years ago

I wasn't able to reproduce the issue first, but after adding a trailing slash to import: vault://secret/app/customers which renders import: vault://secret/app/customers/ is causing the issue. I'm currently not sure how to be more lenient. Maybe we can do something about it.

mp911de commented 3 years ago

Validating the location makes right now the most sense as importing Vault directories as context path isn't supported.

pavankjadda commented 3 years ago

I wasn't able to reproduce the issue first, but after adding a trailing slash to import: vault://secret/app/customers which renders import: vault://secret/app/customers/ is causing the issue. I'm currently not sure how to be more lenient. Maybe we can do something about it.

So when you start the application and go to URL http://localhost:8081 you see placeholders being replaced with vault values? Because I still can't see it. I see ${customers.elk.logging.rabbitmq.username} and ${customers.elk.logging.rabbitmq.password} in console.

mp911de commented 3 years ago

Yes. I basically replicated your sample and I missed the trailing slash in the first variant. After adding the trailing slash, I was able to reproduce the issue with non-replaced placeholders.

pavankjadda commented 3 years ago

Yes. I basically replicated your sample and I missed the trailing slash in the first variant. After adding the trailing slash, I was able to reproduce the issue with non-replaced placeholders.

Now I am confused. You were able reproduce the issue regarding place holders, meaning your code doesn't replace the values but still closed the ticket? Isn't it supposed to replace place holders with values from vault?

And I see the issue with trailing slash which you are working on now.

mp911de commented 3 years ago

I was able to reproduce the problem by adding a trailing slash. Without the trailing slash on the vault path, the problem goes away. The ticket is resolved by adding path validation so that trailing slashes are reported with an exception instead of going unnoticed. Users can then fix the path on their side.

Does this make sense?

pavankjadda commented 3 years ago

@mp911de understood. My original issue was Properties file place holders are not being replaced with values from Vault and then you changed the title reflecting the issue with Spring Cloud vault. I thought we were talking my issue. Anyway I felt my issue belong in StackOverFlow, so I created new issue there. This stops me from adopting new Spring Boot config API.

mp911de commented 3 years ago

There are maybe two sides:

  1. I was able to reproduce the issue by adding a trailing slash and I fixed the problem by removing that trailing slash
  2. You still seem to have trouble with loading config data and I'm not able to reproduce the problem. Can you provide a full sample, ideally a preparation script for Vault and a boot app that doesn't require further tweaking that is able to reproduce the issue? If so, we can revisit the issue and check what's going on. Sorry if that wasn't clear earlier on.