Property sources returned in wrong order and in a Collection (precedence order is wrong)

[marnee01]

Defect Summary: The precedence of property sources returned by spring config server puts application.properties as higher precedence than app-specific properties file. Therefore, when one is using application.properties to share common configs between apps, one can never override something in application.properties for a specific application.

Details:

So, we have groups of applications that will share properties using an application.properties.

We have a single git repository that will have the following file hierarchy:

.repo root
     + services
              application-default.properties
              application-dev.properties
              application-prd.properties
              + service1
                    service1-default.properties
                    service1-dev.properties
                    service1-prd.properties
              + service2
                    service2-default.properties
                    service2-dev.properties
                    service2-prd.properties

Our spring config service will have a bootstrap.yml file as follows:

spring:
  application:
    name: configservice
  cloud:
    config:
      server:
        git:
          uri: ssh://git@bitbucket.pearson.com/sharedRepo/applicationconfigurations.git
          # The following is not relevant for this particular issue, but it is what we do have in our file.
          searchPaths: '{application}'
          repos:
            Services:
              pattern: service1, service2
              searchPaths: 'service/, service/{application}'
              uri: ssh://git@bitbucket.pearson.com/sharedRepo/applicationconfigurations.git

For the properties under the "service" folder, the client side is not a spring app (and not a web service). We will specify the spring config properties via VM options, not that it really matters. But we are specifying it like:

-Dspring.profiles.active=dev,default
-Dspring.cloud.config.name=service1

We have to implement client-side code to read these property sources and produce a properties map.

Sample code:

final CompositePropertySource compositePropertySource 
    = (CompositePropertySource)environment.getPropertySources().get("configService");

if (compositePropertySource != null) {
    if (compositePropertySource.getPropertySources() != null) {

        for (final Object mapPropertySourceObject : compositePropertySource.getPropertySources()) {
            final MapPropertySource propertySource = (MapPropertySource)mapPropertySourceObject;

            if (propertySource != null) {
                p.putAll(propertySource.getSource());
            }
        }
    }
}

There are two related problems.

1) The first is that the CompositePropertySource.getPropertySources() returns a Collection, even though it stores the property sources internally in a LinkedHashSet. That means that to the caller of that method, there is no order to the property sources (even though in the current implementation of CompositePropertySource it does maintain an order internally. And there is no other method that allows for explicitly testing the precedence of a given property source.

2) The big issue is that the order they are returned can not be correct. For service1, the property sources are in the following order. ... service1-default.properties application-default.properties service1-dev.properties application-dev.properties ...

The last one wins, which means that application-default.properties overrides service1-default.properties.

That makes no sense and can not be what was intended.

If we have:

application-default.properties: securityCheckLoggingInterval=120

service1-default.properties: securityCheckLoggingInterval=60

service3-default.properties: securityCheckLoggingInterval=20

Then I expect the value will be: service1: 60 service2: 120 (default) service3: 20 service4: 120 (default) service5: 120 (default) service6: 120 (default)

But instead, application.properties takes precedence, so the value will be 120 for all the services. Therefore, there is actually no way to override a shared value. Instead, if the shared value must differ for one app, you simply can't put it in application.properties. You have to copy the property into each and every service*-default.properties file, with most of the values being the same.

Note that we have dozens of these old services. And we have 9 different environments (so 9 different profiles). It is imperative that we be able to share properties, but also be able to override when needed.

