Skip to content

Commit

Permalink
Remove support for migrationVersion leapfrogging
Browse files Browse the repository at this point in the history
  • Loading branch information
Joe Portner committed Dec 2, 2021
1 parent 35fff52 commit 060dff5
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 67 deletions.
2 changes: 2 additions & 0 deletions dev_docs/tutorials/saved_objects.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ Having said that, if a document is encountered that is not in the expected shape
fail an upgrade than to silently ignore a corrupt document which can cause unexpected behaviour at some future point in time. When such a scenario is encountered,
the error should be verbose and informative so that the corrupt document can be corrected, if possible.

**WARNING:** Do not attempt to change the `migrationVersion`, `id`, or `type` fields within a migration function, this is not supported.

### Testing Migrations

Bugs in a migration function cause downtime for our users and therefore have a very high impact. Follow the <DocLink id="kibDevTutorialTestingPlugins" section="saved-objects-migrations" text="Saved Object migrations section in the plugin testing guide"/>.
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@ upgrade. In most scenarios, it is better to fail an upgrade than to silently
ignore a corrupt document which can cause unexpected behaviour at some future
point in time.

WARNING: Do not attempt to change the `migrationVersion`, `id`, or `type` fields
within a migration function, this is not supported.

It is critical that you have extensive tests to ensure that migrations behave
as expected with all possible input documents. Given how simple it is to test
all the branch conditions in a migration function and the high impact of a bug
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,58 +664,6 @@ describe('DocumentMigrator', () => {
);
});

it('allows updating a migrationVersion prop to a later version', () => {
const migrator = new DocumentMigrator({
...testOpts(),
typeRegistry: createRegistry({
name: 'cat',
namespaceType: 'multiple',
migrations: {
'1.0.0': setAttr('migrationVersion.cat', '2.9.1'),
'2.0.0': () => {
throw new Error('POW!');
},
'2.9.1': () => {
throw new Error('BANG!');
},
'3.0.0': setAttr('attributes.name', 'Shiny'),
},
convertToMultiNamespaceTypeVersion: '2.9.0', // intentionally less than 2.9.1
}),
});
migrator.prepareMigrations();
const actual = migrator.migrate({
id: 'smelly',
type: 'cat',
attributes: { name: 'Boo' },
migrationVersion: { cat: '0.5.6' },
coreMigrationVersion: undefined, // this is intentional
});
// Transforms include, in order:
// 1. migration [1.0.0]
// 2. migration [2.0.0]
// 3. reference [2.9.0]
// 4. conversion [2.9.0]
// 5. migration [2.9.1]
// 6. migration [3.0.0]
// The outer loop (`applyMigrations`) enumerates all properties on the object to see if the object is outdated, and which transforms
// need to be applied for each property.
// Initially, all transforms are applicable. An inner loop starts to apply each of them. Transform (1) gets applied, which increases
// the document's migrationVersion. We detect this before applying transform (2) and break out of the inner loop.
// The outer loop continues, and it sees that the object is still outdated and that transforms (3) and (6) need to be applied. Another
// inner loop starts to apply each of these. Transform (3) is correctly applied (there are no references to update, but the
// object's coreMigrationVersion is set to 2.9.0). Then transform (6) is applied, and the inner loop ends. The outer loop detects that
// the object is no longer outdated, and the outer loop ends, which sets the object's coreMigrationVersion to the current Kibana
// version (3.0.0).
expect(actual).toEqual({
id: 'smelly',
type: 'cat',
attributes: { name: 'Shiny' },
migrationVersion: { cat: '3.0.0' },
coreMigrationVersion: kibanaVersion,
});
});

it('allows adding props to migrationVersion', () => {
const migrator = new DocumentMigrator({
...testOpts(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,7 @@
* handle property addition / deletion / renaming.
*
* A caveat is that this means we must restrict what a migration can do to the doc's
* migrationVersion itself. We allow only these kinds of changes:
*
* - Add a new property to migrationVersion
* - Move a migrationVersion property forward to a later version
*
* Migrations *cannot* move a migrationVersion property backwards (e.g. from 2.0.0 to 1.0.0), and they
* cannot clear a migrationVersion property, as allowing either of these could produce infinite loops.
* However, we do wish to allow migrations to modify migrationVersion if they wish, so that
* they could transform a type from "foo 1.0.0" to "bar 3.0.0".
* migrationVersion itself. Migrations should *not* make any changes to the migrationVersion property.
*
* One last gotcha is that any docs which have no migrationVersion are assumed to be up-to-date.
* This is because Kibana UI and other clients really can't be expected build the migrationVersion
Expand Down Expand Up @@ -753,12 +745,6 @@ function migrateProp(
let additionalDocs: SavedObjectUnsanitizedDoc[] = [];

for (const { version, transform, transformType } of applicableTransforms(migrations, doc, prop)) {
const currentVersion = propVersion(doc, prop);
if (currentVersion && Semver.gt(currentVersion, version) && transformType !== 'reference') {
// the previous transform function increased the object's migrationVersion; break out of the loop
break;
}

if (convertNamespaceTypes || (transformType !== 'convert' && transformType !== 'reference')) {
// migrate transforms are always applied, but conversion transforms and reference transforms are only applied during index migrations
const result = transform(doc);
Expand Down

0 comments on commit 060dff5

Please sign in to comment.