Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for config data api and consul backend #580

Closed
wants to merge 1 commit into from

Conversation

krisiye
Copy link
Contributor

@krisiye krisiye commented Mar 4, 2021

Register SecretBackendMetadataFactory for consul under spring.factories for config data API to discover consul backend.

Fixes #579

…es for config data api to discover consull backend
@mp911de
Copy link
Member

mp911de commented Mar 10, 2021

With the Config Data API, we need to inject an ApplicationEventPublisher into ConsulSecretBackendMetadataFactory so that we can rebind changed config properties.

mp911de pushed a commit that referenced this pull request Mar 10, 2021
Allow usage of Consul through the Config Data API.

Closes gh-579
Original pull request: #580.
mp911de added a commit that referenced this pull request Mar 10, 2021
Allow SecretBackendMetadata to implement ApplicationEventPublisherAware. Register application event publisher after boostrap with ConsulBackendMetadata.

Add integration tests.

Closes gh-579
Original pull request: gh-580
@mp911de
Copy link
Member

mp911de commented Mar 10, 2021

Thank you for your contribution. That's merged and polished now.

@krisiye
Copy link
Contributor Author

krisiye commented Mar 10, 2021

@mp911de - How do we ensure the ordering of the data loaders? For example spring.config.import: vault://,consul:// ? With the vault consul backend enabled, we need to get the consul acl-token before the consul data loader attempts to load. Otherwise, it fails right away with a 403 Forbidden. Any Thoughts?

@krisiye krisiye deleted the issue-579 branch March 10, 2021 15:53
@spencergibb
Copy link
Member

@krisiye AFAIK it's the ordering of the imports. That's all handled by Spring Boot. @philwebb or @mbhave could provide some more ordering insight.

@krisiye
Copy link
Contributor Author

krisiye commented Mar 11, 2021

@philwebb @mbhave -Any thoughts on how we could introduce dependencies in the config import for the use cases such as vault and consul being used in an integrated fashion? I am not sure if such capability exists. But please let me know if you would like me to raise an issue under spring-boot. Thanks for your help!

@mbhave
Copy link

mbhave commented Mar 11, 2021

@krisiye Can you provide more details on what the use case is and how it's expected to work? I'm not sure I understand what is supposed to happen so hard to say without that information.

@krisiye
Copy link
Contributor Author

krisiye commented Mar 11, 2021

Sure @mbhave. I have a spring boot 2.4.3 app set up with spring cloud 2020.0.1 where I use vault for secret management and consul for service configuration. The configuration I use for the config data API is spring.config.import: vault://,consul://. This works great if consul was setup for open access (without ACL). However, in any production configuration, we expect consul to be locked down and be behind ACL and that we expect vault to provide the consul acl token through the vault consul backend (had some issues but was addressed in this bug above). I noticed that the data loaders do not have any dependencies established and hence we end up loading consul even before the acl-token is made available in the property sources and thus end up with a 403 Forbidden.

In the legacy bootstrap strategy, we had a workaround that is documented here: https://gist.github.com/mp911de/17f550ffecdc9e8f22061bfdf896bbb4

The question here is if this can be fixed for the config data api with any dependency configuration for the imports? I could not find anything in the documentation that indicates such a capability exists. Please let me know if you need anymore information.

@philwebb
Copy link

@krisiye Looking at the code, I think that import are loaded in reverse order. So if you have vault://,consul:// then consul:// is loaded first and then vault://. If I remember correctly, things are done this way to ensure that later elements in the list win. I think we were trying to be consistent with the file itself (where if you have duplicate key, the lower one wins).

You could try switching the order to see if that solves your issue.

@krisiye
Copy link
Contributor Author

krisiye commented Mar 12, 2021

Thanks @philwebb. Tried reversing the order here to spring.config.import: consul://,vault:// but no luck. Still seeing the same behaviour resulting in a 403 Forbidden. Should I raise an issue for this under spring-boot?

@philwebb
Copy link

@krisiye Yes please. It would also be helpful if you're able to provide a sample that shows the problem.

@krisiye
Copy link
Contributor Author

krisiye commented Mar 17, 2021

@philwebb Logged an issue for this - spring-projects/spring-boot#25705

@krisiye
Copy link
Contributor Author

krisiye commented Mar 22, 2021

@philwebb @mbhave - Updated spring-projects/spring-boot#25705 and provided a test case for this issue. Sorry, this took a while as it was a little challenging to get a self-contained example with test containers and roles for demonstrating this issue.

spencergibb pushed a commit that referenced this pull request Sep 14, 2023
Allow usage of Consul through the Config Data API.

Closes gh-579
Original pull request: #580.
spencergibb pushed a commit that referenced this pull request Sep 14, 2023
Allow SecretBackendMetadata to implement ApplicationEventPublisherAware. Register application event publisher after boostrap with ConsulBackendMetadata.

Add integration tests.

Closes gh-579
Original pull request: gh-580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consul Secret Backend is not discovered with config data api
5 participants