Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue234 Import all keys from specific path #238

Closed
wants to merge 7 commits into from
Closed

Issue234 Import all keys from specific path #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cc9c383
Update and rename build.yml to manual-build.yml
slemme1 Jul 27, 2021
f09a7fe
Initial check-in of wildcard to get all secrets in path (Issue#234)
slemme1 Jul 27, 2021
6c9a25f
Merge branch 'master' of https://github.com/slemme1/vault-action
slemme1 Jul 27, 2021
a729d0a
Fix wildcard for K/V v2 and Cubbyhole. Add more tests
slemme1 Jul 28, 2021
755188e
Update README.md
slemme1 Jul 28, 2021
8bf66ca
Update and rename manual-build.yml to build.yml
slemme1 Jul 28, 2021
5ecd962
Merge branch 'main' into master
slemme1 Mar 2, 2023
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
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,13 @@ with:
secret/data/ci/aws accessKey | AWS_ACCESS_KEY_ID ;
secret/data/ci/aws secretKey | AWS_SECRET_ACCESS_KEY
```
You can specify a wildcard * for the key name to get all keys in the path. If you provide an output name with the wildcard, the name will be prepended to the key name:

```yaml
with:
secrets: |
secret/data/ci/aws * | MYAPP_ ;
```

## Other Secret Engines

Expand Down
59 changes: 59 additions & 0 deletions integrationTests/basic/integration.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,26 @@ describe('integration', () => {
expect(core.exportVariable).toBeCalledWith('OTHERSECRETDASH', 'OTHERSUPERSECRET');
});

it('get wildcard secrets', async () => {
mockInput(`secret/data/test * ;`);

await exportSecrets();

expect(core.exportVariable).toBeCalledTimes(1);

expect(core.exportVariable).toBeCalledWith('SECRET', 'SUPERSECRET');
});

it('get wildcard secrets with name prefix', async () => {
mockInput(`secret/data/test * | GROUP_ ;`);

await exportSecrets();

expect(core.exportVariable).toBeCalledTimes(1);

expect(core.exportVariable).toBeCalledWith('GROUP_SECRET', 'SUPERSECRET');
});

it('leading slash kvv2', async () => {
mockInput('/secret/data/foobar fookv2');

Expand All @@ -195,6 +215,34 @@ describe('integration', () => {
expect(core.exportVariable).toBeCalledWith('OTHERSECRETDASH', 'OTHERCUSTOMSECRET');
});

it('get K/V v1 wildcard secrets', async () => {
mockInput(`secret-kv1/test * ;`);

await exportSecrets();

expect(core.exportVariable).toBeCalledTimes(1);

expect(core.exportVariable).toBeCalledWith('SECRET', 'CUSTOMSECRET');
});

it('get K/V v1 wildcard secrets with name prefix', async () => {
mockInput(`secret-kv1/test * | GROUP_ ;`);

await exportSecrets();

expect(core.exportVariable).toBeCalledTimes(1);

expect(core.exportVariable).toBeCalledWith('GROUP_SECRET', 'CUSTOMSECRET');
});

it('get wildcard nested secret from K/V v1', async () => {
mockInput('secret-kv1/nested/test *');

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('OTHERSECRETDASH', 'OTHERCUSTOMSECRET');
});

it('leading slash kvv1', async () => {
mockInput('/secret-kv1/foobar fookv1');

Expand Down Expand Up @@ -225,6 +273,17 @@ describe('integration', () => {
expect(core.exportVariable).toBeCalledWith('FOO', 'bar');
});

it('wildcard supports cubbyhole', async () => {
mockInput('/cubbyhole/test *');

await exportSecrets();

expect(core.exportVariable).toBeCalledTimes(2);

expect(core.exportVariable).toBeCalledWith('FOO', 'bar');
expect(core.exportVariable).toBeCalledWith('ZIP', 'zap');
});

it('caches responses', async () => {
mockInput(`
/cubbyhole/test foo ;
Expand Down
32 changes: 32 additions & 0 deletions integrationTests/enterprise/enterprise.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ describe('integration', () => {
expect(core.exportVariable).toBeCalledWith('TEST_KEY', 'SUPERSECRET_IN_NAMESPACE');
});

it('get wildcard secrets', async () => {
mockInput('secret/data/test *');

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('SECRET', 'SUPERSECRET_IN_NAMESPACE');
});

it('get wildcard secrets with name prefix', async () => {
mockInput('secret/data/test * | GROUP_');

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('GROUP_SECRET', 'SUPERSECRET_IN_NAMESPACE');
});

it('get nested secret', async () => {
mockInput('secret/data/nested/test otherSecret');

Expand Down Expand Up @@ -103,6 +119,22 @@ describe('integration', () => {
expect(core.exportVariable).toBeCalledWith('SECRET', 'CUSTOMSECRET_IN_NAMESPACE');
});

it('get wildcard secrets from K/V v1', async () => {
mockInput('my-secret/test *');

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('SECRET', 'CUSTOMSECRET_IN_NAMESPACE');
});

it('get wildcard secrets from K/V v1 with name prefix', async () => {
mockInput('my-secret/test * | GROUP_');

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('GROUP_SECRET', 'CUSTOMSECRET_IN_NAMESPACE');
});

it('get nested secret from K/V v1', async () => {
mockInput('my-secret/nested/test otherSecret');

Expand Down
45 changes: 28 additions & 17 deletions src/action.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,25 @@ const core = require('@actions/core');
const command = require('@actions/core/lib/command');
const got = require('got').default;
const jsonata = require('jsonata');
module.exports = {};
const wildcard = '*';
module.exports.wildcard = wildcard;

/**
* Replaces any dot chars to __ and removes non-ascii charts
* @param {string} dataKey
* @param {boolean=} isEnvVar
*/
function normalizeOutputKey(dataKey, isEnvVar = false) {
let outputKey = dataKey
.replace('.', '__').replace(new RegExp('-', 'g'), '').replace(/[^\p{L}\p{N}_-]/gu, '');
if (isEnvVar) {
outputKey = outputKey.toUpperCase();
}
return outputKey;
}
module.exports.normalizeOutputKey = normalizeOutputKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it causes circular dependency problems, it'd be better to move the normalizeOutputKey method into it's own normalization.js or utils.js file so it can be imported and used by both secrets.js & actions.js.

It's usually better to rethink how a project is compartmentalised rather than hack the exports.


const { auth: { retrieveToken }, secrets: { getSecrets } } = require('./index');

const AUTH_METHODS = ['approle', 'token', 'github', 'jwt', 'kubernetes'];
Expand Down Expand Up @@ -113,6 +132,7 @@ async function exportSecrets() {
core.debug(`✔ ${request.path} => outputs.${request.outputVarName}${exportEnv ? ` | env.${request.envVarName}` : ''}`);
}
};
module.exports.exportSecrets = exportSecrets;

/** @typedef {Object} SecretRequest
* @property {string} path
Expand Down Expand Up @@ -167,7 +187,7 @@ function parseSecretsInput(secretsInput) {
const selectorAst = jsonata(selectorQuoted).ast();
const selector = selectorQuoted.replace(new RegExp('"', 'g'), '');

if ((selectorAst.type !== "path" || selectorAst.steps[0].stages) && selectorAst.type !== "string" && !outputVarName) {
if (selector !== wildcard && (selectorAst.type !== "path" || selectorAst.steps[0].stages) && selectorAst.type !== "string" && !outputVarName) {
throw Error(`You must provide a name for the output key when using json selectors. Input: "${secret}"`);
}

Expand All @@ -186,20 +206,7 @@ function parseSecretsInput(secretsInput) {
}
return output;
}

/**
* Replaces any dot chars to __ and removes non-ascii charts
* @param {string} dataKey
* @param {boolean=} isEnvVar
*/
function normalizeOutputKey(dataKey, isEnvVar = false) {
let outputKey = dataKey
.replace('.', '__').replace(new RegExp('-', 'g'), '').replace(/[^\p{L}\p{N}_-]/gu, '');
if (isEnvVar) {
outputKey = outputKey.toUpperCase();
}
return outputKey;
}
module.exports.parseSecretsInput = parseSecretsInput;

/**
* @param {string} inputKey
Expand All @@ -225,10 +232,14 @@ function parseHeadersInput(inputKey, inputOptions) {
return map;
}, new Map());
}

module.exports.parseHeadersInput = parseHeadersInput;
// restructured module.exports to avoid circular dependency when secrets imports this.
/*
module.exports = {
exportSecrets,
parseSecretsInput,
normalizeOutputKey,
parseHeadersInput
parseHeadersInput,
wildcard
};
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go ahead and move the normalization method in its own file well be able to keep this unique export block which I think is more "standard" than exporting methods one at a time throughout the file.

80 changes: 66 additions & 14 deletions src/secrets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const jsonata = require("jsonata");


const { normalizeOutputKey, wildcard} = require('./action');
/**
* @typedef {Object} SecretRequest
* @property {string} path
Expand All @@ -24,6 +23,7 @@ const jsonata = require("jsonata");
async function getSecrets(secretRequests, client) {
const responseCache = new Map();
const results = [];

for (const secretRequest of secretRequests) {
let { path, selector } = secretRequest;

Expand All @@ -46,25 +46,77 @@ async function getSecrets(secretRequests, client) {
throw error
}
}
if (!selector.match(/.*[\.].*/)) {
selector = '"' + selector + '"'

if (selector == wildcard) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe strict equality with === should be used unless there is a specific reason not to.

body = JSON.parse(body);
let keys = body.data;
if (body.data["data"] != undefined) {
keys = keys.data;
}

for (let key in keys) {
let newRequest = Object.assign({},secretRequest);
newRequest.selector = key;
if (secretRequest.selector === secretRequest.outputVarName) {
newRequest.outputVarName = key;
newRequest.envVarName = key;
}
else {
newRequest.outputVarName = secretRequest.outputVarName+key;
newRequest.envVarName = secretRequest.envVarName+key;
}
newRequest.outputVarName = normalizeOutputKey(newRequest.outputVarName);
newRequest.envVarName = normalizeOutputKey(newRequest.envVarName,true);

selector = key;

//This code (with exception of parsing body again and using newRequest instead of secretRequest) should match the else code for a single key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to isolate the shared code in a method instead of hoping maintainers see this comment and think to copy-paste changes between the wildcard and single-key paths?

if (!selector.match(/.*[\.].*/)) {
selector = '"' + selector + '"'
}
selector = "data." + selector
//body = JSON.parse(body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover comment, should it be deleted or it's important?

if (body.data["data"] != undefined) {
selector = "data." + selector
}
const value = selectData(body, selector);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectData was changed to be async due to the recent bump to Jsonata 2.x, you'll need to await the response otherwise the code does not work and tries to manipulate a promise.

const value = await selectData(body, selector);

results.push({
request: newRequest,
value,
cachedResponse
});

//DEBUG
//console.log("After", newRequest, value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try not to leave leftover debug code or console.logs even if commented out.


// used cachedResponse for first entry in wildcard list and set to true for the rest
cachedResponse = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think setting this to true does anything here? It gets reset to false at the beginning of every loops before we check for a cache hit.

}

}
selector = "data." + selector
body = JSON.parse(body)
if (body.data["data"] != undefined) {
else {
if (!selector.match(/.*[\.].*/)) {
selector = '"' + selector + '"'
}
selector = "data." + selector
}
body = JSON.parse(body)
if (body.data["data"] != undefined) {
selector = "data." + selector
}

const value = await selectData(body, selector);
results.push({
request: secretRequest,
value,
cachedResponse
});

const value = await selectData(body, selector);
results.push({
request: secretRequest,
value,
cachedResponse
});
}
}
return results;
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra empty line can be removed?

/**
* Uses a Jsonata selector retrieve a bit of data from the result
* @param {object} data
Expand Down