mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-30 08:17:06 +02:00
feat(core): Adapt breaking changes report data to UI needs (#21442)
This commit is contained in:
parent
8270f37df5
commit
a2a484ecf2
|
|
@ -113,3 +113,15 @@ export type {
|
|||
} from './schemas/external-secrets.schema';
|
||||
|
||||
export type { UsageState } from './schemas/usage.schema';
|
||||
|
||||
export type {
|
||||
BreakingChangeRuleSeverity,
|
||||
BreakingChangeRecommendation,
|
||||
BreakingChangeAffectedWorkflow,
|
||||
BreakingChangeInstanceIssue,
|
||||
BreakingChangeWorkflowIssue,
|
||||
BreakingChangeInstanceRuleResult,
|
||||
BreakingChangeWorkflowRuleResult,
|
||||
BreakingChangeReportResult,
|
||||
BreakingChangeLightReportResult,
|
||||
} from './schemas/breaking-changes.schema';
|
||||
|
|
|
|||
|
|
@ -0,0 +1,95 @@
|
|||
import { z } from 'zod';
|
||||
|
||||
// Enums
|
||||
export const breakingChangeRuleSeveritySchema = z.enum(['low', 'medium', 'high', 'critical']);
|
||||
export type BreakingChangeRuleSeverity = z.infer<typeof breakingChangeRuleSeveritySchema>;
|
||||
|
||||
export const breakingChangeIssueLevelSchema = z.enum(['info', 'warning', 'error']);
|
||||
|
||||
// Common schemas
|
||||
const recommendationSchema = z.object({
|
||||
action: z.string(),
|
||||
description: z.string(),
|
||||
});
|
||||
export type BreakingChangeRecommendation = z.infer<typeof recommendationSchema>;
|
||||
|
||||
const instanceIssueSchema = z.object({
|
||||
title: z.string(),
|
||||
description: z.string(),
|
||||
level: breakingChangeIssueLevelSchema,
|
||||
});
|
||||
export type BreakingChangeInstanceIssue = z.infer<typeof instanceIssueSchema>;
|
||||
|
||||
const workflowIssueSchema = instanceIssueSchema.extend({
|
||||
nodeId: z.string().optional(),
|
||||
nodeName: z.string().optional(),
|
||||
});
|
||||
export type BreakingChangeWorkflowIssue = z.infer<typeof workflowIssueSchema>;
|
||||
|
||||
const affectedWorkflowSchema = z.object({
|
||||
id: z.string(),
|
||||
name: z.string(),
|
||||
active: z.boolean(),
|
||||
numberOfExecutions: z.number(),
|
||||
lastUpdatedAt: z.date(),
|
||||
lastExecutedAt: z.date().nullable(),
|
||||
issues: z.array(workflowIssueSchema),
|
||||
});
|
||||
export type BreakingChangeAffectedWorkflow = z.infer<typeof affectedWorkflowSchema>;
|
||||
|
||||
const ruleResultBaseSchema = z.object({
|
||||
ruleId: z.string(),
|
||||
ruleTitle: z.string(),
|
||||
ruleDescription: z.string(),
|
||||
ruleSeverity: breakingChangeRuleSeveritySchema,
|
||||
ruleDocumentationUrl: z.string().optional(),
|
||||
recommendations: z.array(recommendationSchema),
|
||||
});
|
||||
|
||||
const instanceRuleResultsSchema = ruleResultBaseSchema.extend({
|
||||
instanceIssues: z.array(instanceIssueSchema),
|
||||
});
|
||||
export type BreakingChangeInstanceRuleResult = z.infer<typeof instanceRuleResultsSchema>;
|
||||
|
||||
const workflowRuleResultsSchema = ruleResultBaseSchema.extend({
|
||||
affectedWorkflows: z.array(affectedWorkflowSchema),
|
||||
});
|
||||
export type BreakingChangeWorkflowRuleResult = z.infer<typeof workflowRuleResultsSchema>;
|
||||
|
||||
const breakingChangeReportDataSchema = {
|
||||
generatedAt: z.date(),
|
||||
targetVersion: z.string(),
|
||||
currentVersion: z.string(),
|
||||
instanceResults: z.array(instanceRuleResultsSchema),
|
||||
workflowResults: z.array(workflowRuleResultsSchema),
|
||||
} as const;
|
||||
|
||||
const breakingChangeReportSchema = z.object(breakingChangeReportDataSchema).strict();
|
||||
|
||||
const breakingChangeLightReportDataSchema = {
|
||||
generatedAt: z.date(),
|
||||
targetVersion: z.string(),
|
||||
currentVersion: z.string(),
|
||||
instanceResults: z.array(instanceRuleResultsSchema),
|
||||
workflowResults: z.array(
|
||||
workflowRuleResultsSchema.omit({ affectedWorkflows: true }).extend({
|
||||
nbAffectedWorkflows: z.number(),
|
||||
}),
|
||||
),
|
||||
} as const;
|
||||
|
||||
const breakingChangeLightReportSchema = z.object(breakingChangeLightReportDataSchema).strict();
|
||||
|
||||
const breakingChangeReportResultDataSchema = z.object({
|
||||
report: breakingChangeReportSchema,
|
||||
shouldCache: z.boolean(),
|
||||
});
|
||||
export type BreakingChangeReportResult = z.infer<typeof breakingChangeReportResultDataSchema>;
|
||||
|
||||
const breakingChangeLightReportResultDataSchema = z.object({
|
||||
report: breakingChangeLightReportSchema,
|
||||
shouldCache: z.boolean(),
|
||||
});
|
||||
export type BreakingChangeLightReportResult = z.infer<
|
||||
typeof breakingChangeLightReportResultDataSchema
|
||||
>;
|
||||
|
|
@ -15,7 +15,6 @@ breaking-changes/
|
|||
detection.types.ts # Report types
|
||||
index.ts
|
||||
rules/
|
||||
abstract-rule.ts # Base class for all rules
|
||||
v2/ # V2-specific rules
|
||||
removed-nodes.rule.ts
|
||||
process-env-access.rule.ts
|
||||
|
|
@ -32,42 +31,98 @@ breaking-changes/
|
|||
|
||||
### Detect Breaking Changes
|
||||
```
|
||||
GET /breaking-changes?version=v2
|
||||
GET /breaking-changes/report?version=v2
|
||||
```
|
||||
|
||||
Returns:
|
||||
```json
|
||||
{
|
||||
"report": {
|
||||
"generatedAt": "2025-10-17T00:00:00.000Z",
|
||||
"targetVersion": "v2",
|
||||
"currentVersion": "1.x.x",
|
||||
"totalIssues": 5,
|
||||
"criticalIssues": 2,
|
||||
"affectedWorkflowCount": 4,
|
||||
"results": [
|
||||
{
|
||||
"ruleId": "removed-nodes-v2",
|
||||
"isAffected": true,
|
||||
"affectedWorkflows": [
|
||||
{
|
||||
"id": "wf-001",
|
||||
"name": "Customer Onboarding",
|
||||
"active": true,
|
||||
"reason": "Uses removed nodes: n8n-nodes-base.spontit"
|
||||
}
|
||||
],
|
||||
"instanceIssues": [],
|
||||
"recommendations": [
|
||||
{
|
||||
"action": "Update affected workflows",
|
||||
"description": "Replace removed nodes with alternatives"
|
||||
}
|
||||
"instanceResults": [
|
||||
{
|
||||
"ruleId": "process-env-access-v2",
|
||||
"ruleTitle": "Process Environment Access Restrictions",
|
||||
"ruleDescription": "Access to process.env is now restricted",
|
||||
"ruleSeverity": "high",
|
||||
"instanceIssues": [
|
||||
{
|
||||
"title": "Environment access detected",
|
||||
"description": "Workflows using process.env may be affected",
|
||||
"level": "warning"
|
||||
}
|
||||
],
|
||||
"recommendations": [
|
||||
{
|
||||
"action": "Review environment variable usage",
|
||||
"description": "Update workflows to use secure environment variable access"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"workflowResults": [
|
||||
{
|
||||
"ruleId": "removed-nodes-v2",
|
||||
"ruleTitle": "Removed Deprecated Nodes",
|
||||
"ruleDescription": "Several deprecated nodes have been removed",
|
||||
"ruleSeverity": "critical",
|
||||
"affectedWorkflows": [
|
||||
{
|
||||
"id": "wf-001",
|
||||
"name": "Customer Onboarding",
|
||||
"active": true,
|
||||
"numberOfExecutions": 142,
|
||||
"lastUpdatedAt": "2025-10-15T10:30:00.000Z",
|
||||
"lastExecutedAt": "2025-10-16T14:22:00.000Z",
|
||||
"issues": [
|
||||
{
|
||||
"title": "Node 'n8n-nodes-base.spontit' with name 'Spontit' has been removed",
|
||||
"description": "The node type 'n8n-nodes-base.spontit' is no longer available",
|
||||
"level": "error",
|
||||
"nodeId": "node-123",
|
||||
"nodeName": "Spontit"
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
],
|
||||
"recommendations": [
|
||||
{
|
||||
"action": "Update affected workflows",
|
||||
"description": "Replace removed nodes with their updated versions or alternatives"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
},
|
||||
"shouldCache": false
|
||||
}
|
||||
```
|
||||
|
||||
## Rule Types
|
||||
|
||||
The system supports two types of rules:
|
||||
|
||||
### Workflow Rules (`IBreakingChangeWorkflowRule`)
|
||||
|
||||
- **Purpose**: Check individual workflows for breaking changes
|
||||
- **Methods**:
|
||||
- `getMetadata()`: Returns rule metadata (version, title, description, severity, etc.)
|
||||
- `detectWorkflow(workflow, nodesGroupedByType)`: Checks a single workflow and returns issues
|
||||
- `getRecommendations(workflowResults)`: Returns recommendations based on detected issues
|
||||
- **Returns**: `WorkflowDetectionReport` with workflow-specific issues
|
||||
- **Example Use Cases**: Removed nodes, deprecated node features, changed node behavior
|
||||
|
||||
### Instance Rules (`IBreakingChangeInstanceRule`)
|
||||
|
||||
- **Purpose**: Check instance-level configuration and environment
|
||||
- **Methods**:
|
||||
- `getMetadata()`: Returns rule metadata (version, title, description, severity, etc.)
|
||||
- `detect()`: Checks the entire instance and returns issues
|
||||
- **Returns**: `InstanceDetectionReport` with instance-level issues and recommendations
|
||||
- **Example Use Cases**: Environment variable requirements, database version checks, configuration changes
|
||||
|
||||
## Adding Rules to the System
|
||||
|
||||
### Option 1: Adding a Rule to an Existing Version
|
||||
|
|
@ -76,64 +131,131 @@ If you're adding a new breaking change rule for an existing version (e.g., v2),
|
|||
|
||||
#### Step 1: Create the Rule Class
|
||||
|
||||
Create your rule file in the appropriate version directory (e.g., `rules/v2/my-new-rule.rule.ts`):
|
||||
There are two types of rules you can create:
|
||||
|
||||
##### A. Workflow Rules (for checking workflows)
|
||||
|
||||
Create your rule file in the appropriate version directory (e.g., `rules/v2/my-workflow-rule.rule.ts`):
|
||||
|
||||
```typescript
|
||||
import { BreakingChangeRecommendation } from '@n8n/api-types';
|
||||
import type { WorkflowEntity } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { ErrorReporter } from 'n8n-core';
|
||||
import { INode } from 'n8n-workflow';
|
||||
|
||||
import type { DetectionResult, BreakingChangeMetadata } from '../../types';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory } from '../../types';
|
||||
import type {
|
||||
BreakingChangeRuleMetadata,
|
||||
IBreakingChangeWorkflowRule,
|
||||
WorkflowDetectionReport,
|
||||
} from '../../types';
|
||||
import { BreakingChangeCategory } from '../../types';
|
||||
|
||||
@Service()
|
||||
export class MyNewRule extends IBreakingChangeInstanceRule {
|
||||
constructor(
|
||||
protected readonly logger: Logger,
|
||||
protected errorReporter: ErrorReporter,
|
||||
) {
|
||||
super(logger);
|
||||
}
|
||||
export class MyWorkflowRule implements IBreakingChangeWorkflowRule {
|
||||
id: string = 'my-workflow-rule-v2';
|
||||
|
||||
getMetadata(): BreakingChangeMetadata {
|
||||
return {
|
||||
id: 'my-rule-v2',
|
||||
version: 'v2',
|
||||
title: 'My Breaking Change',
|
||||
description: 'Description of what changed',
|
||||
category: BreakingChangeCategory.WORKFLOW,
|
||||
severity: BreakingChangeSeverity.HIGH,
|
||||
documentationUrl: 'https://docs.n8n.io/migration/v2/...',
|
||||
};
|
||||
}
|
||||
getMetadata(): BreakingChangeRuleMetadata {
|
||||
return {
|
||||
version: 'v2',
|
||||
title: 'My Workflow Breaking Change',
|
||||
description: 'Description of what changed in workflows',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: 'high',
|
||||
documentationUrl: 'https://docs.n8n.io/migration/v2/...',
|
||||
};
|
||||
}
|
||||
|
||||
async detect(): Promise<DetectionResult> {
|
||||
const result = this.createEmptyResult(this.getMetadata().id);
|
||||
async getRecommendations(): Promise<BreakingChangeRecommendation[]> {
|
||||
return [
|
||||
{
|
||||
action: 'Update affected workflows',
|
||||
description: 'Replace or update the affected nodes',
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
try {
|
||||
// Instance-level issues
|
||||
result.instanceIssues.push({
|
||||
type: 'configuration',
|
||||
description: 'Database version XYZ is deprecated',
|
||||
severity: BreakingChangeSeverity.HIGH,
|
||||
});
|
||||
async detectWorkflow(
|
||||
_workflow: WorkflowEntity,
|
||||
nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<WorkflowDetectionReport> {
|
||||
// Check if workflow uses specific node types
|
||||
const affectedNodes = nodesGroupedByType.get('n8n-nodes-base.someNode') ?? [];
|
||||
|
||||
result.isAffected =
|
||||
result.instanceIssues.length > 0;
|
||||
if (affectedNodes.length === 0) {
|
||||
return { isAffected: false, issues: [] };
|
||||
}
|
||||
|
||||
if (result.isAffected) {
|
||||
result.recommendations.push({
|
||||
action: 'What to do',
|
||||
description: 'How to fix it',
|
||||
documentationUrl: 'https://docs.n8n.io/...',
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
this.errorReporter.error(error, { shouldBeLogged: true});
|
||||
}
|
||||
return {
|
||||
isAffected: true,
|
||||
issues: affectedNodes.map((node) => ({
|
||||
title: `Node '${node.type}' with name '${node.name}' is affected`,
|
||||
description: 'This node requires updates for v2 compatibility',
|
||||
level: 'warning',
|
||||
nodeId: node.id,
|
||||
nodeName: node.name,
|
||||
})),
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
return result;
|
||||
}
|
||||
##### B. Instance Rules (for checking instance configuration)
|
||||
|
||||
Create your rule file in the appropriate version directory (e.g., `rules/v2/my-instance-rule.rule.ts`):
|
||||
|
||||
```typescript
|
||||
import { BreakingChangeRecommendation } from '@n8n/api-types';
|
||||
import { Service } from '@n8n/di';
|
||||
|
||||
import type {
|
||||
BreakingChangeRuleMetadata,
|
||||
IBreakingChangeInstanceRule,
|
||||
InstanceDetectionReport,
|
||||
} from '../../types';
|
||||
import { BreakingChangeCategory } from '../../types';
|
||||
|
||||
@Service()
|
||||
export class MyInstanceRule implements IBreakingChangeInstanceRule {
|
||||
id: string = 'my-instance-rule-v2';
|
||||
|
||||
getMetadata(): BreakingChangeRuleMetadata {
|
||||
return {
|
||||
version: 'v2',
|
||||
title: 'My Instance Breaking Change',
|
||||
description: 'Description of what changed at instance level',
|
||||
category: BreakingChangeCategory.instance,
|
||||
severity: 'critical',
|
||||
documentationUrl: 'https://docs.n8n.io/migration/v2/...',
|
||||
};
|
||||
}
|
||||
|
||||
async detect(): Promise<InstanceDetectionReport> {
|
||||
const instanceIssues = [];
|
||||
const recommendations = [];
|
||||
|
||||
// Check instance-level configuration
|
||||
// Example: check environment variables, database version, etc.
|
||||
const hasIssue = false; // Your detection logic here
|
||||
|
||||
if (hasIssue) {
|
||||
instanceIssues.push({
|
||||
title: 'Configuration issue detected',
|
||||
description: 'Database version XYZ is no longer supported',
|
||||
level: 'error',
|
||||
});
|
||||
|
||||
recommendations.push({
|
||||
action: 'Upgrade database',
|
||||
description: 'Update to database version ABC or higher',
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
isAffected: instanceIssues.length > 0,
|
||||
instanceIssues,
|
||||
recommendations,
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
|
|
@ -148,10 +270,10 @@ import { RemovedNodesRule } from './removed-nodes.rule';
|
|||
import { MyNewRule } from './my-new-rule.rule'; // Import your rule
|
||||
|
||||
const v2Rules = [
|
||||
RemovedNodesRule,
|
||||
ProcessEnvAccessRule,
|
||||
FileAccessRule,
|
||||
MyNewRule, // Add to array
|
||||
RemovedNodesRule,
|
||||
ProcessEnvAccessRule,
|
||||
FileAccessRule,
|
||||
MyNewRule, // Add to array
|
||||
];
|
||||
|
||||
export { v2Rules };
|
||||
|
|
@ -187,9 +309,9 @@ import { MySecondV3Rule } from './my-second-v3-rule.rule';
|
|||
// Import all v3 rules
|
||||
|
||||
const v3Rules = [
|
||||
MyFirstV3Rule,
|
||||
MySecondV3Rule,
|
||||
// Add all v3 rules here
|
||||
MyFirstV3Rule,
|
||||
MySecondV3Rule,
|
||||
// Add all v3 rules here
|
||||
];
|
||||
|
||||
export { v3Rules };
|
||||
|
|
@ -211,24 +333,3 @@ type RuleInstances = InstanceType<RuleConstructors>;
|
|||
|
||||
export { allRules, type RuleInstances };
|
||||
```
|
||||
|
||||
## Rule Categories
|
||||
|
||||
- **WORKFLOW**: Changes affecting workflow execution
|
||||
- **INSTANCE**: Changes to instance configuration
|
||||
- **ENVIRONMENT**: Changes to environment variables
|
||||
- **DATABASE**: Changes to database requirements
|
||||
- **INFRASTRUCTURE**: Changes to deployment/infrastructure
|
||||
|
||||
## Severity Levels
|
||||
|
||||
- **CRITICAL**: Will cause immediate workflow failures
|
||||
- **HIGH**: May cause silent failures or require urgent action
|
||||
- **MEDIUM**: Should be addressed but not urgent
|
||||
- **LOW**: Optional improvements or deprecation notices
|
||||
|
||||
## Permissions
|
||||
|
||||
The `breakingChanges:list` scope is automatically granted to:
|
||||
- **Global Owners**
|
||||
- **Global Admins**
|
||||
|
|
|
|||
|
|
@ -4,10 +4,10 @@ import { mock } from 'jest-mock-extended';
|
|||
import { RuleRegistry } from '../breaking-changes.rule-registry.service';
|
||||
import type {
|
||||
IBreakingChangeRule,
|
||||
BreakingChangeMetadata,
|
||||
InstanceDetectionResult,
|
||||
BreakingChangeRuleMetadata,
|
||||
InstanceDetectionReport,
|
||||
} from '../types';
|
||||
import { BreakingChangeCategory, BreakingChangeSeverity } from '../types';
|
||||
import { BreakingChangeCategory } from '../types';
|
||||
|
||||
describe('RuleRegistry', () => {
|
||||
const logger = mock<Logger>({
|
||||
|
|
@ -26,12 +26,12 @@ describe('RuleRegistry', () => {
|
|||
version: string = 'v2',
|
||||
title: string = 'Test Rule',
|
||||
): IBreakingChangeRule => {
|
||||
const metadata: BreakingChangeMetadata = {
|
||||
const metadata: BreakingChangeRuleMetadata = {
|
||||
version: version as 'v2',
|
||||
title,
|
||||
description: `Description for ${title}`,
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.medium,
|
||||
severity: 'medium',
|
||||
};
|
||||
|
||||
const mockRule: IBreakingChangeRule = {
|
||||
|
|
@ -42,7 +42,7 @@ describe('RuleRegistry', () => {
|
|||
isAffected: false,
|
||||
instanceIssues: [],
|
||||
recommendations: [],
|
||||
} as InstanceDetectionResult);
|
||||
} as InstanceDetectionReport);
|
||||
}),
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -1,8 +1,11 @@
|
|||
import type { BreakingChangeWorkflowRuleResult } from '@n8n/api-types';
|
||||
import { mockLogger } from '@n8n/backend-test-utils';
|
||||
import type { WorkflowRepository } from '@n8n/db';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import type { ErrorReporter } from 'n8n-core';
|
||||
|
||||
import type { CacheService } from '@/services/cache/cache.service';
|
||||
|
||||
import { N8N_VERSION } from '../../../constants';
|
||||
import { RuleRegistry } from '../breaking-changes.rule-registry.service';
|
||||
import { BreakingChangeService } from '../breaking-changes.service';
|
||||
|
|
@ -11,8 +14,6 @@ import { FileAccessRule } from '../rules/v2/file-access.rule';
|
|||
import { ProcessEnvAccessRule } from '../rules/v2/process-env-access.rule';
|
||||
import { RemovedNodesRule } from '../rules/v2/removed-nodes.rule';
|
||||
|
||||
import type { CacheService } from '@/services/cache/cache.service';
|
||||
|
||||
describe('BreakingChangeService', () => {
|
||||
const logger = mockLogger();
|
||||
|
||||
|
|
@ -61,16 +62,13 @@ describe('BreakingChangeService', () => {
|
|||
|
||||
const report = await service.detect('v2');
|
||||
|
||||
expect(report).toMatchObject({
|
||||
expect(report.report).toMatchObject({
|
||||
targetVersion: 'v2',
|
||||
currentVersion: N8N_VERSION,
|
||||
summary: {
|
||||
totalIssues: 0,
|
||||
criticalIssues: 0,
|
||||
},
|
||||
results: [],
|
||||
instanceResults: [],
|
||||
workflowResults: [],
|
||||
});
|
||||
expect(report.generatedAt).toBeInstanceOf(Date);
|
||||
expect(report.report.generatedAt).toBeInstanceOf(Date);
|
||||
});
|
||||
|
||||
it('should aggregate results from multiple rules', async () => {
|
||||
|
|
@ -89,57 +87,38 @@ describe('BreakingChangeService', () => {
|
|||
const report = await service.detect('v2');
|
||||
|
||||
// Verify report structure
|
||||
expect(report.targetVersion).toBe('v2');
|
||||
expect(report.currentVersion).toBe(N8N_VERSION);
|
||||
expect(report.generatedAt).toBeInstanceOf(Date);
|
||||
|
||||
// Verify all three rules detected issues
|
||||
expect(report.summary.totalIssues).toBe(3);
|
||||
expect(report.summary.criticalIssues).toBe(1); // Only RemovedNodesRule is critical
|
||||
expect(report.report.targetVersion).toBe('v2');
|
||||
expect(report.report.currentVersion).toBe(N8N_VERSION);
|
||||
expect(report.report.generatedAt).toBeInstanceOf(Date);
|
||||
|
||||
// Verify each rule's result is in the report
|
||||
const removedNodesResult = report.results.find((r) => r.ruleId === 'removed-nodes-v2');
|
||||
const removedNodesResult = report.report.workflowResults.find(
|
||||
(r) => r.ruleId === 'removed-nodes-v2',
|
||||
);
|
||||
expect(removedNodesResult?.affectedWorkflows).toHaveLength(1);
|
||||
|
||||
const processEnvResult = report.results.find((r) => r.ruleId === 'process-env-access-v2');
|
||||
const processEnvResult = report.report.workflowResults.find(
|
||||
(r) => r.ruleId === 'process-env-access-v2',
|
||||
);
|
||||
expect(processEnvResult?.affectedWorkflows).toHaveLength(1);
|
||||
|
||||
const fileAccessResult = report.results.find(
|
||||
const fileAccessResult = report.report.workflowResults.find(
|
||||
(r) => r.ruleId === 'file-access-restriction-v2',
|
||||
);
|
||||
expect(fileAccessResult?.affectedWorkflows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should correctly count critical issues', async () => {
|
||||
const { workflow: workflow1 } = createWorkflow('wf-1', 'Workflow 1', [
|
||||
createNode('Spontit Node', 'n8n-nodes-base.spontit'), // Critical severity
|
||||
]);
|
||||
const { workflow: workflow2 } = createWorkflow('wf-2', 'Workflow 2', [
|
||||
createNode('File Node', 'n8n-nodes-base.readWriteFile'), // High severity (not critical)
|
||||
]);
|
||||
workflowRepository.find.mockResolvedValue([workflow1, workflow2]);
|
||||
workflowRepository.count.mockResolvedValue(2);
|
||||
|
||||
const report = await service.detect('v2');
|
||||
|
||||
expect(report.summary.totalIssues).toBe(2);
|
||||
expect(report.summary.criticalIssues).toBe(1); // Only removed nodes is critical
|
||||
});
|
||||
|
||||
it('should include all required fields in the report', async () => {
|
||||
workflowRepository.find.mockResolvedValue([]);
|
||||
workflowRepository.count.mockResolvedValue(0);
|
||||
|
||||
const report = await service.detect('v2');
|
||||
|
||||
expect(report).toHaveProperty('generatedAt');
|
||||
expect(report).toHaveProperty('targetVersion', 'v2');
|
||||
expect(report).toHaveProperty('currentVersion', N8N_VERSION);
|
||||
expect(report).toHaveProperty('summary');
|
||||
expect(report.summary).toHaveProperty('totalIssues');
|
||||
expect(report.summary).toHaveProperty('criticalIssues');
|
||||
expect(report).toHaveProperty('results');
|
||||
expect(Array.isArray(report.results)).toBe(true);
|
||||
expect(report.report).toHaveProperty('generatedAt');
|
||||
expect(report.report).toHaveProperty('targetVersion', 'v2');
|
||||
expect(report.report).toHaveProperty('currentVersion', N8N_VERSION);
|
||||
expect(report.report).toHaveProperty('workflowResults');
|
||||
expect(Array.isArray(report.report.workflowResults)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -182,4 +161,27 @@ describe('BreakingChangeService', () => {
|
|||
expect(detectSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getDetectionReportForRule()', () => {
|
||||
it('should return undefined for unknown rule ID', async () => {
|
||||
const result = await service.getDetectionReportForRule('unknown-rule-id');
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should return correct report for a known workflow-level rule', async () => {
|
||||
const { workflow } = createWorkflow('wf-1', 'Test Workflow', [
|
||||
createNode('Spontit Node', 'n8n-nodes-base.spontit'),
|
||||
]);
|
||||
|
||||
workflowRepository.find.mockResolvedValue([workflow as never]);
|
||||
workflowRepository.count.mockResolvedValue(1);
|
||||
|
||||
const result = (await service.getDetectionReportForRule(
|
||||
'removed-nodes-v2',
|
||||
)) as BreakingChangeWorkflowRuleResult;
|
||||
expect(result).toBeDefined();
|
||||
expect(result?.ruleId).toBe('removed-nodes-v2');
|
||||
expect(result?.affectedWorkflows).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { WorkflowEntity } from '@n8n/db';
|
||||
import type { WorkflowEntity, WorkflowStatistics } from '@n8n/db';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
|
||||
export const createWorkflow = (id: string, name: string, nodes: INode[], active = true) => ({
|
||||
|
|
@ -7,6 +7,13 @@ export const createWorkflow = (id: string, name: string, nodes: INode[], active
|
|||
name,
|
||||
active,
|
||||
nodes,
|
||||
statistics: [
|
||||
{
|
||||
count: 1,
|
||||
rootCount: 1,
|
||||
latestEvent: new Date(),
|
||||
} as WorkflowStatistics,
|
||||
],
|
||||
} as WorkflowEntity,
|
||||
nodesGroupedByType: nodes.reduce((map, node) => {
|
||||
if (!map.has(node.type)) {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,13 @@
|
|||
import { Get, RestController, GlobalScope, Query, Post } from '@n8n/decorators';
|
||||
import {
|
||||
BreakingChangeInstanceRuleResult,
|
||||
BreakingChangeLightReportResult,
|
||||
BreakingChangeReportResult,
|
||||
BreakingChangeWorkflowRuleResult,
|
||||
} from '@n8n/api-types';
|
||||
import { AuthenticatedRequest } from '@n8n/db';
|
||||
import { Get, RestController, GlobalScope, Query, Post, Param } from '@n8n/decorators';
|
||||
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
|
||||
import { BreakingChangeService } from './breaking-changes.service';
|
||||
import { BreakingChangeVersion } from './types';
|
||||
|
|
@ -7,18 +16,59 @@ import { BreakingChangeVersion } from './types';
|
|||
export class BreakingChangesController {
|
||||
constructor(private readonly service: BreakingChangeService) {}
|
||||
|
||||
/**
|
||||
* Get all registered breaking change rules
|
||||
*/
|
||||
@Get('/')
|
||||
@GlobalScope('breakingChanges:list')
|
||||
async listRules(@Query query: { version?: BreakingChangeVersion }) {
|
||||
return await this.service.getDetectionResults(query.version ?? 'v2');
|
||||
private getLightDetectionResults(
|
||||
report: BreakingChangeReportResult['report'],
|
||||
): BreakingChangeLightReportResult['report'] {
|
||||
return {
|
||||
...report,
|
||||
workflowResults: report.workflowResults.map((r) => {
|
||||
const { affectedWorkflows, ...otherFields } = r;
|
||||
return { ...otherFields, nbAffectedWorkflows: affectedWorkflows.length };
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
@Post('/refresh')
|
||||
/**
|
||||
* Get all registered breaking change rules results
|
||||
*/
|
||||
@Get('/report')
|
||||
@GlobalScope('breakingChanges:list')
|
||||
async refreshCache(@Query query: { version?: BreakingChangeVersion }) {
|
||||
return await this.service.refreshDetectionResults(query.version ?? 'v2');
|
||||
async getDetectionReport(
|
||||
@Query query: { version?: BreakingChangeVersion },
|
||||
): Promise<BreakingChangeLightReportResult> {
|
||||
const report = await this.service.getDetectionResults(query.version ?? 'v2');
|
||||
return {
|
||||
...report,
|
||||
report: this.getLightDetectionResults(report.report),
|
||||
};
|
||||
}
|
||||
|
||||
@Post('/report/refresh')
|
||||
@GlobalScope('breakingChanges:list')
|
||||
async refreshCache(
|
||||
@Query query: { version?: BreakingChangeVersion },
|
||||
): Promise<BreakingChangeLightReportResult> {
|
||||
const report = await this.service.refreshDetectionResults(query.version ?? 'v2');
|
||||
return {
|
||||
...report,
|
||||
report: this.getLightDetectionResults(report.report),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Get specific breaking change rules
|
||||
*/
|
||||
@Get('/report/:ruleId')
|
||||
@GlobalScope('breakingChanges:list')
|
||||
async getDetectionReportForRule(
|
||||
_req: AuthenticatedRequest,
|
||||
_res: Response,
|
||||
@Param('ruleId') ruleId: string,
|
||||
): Promise<BreakingChangeInstanceRuleResult | BreakingChangeWorkflowRuleResult> {
|
||||
const result = await this.service.getDetectionReportForRule(ruleId);
|
||||
if (!result) {
|
||||
throw new NotFoundError(`Breaking change rule with ID '${ruleId}' not found.`);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,31 +1,35 @@
|
|||
import {
|
||||
BreakingChangeAffectedWorkflow,
|
||||
BreakingChangeInstanceRuleResult,
|
||||
BreakingChangeReportResult,
|
||||
BreakingChangeWorkflowRuleResult,
|
||||
} from '@n8n/api-types';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { Time } from '@n8n/constants';
|
||||
import { WorkflowRepository } from '@n8n/db';
|
||||
import { Container, Service } from '@n8n/di';
|
||||
import { ErrorReporter } from 'n8n-core';
|
||||
import { INode } from 'n8n-workflow';
|
||||
import { strict as assert } from 'node:assert';
|
||||
|
||||
import { CacheService } from '@/services/cache/cache.service';
|
||||
|
||||
import { RuleRegistry } from './breaking-changes.rule-registry.service';
|
||||
import { allRules, RuleInstances } from './rules';
|
||||
import type {
|
||||
DetectionReport,
|
||||
BreakingChangeVersion,
|
||||
DetectionResult,
|
||||
IBreakingChangeRule,
|
||||
AffectedWorkflow,
|
||||
IBreakingChangeWorkflowRule,
|
||||
IBreakingChangeInstanceRule,
|
||||
} from './types';
|
||||
import { BreakingChangeSeverity } from './types';
|
||||
import { N8N_VERSION } from '../../constants';
|
||||
|
||||
import { CacheService } from '@/services/cache/cache.service';
|
||||
|
||||
@Service()
|
||||
export class BreakingChangeService {
|
||||
private readonly batchSize = 100;
|
||||
private static readonly CACHE_KEY_PREFIX = 'breaking-changes:results:';
|
||||
private readonly ongoingDetections = new Map<BreakingChangeVersion, Promise<DetectionReport>>();
|
||||
private readonly ongoingDetections = new Map<
|
||||
BreakingChangeVersion,
|
||||
Promise<BreakingChangeReportResult>
|
||||
>();
|
||||
|
||||
constructor(
|
||||
private readonly ruleRegistry: RuleRegistry,
|
||||
|
|
@ -47,20 +51,25 @@ export class BreakingChangeService {
|
|||
|
||||
async getAllInstanceRulesResults(
|
||||
instanceLevelRules: IBreakingChangeInstanceRule[],
|
||||
): Promise<DetectionResult[]> {
|
||||
const instanceLevelResults: DetectionResult[] = [];
|
||||
): Promise<BreakingChangeInstanceRuleResult[]> {
|
||||
const instanceLevelResults: BreakingChangeInstanceRuleResult[] = [];
|
||||
for (const rule of instanceLevelRules) {
|
||||
try {
|
||||
const ruleResult = await rule.detect();
|
||||
console.log('ruleResult', ruleResult);
|
||||
if (ruleResult.isAffected) {
|
||||
instanceLevelResults.push({
|
||||
ruleId: rule.id,
|
||||
affectedWorkflows: [],
|
||||
ruleTitle: rule.getMetadata().title,
|
||||
ruleDescription: rule.getMetadata().description,
|
||||
ruleSeverity: rule.getMetadata().severity,
|
||||
ruleDocumentationUrl: rule.getMetadata().documentationUrl,
|
||||
instanceIssues: ruleResult.instanceIssues,
|
||||
recommendations: ruleResult.recommendations,
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
console.log('error', error);
|
||||
this.errorReporter.error(error, { shouldBeLogged: true });
|
||||
}
|
||||
}
|
||||
|
|
@ -69,10 +78,10 @@ export class BreakingChangeService {
|
|||
|
||||
private async getAllWorkflowRulesResults(
|
||||
workflowLevelRules: IBreakingChangeWorkflowRule[],
|
||||
): Promise<DetectionResult[]> {
|
||||
): Promise<BreakingChangeWorkflowRuleResult[]> {
|
||||
const totalWorkflows = await this.workflowRepository.count();
|
||||
const allAffectedWorkflowsByRule: Map<string, AffectedWorkflow[]> = new Map();
|
||||
const allResults: DetectionResult[] = [];
|
||||
const allAffectedWorkflowsByRule: Map<string, BreakingChangeAffectedWorkflow[]> = new Map();
|
||||
const allResults: BreakingChangeWorkflowRuleResult[] = [];
|
||||
|
||||
this.logger.info('Processing workflows in batches', {
|
||||
totalWorkflows,
|
||||
|
|
@ -82,10 +91,13 @@ export class BreakingChangeService {
|
|||
// Process workflows in batches
|
||||
for (let skip = 0; skip < totalWorkflows; skip += this.batchSize) {
|
||||
const workflows = await this.workflowRepository.find({
|
||||
select: ['id', 'name', 'active', 'nodes'],
|
||||
select: ['id', 'name', 'active', 'nodes', 'updatedAt', 'statistics'],
|
||||
skip,
|
||||
take: this.batchSize,
|
||||
order: { id: 'ASC' },
|
||||
relations: {
|
||||
statistics: true,
|
||||
},
|
||||
});
|
||||
|
||||
this.logger.debug('Processing batch', {
|
||||
|
|
@ -104,11 +116,19 @@ export class BreakingChangeService {
|
|||
for (const rule of workflowLevelRules) {
|
||||
const workflowDetectionResult = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
if (workflowDetectionResult.isAffected) {
|
||||
const affectedWorkflow = {
|
||||
const affectedWorkflow: BreakingChangeAffectedWorkflow = {
|
||||
id: workflow.id,
|
||||
name: workflow.name,
|
||||
active: workflow.active,
|
||||
issues: workflowDetectionResult.issues,
|
||||
numberOfExecutions: workflow.statistics.reduce(
|
||||
(acc, cur) => acc + (cur.count || 0),
|
||||
0,
|
||||
),
|
||||
lastExecutedAt: workflow.statistics.sort(
|
||||
(a, b) => b.latestEvent.getTime() - a.latestEvent.getTime(),
|
||||
)[0]?.latestEvent,
|
||||
lastUpdatedAt: workflow.updatedAt,
|
||||
};
|
||||
if (!allAffectedWorkflowsByRule.has(rule.id)) {
|
||||
allAffectedWorkflowsByRule.set(rule.id, [affectedWorkflow]);
|
||||
|
|
@ -128,8 +148,10 @@ export class BreakingChangeService {
|
|||
if (isAffected) {
|
||||
allResults.push({
|
||||
ruleId: rule.id,
|
||||
ruleTitle: rule.getMetadata().title,
|
||||
ruleDescription: rule.getMetadata().description,
|
||||
ruleSeverity: rule.getMetadata().severity,
|
||||
affectedWorkflows: workflowResults,
|
||||
instanceIssues: [],
|
||||
recommendations: await rule.getRecommendations(workflowResults),
|
||||
});
|
||||
}
|
||||
|
|
@ -139,42 +161,60 @@ export class BreakingChangeService {
|
|||
|
||||
async refreshDetectionResults(
|
||||
targetVersion: BreakingChangeVersion,
|
||||
): Promise<DetectionReport | undefined> {
|
||||
await this.cacheService.deleteFromHash(BreakingChangeService.CACHE_KEY_PREFIX, targetVersion);
|
||||
): Promise<BreakingChangeReportResult> {
|
||||
await this.cacheService.delete(`${BreakingChangeService.CACHE_KEY_PREFIX}_${targetVersion}`);
|
||||
return await this.getDetectionResults(targetVersion);
|
||||
}
|
||||
|
||||
async getDetectionResults(
|
||||
targetVersion: BreakingChangeVersion,
|
||||
): Promise<DetectionReport | undefined> {
|
||||
return await this.cacheService.getHashValue<DetectionReport>(
|
||||
BreakingChangeService.CACHE_KEY_PREFIX,
|
||||
targetVersion,
|
||||
{
|
||||
refreshFn: async () => {
|
||||
// Check if there's already an ongoing detection for this version
|
||||
const existingDetection = this.ongoingDetections.get(targetVersion);
|
||||
if (existingDetection) {
|
||||
this.logger.debug('Reusing ongoing detection', { targetVersion });
|
||||
return await existingDetection;
|
||||
}
|
||||
): Promise<BreakingChangeReportResult> {
|
||||
// Check if there's already an ongoing detection for this version
|
||||
const existingDetection = this.ongoingDetections.get(targetVersion);
|
||||
if (existingDetection) {
|
||||
this.logger.debug('Reusing ongoing detection', { targetVersion });
|
||||
return await existingDetection;
|
||||
}
|
||||
|
||||
// Start a new detection and store the promise
|
||||
const detectionPromise = this.detect(targetVersion);
|
||||
this.ongoingDetections.set(targetVersion, detectionPromise);
|
||||
const cacheKey = `${BreakingChangeService.CACHE_KEY_PREFIX}_${targetVersion}`;
|
||||
|
||||
try {
|
||||
return await detectionPromise;
|
||||
} finally {
|
||||
// Clean up the promise after completion (success or failure)
|
||||
this.ongoingDetections.delete(targetVersion);
|
||||
}
|
||||
},
|
||||
},
|
||||
);
|
||||
// Start a new detection and store the promise
|
||||
const detectionPromise: Promise<BreakingChangeReportResult> = new Promise((resolve) => {
|
||||
void (async () => {
|
||||
// Check cache first
|
||||
const cachedResult = await this.cacheService.get<BreakingChangeReportResult>(cacheKey);
|
||||
if (cachedResult) {
|
||||
this.logger.info('Using cached breaking change detection results', {
|
||||
targetVersion,
|
||||
});
|
||||
return resolve(cachedResult);
|
||||
}
|
||||
|
||||
// Perform detection
|
||||
const detectionResult = await this.detect(targetVersion);
|
||||
return resolve(detectionResult);
|
||||
})();
|
||||
});
|
||||
this.ongoingDetections.set(targetVersion, detectionPromise);
|
||||
|
||||
try {
|
||||
const result = await detectionPromise;
|
||||
// Store in cache if detection took significant time
|
||||
if (result.shouldCache) {
|
||||
await this.cacheService.set(cacheKey, result);
|
||||
}
|
||||
return result;
|
||||
} finally {
|
||||
// Clean up the promise after completion (success or failure)
|
||||
this.ongoingDetections.delete(targetVersion);
|
||||
}
|
||||
}
|
||||
|
||||
async detect(targetVersion: BreakingChangeVersion): Promise<DetectionReport> {
|
||||
private shouldCacheDetection(durationMs: number): boolean {
|
||||
return durationMs > Time.seconds.toMilliseconds * 10;
|
||||
}
|
||||
|
||||
async detect(targetVersion: BreakingChangeVersion): Promise<BreakingChangeReportResult> {
|
||||
const startTime = Date.now();
|
||||
this.logger.info('Starting breaking change detection', { targetVersion });
|
||||
|
||||
|
|
@ -190,41 +230,43 @@ export class BreakingChangeService {
|
|||
|
||||
const report = this.createDetectionReport(
|
||||
targetVersion,
|
||||
instanceLevelResults.concat(workflowLevelResults),
|
||||
rules,
|
||||
instanceLevelResults,
|
||||
workflowLevelResults,
|
||||
);
|
||||
|
||||
const duration = Date.now() - startTime;
|
||||
this.logger.info('Breaking change detection completed', {
|
||||
duration,
|
||||
totalIssues: report.summary.totalIssues,
|
||||
criticalIssues: report.summary.criticalIssues,
|
||||
});
|
||||
|
||||
return report;
|
||||
return { report, shouldCache: this.shouldCacheDetection(duration) };
|
||||
}
|
||||
|
||||
async getDetectionReportForRule(
|
||||
ruleId: string,
|
||||
): Promise<BreakingChangeInstanceRuleResult | BreakingChangeWorkflowRuleResult | undefined> {
|
||||
const rule = this.ruleRegistry.getRule(ruleId);
|
||||
if (!rule) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if ('detectWorkflow' in rule) {
|
||||
return (await this.getAllWorkflowRulesResults([rule]))[0];
|
||||
}
|
||||
return (await this.getAllInstanceRulesResults([rule]))[0];
|
||||
}
|
||||
|
||||
private createDetectionReport(
|
||||
targetVersion: BreakingChangeVersion,
|
||||
results: DetectionResult[],
|
||||
rules: IBreakingChangeRule[],
|
||||
): DetectionReport {
|
||||
const criticalIssues = results.filter((r) => {
|
||||
const rule = rules.find((rule) => rule.id === r.ruleId);
|
||||
assert(rule);
|
||||
|
||||
return rule.getMetadata().severity === BreakingChangeSeverity.critical;
|
||||
}).length;
|
||||
|
||||
instanceResults: BreakingChangeInstanceRuleResult[],
|
||||
workflowResults: BreakingChangeWorkflowRuleResult[],
|
||||
): BreakingChangeReportResult['report'] {
|
||||
return {
|
||||
generatedAt: new Date(),
|
||||
targetVersion,
|
||||
currentVersion: N8N_VERSION,
|
||||
summary: {
|
||||
totalIssues: results.length,
|
||||
criticalIssues,
|
||||
},
|
||||
results,
|
||||
workflowResults,
|
||||
instanceResults,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { createNode, createWorkflow } from '../../../__tests__/test-helpers';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../../types';
|
||||
import { BreakingChangeCategory } from '../../../types';
|
||||
import { FileAccessRule } from '../file-access.rule';
|
||||
|
||||
describe('FileAccessRule', () => {
|
||||
|
|
@ -19,7 +19,7 @@ describe('FileAccessRule', () => {
|
|||
title: 'File Access Restrictions',
|
||||
description: 'File access is now restricted to a default directory for security purposes',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.high,
|
||||
severity: 'high',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -61,7 +61,7 @@ describe('FileAccessRule', () => {
|
|||
expect(result.issues[0]).toMatchObject({
|
||||
title: "File access node 'n8n-nodes-base.readWriteFile' with name 'Read File' affected",
|
||||
description: 'File access for this node is now restricted to configured directories.',
|
||||
level: IssueLevel.warning,
|
||||
level: 'warning',
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -75,7 +75,7 @@ describe('FileAccessRule', () => {
|
|||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues[0]).toMatchObject({
|
||||
title: "File access node 'n8n-nodes-base.readBinaryFiles' with name 'Read Binary' affected",
|
||||
level: IssueLevel.warning,
|
||||
level: 'warning',
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { createNode, createWorkflow } from '../../../__tests__/test-helpers';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../../types';
|
||||
import { BreakingChangeCategory } from '../../../types';
|
||||
import { ProcessEnvAccessRule } from '../process-env-access.rule';
|
||||
|
||||
describe('ProcessEnvAccessRule', () => {
|
||||
|
|
@ -19,7 +19,7 @@ describe('ProcessEnvAccessRule', () => {
|
|||
title: 'Block process.env Access in Expressions and Code nodes',
|
||||
description: 'Direct access to process.env is blocked by default for security',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.high,
|
||||
severity: 'high',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -77,8 +77,8 @@ describe('ProcessEnvAccessRule', () => {
|
|||
expect(result.issues[0]).toMatchObject({
|
||||
title: 'process.env access detected',
|
||||
description:
|
||||
"The following nodes contain process.env access: 'Code'. This will be blocked by default in v2.0.0.",
|
||||
level: IssueLevel.error,
|
||||
"Node with name 'Code' accesses process.env which is blocked by default for security reasons.",
|
||||
level: 'error',
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -95,7 +95,7 @@ describe('ProcessEnvAccessRule', () => {
|
|||
expect(result.isAffected).toBe(true);
|
||||
expect(result.issues[0]).toMatchObject({
|
||||
title: 'process.env access detected',
|
||||
level: IssueLevel.error,
|
||||
level: 'error',
|
||||
});
|
||||
expect(result.issues[0].description).toContain('HTTP');
|
||||
});
|
||||
|
|
@ -116,8 +116,8 @@ describe('ProcessEnvAccessRule', () => {
|
|||
const result = await rule.detectWorkflow(workflow, nodesGroupedByType);
|
||||
|
||||
expect(result.issues[0].description).toContain('Code');
|
||||
expect(result.issues[0].description).toContain('HTTP');
|
||||
expect(result.issues[0].description).not.toContain('Set');
|
||||
expect(result.issues[1].description).toContain('HTTP');
|
||||
expect(result.issues).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should not detect false positives', async () => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { createNode, createWorkflow } from '../../../__tests__/test-helpers';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../../types';
|
||||
import { BreakingChangeCategory } from '../../../types';
|
||||
import { RemovedNodesRule } from '../removed-nodes.rule';
|
||||
|
||||
describe('RemovedNodesRule', () => {
|
||||
|
|
@ -19,7 +19,7 @@ describe('RemovedNodesRule', () => {
|
|||
title: 'Removed Deprecated Nodes',
|
||||
description: 'Several deprecated nodes have been removed and will no longer work',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.critical,
|
||||
severity: 'critical',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -77,7 +77,7 @@ describe('RemovedNodesRule', () => {
|
|||
expect(result.issues[0].description).toBe(
|
||||
`The node type '${nodeType}' is no longer available. Please replace it with an alternative.`,
|
||||
);
|
||||
expect(result.issues[0].level).toBe(IssueLevel.error);
|
||||
expect(result.issues[0].level).toBe('error');
|
||||
});
|
||||
|
||||
it('should detect multiple removed nodes in the same workflow', async () => {
|
||||
|
|
|
|||
|
|
@ -1,14 +1,14 @@
|
|||
import { BreakingChangeRecommendation } from '@n8n/api-types';
|
||||
import { WorkflowEntity } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { INode } from 'n8n-workflow';
|
||||
|
||||
import type {
|
||||
BreakingChangeMetadata,
|
||||
WorkflowDetectionResult,
|
||||
Recommendation,
|
||||
BreakingChangeRuleMetadata,
|
||||
IBreakingChangeWorkflowRule,
|
||||
WorkflowDetectionReport,
|
||||
} from '../../types';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../types';
|
||||
import { BreakingChangeCategory } from '../../types';
|
||||
|
||||
@Service()
|
||||
export class FileAccessRule implements IBreakingChangeWorkflowRule {
|
||||
|
|
@ -16,23 +16,22 @@ export class FileAccessRule implements IBreakingChangeWorkflowRule {
|
|||
|
||||
id: string = 'file-access-restriction-v2';
|
||||
|
||||
getMetadata(): BreakingChangeMetadata {
|
||||
getMetadata(): BreakingChangeRuleMetadata {
|
||||
return {
|
||||
version: 'v2',
|
||||
title: 'File Access Restrictions',
|
||||
description: 'File access is now restricted to a default directory for security purposes',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.high,
|
||||
severity: 'high',
|
||||
};
|
||||
}
|
||||
|
||||
async getRecommendations(): Promise<Recommendation[]> {
|
||||
async getRecommendations(): Promise<BreakingChangeRecommendation[]> {
|
||||
return [
|
||||
{
|
||||
action: 'Configure file access paths',
|
||||
description:
|
||||
'Set N8N_RESTRICT_FILE_ACCESS_TO to a semicolon-separated list of allowed paths if workflows need to access files outside the default directory',
|
||||
documentationUrl: this.getMetadata().documentationUrl,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
|
@ -40,7 +39,7 @@ export class FileAccessRule implements IBreakingChangeWorkflowRule {
|
|||
async detectWorkflow(
|
||||
_workflow: WorkflowEntity,
|
||||
nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<WorkflowDetectionResult> {
|
||||
): Promise<WorkflowDetectionReport> {
|
||||
const fileNodes = this.FILE_NODES.flatMap((nodeType) => nodesGroupedByType.get(nodeType) ?? []);
|
||||
if (fileNodes.length === 0) return { isAffected: false, issues: [] };
|
||||
|
||||
|
|
@ -49,7 +48,9 @@ export class FileAccessRule implements IBreakingChangeWorkflowRule {
|
|||
issues: fileNodes.map((node) => ({
|
||||
title: `File access node '${node.type}' with name '${node.name}' affected`,
|
||||
description: 'File access for this node is now restricted to configured directories.',
|
||||
level: IssueLevel.warning,
|
||||
level: 'warning',
|
||||
nodeId: node.id,
|
||||
nodeName: node.name,
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,51 +1,51 @@
|
|||
import { BreakingChangeRecommendation } from '@n8n/api-types';
|
||||
import { WorkflowEntity } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { INode } from 'n8n-workflow';
|
||||
|
||||
import type {
|
||||
BreakingChangeMetadata,
|
||||
BreakingChangeRuleMetadata,
|
||||
IBreakingChangeWorkflowRule,
|
||||
Recommendation,
|
||||
WorkflowDetectionResult,
|
||||
WorkflowDetectionReport,
|
||||
} from '../../types';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../types';
|
||||
import { BreakingChangeCategory } from '../../types';
|
||||
|
||||
@Service()
|
||||
export class ProcessEnvAccessRule implements IBreakingChangeWorkflowRule {
|
||||
id: string = 'process-env-access-v2';
|
||||
getMetadata(): BreakingChangeMetadata {
|
||||
getMetadata(): BreakingChangeRuleMetadata {
|
||||
return {
|
||||
version: 'v2',
|
||||
title: 'Block process.env Access in Expressions and Code nodes',
|
||||
description: 'Direct access to process.env is blocked by default for security',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.high,
|
||||
severity: 'high',
|
||||
};
|
||||
}
|
||||
|
||||
async detectWorkflow(
|
||||
workflow: WorkflowEntity,
|
||||
_nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<WorkflowDetectionResult> {
|
||||
): Promise<WorkflowDetectionReport> {
|
||||
// Match process.env with optional whitespace, newlines, comments between 'process' and '.env'
|
||||
// This covers: process.env, process .env, process/* comment */.env, process\n.env, etc.
|
||||
// Also matches optional chaining: process?.env
|
||||
const processEnvPattern = /process\s*(?:\/\*[\s\S]*?\*\/)?\s*\??\.?\s*env\b/;
|
||||
|
||||
const affectedNodes: string[] = [];
|
||||
const affectedNodes: Array<{ nodeId: string; nodeName: string }> = [];
|
||||
|
||||
workflow.nodes.forEach((node) => {
|
||||
// Check in Code nodes
|
||||
if (node.type === 'n8n-nodes-base.code') {
|
||||
const code = typeof node.parameters?.code === 'string' ? node.parameters.code : undefined;
|
||||
if (code && processEnvPattern.test(code)) {
|
||||
affectedNodes.push(node.name);
|
||||
affectedNodes.push({ nodeId: node.id, nodeName: node.name });
|
||||
}
|
||||
} else {
|
||||
// Check in expressions
|
||||
const nodeJson = JSON.stringify(node.parameters);
|
||||
if (processEnvPattern.test(nodeJson) && !affectedNodes.includes(node.name)) {
|
||||
affectedNodes.push(node.name);
|
||||
if (processEnvPattern.test(nodeJson) && !affectedNodes.some((n) => n.nodeId === node.id)) {
|
||||
affectedNodes.push({ nodeId: node.id, nodeName: node.name });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
@ -53,19 +53,17 @@ export class ProcessEnvAccessRule implements IBreakingChangeWorkflowRule {
|
|||
return {
|
||||
isAffected: affectedNodes.length > 0,
|
||||
issues:
|
||||
affectedNodes.length > 0
|
||||
? [
|
||||
{
|
||||
title: 'process.env access detected',
|
||||
description: `The following nodes contain process.env access: '${affectedNodes.join(', ')}'. This will be blocked by default in v2.0.0.`,
|
||||
level: IssueLevel.error,
|
||||
},
|
||||
]
|
||||
: [],
|
||||
affectedNodes.map((n) => ({
|
||||
title: 'process.env access detected',
|
||||
description: `Node with name '${n.nodeName}' accesses process.env which is blocked by default for security reasons.`,
|
||||
level: 'error',
|
||||
nodeId: n.nodeId,
|
||||
nodeName: n.nodeName,
|
||||
})) || [],
|
||||
};
|
||||
}
|
||||
|
||||
async getRecommendations(): Promise<Recommendation[]> {
|
||||
async getRecommendations(): Promise<BreakingChangeRecommendation[]> {
|
||||
return [
|
||||
{
|
||||
action: 'Remove process.env usage',
|
||||
|
|
|
|||
|
|
@ -1,14 +1,14 @@
|
|||
import { BreakingChangeRecommendation } from '@n8n/api-types';
|
||||
import type { WorkflowEntity } from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
import { INode } from 'n8n-workflow';
|
||||
|
||||
import type {
|
||||
BreakingChangeMetadata,
|
||||
WorkflowDetectionResult,
|
||||
Recommendation,
|
||||
BreakingChangeRuleMetadata,
|
||||
IBreakingChangeWorkflowRule,
|
||||
WorkflowDetectionReport,
|
||||
} from '../../types';
|
||||
import { BreakingChangeSeverity, BreakingChangeCategory, IssueLevel } from '../../types';
|
||||
import { BreakingChangeCategory } from '../../types';
|
||||
|
||||
@Service()
|
||||
export class RemovedNodesRule implements IBreakingChangeWorkflowRule {
|
||||
|
|
@ -20,22 +20,21 @@ export class RemovedNodesRule implements IBreakingChangeWorkflowRule {
|
|||
|
||||
id: string = 'removed-nodes-v2';
|
||||
|
||||
getMetadata(): BreakingChangeMetadata {
|
||||
getMetadata(): BreakingChangeRuleMetadata {
|
||||
return {
|
||||
version: 'v2',
|
||||
title: 'Removed Deprecated Nodes',
|
||||
description: 'Several deprecated nodes have been removed and will no longer work',
|
||||
category: BreakingChangeCategory.workflow,
|
||||
severity: BreakingChangeSeverity.critical,
|
||||
severity: 'critical',
|
||||
};
|
||||
}
|
||||
|
||||
async getRecommendations(): Promise<Recommendation[]> {
|
||||
async getRecommendations(): Promise<BreakingChangeRecommendation[]> {
|
||||
return [
|
||||
{
|
||||
action: 'Update affected workflows',
|
||||
description: 'Replace removed nodes with their updated versions or alternatives',
|
||||
documentationUrl: this.getMetadata().documentationUrl,
|
||||
},
|
||||
];
|
||||
}
|
||||
|
|
@ -43,7 +42,7 @@ export class RemovedNodesRule implements IBreakingChangeWorkflowRule {
|
|||
async detectWorkflow(
|
||||
_workflow: WorkflowEntity,
|
||||
nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<WorkflowDetectionResult> {
|
||||
): Promise<WorkflowDetectionReport> {
|
||||
const removedNodes = this.REMOVED_NODES.flatMap((type) => nodesGroupedByType.get(type) ?? []);
|
||||
if (removedNodes.length === 0) return { isAffected: false, issues: [] };
|
||||
|
||||
|
|
@ -52,7 +51,9 @@ export class RemovedNodesRule implements IBreakingChangeWorkflowRule {
|
|||
issues: removedNodes.map((node) => ({
|
||||
title: `Node '${node.type}' with name '${node.name}' has been removed`,
|
||||
description: `The node type '${node.type}' is no longer available. Please replace it with an alternative.`,
|
||||
level: IssueLevel.error,
|
||||
level: 'error',
|
||||
nodeId: node.id,
|
||||
nodeName: node.name,
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,15 +1,16 @@
|
|||
import type { DetectionResult } from './rule.types';
|
||||
import type {
|
||||
BreakingChangeInstanceIssue,
|
||||
BreakingChangeRecommendation,
|
||||
BreakingChangeWorkflowIssue,
|
||||
} from '@n8n/api-types';
|
||||
|
||||
/**
|
||||
* Detection report matching the UI structure
|
||||
*/
|
||||
export interface DetectionReport {
|
||||
generatedAt: Date;
|
||||
targetVersion: string;
|
||||
currentVersion: string;
|
||||
summary: {
|
||||
totalIssues: number;
|
||||
criticalIssues: number;
|
||||
};
|
||||
results: DetectionResult[]; // Keep raw results for debugging
|
||||
export interface WorkflowDetectionReport {
|
||||
isAffected: boolean;
|
||||
issues: BreakingChangeWorkflowIssue[]; // List of issues affecting this workflow
|
||||
}
|
||||
|
||||
export interface InstanceDetectionReport {
|
||||
isAffected: boolean;
|
||||
instanceIssues: BreakingChangeInstanceIssue[];
|
||||
recommendations: BreakingChangeRecommendation[];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,12 +1,12 @@
|
|||
import type {
|
||||
BreakingChangeAffectedWorkflow,
|
||||
BreakingChangeRecommendation,
|
||||
BreakingChangeRuleSeverity,
|
||||
} from '@n8n/api-types';
|
||||
import type { WorkflowEntity } from '@n8n/db';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
|
||||
export const enum BreakingChangeSeverity {
|
||||
critical = 'critical',
|
||||
high = 'high',
|
||||
medium = 'medium',
|
||||
low = 'low',
|
||||
}
|
||||
import type { InstanceDetectionReport, WorkflowDetectionReport } from './detection.types';
|
||||
|
||||
export const enum BreakingChangeCategory {
|
||||
workflow = 'workflow',
|
||||
|
|
@ -16,74 +16,34 @@ export const enum BreakingChangeCategory {
|
|||
infrastructure = 'infrastructure',
|
||||
}
|
||||
|
||||
export const enum IssueLevel {
|
||||
error = 'error',
|
||||
warning = 'warning',
|
||||
info = 'info',
|
||||
}
|
||||
|
||||
export type BreakingChangeVersion = 'v2';
|
||||
|
||||
export interface BreakingChangeMetadata {
|
||||
export interface BreakingChangeRuleMetadata {
|
||||
version: BreakingChangeVersion;
|
||||
title: string;
|
||||
description: string;
|
||||
category: BreakingChangeCategory;
|
||||
severity: BreakingChangeSeverity;
|
||||
documentationUrl?: string;
|
||||
}
|
||||
|
||||
export interface WorkflowDetectionResult {
|
||||
isAffected: boolean;
|
||||
issues: DetectionIssue[]; // List of issues affecting this workflow
|
||||
}
|
||||
|
||||
export interface InstanceDetectionResult {
|
||||
isAffected: boolean;
|
||||
instanceIssues: DetectionIssue[];
|
||||
recommendations: Recommendation[];
|
||||
}
|
||||
|
||||
export type AffectedWorkflow = Omit<WorkflowDetectionResult, 'isAffected'> & {
|
||||
id: string;
|
||||
name: string;
|
||||
active: boolean;
|
||||
};
|
||||
|
||||
export interface DetectionResult {
|
||||
ruleId: string;
|
||||
affectedWorkflows: AffectedWorkflow[];
|
||||
instanceIssues: DetectionIssue[];
|
||||
recommendations: Recommendation[];
|
||||
}
|
||||
|
||||
export interface DetectionIssue {
|
||||
title: string; // e.g., "Environment Variables", "Database Configuration"
|
||||
description: string;
|
||||
level: IssueLevel; // error, warning, info
|
||||
}
|
||||
|
||||
export interface Recommendation {
|
||||
action: string;
|
||||
description: string;
|
||||
severity: BreakingChangeRuleSeverity;
|
||||
documentationUrl?: string;
|
||||
}
|
||||
|
||||
export interface IBreakingChangeInstanceRule {
|
||||
id: string;
|
||||
getMetadata(): BreakingChangeMetadata;
|
||||
detect(): Promise<InstanceDetectionResult>;
|
||||
getMetadata(): BreakingChangeRuleMetadata;
|
||||
detect(): Promise<InstanceDetectionReport>;
|
||||
}
|
||||
|
||||
export interface IBreakingChangeWorkflowRule {
|
||||
id: string;
|
||||
getMetadata(): BreakingChangeMetadata;
|
||||
getRecommendations(workflowResults: AffectedWorkflow[]): Promise<Recommendation[]>;
|
||||
getMetadata(): BreakingChangeRuleMetadata;
|
||||
getRecommendations(
|
||||
workflowResults: BreakingChangeAffectedWorkflow[],
|
||||
): Promise<BreakingChangeRecommendation[]>;
|
||||
// The detectWorkflow function includes the nodes grouped by type for more efficient processing
|
||||
detectWorkflow(
|
||||
workflow: WorkflowEntity,
|
||||
nodesGroupedByType: Map<string, INode[]>,
|
||||
): Promise<WorkflowDetectionResult>;
|
||||
): Promise<WorkflowDetectionReport>;
|
||||
}
|
||||
|
||||
export type IBreakingChangeRule = IBreakingChangeInstanceRule | IBreakingChangeWorkflowRule;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user