Is this just a configuration issue on my part, or is it a defect in spring config server? (It can't be the expected behavior.)

[spencergibb]

CompositePropertySource is from spring framework core, so not created in the config server project.

If you go to http://<configserver>/service1/dev,default I'm sure you'd see the correct order.

We have to implement client-side code to read these property sources and produce a properties map.

Assuming config server returns the correct order, the issue must be in your client code.

[marnee01]

CompositePropertySource is from spring framework core, so not created in the config server project.

If you go to http://<configserver>/service1/dev,default I'm sure you'd see the correct order.

We have to implement client-side code to read these property sources and produce a properties map.

Assuming config server returns the correct order, the issue must be in your client code.

It doesn't return in the correct order. It returns it as follows:

service1-default.properties
application-default.properties
service1-dev.properties
application-dev.properties

[spencergibb]

Please show the output of the call to config server.

[ryanjbaxter]

Do not use screenshots, please put the output in the issue directly.

[marnee01]

I was not sure what you wanted, other than the actual list of property sources that is returned, which I had already included. Here is the output, pretty-printed, with data redacted, and the names altered to match my example:

{
  "name": "service1",
  "profiles": [
    "st1,default"
  ],
  "label": "feature/Marnee/SpringConfigPOC1",
  "version": "81baca37ac4ea55d9135a90b35be19c18214bfd7",
  "state": null,
  "propertySources": [
    {
      "name": "ssh://<redacted>/hub/service1/service1-default.properties",
      "source": {
<redacted>
      }
    },
    {
      "name": "ssh://<redacted>/hub/application-default.properties",
      "source": {
<redacted>
      }
    },
    {
      "name": "ssh://<redacted>/hub/service1/service1-st1.properties",
      "source": {
<redacted>
      }
    },
    {
      "name": "ssh://<redacted>/hub/application-st1.properties",
      "source": {
<redacted>
      }
    }
  ]
}

[marnee01]

I found a resolution. I changed:

searchPaths: 'service/, service/{application}

to:

searchPaths: 'service/{application}, service/

Now the service returns the following, which is what I would expect.

{
  "name": "service1",
  "profiles": [
    "st1,default"
  ],
  "label": "release/2",
  "version": "623a82dac38017245b60dc831173d44359e43aae",
  "state": null,
  "propertySources": [
    {
      "name": "ssh://<redacted>.git/hub/application-default.properties",
      "source": {<redacted>}
    },
    {
      "name": "ssh://<redacted>.git/hub/service1/service1-default.properties",
      "source": {<redacted>}
    },
    {
      "name": "ssh://<redacted>.git/hub/application-st1.properties",
      "source":  {<redacted>}
    },
    {
      "name": "ssh://<redacted>.git/hub/service1/service1-st1.properties",
      "source":  {<redacted>}
    }
  ]
}

Well, I think it's what I expect, but the most basic question that should be answered first is, is my assumption correct, that a source later in the list should take precedence over a source earlier in the list? For example, in the above, service1-st1.properties would take precedence over application-st1.properties. Is that correct?

So, I think that I've resolved the main issue, but there are still issues. I believe this all needs to be documented better.

1. There is still the issue of CompositePropertySource.getPropertySources() returning a Collection, which API does not provide for predictable iteration order. In reality, we can iterate over it, and it does have a predictable iteration order because the underlying collection is ordered (LinkedHashSet), but the caller can't "know" that it is ordered, since it is just a Collection. If the fact that these are ordered is something the caller of the method should be able to expect, the method should return a type that is ordered. Currently, there is nothing stopping a future release to change the underlying LinkedHashSet to an unordered set, which would likely break all of our apps.

2. In my example, I named my files service1-default.properties and application-default.properties. That was to get around this issue: If I name my default files as service1.properties and application.properties, then the order returned is still not what one would expect. See the service response below. service1 now does have higher precedence over application, as expected. However, the basic properties have precedence over the profile-specific properties, which is not what I would expect. Is this the expected behavior? If so, it should be documented. Also, if so, what is the intended use of profiles if they don't override the basic properties?

{ "name": "service1", "profiles": [ "st1,default" ], "label": "release/2", "version": "9996282d7004d72e84af48e9efab4c8c52f0020d", "state": null, "propertySources": [ { "name": "ssh://.git/hub/application-st1.properties", "source": {

} }, { "name": "ssh://.git/hub/service1/service1-st1.properties", "source": { } }, { "name": "ssh://.git/hub/application.properties", "source": { } }, { "name": "ssh://.git/hub/service1/service1.properties", "source": { } } ] } ```

[ryanjbaxter]

1. There is still the issue of CompositePropertySource.getPropertySources() returning a Collection

As @spencergibb stated above this is part of Spring Framework not part of Spring Cloud Config.

2. This is based on the assumption that the propertySources are ordered for precedence and I am not sure that is really the case. The client could easily look at this set then look at the active profiles and determine which ones should take priority over each other. I dont think the server should be determining the order for the client

[spring-projects-issues]

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.

[marnee01]

This is based on the assumption that the propertySources are ordered for precedence and I am not sure that is really the case. The client could easily look at this set then look at the active profiles and determine which ones should take priority over each other. I dont think the server should be determining the order for the client

The MutablePropertySources does track and provide precedence of Spring property sources. It has a precedenceOf method. It seems inconsistent that CompositePropertySource would not provide precedence. Also, if CompositePropertySource does not track precedence order, why does it maintain the property sources in a LinkedHashSet and have method addFirstPropertySource(...)?

In addition, CompositePropertySource has method getProperty(String name), which returns the first property value it finds while iterating over the internal ordered propertySources. Seems like a dangerous method to expose if CompositePropertySource is not supposed to "know" about precedence. This method implies that property sources earlier in the list take precedence over property sources later in this, which is in fact what I've observed.

At the moment, this is how we've solved this:

1. Set profiles to "default,st1".
2. After retrieving the CompositePropertySource, treat the property sources earlier in the list as having higher precedence since the properties will be returned in this order:

service-st1.properties application-st1.properties service-default.properties application-default.properties

We'd like to get away from -default and just have:

service-st1.properties application-st1.properties service.properties application.properties

As for the client side implementing the precedence rules, could you point me to the Spring code that does so for Spring boot apps? (Spring boot apps get that for free, is that correct? So there must be Spring client code already that implements the precedence rules.)

[marnee01]

Correction to my previous comment: With the solution we have implemented currently, we don't need to use a "default" profile and have *-default.properties files. With application.properties and service.properties files, just setting profiles to "st1", the sources are returned in this order:

service-st1.properties application-st1.properties service.properties application.properties

Apologies...I had tried so many different combinations in the past. I thought I had tried the above, but must not have after finding this particular solution.

[spencergibb]

Closing as resolved

[marnee01]

Issue #2 was resolved, but Issue #1 was not:

1. The first is that the CompositePropertySource.getPropertySources() returns a Collection, even though it stores the property sources internally in a LinkedHashSet. That means that to the caller of that method, there is no order to the property sources (even though in the current implementation of CompositePropertySource it does maintain an order internally. And there is no other method that allows for explicitly testing the precedence of a given property source.

There was some discussion with additional details and I clarified my question. It has not yet been fully addressed. It does not seem reasonable to expect every client of Config Server to implement applying precedence on the client side, but perhaps non-native spring apps are rare enough that no client-side code is provided for that. For native spring apps, it seems the logic already is implemented.

If we do need to implement (duplicate) that logic for our non-native spring clients, then we will want to know what the established rules are and see the existing code. Otherwise, we may implement in a non-standard way that doesn't match the existing implementation for the native spring apps.

In fact, we are already in production with this. It's frightening to think that we may need to change the precedence logic and thus re-test across some subset of the approximately 600 versions of applications (including all the varieties of profiles we have) across all of our environments. We still have a few applications left to migrate (80 variations across all environments). The sooner we get a definitive answer, the better.

[spencergibb]

Sorry, reading over everything, I thought it was all resolved.

If I understand correctly you are concerned about the order for non-spring apps?

I can do nothing about composite property source since it lives in framework.

[marnee01]

Yes, that is correct. I'm concerned about the order on the client side for non-spring apps.

I can do nothing about composite property source since it lives in framework.

Who can answer questions and possibly address any changes or add documentation about that framework?

[spencergibb]

So from our perspective, the order received from config server is the correct order.

We can make no guarantees beyond that.

https://github.com/spring-projects/spring-framework

[marnee01]

I might be misunderstanding you...The previous comments in response to my post stated that I could not rely on the order that the property sources were returned from the config server.

In addition, as I documented, the framework does not publicize in the API that there even is an order (since the method returns Collection). If this isn't the right place to get someone to address this, do you know where I can post this question/issue? (Sorry if I'm misunderstanding.)

[spencergibb]

I don't know what to tell you. I can't address the return type of a class in Spring Framework. I can tell you we rely on the ordering in the class and if it breaks, we break.

[marnee01]

You're saying "we rely". That means in the client-side code for spring cloud config? Can you point me to that code? If I can see what the logic is there, then maybe it will help resolve some of the questions I have with our own client-side code.

[spencergibb]

On the server side.