-
Notifications
You must be signed in to change notification settings - Fork 266
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
added formatted proxy URL #834
Changes from all commits
4061352
ea8a877
1663ffa
645bf43
85841c9
77c1a29
dc107e6
b54e925
458f4a1
aaecad0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1839,11 +1839,33 @@ export function findMatch(defaultRoot: string, patterns: string[] | string, find | |
|
||
export interface ProxyConfiguration { | ||
proxyUrl: string; | ||
/** | ||
* Proxy URI formated as: protocol://username:password@hostname:port | ||
* | ||
* For tools that require setting proxy configuration in the single environment variable | ||
*/ | ||
proxyFormattedUrl: string; | ||
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. What does proxyFormattedUrl mean - what is the format/purpose/how to use it? Could it make sense to add JSDoc comments 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. JSDoc added 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. This field is not orthogonal to the other fields, so it could be error-prone in future (it depends on values of the fields like proxyUrl/proxyUsername, so if they will be changed - we would need to change it as well). 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. Resolved per offline discussion - since we considering that this is intended to be read-only object (containing current proxy settings) - it should be good to keep in a separate field |
||
proxyUsername?: string; | ||
proxyPassword?: string; | ||
proxyBypassHosts?: string[]; | ||
} | ||
|
||
/** | ||
* Build Proxy URL in the following format: protocol://username:password@hostname:port | ||
* @param proxyUrl Url address of the proxy server (eg: http://example.com) | ||
* @param proxyUsername Proxy username (optional) | ||
* @param proxyPassword Proxy password (optional) | ||
* @returns string | ||
*/ | ||
function getProxyFormattedUrl(proxyUrl: string, proxyUsername: string | undefined, proxyPassword: string | undefined): string { | ||
kirill-ivlev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const parsedUrl: URL = new URL(proxyUrl); | ||
let proxyAddress: string = `${parsedUrl.protocol}//${parsedUrl.host}`; | ||
if (proxyUsername) { | ||
proxyAddress = `${parsedUrl.protocol}//${proxyUsername}:${proxyPassword}@${parsedUrl.host}`; | ||
} | ||
return proxyAddress; | ||
} | ||
|
||
/** | ||
* Gets http proxy configuration used by Build/Release agent | ||
* | ||
|
@@ -1869,11 +1891,13 @@ export function getHttpProxyConfiguration(requestUrl?: string): ProxyConfigurati | |
return null; | ||
} | ||
else { | ||
const proxyAddress = getProxyFormattedUrl(proxyUrl, proxyUsername, proxyPassword) | ||
return { | ||
proxyUrl: proxyUrl, | ||
proxyUsername: proxyUsername, | ||
proxyPassword: proxyPassword, | ||
proxyBypassHosts: proxyBypassHosts | ||
proxyBypassHosts: proxyBypassHosts, | ||
proxyFormattedUrl: proxyAddress | ||
}; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import * as assert from 'assert'; | ||
import * as tl from '../_build/task'; | ||
|
||
enum ProxyEnvironmentEnum { | ||
proxyUrl = 'AGENT_PROXYURL', | ||
proxyUsername = 'AGENT_PROXYUSERNAME', | ||
proxyPassword = 'AGENT_PROXYPASSWORD', | ||
proxyBypass = 'AGENT_PROXYBYPASSLIST' | ||
} | ||
|
||
describe('GetHttpProxyConfiguration Tests', () => { | ||
const proxyHost: string = 'proxy.example.com'; | ||
const proxyPort: number = 8888; | ||
const proxyUrl: string = `http://${proxyHost}:${proxyPort}`; | ||
const proxyUsername: string = 'proxyUser'; | ||
const proxyPassword: string = 'proxyPassword'; | ||
const proxyByPass: string[] = ['http://bypass.example.com']; | ||
const formatedUrlWithoutCrednetials = proxyUrl; | ||
const fortmatedUrlWithCredentials = `http://${proxyUsername}:${proxyPassword}@${proxyHost}:${proxyPort}`; | ||
|
||
it('returns a valid proxy configuration if no credentials set', () => { | ||
process.env[ProxyEnvironmentEnum.proxyUrl] = proxyUrl; | ||
process.env[ProxyEnvironmentEnum.proxyBypass] = JSON.stringify(proxyByPass); | ||
const expected: tl.ProxyConfiguration = { | ||
proxyUrl: proxyUrl, | ||
proxyBypassHosts: proxyByPass, | ||
proxyUsername: undefined, | ||
proxyPassword: undefined, | ||
proxyFormattedUrl: formatedUrlWithoutCrednetials | ||
} | ||
const result = tl.getHttpProxyConfiguration(); | ||
assert.deepStrictEqual(result, expected, 'it should have valid configuration'); | ||
}); | ||
|
||
it('returns valid proxy configuration if credentials set', () => { | ||
process.env[ProxyEnvironmentEnum.proxyUrl] = proxyUrl; | ||
process.env[ProxyEnvironmentEnum.proxyUsername] = proxyUsername; | ||
process.env[ProxyEnvironmentEnum.proxyPassword] = proxyPassword; | ||
process.env[ProxyEnvironmentEnum.proxyBypass] = JSON.stringify(proxyByPass); | ||
const expected: tl.ProxyConfiguration = { | ||
proxyUrl: proxyUrl, | ||
proxyBypassHosts: proxyByPass, | ||
proxyFormattedUrl: fortmatedUrlWithCredentials, | ||
proxyPassword: proxyPassword, | ||
proxyUsername: proxyUsername | ||
} | ||
const result = tl.getHttpProxyConfiguration(); | ||
assert.deepStrictEqual(result, expected, 'it should have credentials in formatted url'); | ||
}); | ||
|
||
it('returns null if host should be bypassed', () => { | ||
process.env[ProxyEnvironmentEnum.proxyUrl] = proxyUrl; | ||
const result = tl.getHttpProxyConfiguration(proxyByPass[0]); | ||
assert.strictEqual(result, null, 'result should be null'); | ||
}); | ||
}) |
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.
nit:
const