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

feat: add proxy checks to doctor #848

Merged
merged 2 commits into from
Aug 27, 2024
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
4 changes: 4 additions & 0 deletions messages/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@ Warning: the [%s] plugin is linked.
# uninstallSuggestion

Uninstall the deprecated Salesforce CLI (%s) version %s and install the @salesforce/cli version 2. See https://developer.salesforce.com/docs/atlas.en-us.sfdx_setup.meta/sfdx_setup/sfdx_setup_uninstall.htm for uninstall instructions.

# matchProxyEnvVarSuggestion

Modify the values for %s and %s environment variables to match.
50 changes: 50 additions & 0 deletions src/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,54 @@ export class Diagnostics {
status,
});
}

/**
* Checks and warns if proxy env vars conflict.
*/
public async proxyEnvVarsCheck(): Promise<void> {
const httpProxyEnvVars: string[] = [];
const httpsProxyEnvVars: string[] = [];
const noProxyEnvVars: string[] = [];
this.diagnosis.proxyEnvVars.forEach((pev) => {
if (['http_proxy', 'HTTP_PROXY'].includes(pev[0])) {
httpProxyEnvVars.push(pev[1]);
}
if (['https_proxy', 'HTTPS_PROXY'].includes(pev[0])) {
httpsProxyEnvVars.push(pev[1]);
}
if (['no_proxy', 'NO_PROXY'].includes(pev[0])) {
noProxyEnvVars.push(pev[1]);
}
});

const getStatus = (envVars: string[]): DiagnosticStatus['status'] =>
(envVars[0] && envVars.length === 1) || envVars[0] === envVars[1] ? 'pass' : 'fail';

const httpProxyEnvVarStatus = getStatus(httpProxyEnvVars);
const httpsProxyEnvVarStatus = getStatus(httpsProxyEnvVars);
const noProxyEnvVarStatus = getStatus(noProxyEnvVars);

await Lifecycle.getInstance().emit('Doctor:diagnostic', {
testName: 'http_proxy and HTTP_proxy environment variables match',
status: httpProxyEnvVarStatus,
});
await Lifecycle.getInstance().emit('Doctor:diagnostic', {
testName: 'https_proxy and HTTPS_PROXY environment variables match',
status: httpsProxyEnvVarStatus,
});
await Lifecycle.getInstance().emit('Doctor:diagnostic', {
testName: 'no_proxy and NO_PROXY environment variables match',
status: noProxyEnvVarStatus,
});

if (httpProxyEnvVarStatus === 'fail') {
this.doctor.addSuggestion(messages.getMessage('matchProxyEnvVarSuggestion', ['http_proxy', 'HTTP_PROXY']));
}
if (httpsProxyEnvVarStatus === 'fail') {
this.doctor.addSuggestion(messages.getMessage('matchProxyEnvVarSuggestion', ['https_proxy', 'HTTPS_PROXY']));
}
if (noProxyEnvVarStatus === 'fail') {
this.doctor.addSuggestion(messages.getMessage('matchProxyEnvVarSuggestion', ['no_proxy', 'NO_PROXY']));
}
}
}
5 changes: 5 additions & 0 deletions src/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export type SfDoctorDiagnosis = {
versionDetail: Omit<Interfaces.VersionDetails, 'pluginVersions'> & { pluginVersions: string[] };
sfdxEnvVars: Array<KeyValue<string>>;
sfEnvVars: Array<KeyValue<string>>;
proxyEnvVars: Array<KeyValue<string>>;
cliConfig: CliConfig;
pluginSpecificData: { [pluginName: string]: AnyJson[] };
diagnosticResults: DiagnosticStatus[];
Expand Down Expand Up @@ -79,6 +80,9 @@ export class Doctor implements SfDoctor {
__cliConfig = config;
const sfdxEnvVars = new Env().entries().filter((e) => e[0].startsWith('SFDX_'));
const sfEnvVars = new Env().entries().filter((e) => e[0].startsWith('SF_'));
const proxyEnvVars = new Env()
.entries()
.filter((e) => ['http_proxy', 'https_proxy', 'no_proxy'].includes(e[0].toLowerCase()));
const cliConfig = omit(config, [
'plugins',
'pjson',
Expand All @@ -96,6 +100,7 @@ export class Doctor implements SfDoctor {
versionDetail: { ...versionDetails, pluginVersions: formatPlugins(config, pluginVersions ?? {}) },
sfdxEnvVars,
sfEnvVars,
proxyEnvVars,
cliConfig,
pluginSpecificData: {},
diagnosticResults: [],
Expand Down
46 changes: 43 additions & 3 deletions test/commands/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ describe('Doctor Command', () => {
delete Doctor.instance;
});

const resetProxyEnvVars = () => {
delete process.env.http_proxy;
delete process.env.https_proxy;
delete process.env.HTTP_PROXY;
delete process.env.HTTPS_PROXY;
delete process.env.no_proxy;
delete process.env.NO_PROXY;
};

const verifySuggestions = (result: SfDoctorDiagnosis, suggestions: string[] = []) => {
const defaults = [
messages.getMessage('pinnedSuggestions.checkGitHubIssues'),
Expand All @@ -183,13 +192,16 @@ describe('Doctor Command', () => {
});
};

const verifyEnvVars = (result: SfDoctorDiagnosis) => {
result.sfdxEnvVars.every((envVar) => {
const verifyEnvVars = (result: SfDoctorDiagnosis, expectedProxyEnvVars: string[] | string[][] = []) => {
result.sfdxEnvVars.forEach((envVar) => {
expect(envVar[0].startsWith('SFDX_')).to.be.true;
});
result.sfEnvVars.every((envVar) => {
result.sfEnvVars.forEach((envVar) => {
expect(envVar[0].startsWith('SF_')).to.be.true;
});
result.proxyEnvVars.forEach((envVar) => {
expect(expectedProxyEnvVars).to.deep.contain(envVar);
});
};

it('runs doctor command with no flags', async () => {
Expand Down Expand Up @@ -227,6 +239,34 @@ describe('Doctor Command', () => {
]);
});

it('reports on uppercase and lowercase proxy env vars', async () => {
const httpProxyEnv = ['http_proxy', 'test.http.proxy'];
const httpsProxyEnv = ['HTTPS_PROXY', 'test.HTTPS.proxy'];
const noProxyEnv = ['no_proxy', 'test.blah.com'];
process.env[httpProxyEnv[0]] = httpProxyEnv[1];
process.env[httpsProxyEnv[0]] = httpsProxyEnv[1];
process.env[noProxyEnv[0]] = noProxyEnv[1];

const suggestion = 'work smarter, not faster';
fsExistsSyncStub.returns(true);
const diagnosticStatus = { testName: 'doctor test', status: 'pass' };
Doctor.init(oclifConfig);
diagnosticsRunStub.callsFake(() => {
const dr = Doctor.getInstance();
const promise = Lifecycle.getInstance().emit('Doctor:diagnostic', diagnosticStatus);
dr.addSuggestion(suggestion);
return [promise];
});

// set proxy env vars
try {
const result = await runDoctorCmd([]);
verifyEnvVars(result, [httpProxyEnv, httpsProxyEnv, noProxyEnv]);
} finally {
resetProxyEnvVars();
}
});

it('runs doctor command with outputdir flag (existing dir)', async () => {
const outputdir = path.resolve('foo', 'bar', 'test');
fsExistsSyncStub.returns(true);
Expand Down
74 changes: 73 additions & 1 deletion test/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,86 @@ describe('Diagnostics', () => {
const results = diagnostics.run();

// This will have to be updated with each new test
expect(results.length).to.equal(5);
expect(results.length).to.equal(6);
expect(childProcessExecStub.called).to.be.true;
expect(lifecycleEmitSpy.called).to.be.true;
expect(lifecycleEmitSpy.args[0][0]).to.equal('Doctor:diagnostic');
expect(lifecycleEmitSpy.args[0][1]).to.have.property('testName');
expect(lifecycleEmitSpy.args[0][1]).to.have.property('status');
});

describe('proxyEnvVarsCheck', () => {
const httpProxy = 'http://test.dr.diagnostics';
const httpsProxy = 'https://test.dr.diagnostics';
const noProxy = 'point.break';

it('passes with no proxy env vars', async () => {
const dr = Doctor.init(oclifConfig);
stubMethod(sandbox, dr, 'getDiagnosis').returns({
proxyEnvVars: [],
});
const diagnostics = new Diagnostics(dr, oclifConfig);
await diagnostics.proxyEnvVarsCheck();

expect(lifecycleEmitSpy.callCount, 'Expected "Doctor:diagnostic" event fired 3 times').to.equal(3);
expect(drAddSuggestionSpy.called, 'Expected no suggestions to be added').to.be.false;
});

it('passes with matching proxy env vars', async () => {
const dr = Doctor.init(oclifConfig);
stubMethod(sandbox, dr, 'getDiagnosis').returns({
proxyEnvVars: [
['http_proxy', httpProxy],
['https_proxy', httpsProxy],
['no_proxy', noProxy],
['HTTP_PROXY', httpProxy],
['HTTPS_PROXY', httpsProxy],
['NO_PROXY', noProxy],
],
});
const diagnostics = new Diagnostics(dr, oclifConfig);
await diagnostics.proxyEnvVarsCheck();

expect(lifecycleEmitSpy.callCount, 'Expected "Doctor:diagnostic" event fired 3 times').to.equal(3);
expect(drAddSuggestionSpy.called, 'Expected no suggestions to be added').to.be.false;
});

it('passes with proxy env vars in only lowercase', async () => {
const dr = Doctor.init(oclifConfig);
stubMethod(sandbox, dr, 'getDiagnosis').returns({
proxyEnvVars: [
['http_proxy', httpProxy],
['https_proxy', httpsProxy],
['no_proxy', noProxy],
],
});
const diagnostics = new Diagnostics(dr, oclifConfig);
await diagnostics.proxyEnvVarsCheck();

expect(lifecycleEmitSpy.callCount, 'Expected "Doctor:diagnostic" event fired 3 times').to.equal(3);
expect(drAddSuggestionSpy.called, 'Expected no suggestions to be added').to.be.false;
});

it('fails with non-matching proxy env vars', async () => {
const dr = Doctor.init(oclifConfig);
stubMethod(sandbox, dr, 'getDiagnosis').returns({
proxyEnvVars: [
['http_proxy', httpProxy],
['https_proxy', httpsProxy],
['no_proxy', noProxy],
['HTTP_PROXY', 'http://test.foo.diagnostics'],
['HTTPS_PROXY', 'https://test.bar.diagnostics'],
['NO_PROXY', 'something'],
],
});
const diagnostics = new Diagnostics(dr, oclifConfig);
await diagnostics.proxyEnvVarsCheck();

expect(lifecycleEmitSpy.callCount, 'Expected "Doctor:diagnostic" event fired 3 times').to.equal(3);
expect(drAddSuggestionSpy.callCount, 'Expected 3 suggestions to be added').to.equal(3);
});
});

describe('networkCheck', () => {
it('passes when all URLs can be reached', async () => {
stubMethod(sandbox, Connection.prototype, 'request').resolves('{}');
Expand Down