Support for cross-type Saved Object migration

[gmmorris]

As part of the Alerting Terminology issue we wish to migrate some of our Saved Objects from one type to another.

I spoke to @rudolf and my understanding is that this is supposed to work, but he isn't sure how reliable/sustainable this is as it has never actually been used.
I'm submitting this issue for us to verify whether the existing implementation can support our requirements.

Use Case

At the moment we have three Saved Object types: alert (in the alerts plugin), action and action_task_params (in the actions plugin).

We wish to rename them as part of the terminology update, as it will become very confusing having an alert SO type that is used for rule and action which is used for connector. There's also a scenario in which we'll need to introduce a SO for alert (what we current refer to as an alert instance), and that will make things worse as we'd have an alert entity that isn't backed by an alert SO (as our current alert SO would be backing the rule entity).

For this reason, our requirements are two fold:

1. (Must Have) Support migration from type A to type B. We would need this to migrate alert to rule as is.
2. (Nice to Have) Support for reintroduction of type A at a later point. We would need this to introduce a SO that represents an alert, though we aren't sure we'll actually need this, and worse that happens we can introduce it as a new name such as rule-alert or something like that.

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[rudolf]

One of the reason this won't work today is because changing a document's type requires changing it's raw _id and this isn't possible in v2 migrations. This is the same problem for supporting multi-namespace migrations so it will be addressed as part of #86247

Update: confirmed that #91144 works with v1 migrations

      {
        "_index" : ".kibana_2",
        "_id" : "rule:74f3e6d7-b7bb-477d-ac28-92ee22728e6e",
        "_score" : 1.0,
        "_source" : {
          "rule" : {
            "actions" : [ ],
            "alertTypeId" : "example.always-firing",
            "apiKey" : "SV9UcltySECMh5m5raExldEJhG+KZpV1iCQOVdBqves6TuEYmjit7+JeCaML/yoM+2/6UcEUY2vL9yCs7jDPBQ5VOwDp1eelfsscZNy6tuwkWrLrlRpDaiunMXW6T5f3Z4KOQx+5JOeAGoCXfzYPqKwRafmyNWY=",
            "apiKeyOwner" : "elastic",
            "consumer" : "alerts",
            "createdAt" : "2020-06-17T15:35:38.497Z",
            "createdBy" : "elastic",
            "enabled" : true,
            "muteAll" : false,
            "mutedInstanceIds" : [ ],
            "name" : "always-firing-alert",
            "params" : { },
            "schedule" : {
              "interval" : "1m"
            },
            "scheduledTaskId" : "329798f0-b0b0-11ea-9510-fdf248d5f2a4",
            "tags" : [ ],
            "throttle" : null,
            "updatedBy" : "elastic",
            "meta" : {
              "versionApiKeyLastmodified" : "pre-7.10.0"
            },
            "executionStatus" : {
              "status" : "pending",
              "lastExecutionDate" : "2021-02-24T11:09:47.771Z",
              "error" : null
            },
            "updatedAt" : "2020-06-17T15:35:39.839Z",
            "notifyWhen" : "onActiveAlert"
          },
          "type" : "rule",
          "references" : [ ],
          "migrationVersion" : {
            "alert" : "7.12.0"
          },
          "coreMigrationVersion" : "8.0.0",
          "updated_at" : "2020-06-17T15:35:39.839Z"
        }
      },

So once #86247 is done the only remaining work would be to add an integration test for renaming types.

[mikecote]

I see #86247 is now closed. Does that mean we can try again to change the SO type via migrations v2?

[pgayvallet]

I see #86247 is now closed. Does that mean we can try again to change the SO type via migrations v2?

Sorry, I missed the ping. No, #86247 only handles the id renaming for the multi-space migration (even if it is a prerequisite for current issue)

[pgayvallet]

Even now that we technically can change an object's id, there are a couple things very challenging if we want to allow type renaming.

Outdated documents query and document migration

First, if a type is renamed, we'll need to keep track of its previous type(s) to properly handle migration.

E.g if a type old was renamed new, we need to know at which version this occurred, to have the document migrator be aware of it.

const myType: SavedObjectType = {
  name: 'new',
  typeChanges: [
    { version: '7.15.0', previousName: 'old' }
  ],
  migrations: {
    '7.13.0': migrateTo7_1_3,
    '7.17.0': migrateTo7_1_7
  }
}

Given that, the document migrator would be able to split the migrations between the old and new type (and to be aware of previously used type names to accept to migrate them)

This could be done by either spliting the migrations between the old and new types during prepareMigration or, instead by having nextUnmigratedProp be 'smart' about the type change. The document migrator's code is quite complex, I'm not sure which approach would be the best atm though.

We would also need to update the outdatedDocumentsQuery to also include all the previously used type names.

