Include the default application in the Redis backend - #3282
Conversation
ryanjbaxter
left a comment
There was a problem hiding this comment.
Can you submit this against the 5.0.x branch?
| private List<String> addKeys(String application, List<String> profiles) { | ||
| List<String> keys = new ArrayList<>(); | ||
| keys.add(application); | ||
| List<String> applications = new ArrayList<>( |
There was a problem hiding this comment.
I am pretty sure application can be null and can be comma delimated, both of which are not handled here
There was a problem hiding this comment.
Handled in 1411856, following CredhubEnvironmentRepository#normalize: application goes through commaDelimitedListToStringArray, trimmed, filtered on hasText and de-duplicated, with application prepended. So null resolves to the shared hash only, and app1,app2 reads every listed hash with later names taking precedence, the same order the Credhub backend produces. Two integration tests cover both cases (null -> [application-prod, application]; "app1, app2,app1" -> [app2-prod, app1-prod, application-prod, app2, app1, application]). Reverting the source change fails both.
f5d4c2a to
073e4fe
Compare
RedisEnvironmentRepository only read the hashes named after the requested application, so properties stored under the shared "application" key were never returned. The other backends include them: JdbcEnvironmentRepository, MongoDbEnvironmentRepository, CredhubEnvironmentRepository, NativeEnvironmentRepository and AwsS3EnvironmentRepository. Add the default application keys and order them the way jdbc and credhub do, so the requested application still takes precedence over the shared defaults. When the requested application is "application" the key list is unchanged. Fixes spring-cloudgh-3134 Signed-off-by: HDPark95 <qkrgusen456@gmail.com>
073e4fe to
7967ffc
Compare
|
@ryanjbaxter the PR is targeting |
Mirrors CredhubEnvironmentRepository#normalize: a null application resolves to the shared `application` hash only, and a comma-delimited list such as `app1,app2` reads every listed hash with later names taking precedence, the same order the other backends use. Signed-off-by: HDPark95 <qkrgusen456@gmail.com>
1411856 to
2106eed
Compare
RedisEnvironmentRepositoryonly reads the hashes named after the requested application (app,app-<profile>), so properties under the sharedapplicationkey are never returned. The other backends include them: jdbc, mongodb, credhub, native and aws-s3.This adds the default application keys, ordered the way jdbc and credhub do, so the requested application still wins over the shared defaults:
app-p2,application-p2,app-p1,application-p1,app,application. When the requested application isapplicationthe key list is unchanged.New test in
RedisEnvironmentRepositoryIntegrationTests: on 5.0.x it fails with["redis:myapp-prod", "redis:myapp"]and passes with the change.Fixes gh-3134