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

RHINENG-14704: Pass request as param instead of using cls #574

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

RHINENG-14704: Pass request as param instead of using cls #574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 5 additions & 6 deletions src/connectors/Connector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,19 @@ module.exports = class Connector {
return uri;
}

async doHttp (options, caching, metrics = false, responseTransformer) {
async doHttp (req, options, caching, metrics = false, responseTransformer) {
try {
const result = await http.request(options, caching, metrics, responseTransformer);
const result = await http.request(req, options, caching, metrics, responseTransformer);
return result;
} catch (e) {
log.trace(e, 'dependency error');
metrics && metrics.error.inc();
throw errors.internal.dependencyError(e, this);
throw errors.internal.dependencyError(req, e, this);
}
}

getForwardedHeaders (identity = true) {
const req = cls.getReq();
assert(req, 'request not available in CLS');
getForwardedHeaders (req, identity = true) {
// const req1 = cls.getReq();
const toPick = [REQ_ID_HEADER];
if (identity) {
toPick.push(IDENTITY_HEADER);
Expand Down
11 changes: 10 additions & 1 deletion src/connectors/Connector.unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,20 @@ const StatusCodeError = require('./StatusCodeError');
const errors = require('../errors');
const { mockRequest } = require('./testUtils');

const REQ = {
headers: {
'x-rh-identity': 'identity',
'x-rh-insights-request-id': 'request-id'
},
identity: { type: 'test' },
user: { username: 'test', account_number: 'test' }
};

describe('Connector', function () {

test('wraps errors', async function () {
mockRequest();
base.getSandbox().stub(http, 'request').rejects(new StatusCodeError(500));
await expect(vmaas.getCve('id')).rejects.toThrow(errors.DependencyError);
await expect(vmaas.getCve(REQ, 'id')).rejects.toThrow(errors.DependencyError);
});
});
22 changes: 11 additions & 11 deletions src/connectors/advisor/impl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ module.exports = new class extends Connector {
return this.buildUri(host, 'advisor', 'v1');
}

getRule (id, refresh = false) {
getRule (req, id, refresh = false) {
const uri = this.buildBaseUri(host, 'advisor', 'v1');
uri.segment('rule');
uri.segment(id);

return this.doHttp({
return this.doHttp(req, {
uri: uri.toString(),
method: 'GET',
json: true,
rejectUnauthorized: !insecure,
headers: {
...this.getForwardedHeaders()
...this.getForwardedHeaders(req)
}
}, {
refresh,
Expand All @@ -45,7 +45,7 @@ module.exports = new class extends Connector {
this.ruleMetrics);
}

async getDiagnosis (system, branchId = null) {
async getDiagnosis (req, system, branchId = null) {
const uri = this.buildBaseUri();
uri.segment('system');
uri.segment(system);
Expand All @@ -56,13 +56,13 @@ module.exports = new class extends Connector {
uri.query({branch_id: branchId});
}

const data = await this.doHttp({
const data = await this.doHttp(req, {
uri: uri.toString(),
method: 'GET',
json: true,
rejectUnauthorized: !insecure,
headers: {
...this.getForwardedHeaders()
...this.getForwardedHeaders(req)
}
}, false, this.diagnosisMetrics);

Expand All @@ -87,18 +87,18 @@ module.exports = new class extends Connector {
.value();
}

async getSystems (id) {
async getSystems (id, req) {
const uri = this.buildBaseUri(host, 'advisor', 'v1');
uri.segment('rule');
uri.segment(id);
uri.segment('systems');

const data = await this.doHttp({
const data = await this.doHttp(req, {
uri: uri.toString(),
method: 'GET',
json: true,
rejectUnauthorized: !insecure,
headers: this.getForwardedHeaders()
headers: this.getForwardedHeaders(req)
},
false,
this.systemsMetrics);
Expand All @@ -110,8 +110,8 @@ module.exports = new class extends Connector {
return data.host_ids;
}

async ping () {
const result = await this.getRule('network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE', true);
async ping (req) {
const result = await this.getRule(req, 'network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE', true);
assert(result !== null);
}
}();
23 changes: 16 additions & 7 deletions src/connectors/advisor/impl.unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ const { mockRequest, mockCache } = require('../testUtils');
const request = require('../../util/request');
const errors = require('../../errors');

const REQ = {
headers: {
'x-rh-identity': 'identity',
'x-rh-insights-request-id': 'request-id'
},
identity: { type: 'test' },
user: { username: 'test', account_number: 'test' }
};

/* eslint-disable max-len */
describe('advisor impl', function () {

Expand Down Expand Up @@ -68,7 +77,7 @@ describe('advisor impl', function () {
headers: {}
});

const result = await impl.getRule('network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE');
const result = await impl.getRule(REQ, 'network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE');
result.should.have.property('summary', 'Bonding will not fail over to the backup link when bonding options are partially read.\n');

http.callCount.should.equal(1);
Expand All @@ -79,7 +88,7 @@ describe('advisor impl', function () {
cache.get.callCount.should.equal(1);
cache.setex.callCount.should.equal(1);

await impl.getRule('network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE');
await impl.getRule(REQ, 'network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE');
cache.get.callCount.should.equal(2);
cache.setex.callCount.should.equal(1);
});
Expand All @@ -95,7 +104,7 @@ describe('advisor impl', function () {
headers: {}
});

await expect(impl.getRule('unknown-rule')).resolves.toBeNull();
await expect(impl.getRule(REQ, 'unknown-rule')).resolves.toBeNull();

http.callCount.should.equal(1);
cache.get.callCount.should.equal(1);
Expand All @@ -104,15 +113,15 @@ describe('advisor impl', function () {

test('status code handling', async function () {
base.mockRequestStatusCode();
await expect(impl.getRule('network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE')).rejects.toThrow(errors.DependencyError);
await expect(impl.getRule(REQ, 'network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE')).rejects.toThrow(errors.DependencyError);
});
});

describe('getDiagnosis', function() {
test('parses diagnosis reports', async function () {
const spy = base.getSandbox().stub(Connector.prototype, 'doHttp').resolves(data.diagnosis1);

const diagnosis = await impl.getDiagnosis('id', 'branchId');
const diagnosis = await impl.getDiagnosis(REQ, 'id', 'branchId');

spy.callCount.should.equal(1);
diagnosis.should.eql({
Expand Down Expand Up @@ -155,7 +164,7 @@ describe('advisor impl', function () {
headers: {}
});

await expect(impl.getSystems('rule')).resolves.toEqual([
await expect(impl.getSystems('rule', REQ)).resolves.toEqual([
'7de3c608-8553-4625-90c7-d27f1d29327f',
'0d962e11-ff3c-4db1-b06c-c4674d90069b'
]);
Expand All @@ -173,7 +182,7 @@ describe('advisor impl', function () {
headers: {}
});

await expect(impl.getSystems('unknown-rule')).resolves.toEqual([]);
await expect(impl.getSystems('unknown-rule', REQ)).resolves.toEqual([]);

http.callCount.should.equal(1);
cache.get.callCount.should.equal(0);
Expand Down
8 changes: 4 additions & 4 deletions src/connectors/advisor/mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ module.exports = new class extends Connector {
super(module);
}

async getRule (id) {
async getRule (req, id) {
if (_.has(DATA, id)) {
// eslint-disable-next-line security/detect-object-injection
return DATA[id];
Expand All @@ -67,7 +67,7 @@ module.exports = new class extends Connector {
return null;
}

async getDiagnosis (system, branchId) {
async getDiagnosis (req, system, branchId) {
if (system === 'none') {
return {};
}
Expand All @@ -86,7 +86,7 @@ module.exports = new class extends Connector {
];
}

ping () {
return this.getRule('network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE');
ping (req) {
return this.getRule(req, 'network_bond_opts_config_issue|NETWORK_BONDING_OPTS_DOUBLE_QUOTES_ISSUE');
}
}();
23 changes: 13 additions & 10 deletions src/connectors/bop/impl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ module.exports = new class extends Connector {

// Given an array of account numbers, fetch corresponding tenant org_ids from
// backoffice proxy
async getTenantOrgIds (accounts) {
// Might have to look at migrations.. this function is used there without request being passed
async getTenantOrgIds (req, accounts) {
log.info(`Fetching tenant_org_ids for accounts: ${accounts}`);

const EBS_accounts = [].concat(accounts).map(String);
Expand All @@ -43,13 +44,14 @@ module.exports = new class extends Connector {
// TODO: I'm not even sure we need to bother forwarding headers for this internal service...
// if this function was called outside the context of a user request (e.g. from a db migration)
// then skip the forwarded headers.
if (cls.getReq()) {
options.headers = this.getForwardedHeaders(false);
// if (cls.getReq()) {
if (req) {
options.headers = this.getForwardedHeaders(req, false);
}

try {
log.debug(`Request options: ${JSON.stringify(options)}`);
const result = await this.doHttp (options, false, this.orgIdMetrics);
const result = await this.doHttp (req, options, false, this.orgIdMetrics);
log.debug(`POST response: ${JSON.stringify(result)}`);

if (_.isEmpty(result)) {
Expand All @@ -65,7 +67,7 @@ module.exports = new class extends Connector {

// Given an array of tenant org_ids, fetch corresponding EBS account numbers
// from backoffice proxy
async getEBSAccounts (org_ids) {
async getEBSAccounts (req, org_ids) {
log.info(`Fetching EBS Accounts for: ${org_ids}`);

const tenant_org_ids = [].concat(org_ids).map(String);
Expand All @@ -83,13 +85,13 @@ module.exports = new class extends Connector {
method: 'POST',
json: true,
rejectUnauthorized: !insecure,
headers: this.getForwardedHeaders(false),
headers: this.getForwardedHeaders(req, false),
body: tenant_org_ids
};

try {
log.debug(`Request options: ${JSON.stringify(options)}`);
const result = await this.doHttp (options, false, this.EBSAccountMetrics);
const result = await this.doHttp (req, options, false, this.EBSAccountMetrics);
log.debug(`POST response: ${JSON.stringify(result)}`);

if (_.isEmpty(result)) {
Expand All @@ -104,9 +106,10 @@ module.exports = new class extends Connector {
}

// Verify connection to backoffice proxy tenant org_id / EBS account number translation service
async ping () {
const req = cls.getReq();
const result = await this.getEBSAccounts([`${req.identity.internal.org_id}`]);
// Pass req to ping here too
async ping (req) {
// const req = cls.getReq();
const result = await this.getEBSAccounts(req, [`${req.identity.internal.org_id}`]);
assert(result !== null);
}
}();
Loading
Loading