diff --git a/integration-tests/cli/CHANGELOG.md b/integration-tests/cli/CHANGELOG.md index 4fd1d9abd..2037b71e0 100644 --- a/integration-tests/cli/CHANGELOG.md +++ b/integration-tests/cli/CHANGELOG.md @@ -1,5 +1,12 @@ # @openfn/integration-tests-cli +## 1.0.14 + +### Patch Changes + +- Updated dependencies [1bbc8c4] + - @openfn/project@0.14.0 + ## 1.0.13 ### Patch Changes diff --git a/integration-tests/cli/package.json b/integration-tests/cli/package.json index c83014d00..b60284eaa 100644 --- a/integration-tests/cli/package.json +++ b/integration-tests/cli/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-cli", "private": true, - "version": "1.0.13", + "version": "1.0.14", "description": "CLI integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index b10402f5b..f42b3540f 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,18 @@ # @openfn/cli +## 1.28.0 + +### Minor Changes + +- 1bbc8c4: Update credentials to use credential id, not UUID. This enables credentials to sync better with app projects. + + WARNING: existing credential maps will break after pulling after this change. Update your credential maps to index on the new id values rather than the UUIDs. + +### Patch Changes + +- Updated dependencies [1bbc8c4] + - @openfn/project@0.14.0 + ## 1.27.0 ### Minor Changes @@ -8,7 +21,7 @@ ### Patch Changes -- 5d6237d: DEfault endpoint to app.openfn.org and improve error message +- 5d6237d: Default endpoint to app.openfn.org and improve error message - Updated dependencies [92c0b49] - Updated dependencies [dd88099] - @openfn/project@0.13.1 diff --git a/packages/cli/package.json b/packages/cli/package.json index ae4c92fa7..7fdfd0f05 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.27.0", + "version": "1.28.0", "description": "CLI devtools for the OpenFn toolchain", "engines": { "node": ">=18", diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 1e16d969e..8e9754679 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -245,7 +245,7 @@ export async function handler(options: DeployOptions, logger: Logger) { const active = ws.getActiveProject(); const alias = options.alias ?? active?.alias; - // TODO this doesn't have an alias + const localProject = await Project.from('fs', { root: options.workspace || '.', alias, @@ -258,6 +258,8 @@ export async function handler(options: DeployOptions, logger: Logger) { endpoint: config.endpoint, }; } + // generate a credential map + localProject.credentials = localProject.buildCredentialMap(); logger.success(`Loaded local project ${printProjectName(localProject)}`); diff --git a/packages/cli/test/execute/apply-credential-map.test.ts b/packages/cli/test/execute/apply-credential-map.test.ts index 0c8f8af08..1ebf909bc 100644 --- a/packages/cli/test/execute/apply-credential-map.test.ts +++ b/packages/cli/test/execute/apply-credential-map.test.ts @@ -12,11 +12,8 @@ const createWorkflow = (steps?: any[]) => ({ steps: steps ?? [ { id: 'a', - expression: `${fn}fn(() => ({ data: { count: 42 } }));`, - // project_credential_id must map here - // what about keychain_credential_id ? - // Should we map to credential, rather than configuration? I don't think so - configuration: 'A', + expression: `${fn}fn(() => ({ dat'admin@openfn.org-cred': { count: 42 } }));`, + configuration: 'admin@openfn.org-cred', next: { b: true }, }, ], @@ -44,21 +41,41 @@ test('do nothing if map is empty', (t) => { test('apply a credential to a single step', (t) => { const wf = createWorkflow(); const map = { - A: { user: 'Anne Arnold' }, + 'admin@openfn.org-cred': { user: 'Anne Arnold' }, }; - t.is(wf.workflow.steps[0].configuration, 'A'); + t.is(wf.workflow.steps[0].configuration, 'admin@openfn.org-cred'); applyCredentialMap(wf, map); - t.deepEqual(wf.workflow.steps[0].configuration, map.A); + t.deepEqual(wf.workflow.steps[0].configuration, map['admin@openfn.org-cred']); +}); + +test('apply a credential to a single step if UUIDs are used', (t) => { + const uuid = '4227ea57-6df9-4b6c-877f-04f00a6892b5'; + const wf = createWorkflow([ + { + id: 'a', + expression: `fn(s => s)`, + configuration: uuid, + }, + ]); + const map = { + [uuid]: { user: 'Anne Arnold' }, + }; + + t.is(wf.workflow.steps[0].configuration, uuid); + + applyCredentialMap(wf, map); + + t.deepEqual(wf.workflow.steps[0].configuration, map[uuid]); }); test('apply a credential to a single step which already has config', (t) => { const wf = createWorkflow(); wf.workflow.steps[0].configuration = { x: 1, [CREDENTIALS_KEY]: 'A' }; const map = { - A: { user: 'Anne Arnold' }, + 'admin@openfn.org-cred': { user: 'Anne Arnold' }, }; applyCredentialMap(wf, map); @@ -68,21 +85,21 @@ test('apply a credential to a single step which already has config', (t) => { test('apply a credential to several steps', (t) => { const wf = createWorkflow([ - { id: 'a', configuration: 'A' }, - { id: 'b', configuration: 'B' }, + { id: 'a', configuration: 'admin@openfn.org-A' }, + { id: 'b', configuration: 'admin@openfn.org-B' }, ]); const map = { - A: { user: 'Anne Arnold' }, - B: { user: 'Belle Bellvue' }, + 'admin@openfn.org-A': { user: 'Anne Arnold' }, + 'admin@openfn.org-B': { user: 'Belle Bellvue' }, }; - t.is(wf.workflow.steps[0].configuration, 'A'); - t.is(wf.workflow.steps[1].configuration, 'B'); + t.is(wf.workflow.steps[0].configuration, 'admin@openfn.org-A'); + t.is(wf.workflow.steps[1].configuration, 'admin@openfn.org-B'); applyCredentialMap(wf, map); - t.deepEqual(wf.workflow.steps[0].configuration, map.A); - t.deepEqual(wf.workflow.steps[1].configuration, map.B); + t.deepEqual(wf.workflow.steps[0].configuration, map['admin@openfn.org-A']); + t.deepEqual(wf.workflow.steps[1].configuration, map['admin@openfn.org-B']); }); test('wipe string credential if unmapped', (t) => { diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index b87f90832..f8a0433c4 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -193,7 +193,6 @@ export interface Step { // TODO a Step must ALWAYS have an id (util functions can default it) id?: StepId; name?: string; // user-friendly name used in logging - next?: string | Record; previous?: StepId; } diff --git a/packages/lexicon/lightning.d.ts b/packages/lexicon/lightning.d.ts index 695bd7662..a74018206 100644 --- a/packages/lexicon/lightning.d.ts +++ b/packages/lexicon/lightning.d.ts @@ -231,9 +231,7 @@ export namespace Provisioner { workflows: Record; concurrency?: any; // TODO - // TODO typing isn't quite right here either - //project_credentials: Record; - project_credentials: any[]; + project_credentials: Array<{ id: string; name: string; owner: string }>; // this is clearly wrong? //collections: Record; diff --git a/packages/project/CHANGELOG.md b/packages/project/CHANGELOG.md index 98abc8326..009374793 100644 --- a/packages/project/CHANGELOG.md +++ b/packages/project/CHANGELOG.md @@ -1,5 +1,13 @@ # @openfn/project +## 0.14.0 + +### Minor Changes + +- 1bbc8c4: Update credentials to use credential id, not UUID. This enables credentials to sync better with app projects. + + WARNING: existing credential maps will break after pulling after this change. Update your credential maps to index on the new id values rather than the UUIDs. + ## 0.13.1 ### Patch Changes diff --git a/packages/project/package.json b/packages/project/package.json index 83f39fe20..46110dd89 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/project", - "version": "0.13.1", + "version": "0.14.0", "description": "Read, serialize, replicate and sync OpenFn projects", "scripts": { "test": "pnpm ava", diff --git a/packages/project/src/Project.ts b/packages/project/src/Project.ts index 72c3ad004..dea6a5cb1 100644 --- a/packages/project/src/Project.ts +++ b/packages/project/src/Project.ts @@ -15,6 +15,7 @@ import { Workspace } from './Workspace'; import { buildConfig, extractConfig } from './util/config'; import { Provisioner } from '@openfn/lexicon/lightning'; import { SandboxMeta, UUID, WorkspaceConfig } from '@openfn/lexicon'; +import { parse } from './util/get-credential-name'; const maybeCreateWorkflow = (wf: any) => wf instanceof Workflow ? wf : new Workflow(wf); @@ -34,6 +35,14 @@ type CLIMeta = { forked_from?: Record; }; +export type Credential = { + uuid: string; + name: string; + + // TODO having the owner in the credential is controvertial and we may need to rethink this later + owner: string; +}; + export class Project { // what schema version is this? // And how are we tracking this? @@ -70,7 +79,7 @@ export class Project { collections: any; - credentials: string[]; + credentials: Credential[]; sandbox?: SandboxMeta; @@ -241,6 +250,26 @@ export class Project { return result; } + /** + * Find all project credentials referenced in all + * workflows and return it + */ + buildCredentialMap(): Credential[] { + const creds: any = {}; + for (const wf of this.workflows) { + for (const step of wf.steps) { + if ( + typeof step.configuration === 'string' && + !creds[step.configuration] + ) { + const { name, owner } = parse(step.configuration); + creds[step.configuration] = { name, owner }; + } + } + } + return Object.values(creds); + } + // Compare this project with another and return a list of workflow changes diff(project: Project, workflows: string[] = []) { return projectDiff(this, project, workflows); diff --git a/packages/project/src/gen/generator.ts b/packages/project/src/gen/generator.ts index 01e12b15f..c4f2665cf 100644 --- a/packages/project/src/gen/generator.ts +++ b/packages/project/src/gen/generator.ts @@ -35,7 +35,7 @@ const expectedNodeProps = [ // TODO need to clarify adaptor/adaptors confusion 'adaptor', 'adaptors', - + 'configuration', 'expression', 'condition', 'label', diff --git a/packages/project/src/merge/merge-project.ts b/packages/project/src/merge/merge-project.ts index 6a3fe5d34..f1578fdef 100644 --- a/packages/project/src/merge/merge-project.ts +++ b/packages/project/src/merge/merge-project.ts @@ -1,12 +1,13 @@ import { defaultsDeep, isEmpty } from 'lodash-es'; -import { Project } from '../Project'; +import { Credential, Project } from '../Project'; import { mergeWorkflows } from './merge-workflow'; import mapUuids from './map-uuids'; import baseMerge from '../util/base-merge'; import getDuplicates from '../util/get-duplicates'; import Workflow from '../Workflow'; import findChangedWorkflows from '../util/find-changed-workflows'; +import getCredentialName from '../util/get-credential-name'; export const SANDBOX_MERGE = 'sandbox'; @@ -157,7 +158,12 @@ export function merge( name: source.name ?? target.name, alias: source.alias ?? target.alias, description: source.description ?? target.description, - credentials: source.credentials ?? target.credentials, + + // when mapping credentials, we prefer the UUIDs on the target + credentials: replaceCredentials( + source.credentials, + target.credentials + ), collections: source.collections ?? target.collections, }; @@ -166,3 +172,29 @@ export function merge( baseMerge(target, source, ['collections'], assigns as any) ); } + +export const replaceCredentials = ( + sourceCreds: Credential[] = [], + targetCreds: Credential[] = [] +): Credential[] => { + const result = [...targetCreds]; + + // Build an object of existing target credential names for quick lookup + const targetCredNames = targetCreds.reduce((acc, cred) => { + acc[getCredentialName(cred)] = true; + return acc; + }, {} as Record); + + // Find credentials in source that don't exist in target + for (const sourceCred of sourceCreds) { + const credName = getCredentialName(sourceCred); + if (!targetCredNames[credName]) { + // This is a new credential - add it without the source uuid + // (a new UUID will be generated elsewhere) + const { uuid, ...credWithoutUuid } = sourceCred; + result.push(credWithoutUuid as Credential); + } + } + + return result; +}; diff --git a/packages/project/src/parse/from-app-state.ts b/packages/project/src/parse/from-app-state.ts index f9e84a960..4348aeb57 100644 --- a/packages/project/src/parse/from-app-state.ts +++ b/packages/project/src/parse/from-app-state.ts @@ -3,10 +3,11 @@ import * as l from '@openfn/lexicon'; import { Provisioner } from '@openfn/lexicon/lightning'; -import { Project } from '../Project'; +import { Project, Credential } from '../Project'; import renameKeys from '../util/rename-keys'; import slugify from '../util/slugify'; import ensureJson from '../util/ensure-json'; +import getCredentialName from '../util/get-credential-name'; export type fromAppStateConfig = Partial & { format?: 'yaml' | 'json'; @@ -26,7 +27,7 @@ export default ( name, description, workflows, - project_credentials: credentials, + project_credentials = [], collections, inserted_at, updated_at, @@ -34,6 +35,13 @@ export default ( ...options } = stateJson; + // subtle mapping of credentials keys to align with lexicon + const credentials = project_credentials.map((c) => ({ + uuid: c.id, + name: c.name, + owner: c.owner, + })); + const proj: Partial = { name, description: description ?? undefined, @@ -59,7 +67,9 @@ export default ( }; } - proj.workflows = Object.values(stateJson.workflows).map(mapWorkflow); + proj.workflows = Object.values(stateJson.workflows).map((w) => + mapWorkflow(w, proj.credentials) + ); return new Project(proj as l.Project, config); }; @@ -91,7 +101,10 @@ export const mapEdge = (edge: Provisioner.Edge) => { // map a project workflow to a local cli workflow // TODO this probably gets easier if I index everything by name -export const mapWorkflow = (workflow: Provisioner.Workflow) => { +export const mapWorkflow = ( + workflow: Provisioner.Workflow, + credentials: Credential[] = [] +) => { const { jobs, edges, triggers, name, version_history, ...remoteProps } = workflow; const mapped: l.Workflow = { @@ -155,7 +168,14 @@ export const mapWorkflow = (workflow: Provisioner.Workflow) => { openfn: renameKeys(remoteProps, { id: 'uuid' }), }; if (project_credential_id) { - s.configuration = project_credential_id; + const mappedCredential = credentials.find( + (c) => c.uuid == project_credential_id + ); + if (mappedCredential) { + s.configuration = getCredentialName(mappedCredential); + } else { + s.configuration = project_credential_id; + } } if (outboundEdges.length) { diff --git a/packages/project/src/parse/from-project.ts b/packages/project/src/parse/from-project.ts index e99eabcd1..cd1bd8bb9 100644 --- a/packages/project/src/parse/from-project.ts +++ b/packages/project/src/parse/from-project.ts @@ -20,7 +20,7 @@ export type SerializedWorkflow = { id: string; name: string; - steps: WithMeta; + steps: WithMeta>; openfn?: l.ProjectMeta; }; diff --git a/packages/project/src/serialize/to-app-state.ts b/packages/project/src/serialize/to-app-state.ts index 24175317b..737e16fed 100644 --- a/packages/project/src/serialize/to-app-state.ts +++ b/packages/project/src/serialize/to-app-state.ts @@ -2,11 +2,12 @@ import { pick, omitBy, isNil, sortBy } from 'lodash-es'; import { Provisioner } from '@openfn/lexicon/lightning'; import { randomUUID } from 'node:crypto'; -import { Project } from '../Project'; +import { Credential, Project } from '../Project'; import renameKeys from '../util/rename-keys'; import { jsonToYaml } from '../util/yaml'; import Workflow from '../Workflow'; import slugify from '../util/slugify'; +import getCredentialName from '../util/get-credential-name'; type Options = { format?: 'json' | 'yaml' }; @@ -27,6 +28,7 @@ export default function ( env, id /* shouldn't be there but will cause problems if it's set*/, fetched_at /* remove this metadata as it causes problems */, + alias, // shouldn't be written but has been caught in some legacy files ...rest } = project.openfn ?? {}; @@ -39,9 +41,21 @@ export default function ( Object.assign(state, rest, project.options); - state.project_credentials = project.credentials ?? []; + const credentialsWithUuids = + project.credentials?.map((c) => ({ + ...c, + uuid: c.uuid ?? randomUUID(), + })) ?? []; + + state.project_credentials = credentialsWithUuids.map((c) => ({ + // note the subtle conversion here + id: c.uuid, + name: c.name, + owner: c.owner, + })); + state.workflows = project.workflows - .map(mapWorkflow) + .map((w) => mapWorkflow(w, credentialsWithUuids)) .reduce((obj: any, wf) => { obj[slugify(wf.name ?? wf.id)] = wf; return obj; @@ -58,7 +72,10 @@ export default function ( return state; } -export const mapWorkflow = (workflow: Workflow) => { +export const mapWorkflow = ( + workflow: Workflow, + credentials: Credential[] = [] +) => { if (workflow instanceof Workflow) { // @ts-ignore workflow = workflow.toJSON(); @@ -116,12 +133,21 @@ export const mapWorkflow = (workflow: Workflow) => { typeof s.configuration === 'string' && !s.configuration.endsWith('.json') ) { - // TODO do I need to ensure that this gets added to project_credntials? - // not really - if the credential hasn't been added yet, users have to go into - // the app and do it - // Maybe there's a feature-request to auto-add credentials if the user - // has access - otherOpenFnProps.project_credential_id = s.configuration; + let projectCredentialId = s.configuration; + if (projectCredentialId) { + const mappedCredential = credentials.find((c) => { + const name = getCredentialName(c); + return name === projectCredentialId; + }); + if (mappedCredential) { + 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; + } } Object.assign(node, defaultJobProps, otherOpenFnProps); diff --git a/packages/project/src/util/get-credential-name.ts b/packages/project/src/util/get-credential-name.ts new file mode 100644 index 000000000..964eaf44a --- /dev/null +++ b/packages/project/src/util/get-credential-name.ts @@ -0,0 +1,10 @@ +import { Credential } from '../Project'; + +export const DELIMETER = '|'; + +export default (cred: Credential) => `${cred.owner}${DELIMETER}${cred.name}`; + +export const parse = (credentialName: string) => { + const [owner, name] = credentialName.split('|'); + return { owner, name }; +}; diff --git a/packages/project/test/fixtures/sample-v1-project.ts b/packages/project/test/fixtures/sample-v1-project.ts index dcf222ddc..9564bc80f 100644 --- a/packages/project/test/fixtures/sample-v1-project.ts +++ b/packages/project/test/fixtures/sample-v1-project.ts @@ -58,6 +58,14 @@ const state: Provisioner.Project = { export default state; const withCreds = cloneDeep(state); +// TODO I'm not sure about keychain creds hre +withCreds.project_credentials = [ + { + id: 'p', + name: 'My Credential', + owner: 'admin@openfn.org', + }, +]; Object.assign(withCreds.workflows['my-workflow'].jobs['transform-data'], { project_credential_id: 'p', keychain_credential_id: 'k', diff --git a/packages/project/test/fixtures/sample-v2-project.ts b/packages/project/test/fixtures/sample-v2-project.ts index 6b9cbeb8d..76f8177d1 100644 --- a/packages/project/test/fixtures/sample-v2-project.ts +++ b/packages/project/test/fixtures/sample-v2-project.ts @@ -13,13 +13,21 @@ export const json: SerializedProject = { sandbox: { parentId: 'abcd', }, + credentials: [ + { + uuid: 'x', + owner: 'admin@openfn.org', + name: 'My Credential', + }, + ], workflows: [ { steps: [ { name: 'b', id: 'b', - openfn: { uuid: 3, project_credential_id: 'x' }, + configuration: 'admin@openfn.org|My Credential', + openfn: { uuid: 3 }, expression: 'fn()', adaptor: 'common', }, @@ -44,6 +52,10 @@ name: My Project cli: version: 2 description: my lovely project +credentials: + - uuid: x + owner: admin@openfn.org + name: My Credential openfn: uuid: "1234" endpoint: https://app.openfn.org @@ -57,9 +69,9 @@ workflows: name: b openfn: uuid: 3 - project_credential_id: x expression: fn() adaptor: common + configuration: admin@openfn.org|My Credential - id: trigger openfn: uuid: 2 diff --git a/packages/project/test/merge/merge-project.test.ts b/packages/project/test/merge/merge-project.test.ts index 667744223..1ed4af421 100644 --- a/packages/project/test/merge/merge-project.test.ts +++ b/packages/project/test/merge/merge-project.test.ts @@ -1,8 +1,13 @@ import test from 'ava'; import { randomUUID } from 'node:crypto'; import Project from '../../src'; -import { merge, REPLACE_MERGE } from '../../src/merge/merge-project'; +import { + merge, + REPLACE_MERGE, + replaceCredentials, +} from '../../src/merge/merge-project'; import { generateWorkflow } from '../../src/gen/generator'; +import { Credential } from '../../src/Project'; let idgen = 0; @@ -736,3 +741,97 @@ test.todo('options: only changed and 1 workflow'); // this test it's important that the final project includes the unchanged workflow test.todo('options: only changed, and 1 changed, 1 unchanged workflow'); + +test('replaceCredentials: preserves target credentials with their UUIDs', (t) => { + const targetCreds: Credential[] = [ + { uuid: 'target-uuid-1', name: 'cred1', owner: 'user1' }, + { uuid: 'target-uuid-2', name: 'cred2', owner: 'user1' }, + ]; + + const result = replaceCredentials([], targetCreds); + + t.is(result.length, 2); + t.is(result[0].uuid, 'target-uuid-1'); + t.is(result[1].uuid, 'target-uuid-2'); +}); + +test('replaceCredentials: adds new credentials from source without their UUIDs', (t) => { + const sourceCreds: Credential[] = [ + { uuid: 'source-uuid-1', name: 'newcred', owner: 'user1' }, + ]; + const targetCreds: Credential[] = [ + { uuid: 'target-uuid-1', name: 'existingcred', owner: 'user1' }, + ]; + + const result = replaceCredentials(sourceCreds, targetCreds); + + t.is(result.length, 2); + // First credential should be the existing target credential + t.is(result[0].uuid, 'target-uuid-1'); + t.is(result[0].name, 'existingcred'); + + // Second credential should be the new one from source, but without UUID + t.is(result[1].name, 'newcred'); + t.is(result[1].owner, 'user1'); + t.falsy(result[1].uuid); +}); + +test('replaceCredentials: does not duplicate credentials with same name/owner', (t) => { + const sourceCreds: Credential[] = [ + { uuid: 'source-uuid-1', name: 'samecred', owner: 'user1' }, + ]; + const targetCreds: Credential[] = [ + { uuid: 'target-uuid-1', name: 'samecred', owner: 'user1' }, + ]; + + const result = replaceCredentials(sourceCreds, targetCreds); + + // Should only have one credential (from target) + t.is(result.length, 1); + t.is(result[0].uuid, 'target-uuid-1'); + t.is(result[0].name, 'samecred'); + t.is(result[0].owner, 'user1'); +}); + +test('replaceCredentials: treats credentials with different owners as different', (t) => { + const sourceCreds: Credential[] = [ + { uuid: 'source-uuid-1', name: 'cred1', owner: 'user2' }, + ]; + const targetCreds: Credential[] = [ + { uuid: 'target-uuid-1', name: 'cred1', owner: 'user1' }, + ]; + + const result = replaceCredentials(sourceCreds, targetCreds); + + // Should have both credentials (different owners) + t.is(result.length, 2); + t.is(result[0].owner, 'user1'); + t.is(result[1].owner, 'user2'); + t.falsy(result[1].uuid); +}); + +test('replaceCredentials: handles multiple new and existing credentials', (t) => { + const sourceCreds: Credential[] = [ + { uuid: 'source-uuid-1', name: 'existing', owner: 'user1' }, + { uuid: 'source-uuid-2', name: 'new1', owner: 'user1' }, + { uuid: 'source-uuid-3', name: 'new2', owner: 'user2' }, + ]; + const targetCreds: Credential[] = [ + { uuid: 'target-uuid-1', name: 'existing', owner: 'user1' }, + { uuid: 'target-uuid-2', name: 'old', owner: 'user1' }, + ]; + + const result = replaceCredentials(sourceCreds, targetCreds); + + t.is(result.length, 4); + + // First two should be from target with their UUIDs + t.is(result[0].uuid, 'target-uuid-1'); + t.is(result[1].uuid, 'target-uuid-2'); + + // Next two should be new credentials without UUIDs + t.is(result[2].name, 'new1'); + t.falsy(result[2].uuid); + t.is(result[3].name, 'new2'); + t.falsy(result[3].uuid); +}); diff --git a/packages/project/test/parse/from-app-state.test.ts b/packages/project/test/parse/from-app-state.test.ts index 3deffc5a1..e90421796 100644 --- a/packages/project/test/parse/from-app-state.test.ts +++ b/packages/project/test/parse/from-app-state.test.ts @@ -92,6 +92,18 @@ test('should create a Project from prov state with positions', (t) => { }); }); +test('should handle project credentials', (t) => { + const newState = cloneDeep(withCreds); + + const project = fromAppState(newState, meta); + + t.is(project.credentials.length, 1); + t.is( + project.workflows[0].steps[1].configuration, + 'admin@openfn.org|My Credential' + ); +}); + test('should create a Project from prov state with a workflow', (t) => { const project = fromAppState(state, meta); @@ -250,7 +262,7 @@ test('mapWorkflow: map a job with keychain credentials onto .openfn', (t) => { id: 'transform-data', name: 'Transform data', adaptor: '@openfn/language-common@latest', - configuration: 'p', + configuration: 'p', // note that without a credential map, this gets left alone expression: 'fn(s => s)', openfn: { uuid: '66add020-e6eb-4eec-836b-20008afca816', @@ -259,21 +271,28 @@ test('mapWorkflow: map a job with keychain credentials onto .openfn', (t) => { }); }); -test('mapWorkflow: map a job with projcet credentials onto job.configuration', (t) => { +test('mapWorkflow: map a job with project credentials onto job.configuration', (t) => { const wf = withCreds.workflows['my-workflow']; - const mapped = mapWorkflow(wf); + const credentials = [ + { + uuid: 'p', + owner: 'admin', + name: 'cred', + }, + ]; + const mapped = mapWorkflow(wf, credentials); const [_trigger, job] = mapped.steps; // This is the important bit - t.is((job as Job).configuration, 'p'); + t.is((job as Job).configuration, 'admin|cred'); t.deepEqual(job, { id: 'transform-data', name: 'Transform data', adaptor: '@openfn/language-common@latest', expression: 'fn(s => s)', - configuration: 'p', + configuration: 'admin|cred', openfn: { uuid: '66add020-e6eb-4eec-836b-20008afca816', keychain_credential_id: 'k', diff --git a/packages/project/test/parse/from-project.test.ts b/packages/project/test/parse/from-project.test.ts index c41b4dca6..44646b230 100644 --- a/packages/project/test/parse/from-project.test.ts +++ b/packages/project/test/parse/from-project.test.ts @@ -6,7 +6,10 @@ import * as v2 from '../fixtures/sample-v2-project'; const v1_yaml = `id: '1234' name: aaa description: a project -project_credentials: [] +credentials: + - uuid: x + owner: admin@openfn.org + name: My Credential collections: [] inserted_at: 2025-04-23T11:15:59Z updated_at: 2025-04-23T11:15:59Z @@ -90,6 +93,14 @@ test('import from a v2 project as JSON', async (t) => { t.is(proj.workflows.length, 1); + t.deepEqual(proj.credentials, [ + { + uuid: 'x', + owner: 'admin@openfn.org', + name: 'My Credential', + }, + ]); + t.deepEqual(proj.workflows[0].workflow, { id: 'workflow', name: 'Workflow', @@ -135,7 +146,7 @@ test('import from a v2 project with alias', async (t) => { t.is(proj.cli.alias, 'staging'); }); -test('import from a v2 project as YAML', async (t) => { +test.only('import from a v2 project as YAML', async (t) => { const proj = await Project.from('project', v2.yaml); t.is(proj.id, 'my-project'); t.is(proj.name, 'My Project'); @@ -162,9 +173,9 @@ test('import from a v2 project as YAML', async (t) => { id: 'b', expression: 'fn()', adaptor: 'common', + configuration: 'admin@openfn.org|My Credential', openfn: { uuid: 3, - project_credential_id: 'x', }, }, { diff --git a/packages/project/test/project.test.ts b/packages/project/test/project.test.ts index 67d5dccc8..7322ac173 100644 --- a/packages/project/test/project.test.ts +++ b/packages/project/test/project.test.ts @@ -112,7 +112,76 @@ test('should default alias to "main"', (t) => { t.is(project.alias, 'main'); }); -test.only('should convert a state file to a project and back again', async (t) => { +test('should support credentials', (t) => { + const project = new Project({ + credentials: [ + { + uuid: '21345', + name: 'My Credential', + owner: 'admin@openfn.org', + }, + ], + }); + + const [cred] = project.credentials; + t.deepEqual(cred, { + uuid: '21345', + name: 'My Credential', + owner: 'admin@openfn.org', + }); +}); + +test('should generate a credential map', (t) => { + const project = new Project({ + workflows: [ + { + id: 'w', + steps: [ + { + id: 'a', + configuration: 'admin@openfn.org|My Credential', + }, + ], + }, + ], + }); + const creds = project.buildCredentialMap(); + t.deepEqual(creds, [ + { + name: 'My Credential', + owner: 'admin@openfn.org', + }, + ]); +}); + +test('should generate a credential map without duplicates', (t) => { + const project = new Project({ + workflows: [ + { + id: 'w', + steps: [ + { + id: 'a', + configuration: 'admin@openfn.org|My Credential', + }, + { + id: 'b', + configuration: 'admin@openfn.org|My Credential', + }, + ], + }, + ], + }); + const creds = project.buildCredentialMap(); + t.deepEqual(creds, [ + { + name: 'My Credential', + owner: 'admin@openfn.org', + }, + ]); +}); + +test('should convert a state file to a project and back again', async (t) => { const meta = { endpoint: 'app.openfn.org', env: 'test', @@ -163,8 +232,8 @@ test('should return UUIDs for everything', async (t) => { wf1: { self: '72ca3eb0-042c-47a0-a2a1-a545ed4a8406', children: { - trigger: '4a06289c-15aa-4662-8dc6-f0aaacd8a058', - 'trigger-transform-data': 'a9a3adef-b394-4405-814d-3ac4323f4b4b', + webhook: '4a06289c-15aa-4662-8dc6-f0aaacd8a058', + 'webhook-transform-data': 'a9a3adef-b394-4405-814d-3ac4323f4b4b', 'transform-data': '66add020-e6eb-4eec-836b-20008afca816', }, }, diff --git a/packages/project/test/serialize/to-app-state.test.ts b/packages/project/test/serialize/to-app-state.test.ts index c900fb34c..f27adc615 100644 --- a/packages/project/test/serialize/to-app-state.test.ts +++ b/packages/project/test/serialize/to-app-state.test.ts @@ -50,7 +50,13 @@ const state: Provisioner.Project = { }, }, updated_at: '2025-04-23T11:15:59Z', - project_credentials: [''], + project_credentials: [ + { + id: '', + name: 'my cred', + owner: 'admin@openfn.org', + }, + ], scheduled_deletion: null, allow_support_access: false, requires_mfa: false, @@ -232,6 +238,13 @@ test('should write openfn keys to objects', (t) => { test('should handle credentials', (t) => { const data = { id: 'my-project', + credentials: [ + { + uuid: '123', + name: 'cred', + owner: 'admin@openfn.org', + }, + ], workflows: [ { id: 'wf', @@ -247,7 +260,7 @@ test('should handle credentials', (t) => { { id: 'step', expression: '.', - configuration: 'p', + configuration: 'admin@openfn.org|cred', openfn: { keychain_credential_id: 'k', }, @@ -260,7 +273,47 @@ test('should handle credentials', (t) => { const state = toAppState(new Project(data), { format: 'json' }); const { step } = state.workflows['wf'].jobs; t.is(step.keychain_credential_id, 'k'); - t.is(step.project_credential_id, 'p'); + t.is(step.project_credential_id, '123'); +}); + +test('should force a UUID on project credentials', (t) => { + const data = { + id: 'my-project', + credentials: [ + { + name: 'cred', + owner: 'admin@openfn.org', + }, + ], + workflows: [ + { + id: 'wf', + name: 'wf', + steps: [ + { + id: 'trigger', + type: 'webhook', + next: { + step: {}, + }, + }, + { + id: 'step', + expression: '.', + configuration: 'admin@openfn.org|cred', + openfn: { + keychain_credential_id: 'k', + }, + }, + ], + }, + ], + }; + + const state = toAppState(new Project(data), { + format: 'json', + }) as Provisioner.Project_v1; + t.truthy(state.project_credentials[0].id); }); test('should ignore forked_from', (t) => { @@ -427,8 +480,13 @@ test('should convert a project back to app state in json', (t) => { const data = { name: 'aaa', description: 'a project', - // TODO I think we might need more automation of this? - credentials: [''], + credentials: [ + { + uuid: '', + name: 'my cred', + owner: 'admin@openfn.org', + }, + ], collections: [], openfn: { env: 'project', @@ -477,7 +535,7 @@ test('should convert a project back to app state in json', (t) => { name: 'Transform data', expression: 'fn(s => s)', adaptor: '@openfn/language-common@latest', - configuration: '', + configuration: 'admin@openfn.org|my cred', openfn: { uuid: '66add020-e6eb-4eec-836b-20008afca816', }, diff --git a/packages/project/test/serialize/to-project.test.ts b/packages/project/test/serialize/to-project.test.ts index ea4f1cba5..53ebe769e 100644 --- a/packages/project/test/serialize/to-project.test.ts +++ b/packages/project/test/serialize/to-project.test.ts @@ -23,9 +23,16 @@ const createProject = (props: Partial = {}) => { env: 'dev', color: 'red', }, + credentials: [ + { + uuid: 'x', + owner: 'admin@openfn.org', + name: 'My Credential', + }, + ], workflows: [ generateWorkflow( - 'trigger(type=webhook)-b(expression="fn()",adaptor=common,project_credential_id=x)', + 'trigger(type=webhook)-b(expression="fn()",adaptor=common,configuration="admin@openfn.org|My Credential")', { uuidSeed: 1, openfnUuid: true, diff --git a/packages/project/test/util/get-credential-name.test.ts b/packages/project/test/util/get-credential-name.test.ts new file mode 100644 index 000000000..e25374ddc --- /dev/null +++ b/packages/project/test/util/get-credential-name.test.ts @@ -0,0 +1,20 @@ +import test from 'ava'; +import getCredentialName, { parse } from '../../src/util/get-credential-name'; +import { Credential } from '../../src/Project'; + +test('should generate a credential name', (t) => { + const cred: Credential = { + uuid: '', + owner: 'admin@openfn.org', + name: 'my credential', + }; + + const result = getCredentialName(cred); + t.is(result, `admin@openfn.org|my credential`); +}); + +test('should parse credential name', (t) => { + const { owner, name } = parse('admin@openfn.org|my credential'); + t.is(owner, 'admin@openfn.org'); + t.is(name, 'my credential'); +}); diff --git a/packages/project/test/workspace.test.ts b/packages/project/test/workspace.test.ts index 0410c0cd9..7131fff15 100644 --- a/packages/project/test/workspace.test.ts +++ b/packages/project/test/workspace.test.ts @@ -223,9 +223,11 @@ test('workspace-path: invalid workspace path', (t) => { test('workspace-list: list projects in the workspace', (t) => { const ws = new Workspace('/ws'); - t.is(ws.list().length, 1); + const result = ws.list(); + + t.is(result.length, 1); t.deepEqual( - ws.list().map((w) => w.id), + result.map((w) => w.id), ['project-1'] ); });