kibana/src/core/server/saved_objects/migrationsv2/model.ts

Lines 975 to 984 in 26272fa

	const outdatedDocumentsQuery = {
	bool: {
	should: Object.entries(migrationVersionPerType).map(([type, latestVersion]) => ({
	bool: {
	must: { term: { type } },
	must_not: { term: { [`migrationVersion.${type}`]: latestVersion } },
	},
	})),
	},
	};

Note that we probably want to have #97965 implemented too.

Also note that given the complexity of these changes, I'm not very optimistic about being able to support

(Nice to Have) Support for reintroduction of type A at a later point

As having the same name for a renamed type and a newly introduced one seems like a nightmare for both points (outdated query and document migrator implementation).

It would probably doable, by having 'new' types specifying the version they were introduced at, and then generating a 'time chart' of the usage of the type name, but it would probably be even more painful than it sounds.

Just for the record, the definition could look like:

const myType: SavedObjectType = {
  name: 'new',
  typeChanges: [
    { version: '7.15.0', oldName: 'old' }
  ],
}
const newOld: SavedObjectType = {
  name: 'old',
  since: '7.17.0'
}

but the complexity isn't probably worth the cost unless this is a hard requirement.

References rewrite

Another thing we'll need to address is how to rewrite the references to the renamed type, as if we have our old type renamed to new during x.x.x, we'll also need to update all documents having references to object of this type during the same migration.

For the sharing saved objects features, we introduced the concept of reference transform. Hopefully, we would be able to leverage it without much changes and generating additional references transform for each of the type renames. @jportner does that sound realistic?

SO Import

We are migrating objects during the import/creation, so I don't think we'll need to do anything here.

cc @jportner @mshustov @joshdover do you see anything I may have missed?

[jportner]

We would also need to update the outdatedDocumentsQuery to also include all the previously used type names.

I don't think we would need to do that. The query checks if any documents are outdated, then feeds any outdated documents to the DocumentMigrator to have all appropriate transforms applied.

For the sharing saved objects features, we introduced the concept of reference transform. Hopefully, we would be able to leverage it without much changes and generating additional references transform for each of the type renames. @jportner does that sound realistic?

The way this is designed today, in a given version, reference transforms are applied first, then convert transforms are applied, then finally the consumer-defined migrate transforms are applied. I think we can keep this order and augment the reference transform generator, yes. As long as consumers have to define a specific field at registration time like typeChanges as suggested above.

do you see anything I may have missed?

Nothing comes to mind. I agree it should be doable as proposed. I also agree that allowing "Support for reintroduction of type A at a later point" sounds like more trouble than it's worth.

[pgayvallet]

I don't think we would need to do that. The query checks if any documents are outdated

Correct me if I'm wrong, but I think the query checks if any known document type are outdated, atm.

 const outdatedDocumentsQuery = {
    bool: {
      should: Object.entries(migrationVersionPerType).map(([type, latestVersion]) => ({
        bool: {
          must: { term: { type } },
          must_not: { term: { [`migrationVersion.${type}`]: latestVersion } },
        },
      })),
    },
  };

I think we can keep this order and augment the reference transform generator, yes. As long as consumers have to define a specific field at registration time like typeChanges as suggested above.

👍

[jportner]

Correct me if I'm wrong, but I think the query checks if any known document type are outdated, atm.

Ah you're absolutely right, I understand the intent now. Agreed, this would need to be changed.

[pgayvallet]

@gmmorris is this something that is mandatory to be done before 8.0?

[gmmorris]

@gmmorris is this something that is mandatory to be done before 8.0?

Mandatory is a very strong word :)

@mikecote, correct me if I'm wrong, but our only issue blocked on this is #100068, am I right?
If that's the case, then I think we might be able to do this post 8.0, if it doesn't force a breaking change.

Any thoughts?

[mikecote]

@mikecote, correct me if I'm wrong, but our only issue blocked on this is #100068, am I right?
If that's the case, then I think we might be able to do this post 8.0, if it doesn't force a breaking change.

Correct, after speaking with @kobelb, it felt ok to do in a minor so I would no longer say it's needed for 8.0. But hopefully soon so we can address our technical debt with terminology. (especially if we're planning to create another SO as data called "alert" which is currently in use).

[pgayvallet]

Ok, thanks. Asking because of the interactions with #101351.

Ideally, we would still be able to address current issue before 8.0

[rudolf]

As part of #144035 we're no longer reindexing documents but instead re-using the same index #147237. Because the type of a saved object is encoded in it's _id (bad design decision made long ago) we can't change the type of an existing document in an existing index. We would have to create a new document with the new type in it's _id and then delete the old document.

So this adds additional complexity towards achieving this.