[uiSettings] always use the latest config document to create the new one #159649
Conversation
pgayvallet
added
Team:Core
| const matchingResults = savedConfigs.filter((savedConfig) => | ||
| isConfigVersionUpgradeable(savedConfig.id, version) | ||
| ); | ||
| if (findResult) { | ||
| return { id: findResult.id, attributes: findResult.attributes }; | ||
| const mostRecentConfig = getMostRecentConfig(matchingResults); | ||
| if (mostRecentConfig) { | ||
| return { id: mostRecentConfig.id, attributes: mostRecentConfig.attributes }; | ||
| } |
There was a problem hiding this comment.
The root change. The code was making the assumption that sorting by buildNum was sufficient, but this field is indexed as keyword, not number, which cause the order to be wrong in some situations.
E.g
["9517", "16602", "63240"].sort()
>> (3) ['16602', '63240', '9517']So we're now manually comparing the version of each matching document to make sure that the most recent is used.
| export function stripVersionQualifier(version: string): string { | ||
| return version.split('-')[0]; | ||
| } |
There was a problem hiding this comment.
Extracted from the SO service to be reused elsewhere
| type: (isNamespaceScope ? 'config' : 'config-global') as 'config' | 'config-global', | ||
| id: version, | ||
| id: stripVersionQualifier(version), |
There was a problem hiding this comment.
For consistency with how we're treating qualified versions (e.g X.Y.Z-SNAPSHOT or X.Y.Z-RC1) in other parts of core, especially in SO code, we're now stripping qualifiers when providing the version to the uiSettings client.
Note that this isn't directly related to the fix, but it was trivial enough to include it there.
| const config = await kibanaServer.savedObjects.get({ | ||
| id: await kibanaServer.version.get(), | ||
| id: stripVersionQualifier(await kibanaServer.version.get()), | ||
| type: 'config', |
There was a problem hiding this comment.
The only FTR test that was doing direct config access by ID using the SO client. Had to adapt for the version change described in previous comment.
|
Pinging @elastic/kibana-core (Team:Core) |
| const savedObjectsClient = savedObjectsClientMock.create(); | ||
| savedObjectsClient.find.mockResolvedValue({ | ||
| saved_objects: [ | ||
| { id: '7.2.0', attributes: 'foo' }, |
There was a problem hiding this comment.
Could there ever be a scenario where we have an invalid version value in the id field? If so, would be nice to have test coverage for it. If not, ignore this!
There was a problem hiding this comment.
In theory, there shouldn't, but life alway finds a way, so I guess adding a test wouldn't hurt
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Summary
Fix #159646
Fix the config creation-from-previous-one logic by always using the latest config for the new version's creation
Release Note
Fix a bug that could cause old Kibana deployments to loose their uiSettings after an upgrade