Skip to content

Commit

Permalink
[Task Manager] Force validation on all tasks using state (#164574)
Browse files Browse the repository at this point in the history
Resolves #159347
Part of #155764

In this PR, I'm forcing validation on any tasks using state by ensuring
a state schema is defined by the task type. Without this schema and once
this PR merges, Task Manager will now fail validation by throwing
`[TaskValidator] stateSchemaByVersion not defined for task type: XYZ`
errors. This forces an explicit schema to be defined so we can properly
handle state objects in ZDT environments.

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
mikecote and kibanamachine authored Sep 26, 2023
1 parent 9cccd0e commit d86eebd
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 16 deletions.
8 changes: 2 additions & 6 deletions x-pack/plugins/task_manager/server/task_validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ describe('TaskValidator', () => {
expect(result).toEqual(task);
});

// TODO: Remove skip once all task types have defined their state schema.
// https://github.com/elastic/kibana/issues/159347
it.skip(`should fail to validate the state schema when the task type doesn't have stateSchemaByVersion defined`, () => {
it(`should fail to validate the state schema when the task type doesn't have stateSchemaByVersion defined`, () => {
const definitions = new TaskTypeDictionary(mockLogger());
definitions.registerTaskDefinitions({
foo: fooTaskDefinition,
Expand Down Expand Up @@ -322,9 +320,7 @@ describe('TaskValidator', () => {
expect(result).toEqual(task);
});

// TODO: Remove skip once all task types have defined their state schema.
// https://github.com/elastic/kibana/issues/159347
it.skip(`should fail to validate the state schema when the task type doesn't have stateSchemaByVersion defined`, () => {
it(`should fail to validate the state schema when the task type doesn't have stateSchemaByVersion defined`, () => {
const definitions = new TaskTypeDictionary(mockLogger());
definitions.registerTaskDefinitions({
foo: fooTaskDefinition,
Expand Down
17 changes: 7 additions & 10 deletions x-pack/plugins/task_manager/server/task_validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { max, memoize } from 'lodash';
import { max, memoize, isEmpty } from 'lodash';
import type { Logger } from '@kbn/core/server';
import type { ObjectType } from '@kbn/config-schema';
import { TaskTypeDictionary } from './task_type_dictionary';
Expand Down Expand Up @@ -64,14 +64,13 @@ export class TaskValidator {
const taskTypeDef = this.definitions.get(task.taskType);
const latestStateSchema = this.cachedGetLatestStateSchema(taskTypeDef);

// TODO: Remove once all task types have defined their state schema.
// https://github.com/elastic/kibana/issues/159347
// Otherwise, failures on read / write would occur. (don't forget to unskip test)
if (!latestStateSchema) {
let state = task.state;

// Skip validating tasks that don't use state
if (!latestStateSchema && isEmpty(state)) {
return task;
}

let state = task.state;
try {
state = this.getValidatedStateSchema(
this.migrateTaskState(task.state, task.stateVersion, taskTypeDef, latestStateSchema),
Expand Down Expand Up @@ -111,10 +110,8 @@ export class TaskValidator {
const taskTypeDef = this.definitions.get(task.taskType);
const latestStateSchema = this.cachedGetLatestStateSchema(taskTypeDef);

// TODO: Remove once all task types have defined their state schema.
// https://github.com/elastic/kibana/issues/159347
// Otherwise, failures on read / write would occur. (don't forget to unskip test)
if (!latestStateSchema) {
// Skip validating tasks that don't use state
if (!latestStateSchema && isEmpty(task.state)) {
return task;
}

Expand Down

0 comments on commit d86eebd

Please sign in to comment.