Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/orange-pigs-cross.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/cli': minor
---

Allow credential map, as json or yaml, to be passed via --credentials
5 changes: 5 additions & 0 deletions .changeset/public-dragons-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@openfn/project': patch
---

Map project_credential_id to configuration
12 changes: 12 additions & 0 deletions integration-tests/cli/test/execute-workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,18 @@ test.serial(
}
);

test.serial(
`openfn ${jobsPath}/wf-creds.json --credentials ${jobsPath}/creds.json`,
async (t) => {
const { err, stdout, stderr } = await run(t.title);
console.log({ stdout, stderr });
t.falsy(err);

const out = getJSON();
t.is(out.value, 'admin:admin');
}
);

test.serial(
`openfn ${jobsPath}/wf-errors.json -S "{ \\"data\\": { \\"number\\": 2 } }"`,
async (t) => {
Expand Down
4 changes: 4 additions & 0 deletions integration-tests/cli/test/fixtures/creds.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn((s) => {
s.value = `${s.configuration.user}:${s.configuration.password}`;
return s;
});
6 changes: 6 additions & 0 deletions integration-tests/cli/test/fixtures/creds.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"08089249-0890-4a73-8799-e2ec2b9e5d77": {
"user": "admin",
"password": "admin"
}
}
11 changes: 11 additions & 0 deletions integration-tests/cli/test/fixtures/wf-creds.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"workflow": {
"steps": [
{
"adaptor": "common",
"configuration": "08089249-0890-4a73-8799-e2ec2b9e5d77",
"expression": "creds.js"
}
]
}
}
55 changes: 55 additions & 0 deletions packages/cli/src/execute/apply-credential-map.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* utility to take a workflow and a credential map
* and apply credentials to each step
*/

import { ExecutionPlan } from '@openfn/lexicon';
import { Logger } from '../util';

type JobId = string;

export type CredentialMap = Record<JobId, any>;

const applyCredentialMap = (
plan: ExecutionPlan,
map: CredentialMap = {},
logger?: Logger
) => {
const stepsWithCredentialIds = plan.workflow.steps.filter(
(step: any) =>
typeof step.configuration === 'string' &&
!step.configuration.endsWith('.json')
) as { configuration: string; name?: string; id: string }[];

const unmapped: Record<string, true> = {};

for (const step of stepsWithCredentialIds) {
if (map[step.configuration]) {
logger?.debug(
`Applying credential ${step.configuration} to "${step.name ?? step.id}"`
);
step.configuration = map[step.configuration];
} else {
unmapped[step.configuration] = true;
// @ts-ignore
delete step.configuration;
}
}

if (Object.keys(unmapped).length) {
logger?.warn(
`WARNING: credential IDs were found in the workflow, but values have not been provided:`
);
logger?.warn(' ', Object.keys(unmapped).join(','));
if (map) {
logger?.warn(
'If the workflow fails, add these credentials to the credential map'
);
} else {
// TODO if running from project file this might be bad advice
logger?.warn('Pass a credential map with --credentials');
}
}
};

