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

Allow values from configtree to resolve nested placeholders #34195

Closed
ChristianAnke opened this issue Feb 15, 2023 · 9 comments · May be fixed by #40862
Closed

Allow values from configtree to resolve nested placeholders #34195

ChristianAnke opened this issue Feb 15, 2023 · 9 comments · May be fixed by #40862
Labels
status: superseded An issue that has been superseded by another

Comments

@ChristianAnke
Copy link

It would be helpful if for properties which come from a configtree the placeholders will be replaced. Currently this seems to be blocked by the ConfigTreePropertySource producing non String results when getting the value of a property.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 15, 2023
@wilkinsona
Copy link
Member

Thanks for the suggestion.

This is a little bit similar to #29642. A notable difference here, however, is that org.springframework.boot.env.ConfigTreePropertySource.Value is part of the property source implementation and, unlike the String[] in #29642, something that cannot be controlled. It feels like placeholder replacement should be possible, but it also doesn't feel right for PropertySourcesPlaceholdersResolver to know about org.springframework.boot.env.ConfigTreePropertySource.Value.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 20, 2023
@ChristianAnke
Copy link
Author

Thanks for the answer!

Sounds legit to me. Surely it is not easy to to simply support this for all the various property source providers. Somehow it has to be unified how replacements can be performed with the different providers.

I see minimum two ways to solve this:
A: It is possible to convert a value to a string (by asking the property provider) and handle then the nested placeholders
B: let the provider itself perform the nested replacement
C: ???

what do you think?

@philwebb
Copy link
Member

philwebb commented Mar 2, 2023

We might be able to change PropertySourcesPlaceholdersResolver to check for a CharSequence rather than a String since ConfigTreePropertySource.Value implements CharSequence. There is a risk a configtree file might be binary and we accidentally change the contents.

@wilkinsona wilkinsona added type: enhancement A general enhancement and removed for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged labels Jun 21, 2023
@wilkinsona wilkinsona added this to the 3.x milestone Jun 21, 2023
@wilkinsona
Copy link
Member

We're going to try the CharSequence check.

@lazystone
Copy link

I tried to use env variables in configtree in SB 3.2.5 and it does not seem to work unfortunately.

Is there any progress at the moment?

@philwebb
Copy link
Member

Not as yet I'm afraid @lazystone

@lazystone
Copy link

Ok, i understand the struggle here(we don't want to accidentally replace something in binary files), but I also realized that my problem was wrong from the beginning, so I'll describe it here for others.

Initially we used configmaps in k8s environments to provide configuration overrides for SB. And standard way to pass secrets to it was to read them from k8s secrets, put them in environment variables and SB would resolve them later.

Then, with introduction of configtree in SB we started to use it. And we did exactly the same thing as before - tried to use placeholders for secrets. One thing that we were missing I think - the whole purpose of configtree approach.

Using configtree we could mount secrets as we mount configmaps in k8s pod and use them directly as configuration overrides.

So, whoever faces this problem - have another look on your use case. Maybe you don't need this feature that much.

@quaff
Copy link
Contributor

quaff commented May 22, 2024

We're going to try the CharSequence check.

See #40862

quaff added a commit to quaff/spring-boot that referenced this issue May 22, 2024
@mhalbritter
Copy link
Contributor

Superseded by #40862.

@mhalbritter mhalbritter closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2024
@mhalbritter mhalbritter removed this from the 3.x milestone May 22, 2024
@mhalbritter mhalbritter added status: superseded An issue that has been superseded by another and removed type: enhancement A general enhancement labels May 22, 2024
quaff added a commit to quaff/spring-framework that referenced this issue May 23, 2024
quaff added a commit to quaff/spring-boot that referenced this issue May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants