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

Support Additional spring.config.import Statements In EnvironmentRepositories #2097

Open
piercer opened this issue May 26, 2022 · 9 comments

Comments

@piercer
Copy link

piercer commented May 26, 2022

Describe the bug
We are using the spring cloud config server to load configuration into our spring boot app using git and hashicorp vault.
The configuration for the server is like this

spring:
  profiles:
    active: git,vault
  cloud:
    config:
      server:
        native:
        git:
          uri: https://our-git-uri
          searchPaths:
            - '{application}'
        vault:
          host: our-vault-host
          scheme: https
          port: 443
          authentication: our-authentication
          kv-version: 2

If we ask for the config for the application 'app' with the active profile 'prof' it will load the 'app-prof.yml' configuration from git as expected. If the 'app-prof.yml' configuration being loaded contains vault imports like the following

spring:
  config:
    import: >-
       vault://arbitrary/secret/path1
       vault://arbitrary/secret/path2

Then the Spring cloud config server loads the configuration successfully and then proceeds to import the properties from the vault paths specified in the configuration, which is exactly what we want.

** The single bug is this - I would expect the properties loaded from the vault paths to be included in the configuration returned to the application that requested the configuration, but they are not. **

I investigated and below are explained some of the reasons I believe the properties are not returned. I do not know what the desired functionality is of each part and so do not know if what I list below are bugs, however they do result in the bug presented above manifesting.

There seem to be two barriers to these properties being returned in the response to the configuration request, both of which contribute to the single bug.

The PassthruEnvironmentRepository only 'passes through' configuration in MapPropertySources but the vault properties are stored in EnumerablePropertySources. it is possible to change this behaviour with the following code change

in PassthruEnvironmentRepository::findOne change MapPropertySource to EnumerablePropertySource

			if (!this.standardSources.contains(name) && source instanceof EnumerablePropertySource<?>) {
				result.add(
						new PropertySource(name, getMap((EnumerablePropertySource<?>) source, includeOrigin), source));
			}

then PasstheuEnvironmentRepository::getMap can be simplified and still work the same

	private Map<?, ?> getMap(EnumerablePropertySource<?> source, boolean includeOrigin) {
		Map<Object, Object> map = new LinkedHashMap<>();
		if (includeOrigin && source instanceof OriginLookup) {
			Map<?, ?> input = (Map<?, ?>) source.getSource();
			OriginLookup<String> originLookup = (OriginLookup<String>) source;
			for (Object key : input.keySet()) {
				Origin origin = originLookup.getOrigin(key.toString());
				if (origin == null) {
					map.put(key, source.getProperty(key.toString()));
					continue;
				}
				String originDesc;
				if (origin instanceof TextResourceOrigin) {
					TextResourceOrigin tro = (TextResourceOrigin) origin;
					originDesc = tro.getLocation().toString();
				}
				else {
					originDesc = origin.toString();
				}
				Object value = source.getProperty(key.toString());
				map.put(key, new PropertyValueDescriptor(value, originDesc));
			}
		}
		else {
			for (String key : source.getPropertyNames()) {
				// Spring Boot wraps the property values in an "origin" detector, so we
				// need
				// to extract the string values
				map.put(key, source.getProperty(key));
			}
		}
		return map;
	}

As MapPropertySource extends EnumerablePropertySource this is backwards compatible and does not change the normal functionality, but now the properties returned from the arbitrary vault path import are available in the property source map.

The second barrier I have found is that the arbitrary vault paths being returned are removed by the NativeEnvironmentRepository. Even though we do not have the native profile enabled the NativeEnvironmentRepository is used to filter out what is returned. the NativeEnvironmentRepository::matchesLocation method does not return true for the arbitrary vault paths and I cannot fathom a clean way to make this actually happen. This is where I got stuck.

I am very happy with the way that the cloud config server chooses which configuration files to load but it we have a requirement that each active profile may load up different properties from arbitrary vault paths that have nothing to do with the application or profile of the configuration file that they are specifies in. We do not even control those paths on the app side.

It is very frustrating that it so nearly works exactly as we want, but that the properties get filtered out at the end because they are not linked to the active profile in the same way.

We would like to use the cloud-config-server as a one-stop-shop for loading configuration but this is stopping us.

Can this be fixed?

@piercer
Copy link
Author

piercer commented May 26, 2022

To contributors who think this is more than one bug, can you please help me understand how I might split it up? I can see that each problem might be a sub-task of the overall bug, but on their own they are an incomplete description of the problem.

@ryanjbaxter
Copy link
Contributor

but it we have a requirement that each active profile may load up different properties from arbitrary vault paths that have nothing to do with the application or profile of the configuration file that they are specifies in

This is generally what the general application name is used for in the EnvironmentRepository it contains properties that should be applies to all apps requesting configuration data. Could that be a solution for your use case?

@spring-cloud-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@piercer
Copy link
Author

piercer commented Dec 5, 2022

I did provide as much information as is possible - please read my original post. It is not possible to provide a project as it requires the project, a git repository and a vault instance to show the problem. If there is something lacking in my description please let me know.

@piercer
Copy link
Author

piercer commented Dec 5, 2022

but it we have a requirement that each active profile may load up different properties from arbitrary vault paths that have nothing to do with the application or profile of the configuration file that they are specifies in

This is generally what the general application name is used for in the EnvironmentRepository it contains properties that should be applies to all apps requesting configuration data. Could that be a solution for your use case?

I am sorry - I don not understand how this solution would work - can you please explain in more detail. I understand that the yml files are linked to the app and profiles - but I do not understand why the vault paths inside the yml have to conform to that pattern as well?

@ryanjbaxter
Copy link
Contributor

@piercer
Copy link
Author

piercer commented Dec 5, 2022

Thank you for your link - I have already read this. It shows exactly my problem - vault secret paths also need to use the 'app'/'profile' pattern - which I think is just wrong. My configuration file names do conform to this standard (which I like), but I have vault path in those configuration files that do not confirm to that standard (which I have to do as I do not control the vault paths).

If you read my post in details you will see that it very nearly works exactly as I would expect it to. The secrets ARE loaded from these custom paths with a tweak to the PassthruEnvironmentRepository. But then they get filtered out at the last moment by the NativeEnvironmentRepository, which just seems wrong. Surely there is a way to stop the filtering of those secrets happening.

Can anybody explain the reasoning behind not allowing custom vault paths from inside config files that follow the standard?

@ryanjbaxter ryanjbaxter changed the title Vault importing does not work as expected Support Additional spring.config.import Statements In EnvironmentRepositories Dec 8, 2022
@ryanjbaxter
Copy link
Contributor

I think I understand this better now.

Honestly it was something we never anticipated supporting to begin with. I am going to open this up for votes to see if other users also have similar use cases.

@piercer
Copy link
Author

piercer commented Dec 9, 2022

Thank you Ryan, I appreciate you taking the time to understand the requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants