-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[One Workflow] feat: Link test executions to workflow #241502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[One Workflow] feat: Link test executions to workflow #241502
Conversation
…specific workflow id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements persistence for all workflow executions by introducing a distinction between test runs and production runs. The key change is adding an isTestRun boolean field to workflow executions instead of using the executionType field, allowing both test and production executions to be persisted in the database.
Key Changes
- Modified filtering logic to use
isTestRunfield instead ofexecutionTypefor distinguishing test and production workflow executions - Added support for testing workflows either by ID or YAML content
- Updated the UI to intelligently choose between test execution (for modified workflows) and production execution (for unmodified workflows)
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
workflows_management_service.ts |
Updated execution filtering to use isTestRun boolean field instead of executionType |
workflows_management_service.test.ts |
Added comprehensive tests for the new filtering logic |
workflows_management_api.ts |
Enhanced testWorkflow to accept either workflowId or workflowYaml parameters |
workflows_management_api.test.ts |
Added extensive test coverage for the updated test workflow functionality |
routes/types.ts |
Added parseExecutionTypes helper function for parsing execution type query parameters |
route_error_handlers.ts |
Added handling for WorkflowNotFoundError |
post_test_workflow.ts |
Updated route validation to accept optional workflowId and workflowYaml |
post_test_workflow.test.ts |
Updated tests to match new API signature |
get_workflow_executions.ts |
Enabled execution type filtering |
test_workflow_thunk.ts |
Modified to include workflowId when available |
test_workflow_thunk.test.ts |
Added tests for workflow ID inclusion |
run_workflow_thunk.ts |
New file implementing production workflow execution |
run_workflow_thunk.test.ts |
New file with tests for production workflow execution |
workflow_detail_test_modal.tsx |
Updated to conditionally use test or run workflow based on changes |
workflow_detail_test_modal.test.tsx |
Updated tests for conditional execution logic |
workflow_execution_repository.ts |
Changed to use createOrUpdateIndex for index management |
workflows_execution_engine/common/index.ts |
Added isTestRun field to execution index mappings |
workflow_not_found_error.ts |
New error class for missing workflows |
errors/index.ts |
New index file exporting workflow error classes |
src/platform/plugins/shared/workflows_management/server/workflows_management/routes/types.ts
Outdated
Show resolved
Hide resolved
| type: 'text', | ||
| index: false, | ||
| }, | ||
| isTestRun: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding isTestRun to mapping so that we can filter on in
| if (this.indexInitialized) return; // Only 1 boolean check after first time | ||
|
|
||
| await createIndexWithMappings({ | ||
| await createOrUpdateIndex({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing create with createOrUpdateIndex so that we can update index mapping with newly added fields.
It will fail upon attempt to change mapping type of an existing mapping field. Just FYI
.../plugins/shared/workflows_management/server/workflows_management/workflows_management_api.ts
Show resolved
Hide resolved
…concept' of https://github.com/skynetigor/kibana into 14414-Persist-all-workflow-execution-&-remove-test-run-concept
…e-test-run-concept
…concept' of https://github.com/skynetigor/kibana into 14414-Persist-all-workflow-execution-&-remove-test-run-concept
…e-test-run-concept
…concept' of https://github.com/skynetigor/kibana into 14414-Persist-all-workflow-execution-&-remove-test-run-concept
…e-test-run-concept
💔 Build Failed
Failed CI StepsHistory
cc @skynetigor |
Summary
closes: https://github.com/elastic/security-team/issues/14414
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesbackport:*labels.Identify risks
Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.
Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.