Repository navigation
Let the last profile win in the AWS S3 environment repository - #3348
sharanggupta wants to merge 2 commits into
Conversation
The other environment repositories give the last requested profile the highest precedence. AwsS3EnvironmentRepository reversed applications and labels but not profiles, so with profiles `profile1,profile2` the `foo-profile1` file won over `foo-profile2`. The documents of a file that is not profile specific were also added per profile, each followed by the content that applies to every profile. The document of the second profile therefore ranked below that content and could never override it. Documents with negated profile expressions were added after that content as well. * Add the profile specific files and documents with the last profile first. * Add the profile and negated profile documents before the content that applies to every profile. The version of the environment is still the one of the last added property source, so it can change with the new order. Signed-off-by: Sharang Gupta <sharang@sharanggupta.dev>
| // documents, including documents with negated profile expressions (e.g. | ||
| // on-profile: "!my-profile"). These take precedence over the content that | ||
| // applies to every profile, so add them first. | ||
| for (String profile : profilesByPrecedence) { |
There was a problem hiding this comment.
This is correct for profile specific files, but when there are multiple documents within the same file Boot will parse them in document order https://docs.spring.io/spring-boot/reference/features/external-config.html#features.external-config.files.multi-document.
I think this would also now allow profile documents within application.yml override properties from application specific documents. For example with foo.yml containing greeting: foo and shared application.yml containing an activated profile1 document with greeting: shared, requesting foo/profile1 now returns shared.
There was a problem hiding this comment.
Thanks, you are right, and your example failed against this version: foo/profile1 returned shared. In 6a3e1f7 I limited the change to profile-specific files and restored the previous order for the documents of files that are not profile specific. I added your example as a test.
Following the document order of a multi-document file, as Spring Boot does, would change how this repository reads YAML documents, so I left it out. I can open a separate issue or PR for it if you want that.
There was a problem hiding this comment.
I would rather this PR address all variations including multi-document files
Following the review, limit the change to profile-specific files. Spring Boot reads the documents of a multi-document file in document order, and adding the profile documents of every file first let a profile document of application.yml override an application-specific file, so restore the previous order for the documents of files that are not profile specific. Replace the tests of that order with one for the example of the review: foo.yml takes precedence over a profile1 document of application.yml. Signed-off-by: Sharang Gupta <sharang@sharanggupta.dev>
This branch has not been deployed
3 participants
The documentation says that, with multiple profiles, the last one wins. Native, Git and SVN repositories follow Spring Boot, and the JDBC, MongoDB, Redis, Vault, CredHub, Parameter Store and Secrets Manager repositories reverse the requested profiles.
AwsS3EnvironmentRepositoryreverses applications and labels but not profiles, so for an applicationfoowith the profilesprofile1,profile2,foo-profile1.propertiescame out abovefoo-profile2.properties.Changes
addPropertySources, soEnvironment.getProfiles()keeps the requested order.application.ymloverride an application-specific file and did not follow the document order that Spring Boot uses for a multi-document file.Visible change
environment.setVersion()runs for every added source, so the version is the S3 version of the last added source. For a request with several profile-specific files, it is now the first profile's file instead of the last. Two existing tests pinned the old value and now expect the new one. I kept the rule as it is rather than invent a new one.Testing
Test first:
lastProfileTakesPrecedenceForProfileSpecificFilesfailed withprofile1aboveprofile2and passes now.applicationSpecificFileTakesPrecedenceOverProfileDocumentOfSharedFilecovers the example from the review:foo.ymlstays above aprofile1document ofapplication.yml. It failed against the first version of this PR and passes now.All 69
AwsS3EnvironmentRepositoryTestsand 7AwsS3IntegrationTestspass against LocalStack. Checkstyle and the Spring Java Format check pass.