-
Notifications
You must be signed in to change notification settings - Fork 68
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.
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
CLI arguments made order invariant #81
Changes from 7 commits
b4736b4
e0a1c4b
6b6b061
f9f34b6
53577b6
ee535e5
ae6438e
74e6afd
a2b0a49
15b87af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"env": { | ||
"browser": false, | ||
"node": true | ||
}, | ||
"parserOptions": { | ||
"ecmaVersion": 2017 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,21 @@ | ||
#!/usr/bin/env node | ||
|
||
var pjson = require('./package.json'); | ||
const pjson = require('./package.json'); | ||
const prog = require('caporal'); | ||
const encrypt = require('./actions/encrypt'); | ||
const regexGuid = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; | ||
|
||
prog | ||
.version(pjson.version) | ||
.command('encrypt', 'Encrypt data') | ||
.argument('<data>','Data to encrypt') | ||
.argument('<service-account>', 'Deployment service account') | ||
.argument('<namespace>', 'Deployment namespace') | ||
.action(encrypt) | ||
.action(encrypt) | ||
.option('--secret <data>','Data to encrypt', prog.REQUIRED) | ||
.option('--service-account <service-account>', 'Deployment service account', prog.REQUIRED) | ||
.option('--namespace <namespace>', 'Deployment namespace', prog.REQUIRED) | ||
.option('--kamus-url <kamusUrl>', 'Kamus URL', prog.REQUIRED) | ||
.option('--auth-tenant <id>', 'Azure tenant id', regexGuid) | ||
.option('--auth-application <id>', 'Azure application id', regexGuid) | ||
.option('--auth-resource <name>', 'Azure resource name', prog.STRING) | ||
.option('--kamus-url <url>', 'Kamus URL', prog.REQUIRED) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to keep it shorter - |
||
.option('--allow-insecure-url', 'Allow insecure (http) Kamus URL', prog.BOOL) | ||
.option('--cert-fingerprint <certFingerprint>', 'Force server certificate to match the given fingerprint', prog.STRING); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
var fs = require('fs'); | ||
const fs = require('fs'); | ||
|
||
var isDocker; | ||
let isDocker; | ||
|
||
function hasDockerEnv() { | ||
try { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,9 @@ const expect = require('chai').expect; | |
const nock = require('nock'); | ||
const sinon = require('sinon'); | ||
|
||
const encrypt = require('../actions/encrypt.js'); | ||
const encrypt = require('../actions/encrypt'); | ||
|
||
const logger = | ||
const logger = | ||
{ | ||
info: sinon.spy(), | ||
error: console.error, | ||
|
@@ -17,16 +17,19 @@ const data = 'super-secret'; | |
const serviceAccount = 'dummy'; | ||
const namespace = 'team-a'; | ||
|
||
let kamusApiScope; | ||
|
||
describe('Encrypt', () => { | ||
beforeEach(() => { | ||
sinon.stub(process, 'exit'); | ||
nock(kamusUrl) | ||
.post('/api/v1/encrypt', { data, "service-account": serviceAccount, namespace}) | ||
kamusApiScope = nock(kamusUrl) | ||
.post('/api/v1/encrypt', { data, ['service-account']: serviceAccount, namespace}) | ||
.reply(200, '123ABC'); | ||
}); | ||
|
||
it('Should return encrypted data', async () => { | ||
await encrypt({data, serviceAccount, namespace}, {kamusUrl}, logger); | ||
await encrypt(null, {data, serviceAccount, namespace, kamusUrl}, logger); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why null is added here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
expect(kamusApiScope.isDone()).to.be.true; | ||
expect(process.exit.called).to.be.true; | ||
expect(process.exit.calledWith(0)).to.be.true; | ||
expect(logger.info.lastCall.lastArg).to.equal('Encrypted data:\n123ABC') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes the arguments global options, and not only for the encrypt commands - correct? Isn't it weird?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes the arguments global. I chained another
command
method and didn't see that arguments in the second command help.Only arguments or options that are set before the first
command
method are global.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if what you're saying is true - all these:
Should move up before the first command...