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 API for PropertySource resource location retrieval #24504

Closed
spencergibb opened this issue Dec 14, 2020 · 4 comments
Closed

Add API for PropertySource resource location retrieval #24504

spencergibb opened this issue Dec 14, 2020 · 4 comments

Comments

@spencergibb
Copy link
Member

@spencergibb spencergibb commented Dec 14, 2020

Spring Cloud Config Server verifies that any property sources it loads are from configured locations for security. To do that has used the property source name to get this information.

In boot 2.3.x we got classpath:/configs/application.yaml from applicationConfig: [classpath:/configs/application.yaml].

In boot 2.4.0 we updated the parsing logic in spring-cloud/spring-cloud-config@854060b

#24428 changed the property source names and hence broke config server. It has been updated again as seen in the issue below, but this is a brittle approach. We likely didn't say anything in the 2.3.x or prior timeline because it didn't change.

I've added support for checking the location by parsing classpath:/configs/ from Config resource 'class path resource [configs/application.yml]' via location 'classpath:/configs/' (document #0) which works for both 2.4.0 and 2.4.1.

spring-cloud/spring-cloud-config#1771

/cc @philwebb

@philwebb
Copy link
Member

@philwebb philwebb commented Dec 15, 2020

This one is a bit tricky. If I've read the code correctly, you ideally want a way to get the ConfigDataLocation from a PropertySource? We unfortunately have an additional PropertySourceLoader abstraction to deal with.

The only ideas I've had so far are to either try to create some kind of registry that links a PropertySource to the ConfigDataLocation that created it, or, to somehow use OrginLookup. Another idea is to perhaps change ConfigDataEnvironmentPostProcessor.applyTo so that it accepts some kind of callback that could be used to track things on your side.

I definitely needs more thought.

@philwebb philwebb added this to the 2.4.2 milestone Dec 16, 2020
@philwebb philwebb changed the title API for PropertySource resource location Add API for PropertySource resource location retrieval Dec 16, 2020
@philwebb philwebb closed this in 38e4c2a Dec 16, 2020
@philwebb
Copy link
Member

@philwebb philwebb commented Dec 16, 2020

@spencergibb I've pushed something to 2.4.x, can you take a look and see if you think it will work.

I hope you can update NativeEnvironmentRepository.findOne to setup tracking. Something like:

TrackingConfigDataEnvironmentUpdateListener listener = new TrackingConfigDataEnvironmentUpdateListener()
ConfigurableEnvironment environment = getEnvironment(config, profile, label, listener);
...
ConfigDataEnvironmentPostProcessor.applyTo(environment, resourceLoader, null,
    StringUtils.commaDelimitedListToStringArray(profile), listener);

The TrackingConfigDataEnvironmentUpdateListener can implement onPropertySourceAdded a stash the propertySource and location in a Map.

You can then hopefully update the clean(...) method to use that Map to get back the location for any given propertySource.

Let me know if that doesn't work and we can have another go.

@spencergibb
Copy link
Member Author

@spencergibb spencergibb commented Dec 16, 2020

I'll take a look in an hour or so.

@philwebb philwebb reopened this Dec 16, 2020
@philwebb
Copy link
Member

@philwebb philwebb commented Dec 16, 2020

Reopening to make StandardConfigDataResource.getResource() public

philwebb added a commit that referenced this issue Dec 17, 2020
Update `StandardConfigDataResource`  to make the `getResource()` method
public so that it can be used by Spring Cloud.

Closes gh-24504
@philwebb philwebb closed this in aeaefbe Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants