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.

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

fix: fix to respect ask env variables for commands #167

Merged
merged 4 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
21 changes: 21 additions & 0 deletions lib/clients/aws-client/abstract-client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const aws = require('aws-sdk');

const CONSTANTS = require('@src/utils/constants');
const stringUtils = require('@src/utils/string-utils');


module.exports = class AbstractClient {
kakhaUrigashvili marked this conversation as resolved.
Show resolved Hide resolved
constructor(configuration) {
const { awsProfile, awsRegion } = configuration;
if (!stringUtils.isNonBlankString(awsProfile) || !stringUtils.isNonBlankString(awsRegion)) {
throw new Error('Invalid awsProfile or Invalid awsRegion');
}
if (awsProfile !== CONSTANTS.PLACEHOLDER.ENVIRONMENT_VAR.AWS_CREDENTIALS) {
aws.config.credentials = new aws.SharedIniFileCredentials({
profile: awsProfile
});
}
this.awsRegion = awsRegion;
kakhaUrigashvili marked this conversation as resolved.
Show resolved Hide resolved
aws.config.region = this.awsRegion;
}
};
13 changes: 3 additions & 10 deletions lib/clients/aws-client/cloudformation-client.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
const aws = require('aws-sdk');
const R = require('ramda');
const stringUtils = require('@src/utils/string-utils');
const AbstractClient = require('./abstract-client');