export default applyCredentialMap;
2 changes: 2 additions & 0 deletions packages/cli/src/execute/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type ExecuteOptions = Required<
| 'cacheSteps'
| 'command'
| 'compile'
| 'credentials'
| 'expandAdaptors'
| 'end'
| 'immutable'
Expand Down Expand Up @@ -46,6 +47,7 @@ const options = [
o.autoinstall,
o.cacheSteps,
o.compile,
o.credentials,
o.end,
o.ignoreImports,
o.immutable,
Expand Down
35 changes: 34 additions & 1 deletion packages/cli/src/execute/handler.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import type { ExecutionPlan } from '@openfn/lexicon';
import { yamlToJson } from '@openfn/project';
import { readFile } from 'node:fs/promises';
import path from 'node:path';

import type { ExecuteOptions } from './command';
import execute from './execute';
import serializeOutput from './serialize-output';
import getAutoinstallTargets from './get-autoinstall-targets';
import applyCredentialMap from './apply-credential-map';

import { install } from '../repo/handler';
import compile from '../compile/compile';
Expand Down Expand Up @@ -44,14 +48,43 @@ const matchStep = (
return '';
};

const loadAndApplyCredentialMap = async (
plan: ExecutionPlan,
options: ExecuteOptions,
logger: Logger
) => {
let creds = {};
if (options.credentials) {
try {
const credsRaw = await readFile(
path.resolve(options.credentials),
'utf8'
);
if (options.credentials.endsWith('.json')) {
creds = JSON.parse(credsRaw);
} else {
creds = yamlToJson(credsRaw);
}
} catch (e) {
logger.error('Error processing credential map:');
logger.error(e);
// probably want to exist if the credential map is invalid
process.exitCode = 1;
return;
}
logger.info('Credential map loaded ');
}
return applyCredentialMap(plan, creds, logger);
};

const executeHandler = async (options: ExecuteOptions, logger: Logger) => {
const start = new Date().getTime();
assertPath(options.path);
await validateAdaptors(options, logger);

let plan = await loadPlan(options, logger);
validatePlan(plan, logger);

await loadAndApplyCredentialMap(plan, options, logger);
if (options.cacheSteps) {
await clearCache(plan, options, logger);
}
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export type Opts = {
compile?: boolean;
configPath?: string;
confirm?: boolean;
credentials?: string;
describe?: string;
end?: string; // workflow end node
env?: string;
Expand Down Expand Up @@ -245,6 +246,14 @@ export const configPath: CLIOption = {
},
};

export const credentials: CLIOption = {
name: 'credentials',
yargs: {
alias: ['creds'],
description: 'A path which points to a credential map',
},
};

export const describe: CLIOption = {
name: 'describe',
yargs: {
Expand Down
95 changes: 95 additions & 0 deletions packages/cli/test/execute/apply-credential-map.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import test from 'ava';
import applyCredentialMap from '../../src/execute/apply-credential-map';
import { createMockLogger } from '@openfn/logger/dist';

const fn = `const fn = (fn) => (s) => fn(s);
`;

const createWorkflow = (steps?: any[]) => ({
workflow: {
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',
next: { b: true },
},
],
},
});

test('do nothing if map is undefined', (t) => {
const wf = createWorkflow();
delete wf.workflow.steps[0].configuration;

applyCredentialMap(wf);

t.falsy(wf.workflow.steps[0].configuration);
});

test('do nothing if map is empty', (t) => {
const wf = createWorkflow();
delete wf.workflow.steps[0].configuration;

applyCredentialMap(wf, {});

t.falsy(wf.workflow.steps[0].configuration);
});

test('apply a credential to a single step', (t) => {
const wf = createWorkflow();
const map = {
A: { user: 'Anne Arnold' },
};

t.is(wf.workflow.steps[0].configuration, 'A');

applyCredentialMap(wf, map);

t.deepEqual(wf.workflow.steps[0].configuration, map.A);
});

test('apply a credential to several steps', (t) => {
const wf = createWorkflow([
{ id: 'a', configuration: 'A' },
{ id: 'b', configuration: 'B' },
]);
const map = {
A: { user: 'Anne Arnold' },
B: { user: 'Belle Bellvue' },
};

t.is(wf.workflow.steps[0].configuration, 'A');
t.is(wf.workflow.steps[1].configuration, 'B');

applyCredentialMap(wf, map);

t.deepEqual(wf.workflow.steps[0].configuration, map.A);
t.deepEqual(wf.workflow.steps[1].configuration, map.B);
});

test('wipe string credential if unmapped', (t) => {
const wf = createWorkflow();

t.truthy(wf.workflow.steps[0].configuration);

applyCredentialMap(wf, {});

t.falsy(wf.workflow.steps[0].configuration);
});

test('warn if credential unmapped', (t) => {
const wf = createWorkflow();

const logger = createMockLogger();
t.truthy(wf.workflow.steps[0].configuration);

applyCredentialMap(wf, {}, logger);

t.truthy(
logger._find('warn', /WARNING: credential IDs were found in the workflow/i)
);
});
74 changes: 74 additions & 0 deletions packages/cli/test/execute/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,80 @@ test.serial('run a workflow', async (t) => {
t.is(result.data.count, 84);
});

test.serial('run a workflow with a JSON credential map', async (t) => {
const workflow = {
workflow: {
steps: [
{
id: 'a',
// The two steps in this workflow will just write the credential to state
expression: `${fn}fn(s => { s.a = s.configuration.password; return s; })`,
configuration: 'A',
next: { b: true },
},
{
id: 'b',
expression: `${fn}fn(s => { s.b = s.configuration.password; return s; })`,
configuration: 'B',
},
],
},
};
mockFs({
'/workflow.json': JSON.stringify(workflow),
'/creds.json': JSON.stringify({
A: { password: 'a' },
B: { password: 'b' },
}),
});

const options = {
...defaultOptions,
workflowPath: '/workflow.json',
credentials: '/creds.json',
};
const result = await handler(options, logger);
t.is(result.a, 'a');
t.is(result.b, 'b');
});

test.serial.skip('run a workflow with a YAML credential map', async (t) => {
const workflow = {
workflow: {
steps: [
{
id: 'a',
// The two steps in this workflow will just write the credential to state
expression: `${fn}fn(s => { s.a = s.configuration.password; return s; })`,
configuration: 'A',
next: { b: true },
},
{
id: 'b',
expression: `${fn}fn(s => { s.b = s.configuration.password; return s; })`,
configuration: 'B',
},
],
},
};
mockFs({
'/workflow.json': JSON.stringify(workflow),
'/creds.yaml': `A:
password: a
B:
password: b`,
});

const options = {
...defaultOptions,
workflowPath: '/workflow.json',
credentials: '/creds.yaml',
};
const result = await handler(options, logger);
t.is(result.a, 'a');
t.is(result.b, 'b');
});

test.serial('run a workflow with state', async (t) => {
const workflow = {
workflow: {
Expand Down
3 changes: 2 additions & 1 deletion packages/engine-multi/test/worker/pool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ test('destroy should handle un-initialised workers', async (t) => {
t.is(pool._pool.length, 0);
});

test('destroy should close all child processes', async (t) => {
// Flaky - see https://github.com/OpenFn/kit/issues/1192
test.skip('destroy should close all child processes', async (t) => {
// warm up a pool
const pool = createPool(workerPath, { capacity: 10 }, logger);

Expand Down
Loading