diff --git a/.changeset/forty-areas-remain.md b/.changeset/forty-areas-remain.md new file mode 100644 index 000000000..c01a3887d --- /dev/null +++ b/.changeset/forty-areas-remain.md @@ -0,0 +1,5 @@ +--- +'@openfn/lightning-mock': patch +--- + +Add validation to the provisioner endpoint diff --git a/.claude/yaml-formats.md b/.claude/yaml-formats.md new file mode 100644 index 000000000..35ecc24dd --- /dev/null +++ b/.claude/yaml-formats.md @@ -0,0 +1,96 @@ +# OpenFn Project YAML Formats + +Two YAML formats are used across the monorepo. The key distinction: **v1** uses objects keyed by ID; **v2** uses arrays. + +## v1 (Lightning app state) + +Used by `packages/deploy` and sent to/from the Lightning API (`Provisioner.Project` type from `@openfn/lexicon/lightning`). + +- `workflows` is a keyed object (`{ [slug]: Workflow }`) +- Each workflow has `jobs`, `triggers`, and `edges` as keyed objects +- Steps are called `jobs`; code is stored in `body` +- Credentials referenced by UUID (`project_credential_id`) +- No version marker — absence of `schema_version`/`cli.version` means v1 + +```yaml +id: abc-123 +name: My Project +project_credentials: + - id: cred-uuid + name: My Credential + owner: admin@openfn.org +workflows: + my-workflow: + id: wf-uuid + name: My Workflow + jobs: + transform-data: + id: job-uuid + name: Transform data + body: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' + project_credential_id: cred-uuid + keychain_credential_id: null + triggers: + webhook: + id: trig-uuid + type: webhook + enabled: true + edges: + trigger->transform-data: + id: edge-uuid + enabled: true + source_trigger_id: trig-uuid + target_job_id: job-uuid +``` + +## v2 (local project state) + +Used by `packages/project` and the CLI project subcommands (`ProjectState` type from `@openfn/lexicon`). + +- Identified by `schema_version` field (current: `'4.0'`) or legacy `cli.version: 2` +- `workflows` is an array +- Each workflow has a `steps` array; triggers are steps with a `type` field +- Code stored in `expression`; edges expressed inline via `next` map on each step +- Credentials referenced by name string (`configuration`) + +```yaml +id: my-project +name: My Project +schema_version: '4.0' +credentials: + - uuid: cred-uuid + name: My Credential + owner: admin@openfn.org +workflows: + - id: my-workflow + name: My Workflow + start: webhook + steps: + - id: webhook + type: webhook + enabled: true + next: + transform-data: + condition: always + - id: transform-data + name: Transform data + expression: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' + configuration: 'admin@openfn.org|My Credential' +``` + +## Detection logic + +Use `detectVersion(data)` from `@openfn/project` — returns `1` or `2`. Accepts YAML/JSON string or pre-parsed object. + +```typescript +import { detectVersion } from '@openfn/project'; +if (detectVersion(json) === 2) { /* v2 */ } +``` + +## Conversion + +- **v2 → v1**: `Project.from('project', json).then(p => p.serialize('state', { format: 'yaml' }))` — see `maybeConvertV2spec` in `packages/cli/src/deploy/handler.ts` +- **v1 → v2**: `Project.from('state', json)` — see `packages/project/src/parse/from-app-state.ts` +- Full conversion logic: `packages/project/src/serialize/to-app-state.ts` (v2→v1) and `packages/project/src/parse/from-app-state.ts` (v1→v2) diff --git a/CLAUDE.md b/CLAUDE.md index c1744b67e..d88984188 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,6 +131,7 @@ cd packages/cli && pnpm test:watch # Watch mode The [.claude](.claude) folder contains detailed guides: - **[event-processor.md](.claude/event-processor.md)** - Worker event processing deep-dive (ordering, batching) — companion to `packages/ws-worker/CLAUDE.md` +- **[yaml-formats.md](.claude/yaml-formats.md)** - v1 vs v2 project YAML formats: structure, detection logic, and conversion paths Key packages also carry their own `CLAUDE.md` (runtime, engine-multi, ws-worker), auto-loaded when you work in them. diff --git a/integration-tests/cli/test/deploy.test.ts b/integration-tests/cli/test/deploy.test.ts index 607cd6b82..0cc5761b9 100644 --- a/integration-tests/cli/test/deploy.test.ts +++ b/integration-tests/cli/test/deploy.test.ts @@ -1,6 +1,6 @@ import test from 'ava'; import path from 'node:path'; -import fs from 'node:fs/promises'; +import fs, { rm } from 'node:fs/promises'; import run from '../src/run'; import createLightningServer from '@openfn/lightning-mock'; import { extractLogs, assertLog } from '../src/util'; @@ -12,6 +12,50 @@ const port = 8967; const endpoint = `http://localhost:${port}`; let tmpDir = path.resolve('tmp/deploy'); +const testProjectV2 = ` +id: my-project +name: My Project +schema_version: '4.0' +workflows: + - id: my-workflow + name: My Workflow + start: webhook + steps: + - id: webhook + type: webhook + enabled: true + next: + transform-data: {} + - id: transform-data + name: Transform data + expression: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' +`.trim(); + +const testProjectV2WithCredential = ` +id: my-project +name: My Project +schema_version: '4.0' +credentials: + - name: http1 + owner: super@openfn.org +workflows: + - id: my-workflow + name: My Workflow + start: webhook + steps: + - id: webhook + type: webhook + enabled: true + next: + transform-data: {} + - id: transform-data + name: Transform data + expression: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' + configuration: super@openfn.org|http1 +`.trim(); + const testProject = ` name: test-project workflows: @@ -97,7 +141,6 @@ test.serial('deploy a local project', async (t) => { --log-json \ -l debug` ); - t.falsy(stderr); const logs = extractLogs(stdout); @@ -269,6 +312,51 @@ test.serial('redirect to v2 protocol if openfn.yaml is present', async (t) => { ); }); +test.serial('deploy a v2 spec file', async (t) => { + const testProjectV2 = ` +name: test-project +schema_version: '4.0' +workflows: + - id: my-workflow + name: My Workflow + start: webhook + steps: + - id: webhook + type: webhook + enabled: true + next: + my-job: {} + - id: my-job + name: My Job + expression: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' +`.trim(); + + await fs.writeFile(path.join(tmpDir, 'project.yaml'), testProjectV2); + + t.is(Object.keys(server.state.projects).length, 0); + + const { stdout, stderr } = await run( + `openfn deploy \ + --project-path ${tmpDir}/project.yaml \ + --state-path ${tmpDir}/.state.json \ + --no-confirm \ + --log-json \ + -l debug` + ); + t.falsy(stderr); + + const logs = extractLogs(stdout); + assertLog(t, logs, /v2 spec/i); + assertLog(t, logs, /Deployed/); + + t.is(Object.keys(server.state.projects).length, 1); + const [project] = Object.values(server.state.projects) as any[]; + t.is(project.name, 'test-project'); + const [workflow] = Object.values(project.workflows) as any[]; + t.is(workflow.name, 'My Workflow'); +}); + test.serial('deploy then pull, changes one workflow, deploy', async (t) => { t.is(Object.keys(server.state.projects).length, 0); @@ -318,7 +406,6 @@ test.serial('deploy then pull, changes one workflow, deploy', async (t) => { // And deploy those changes const { stdout, stderr } = await run(deployCmd); - t.falsy(stderr); const logs = extractLogs(stdout); @@ -332,3 +419,63 @@ test.serial('deploy then pull, changes one workflow, deploy', async (t) => { t.is(Object.keys(server.state.projects).length, 1); t.truthy(server.state.projects[projectId]); }); + +test.serial('deploy a v2 project.yaml', async (t) => { + await fs.writeFile(path.join(tmpDir, 'project.yaml'), testProjectV2); + + const { stdout, stderr } = await run( + `openfn deploy \ + --project-path ${tmpDir}/project.yaml \ + --state-path ${tmpDir}/.state.json \ + --no-confirm \ + --log-json \ + -l debug` + ); + + t.falsy(stderr); + + const logs = extractLogs(stdout); + assertLog(t, logs, /Deployed/); + + t.is(Object.keys(server.state.projects).length, 1); + const [project] = Object.values(server.state.projects) as any[]; + t.is(project.name, 'My Project'); +}); + +test.serial('deploy a new v2 project.yaml with credentials', async (t) => { + await fs.writeFile( + path.join(tmpDir, 'project.yaml'), + testProjectV2WithCredential + ); + + try { + await rm(`${tmpDir}/.state.json`); + } catch (e) { + // ignore + } + + const { stdout, stderr } = await run( + `openfn deploy \ + --project-path ${tmpDir}/project.yaml \ + --no-confirm \ + --log-json \ + -l debug` + ); + + t.falsy(stderr); + + const logs = extractLogs(stdout); + assertLog(t, logs, /Deployed/); + + t.is(Object.keys(server.state.projects).length, 1); + const [project] = Object.values(server.state.projects) as any[]; + t.is(project.name, 'My Project'); + + t.is(project.project_credentials[0].name, 'http1'); + t.is(project.project_credentials[0].owner, 'super@openfn.org'); + + const uuid = project.project_credentials[0].id; + + const workflow: any = Object.values(project.workflows).pop(); + t.is(workflow.jobs[0].project_credential_id, uuid); +}); diff --git a/packages/cli/src/deploy/handler.ts b/packages/cli/src/deploy/handler.ts index 13b0d7fd7..767e59231 100644 --- a/packages/cli/src/deploy/handler.ts +++ b/packages/cli/src/deploy/handler.ts @@ -10,7 +10,7 @@ import { DeployOptions } from './command'; import * as beta from '../projects/deploy'; import path from 'node:path'; import { fileExists } from '../util/file-exists'; -import { yamlToJson } from '@openfn/project'; +import Project, { detectVersion, yamlToJson } from '@openfn/project'; import fs from 'node:fs/promises'; export type DeployFn = typeof deploy; @@ -62,6 +62,15 @@ async function deployHandler( config.endpoint = process.env['OPENFN_ENDPOINT']; } + const rawSpec = await fs.readFile(config.specPath, 'utf-8'); + const convertedSpec = await maybeConvertV2spec(rawSpec); + if (convertedSpec !== rawSpec) { + logger.info( + 'Detected v2 spec file - converting to legacy format; validation will be skipped.' + ); + config.spec = convertedSpec; + } + logger.debug('Deploying with config', config); logger.info(`Deploying`); @@ -137,4 +146,16 @@ const redirectTov2 = async ( ); }; +export const maybeConvertV2spec = async (yaml: string): Promise => { + const json = yamlToJson(yaml) as any; + if (detectVersion(json) > 1) { + const project = await Project.from('project', json); + return project.serialize('state', { + format: 'yaml', + asSpec: true, + }) as string; + } + return yaml; +}; + export default deployHandler; diff --git a/packages/cli/test/deploy/deploy.test.ts b/packages/cli/test/deploy/deploy.test.ts index 0e9ea411d..db0dfe633 100644 --- a/packages/cli/test/deploy/deploy.test.ts +++ b/packages/cli/test/deploy/deploy.test.ts @@ -3,7 +3,11 @@ import test from 'ava'; import mockfs from 'mock-fs'; import { Logger, createMockLogger } from '@openfn/logger'; -import deployHandler, { DeployFn } from '../../src/deploy/handler'; +import deployHandler, { + DeployFn, + maybeConvertV2spec, +} from '../../src/deploy/handler'; +import { yamlToJson } from '@openfn/project'; import { DeployError, type DeployConfig } from '@openfn/deploy'; import { DeployOptions } from '../../src/deploy/command'; @@ -183,3 +187,136 @@ test.serial('catches DeployErrors', async (t) => { t.is(process.exitCode, 10); process.exitCode = origExitCode; }); + +const v1Yaml = `id: '1234' +name: My Project +workflows: + my-workflow: + id: job-1 + name: My Workflow + jobs: + transform-data: + id: job-1 + name: Transform data + body: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' + project_credential_id: null + keychain_credential_id: null + triggers: + webhook: + id: trig-1 + type: webhook + enabled: true + edges: + trigger->transform-data: + id: edge-1 + enabled: true + source_trigger_id: trig-1 + target_job_id: job-1 +project_credentials: [] +`; + +const v2Yaml = `id: my-project +name: My Project +schema_version: '4.0' +credentials: + - name: http1 + owner: super@openfn.org +workflows: + - id: my-workflow + name: My Workflow + start: webhook + steps: + - id: webhook + type: webhook + enabled: true + next: + transform-data: {} + - id: transform-data + name: Transform data + expression: 'fn(s => s)' + adaptor: '@openfn/language-common@latest' + configuration: super@openfn.org|http1 +`; + +test('maybeConvertV2spec: returns v1 yaml unchanged', async (t) => { + const result = await maybeConvertV2spec(v1Yaml); + t.is(result, v1Yaml); +}); + +test('maybeConvertV2spec: converts v2 to v1', async (t) => { + const result = await maybeConvertV2spec(v2Yaml); + const json = yamlToJson(result) as any; + + // v1 has workflows as a keyed object + t.is(typeof json.workflows, 'object'); + t.false(Array.isArray(json.workflows)); + + // v1 uses jobs, not steps + const workflow = Object.values(json.workflows)[0] as any; + t.truthy(workflow.jobs); + t.falsy(workflow.steps); + t.truthy(workflow.triggers); + + // no uuids + const edge = workflow.edges['webhook->transform-data']; + t.is(edge.target_job, 'transform-data'); + t.is(edge.source_trigger, 'webhook'); + t.falsy(workflow.jobs['transform-data'].id); + + // no v2 marker + t.falsy(json.schema_version); +}); + +test('maybeConvertV2spec: converts with credentials', async (t) => { + const result = await maybeConvertV2spec(v2Yaml); + const json = yamlToJson(result) as any; + + t.deepEqual(json.credentials, { + 'super@openfn.org|http1': { name: 'http1', owner: 'super@openfn.org' }, + }); + + t.is( + json.workflows['my-workflow'].jobs['transform-data'].credential, + 'super@openfn.org|http1' + ); +}); + +test('maybeConvertV2spec: converted edges use key references, not UUIDs', async (t) => { + const result = await maybeConvertV2spec(v2Yaml); + const json = yamlToJson(result) as any; + + const workflow = Object.values(json.workflows)[0] as any; + const edge = Object.values(workflow.edges)[0] as any; + + // edge must use spec format (key references) so mergeSpecIntoState can resolve them + t.truthy(edge.source_trigger); + t.truthy(edge.target_job); + t.falsy(edge.source_trigger_id); + t.falsy(edge.target_job_id); + + // source_trigger must match a trigger key; target_job must match a job key + t.truthy(workflow.triggers[edge.source_trigger]); + t.truthy(workflow.jobs[edge.target_job]); +}); + +test('maybeConvertV2spec: converts legacy v2 (cli.version: 2) to v1', async (t) => { + const legacyV2Yaml = `id: my-project +name: My Project +cli: + version: 2 +workflows: + - id: my-workflow + name: My Workflow + start: webhook + steps: + - id: webhook + type: webhook + enabled: true +`; + const result = await maybeConvertV2spec(legacyV2Yaml); + const json = yamlToJson(result) as any; + + t.is(typeof json.workflows, 'object'); + t.false(Array.isArray(json.workflows)); +}); diff --git a/packages/cli/test/projects/deploy.test.ts b/packages/cli/test/projects/deploy.test.ts index b0c1774e5..d6ac61ca8 100644 --- a/packages/cli/test/projects/deploy.test.ts +++ b/packages/cli/test/projects/deploy.test.ts @@ -19,7 +19,6 @@ import { TWO_WORKFLOWS_UUID, } from './fixtures'; import { checkout } from '../../src/projects'; -import { readFileSync } from 'node:fs'; let server: any; const logger = createMockLogger(undefined, { level: 'debug' }); diff --git a/packages/deploy/src/index.ts b/packages/deploy/src/index.ts index d5aaad97e..215aceb38 100644 --- a/packages/deploy/src/index.ts +++ b/packages/deploy/src/index.ts @@ -2,7 +2,7 @@ import { confirm } from '@inquirer/prompts'; import { inspect } from 'node:util'; import { DeployConfig, ProjectState } from './types'; import { readFile, writeFile } from 'fs/promises'; -import { parseAndValidate } from './validator'; +import { parseAndValidate, parseSpec } from './validator'; import jsondiff from 'json-diff'; import { mergeProjectPayloadIntoState, @@ -108,8 +108,8 @@ export async function getSpec(path: string) { export async function deploy(config: DeployConfig, logger: Logger) { const [state, spec] = await Promise.all([ - getState(config.statePath), - getSpec(config.specPath), + config.state ?? getState(config.statePath), + config.spec ? parseSpec(config.spec) : getSpec(config.specPath), ]); logger.debug('spec', spec); @@ -118,7 +118,6 @@ export async function deploy(config: DeployConfig, logger: Logger) { throw new DeployError(`${config.specPath} has errors`, 'VALIDATION_ERROR'); } const nextState = mergeSpecIntoState(state, spec.doc); - validateProjectState(nextState); // Convert the state to a payload for the API. diff --git a/packages/deploy/src/types.ts b/packages/deploy/src/types.ts index d8dbf4444..c53b2886e 100644 --- a/packages/deploy/src/types.ts +++ b/packages/deploy/src/types.ts @@ -193,4 +193,6 @@ export interface DeployConfig { requireConfirmation: boolean; dryRun: boolean; apiKey: string | null; + spec?: string; + state?: ProjectState; } diff --git a/packages/deploy/src/validator.ts b/packages/deploy/src/validator.ts index 6c79750f5..b578ac5a8 100644 --- a/packages/deploy/src/validator.ts +++ b/packages/deploy/src/validator.ts @@ -3,6 +3,10 @@ import { ProjectSpec } from './types'; import { readFile } from 'fs/promises'; import path from 'path'; +export function parseSpec(input: string) { + return { errors: [] as Error[], doc: YAML.parse(input) as ProjectSpec }; +} + export interface Error { context: any; message: string; diff --git a/packages/lightning-mock/src/api-rest.ts b/packages/lightning-mock/src/api-rest.ts index 0727c8f74..6dc8d6e64 100644 --- a/packages/lightning-mock/src/api-rest.ts +++ b/packages/lightning-mock/src/api-rest.ts @@ -83,6 +83,45 @@ workflows: enabled: true `; +// Validates a provisioner payload, returning an error body if invalid or null if valid. +// Mirrors Lightning's error format so deploy code sees realistic rejection responses. +export function validateProvisionPayload( + incoming: any +): Record | null { + const workflowErrors: Record = {}; + + const wfList: any[] = Array.isArray(incoming.workflows) + ? incoming.workflows + : Object.values(incoming.workflows ?? {}); + + for (const wf of wfList) { + const edgeErrors: Record = {}; + const edgeList: any[] = Array.isArray(wf.edges) + ? wf.edges + : Object.values(wf.edges ?? {}); + + for (const edge of edgeList) { + if (!edge.delete && !edge.source_trigger_id && !edge.source_job_id) { + const key = edge.id ?? '->'; + edgeErrors[key] = { + source_job_id: ['source_job_id or source_trigger_id must be present'], + }; + } + } + + if (Object.keys(edgeErrors).length > 0) { + const wfKey = wf.name ?? wf.id ?? 'unknown'; + workflowErrors[wfKey] = { edges: edgeErrors }; + } + } + + if (Object.keys(workflowErrors).length > 0) { + return { errors: { workflows: workflowErrors } }; + } + + return null; +} + export default ( app: DevServer, state: ServerState, @@ -121,6 +160,14 @@ export default ( router.post('/api/provision', (ctx) => { const incoming: any = ctx.request.body; + + const validationErrors = validateProvisionPayload(incoming); + if (validationErrors) { + ctx.response.status = 422; + ctx.response.body = validationErrors; + return; + } + const now = new Date().toISOString(); if (!state.projects[incoming.id]) { diff --git a/packages/lightning-mock/test/rest.test.ts b/packages/lightning-mock/test/rest.test.ts index d5dc67ed9..87db89a96 100644 --- a/packages/lightning-mock/test/rest.test.ts +++ b/packages/lightning-mock/test/rest.test.ts @@ -2,7 +2,7 @@ import test from 'ava'; import { setup } from './util'; -import { DEFAULT_PROJECT_ID } from '../src/api-rest'; +import { DEFAULT_PROJECT_ID, validateProvisionPayload } from '../src/api-rest'; // @ts-ignore let server: any; @@ -84,3 +84,140 @@ test.serial("should return 404 if a collection isn't found", async (t) => { }); test.todo("should return 403 if a collection isn't authorized"); + +test('validateProvisionPayload: returns null for a valid edge with source_trigger_id', (t) => { + const payload = { + id: 'proj-1', + workflows: [ + { + name: 'wf1', + edges: [ + { + id: 'e1', + source_trigger_id: 'trig-uuid', + target_job_id: 'job-uuid', + enabled: true, + }, + ], + }, + ], + }; + t.is(validateProvisionPayload(payload), null); +}); + +test('validateProvisionPayload: returns null for a valid edge with source_job_id', (t) => { + const payload = { + id: 'proj-1', + workflows: [ + { + name: 'wf1', + edges: [ + { + id: 'e1', + source_job_id: 'job-uuid', + target_job_id: 'job-uuid-2', + enabled: true, + }, + ], + }, + ], + }; + t.is(validateProvisionPayload(payload), null); +}); + +test('validateProvisionPayload: returns errors when edge has no source', (t) => { + const payload = { + id: 'proj-1', + workflows: [ + { + name: 'wf1', + edges: [ + { + id: 'edge-1', + source_trigger_id: null, + target_job_id: '', + enabled: true, + }, + ], + }, + ], + }; + const result = validateProvisionPayload(payload); + t.truthy(result); + t.deepEqual(result, { + errors: { + workflows: { + wf1: { + edges: { + 'edge-1': { + source_job_id: [ + 'source_job_id or source_trigger_id must be present', + ], + }, + }, + }, + }, + }, + }); +}); + +test('validateProvisionPayload: returns null for deleted edges', (t) => { + const payload = { + id: 'proj-1', + workflows: [ + { + name: 'wf1', + edges: [ + { + id: 'edge-1', + delete: true, + }, + ], + }, + ], + }; + const result = validateProvisionPayload(payload); + t.falsy(result); +}); + +test('validateProvisionPayload: returns null when there are no edges', (t) => { + const payload = { + id: 'proj-1', + workflows: [{ name: 'wf1', edges: [] }], + }; + t.is(validateProvisionPayload(payload), null); +}); + +test.serial( + 'should return 422 when a workflow edge has no source', + async (t) => { + const response = await fetch(`${endpoint}/api/provision`, { + method: 'POST', + body: JSON.stringify({ + id: 'bad-proj', + name: 'Bad Project', + workflows: [ + { + id: 'wf-uuid', + name: 'wf1', + jobs: [], + triggers: [], + edges: [ + { + id: 'e1', + source_trigger_id: null, + target_job_id: '', + enabled: true, + }, + ], + }, + ], + }), + headers: { 'content-type': 'application/json' }, + }); + + t.is(response.status, 422); + const body = await response.json(); + t.truthy(body.errors?.workflows?.wf1?.edges); + } +); diff --git a/packages/project/src/index.ts b/packages/project/src/index.ts index f80d48072..a46c440f9 100644 --- a/packages/project/src/index.ts +++ b/packages/project/src/index.ts @@ -24,4 +24,6 @@ export { export { mapWorkflow } from './parse/from-app-state'; +export { default as detectVersion } from './util/detect-version'; + export type { MergeProjectOptions } from './merge/merge-project'; diff --git a/packages/project/src/parse/from-project.ts b/packages/project/src/parse/from-project.ts index 0f2e8f765..0e834f697 100644 --- a/packages/project/src/parse/from-project.ts +++ b/packages/project/src/parse/from-project.ts @@ -4,6 +4,7 @@ import Project from '../Project'; import ensureJson from '../util/ensure-json'; import { Provisioner } from '@openfn/lexicon/lightning'; import fromAppState, { fromAppStateConfig } from './from-app-state'; +import detectVersion from '../util/detect-version'; // Load a project from any JSON or yaml representation // This is backwards-compatible with v1 state.json files @@ -21,11 +22,7 @@ export default ( // first ensure the data is in JSON format let rawJson = ensureJson(data); - if ( - rawJson.schema_version || - rawJson.cli?.version === 2 || - rawJson.version /*deprecated*/ - ) { + if (detectVersion(rawJson) > 1) { return new Project(from_v2(rawJson as SerializedProject), config); } diff --git a/packages/project/src/serialize/to-app-state.ts b/packages/project/src/serialize/to-app-state.ts index 7805910a1..5864c2a25 100644 --- a/packages/project/src/serialize/to-app-state.ts +++ b/packages/project/src/serialize/to-app-state.ts @@ -10,7 +10,16 @@ import Workflow from '../Workflow'; import slugify from '../util/slugify'; import getCredentialName from '../util/get-credential-name'; -type Options = { format?: 'json' | 'yaml' }; +type Options = { + format?: 'json' | 'yaml'; + /** + * Serialize the project into a v1 spec format (not state) + * This is awkward and ugly but should only be a temporary solution + * If we decide we need it long term, we should generate a separate + * to-app-spec function which does a more focused job of it. + */ + asSpec?: boolean; +}; const defaultJobProps = { // TODO why does the provisioner throw if these keys are not set? @@ -41,22 +50,34 @@ export default function ( state.id = (uuid as string) ?? randomUUID(); Object.assign(state, rest, project.options); - - const credentialsWithUuids = - project.credentials?.map((c) => ({ - ...c, - uuid: (c as CredentialState).uuid ?? randomUUID(), - })) ?? []; - - state.project_credentials = credentialsWithUuids.map((c) => ({ - // note the subtle conversion here - id: c.uuid as string, - name: c.name, - owner: c.owner, - })); + if (options.asSpec) { + for (const c of project.credentials) { + // note that credentials for a spec file are not the + // the same format as a state file, + // so typings break here + (state as any).credentials ??= {}; + (state as any).credentials[getCredentialName(c)] = { + name: c.name, + owner: c.owner, + }; + } + } else { + const credentialsWithUuids = + project.credentials?.map((c) => ({ + ...c, + uuid: (c as CredentialState).uuid ?? randomUUID(), + })) ?? []; + + state.project_credentials = credentialsWithUuids.map((c) => ({ + // note the subtle conversion here + id: c.uuid as string, + name: c.name, + owner: c.owner, + })); + } state.workflows = project.workflows - .map((w) => mapWorkflow(w, credentialsWithUuids)) + .map((w) => mapWorkflow(w, project.credentials, options)) .reduce((obj: any, wf) => { obj[slugify(wf.name ?? wf.id)] = wf; return obj; @@ -75,8 +96,11 @@ export default function ( export const mapWorkflow = ( workflow: Workflow, - credentials: CredentialState[] = [] + credentials: CredentialState[] = [], + options: Options = {} ) => { + const useUuids = !options.asSpec; + if (workflow instanceof Workflow) { // @ts-ignore workflow = workflow.toJSON(); @@ -85,28 +109,31 @@ export const mapWorkflow = ( const { uuid, ...originalOpenfnProps } = workflow.openfn ?? {}; const wfState = { ...originalOpenfnProps, - id: workflow.openfn?.uuid ?? randomUUID(), jobs: {}, triggers: {}, edges: {}, lock_version: workflow.openfn?.lock_version ?? null, // TODO needs testing } as Provisioner.Workflow; + if (useUuids) { + wfState.id = (workflow.openfn?.uuid ?? randomUUID) as any; + } + if (workflow.name) { wfState.name = workflow.name; } - // lookup of local-ids to project-ids + // lookup of local-ids to project-ids (only needed when using UUIDs) const lookup = workflow.steps.reduce((obj, next) => { - if (!next.openfn?.uuid) { - // If there's no tracked id, we generate one here - // TODO there is no unit test on this - next.openfn ??= {}; - next.openfn.uuid = randomUUID(); + if (useUuids) { + if (!next.openfn?.uuid) { + // If there's no tracked id, we generate one here + next.openfn ??= {}; + next.openfn.uuid = randomUUID(); + } + // @ts-ignore + obj[next.id] = next.openfn.uuid; } - - // @ts-ignore - obj[next.id] = next.openfn.uuid; return obj; }, {}) as Record; @@ -122,13 +149,15 @@ export const mapWorkflow = ( node = { ...rest, type: s.type ?? 'webhook', // this is mostly for tests - ...renameKeys(openfn, { uuid: 'id' }), + ...(useUuids ? renameKeys(openfn, { uuid: 'id' }) : {}), } as Provisioner.Trigger; wfState.triggers[node.type] = node; } else { node = omitBy(pick(s, ['name', 'adaptor']), isNil) as Provisioner.Job; const { uuid, ...otherOpenFnProps } = s.openfn ?? {}; - node.id = uuid; + if (useUuids) { + node.id = uuid; + } if (s.expression) { node.body = s.expression; } @@ -142,19 +171,19 @@ export const mapWorkflow = ( const name = getCredentialName(c); return name === projectCredentialId; }); - if (mappedCredential) { + if (mappedCredential && useUuids) { projectCredentialId = mappedCredential.uuid; } - // else { - // console.warn(`WARING! Failed to map credential ${projectCredentialId} - Lightning may throw an error. - // Ensure the credential exists in project.yaml and try again (maybe ensure the credential is attached to the project in the app and run project fetch)`); - // } - otherOpenFnProps.project_credential_id = projectCredentialId; + if (useUuids) { + otherOpenFnProps.project_credential_id = projectCredentialId; + } else { + otherOpenFnProps.credential = projectCredentialId; + } } } - Object.assign(node, defaultJobProps, otherOpenFnProps); + Object.assign(node, useUuids ? defaultJobProps : {}, otherOpenFnProps); wfState.jobs[s.id ?? slugify(s.name)] = node; } @@ -165,18 +194,31 @@ export const mapWorkflow = ( const { uuid, ...otherOpenFnProps } = rules.openfn ?? {}; - const e = { - id: uuid ?? randomUUID(), - target_job_id: lookup[next], - enabled: !rules.disabled, - source_trigger_id: null, // lightning complains if this isn't set, even if its falsy :( - } as Provisioner.Edge; - Object.assign(e, otherOpenFnProps); - - if (isTrigger) { - e.source_trigger_id = node.id; + let e: any; + if (useUuids) { + e = { + id: uuid ?? randomUUID(), + target_job_id: lookup[next], + enabled: !rules.disabled, + source_trigger_id: null, // lightning complains if this isn't set, even if its falsy :( + } as Provisioner.Edge; + Object.assign(e, otherOpenFnProps); + if (isTrigger) { + e.source_trigger_id = node.id; + } else { + e.source_job_id = node.id; + } } else { - e.source_job_id = node.id; + e = { + enabled: !rules.disabled, + target_job: next, + }; + Object.assign(e, otherOpenFnProps); + if (isTrigger) { + e.source_trigger = s.type; + } else { + e.source_job = s.id; + } } if (rules.label) { @@ -202,16 +244,18 @@ export const mapWorkflow = ( }); }); - // Sort edges by UUID (for more predictable comparisons in test) - wfState.edges = Object.keys(wfState.edges) - // convert edge ids to strings just in case a number creeps in (it might in test) - .sort((a, b) => - `${wfState.edges[a].id}`.localeCompare('' + wfState.edges[b].id) - ) - .reduce((obj: any, key) => { - obj[key] = wfState.edges[key]; - return obj; - }, {}); + if (useUuids) { + // Sort edges by UUID (for more predictable comparisons in test) + wfState.edges = Object.keys(wfState.edges) + // convert edge ids to strings just in case a number creeps in (it might in test) + .sort((a, b) => + `${wfState.edges[a].id}`.localeCompare('' + wfState.edges[b].id) + ) + .reduce((obj: any, key) => { + obj[key] = wfState.edges[key]; + return obj; + }, {}); + } return wfState; }; diff --git a/packages/project/src/util/detect-version.ts b/packages/project/src/util/detect-version.ts new file mode 100644 index 000000000..dacbe7417 --- /dev/null +++ b/packages/project/src/util/detect-version.ts @@ -0,0 +1,14 @@ +import ensureJson from './ensure-json'; + +// Detect whether a project spec is v1 (Lightning app state) or v2 (local project state) +// Accepts YAML/JSON strings or a pre-parsed object +export default function detectVersion(projectSpec: string | object): number { + const json = ensureJson(projectSpec); + if (json.schema_version) { + return parseInt(json.schema_version, 10); + } + if (json.cli?.version === 2 || json.version) { + return 2; + } + return 1; +} diff --git a/packages/project/test/serialize/to-app-state.test.ts b/packages/project/test/serialize/to-app-state.test.ts index c007f114c..efa7990e2 100644 --- a/packages/project/test/serialize/to-app-state.test.ts +++ b/packages/project/test/serialize/to-app-state.test.ts @@ -4,6 +4,7 @@ import toAppState from '../../src/serialize/to-app-state'; import { generateProject } from '../../src/gen/generator'; import type { Provisioner } from '@openfn/lexicon/lightning'; +import { cloneDeep } from 'lodash-es'; const state: Provisioner.Project = { id: 'e16c5f09-f0cb-4ba7-a4c2-73fcb2f29d00', @@ -610,8 +611,97 @@ test('should convert a project back to app state in json', (t) => { t.deepEqual(newState, state); }); -// TODO this test is failing because the order of keys in the yaml have changed! -// We probably need to force alphabetical sorting on yaml keys +const v2ProjectData: any = { + id: 'my-project', + name: 'My Project', + schema_version: '4.0', + workflows: [ + { + id: 'my-workflow', + name: 'My Workflow', + start: 'webhook', + steps: [ + { + id: 'webhook', + type: 'webhook', + enabled: true, + next: { 'transform-data': {} }, + }, + { + id: 'transform-data', + name: 'Transform data', + expression: 'fn(s => s)', + adaptor: '@openfn/language-common@latest', + }, + ], + }, + ], +}; + +test('asSpec:true - edges use source_trigger/target_job keys, not UUIDs', (t) => { + const project = new Project(v2ProjectData, { formats: { project: 'json' } }); + const result = toAppState(project, { format: 'json', asSpec: true }) as any; + + const edge = Object.values(result.workflows['my-workflow'].edges)[0] as any; + t.truthy(edge.source_trigger); + t.truthy(edge.target_job); + t.falsy(edge.source_trigger_id); + t.falsy(edge.target_job_id); + t.falsy(edge.id); +}); + +test('asSpec:true - handle credentials', (t) => { + const data = cloneDeep(v2ProjectData); + data.credentials = [ + { + name: 'x', + owner: 'a@b.org,', + uuid: '123', + }, + ]; + data.workflows[0].steps[1].configuration = `a@b.org|x`; + + const project = new Project(data, { formats: { project: 'json' } }); + const result = toAppState(project, { format: 'json', asSpec: true }) as any; + + t.deepEqual(result.credentials, { + 'a@b.org,|x': { name: 'x', owner: 'a@b.org,' }, + }); + t.is( + result.workflows['my-workflow'].jobs['transform-data'].credential, + 'a@b.org|x' + ); +}); + +test('asSpec:true - source_trigger matches the trigger key', (t) => { + const project = new Project(v2ProjectData, { formats: { project: 'json' } }); + const result = toAppState(project, { format: 'json', asSpec: true }) as any; + + const wf = result.workflows['my-workflow']; + const edge = Object.values(wf.edges)[0] as any; + t.truthy(wf.triggers[edge.source_trigger]); +}); + +test('asSpec:true - target_job matches the job key', (t) => { + const project = new Project(v2ProjectData, { formats: { project: 'json' } }); + const result = toAppState(project, { format: 'json', asSpec: true }) as any; + + const wf = result.workflows['my-workflow']; + const edge = Object.values(wf.edges)[0] as any; + t.truthy(wf.jobs[edge.target_job]); +}); + +test('asSpec:true - triggers and jobs have no generated id', (t) => { + const project = new Project(v2ProjectData, { formats: { project: 'json' } }); + const result = toAppState(project, { format: 'json', asSpec: true }) as any; + + const wf = result.workflows['my-workflow']; + const trigger = Object.values(wf.triggers)[0] as any; + const job = Object.values(wf.jobs)[0] as any; + t.falsy(trigger.id); + t.falsy(job.id); +}); + test.skip('should convert a project back to app state in yaml', (t) => { // this is a serialized project file const data: any = { diff --git a/packages/project/test/util/detect-version.test.ts b/packages/project/test/util/detect-version.test.ts new file mode 100644 index 000000000..d8a20bd1f --- /dev/null +++ b/packages/project/test/util/detect-version.test.ts @@ -0,0 +1,68 @@ +import test from 'ava'; +import detectVersion from '../../src/util/detect-version'; + +test('detects v1 from a JSON object', (t) => { + const project = { + id: '1234', + name: 'My Project', + workflows: {}, + project_credentials: [], + }; + const version = detectVersion(project); + t.is(version, 1); +}); + +test('detects v1 from a YAML string', (t) => { + const project = ` + id: '1234' + name: My Project + workflows: {} + project_credentials: []`; + const version = detectVersion(project); + t.is(version, 1); +}); + +test('detects v1 from a JSON string', (t) => { + const project = JSON.stringify({ + id: '1234', + name: 'My Project', + workflows: {}, + }); + const version = detectVersion(project); + t.is(version, 1); +}); + +test('detects v2 via schema_version from a JSON object', (t) => { + const project = { + id: 'my-project', + name: 'My Project', + schema_version: '4.0', + workflows: [], + }; + const version = detectVersion(project); + t.is(version, 4); +}); + +test('detects v2 via schema_version from a YAML string', (t) => { + const project = `id: my-project\nname: My Project\nschema_version: '4.0'\nworkflows: []\n`; + const version = detectVersion(project); + t.is(version, 4); +}); + +test('detects v2 via cli.version === 2 (legacy format)', (t) => { + const project = { id: 'x', name: 'x', cli: { version: 2 }, workflows: [] }; + const version = detectVersion(project); + t.is(version, 2); +}); + +test('does not detect v2 for cli.version !== 2', (t) => { + const project = { id: 'x', name: 'x', cli: { version: 1 }, workflows: {} }; + const version = detectVersion(project); + t.is(version, 1); +}); + +test('detects v2 via deprecated version field', (t) => { + const project = { id: 'x', name: 'x', version: '1.0', workflows: [] }; + const version = detectVersion(project); + t.is(version, 2); +});