/**
* Class for AWS Cloudformation Client
*/
module.exports = class CloudformationClient {
module.exports = class CloudformationClient extends AbstractClient {
constructor(configuration) {
const { awsProfile, awsRegion } = configuration;
if (!stringUtils.isNonBlankString(awsProfile) || !stringUtils.isNonBlankString(awsRegion)) {
throw new Error('Invalid awsProfile or Invalid awsRegion');
}
aws.config.credentials = new aws.SharedIniFileCredentials({
profile: awsProfile
});
this.awsRegion = awsRegion;
aws.config.region = this.awsRegion;
super(configuration);
this.client = new aws.CloudFormation();
}

Expand Down
15 changes: 4 additions & 11 deletions lib/clients/aws-client/iam-client.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,15 @@
const aws = require('aws-sdk');


const CONSTANTS = require('@src/utils/constants');
const stringUtils = require('@src/utils/string-utils');
const AbstractClient = require('./abstract-client');

/**
* Class for AWS IAM Client
*/
module.exports = class IAMClient {
module.exports = class IAMClient extends AbstractClient {
constructor(configuration) {
const { awsProfile, awsRegion } = configuration;
if (!stringUtils.isNonBlankString(awsProfile) || !stringUtils.isNonBlankString(awsRegion)) {
throw new Error('Invalid awsProfile or Invalid awsRegion');
}
aws.config.credentials = new aws.SharedIniFileCredentials({
profile: awsProfile
});
this.awsRegion = awsRegion;
aws.config.region = this.awsRegion;
super(configuration);
this.client = new aws.IAM();
}

Expand Down
13 changes: 3 additions & 10 deletions lib/clients/aws-client/lambda-client.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
const aws = require('aws-sdk');

const stringUtils = require('@src/utils/string-utils');
const AbstractClient = require('./abstract-client');

/**
* Class for Lambda Client
*/
module.exports = class LambdaClient {
module.exports = class LambdaClient extends AbstractClient {
constructor(configuration) {
const { awsProfile, awsRegion } = configuration;
if (!stringUtils.isNonBlankString(awsProfile) || !stringUtils.isNonBlankString(awsRegion)) {
throw new Error('Invalid awsProfile or Invalid awsRegion');
}
aws.config.credentials = new aws.SharedIniFileCredentials({
profile: awsProfile
});
this.awsRegion = awsRegion;
aws.config.region = this.awsRegion;
super(configuration);
this.client = new aws.Lambda();
}

Expand Down
15 changes: 4 additions & 11 deletions lib/clients/aws-client/s3-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,15 @@ const aws = require('aws-sdk');
const SpinnerView = require('@src/view/spinner-view');
const { ParallelStream } = require('@src/utils/stream-utility');
const CONSTANTS = require('@src/utils/constants');
const stringUtil = require('@src/utils/string-utils');
const AbstractClient = require('./abstract-client');


/**
* Class for S3 client
*/
module.exports = class S3Client {
module.exports = class S3Client extends AbstractClient {
constructor(configuration) {
const { awsProfile, awsRegion } = configuration;
if (!stringUtil.isNonBlankString(awsProfile) || !stringUtil.isNonBlankString(awsRegion)) {
throw new Error('Invalid awsProfile or Invalid awsRegion');
}
aws.config.credentials = new aws.SharedIniFileCredentials({
profile: awsProfile
});
this.awsRegion = awsRegion;
aws.config.region = this.awsRegion;
super(configuration);

const httpClient = new aws.NodeHttpClient();
const httpAgent = httpClient.getAgent(true, {
Expand Down
4 changes: 3 additions & 1 deletion lib/commands/abstract-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const AppConfig = require('@src/model/app-config');
const CONSTANTS = require('@src/utils/constants');
const metricClient = require('@src/utils/metrics');
const Messenger = require('@src/view/messenger');
const profileHelper = require('@src/utils/profile-helper');
kakhaUrigashvili marked this conversation as resolved.
Show resolved Hide resolved

const packageJson = require('@root/package.json');

Expand Down Expand Up @@ -90,7 +91,8 @@ class AbstractCommand {
* Only `ask configure` command should have the eligibility to create the ASK config file (which is handled
* in the configure workflow).
*/
if (commandInstance._name !== 'configure') {
if (commandInstance._name !== 'configure'
&& profileHelper.runtimeProfile() !== CONSTANTS.PLACEHOLDER.ENVIRONMENT_VAR.PROFILE_NAME) {
this._initiateAppConfig();
}
} catch (err) {
Expand Down
47 changes: 47 additions & 0 deletions test/unit/clients/aws-client/abstract-client-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
const { expect } = require('chai');
const aws = require('aws-sdk');
const CONSTANTS = require('@src/utils/constants');

const AbstractClient = require('@src/clients/aws-client/abstract-client');

describe('Clients test - abstract client test', () => {
const TEST_AWS_PROFILE = 'TEST_AWS_PROFILE';
const TEST_AWS_REGION = 'TEST_AWS_REGION';
let configuration;
beforeEach(() => {
aws.config.credentials.profile = null;
configuration = {
awsProfile: TEST_AWS_PROFILE,
awsRegion: TEST_AWS_REGION
};
});

describe('# constructor tests', () => {
it('| should set region and credentials profile', () => {
const client = new AbstractClient(configuration);
expect(client).to.be.instanceof(AbstractClient);
expect(client.awsRegion).eql(TEST_AWS_REGION);
expect(aws.config.region).eql(TEST_AWS_REGION);
expect(aws.config.credentials.profile).eql(TEST_AWS_PROFILE);
});

it('| should not set credentials profile since using env variables', () => {
configuration.awsProfile = CONSTANTS.PLACEHOLDER.ENVIRONMENT_VAR.AWS_CREDENTIALS;
const client = new AbstractClient(configuration);
expect(client).to.be.instanceof(AbstractClient);
expect(client.awsRegion).eql(TEST_AWS_REGION);
expect(aws.config.region).eql(TEST_AWS_REGION);
expect(aws.config.credentials.profile).eql(null);
});

it('| should throw error when aws profile is not specified ', () => {
configuration.awsProfile = null;
expect(() => new AbstractClient(configuration)).to.throw('Invalid awsProfile or Invalid awsRegion');
});

it('| should throw error when aws region is not specified ', () => {
configuration.awsRegion = null;
expect(() => new AbstractClient(configuration)).to.throw('Invalid awsProfile or Invalid awsRegion');
});
});
});
29 changes: 29 additions & 0 deletions test/unit/commands/abstract-command-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const AppConfig = require('@src/model/app-config');
const CONSTANTS = require('@src/utils/constants');
const metricClient = require('@src/utils/metrics');
const Messenger = require('@src/view/messenger');
const profileHelper = require('@src/utils/profile-helper');

const packageJson = require('@root/package.json');

Expand Down Expand Up @@ -436,6 +437,7 @@ It is recommended to use the latest version. Please update using "npm upgrade -g
});

it('| should not be null for other commands', async () => {
sinon.stub(profileHelper, 'runtimeProfile');
class NonConfigureCommand extends AbstractCommand {
constructor(optionModel, handle) {
super(optionModel);
Expand Down Expand Up @@ -464,6 +466,7 @@ It is recommended to use the latest version. Please update using "npm upgrade -g
});

it('| should be null for configure command', async () => {
sinon.stub(profileHelper, 'runtimeProfile');
class ConfigureCommand extends AbstractCommand {
constructor(optionModel, handle) {
super(optionModel);
Expand Down Expand Up @@ -491,6 +494,32 @@ It is recommended to use the latest version. Please update using "npm upgrade -g
await commander.parseAsync(['node', 'mock', 'configure']);
});

it('| should be null when profile from env variables', async () => {
sinon.stub(profileHelper, 'runtimeProfile').returns(CONSTANTS.PLACEHOLDER.ENVIRONMENT_VAR.PROFILE_NAME);
class SomeCommand extends AbstractCommand {
constructor(optionModel, handle) {
super(optionModel);
this.handle = handle;
}

name() {
return 'test';
}

description() {
return 'test description';
}
}

const mockCommand = new SomeCommand(mockOptionModel, (options, cb) => {
expect(AppConfig.getInstance()).eq(null);
cb();
});

mockCommand.createCommand()(commander);
await commander.parseAsync(['node', 'mock', 'test']);
});

afterEach(() => {
sinon.restore();
AppConfig.dispose();
Expand Down
1 change: 1 addition & 0 deletions test/unit/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ require('module-alias/register');
'@test/unit/clients/smapi-client-test',
'@test/unit/clients/git-client-test',
'@test/unit/clients/lwa-auth-code-client-test',
'@test/unit/clients/aws-client/abstract-client-test',
'@test/unit/clients/aws-client/s3-client-test',
'@test/unit/clients/aws-client/cloudformation-client-test',
'@test/unit/clients/aws-client/aws-util-test',
Expand Down