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

Jump to bottom

add proxy support using hpagent #374

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

add proxy support using hpagent #374

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
150 changes: 150 additions & 0 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14552,6 +14552,134 @@ module.exports = header => {
};


/***/ }),

/***/ 737:
/***/ (function(module, __unusedexports, __webpack_require__) {

"use strict";


const https = __webpack_require__(211)
const http = __webpack_require__(605)
const { URL } = __webpack_require__(835)

class HttpProxyAgent extends http.Agent {
constructor (options) {
const { proxy, ...opts } = options
super(opts)
this.proxy = typeof proxy === 'string'
? new URL(proxy)
: proxy
}

createConnection (options, callback) {
const requestOptions = {
method: 'CONNECT',
host: this.proxy.hostname,
port: this.proxy.port,
path: `${options.host}:${options.port}`,
setHost: false,
headers: { connection: this.keepAlive ? 'keep-alive' : 'close', host: `${options.host}:${options.port}` },
agent: false,
timeout: options.timeout || 0
}

if (this.proxy.username || this.proxy.password) {
const base64 = Buffer.from(`${decodeURIComponent(this.proxy.username || '')}:${decodeURIComponent(this.proxy.password || '')}`).toString('base64')
requestOptions.headers['proxy-authorization'] = `Basic ${base64}`
}

if (this.proxy.protocol === 'https:') {
requestOptions.servername = this.proxy.hostname
}

const request = (this.proxy.protocol === 'http:' ? http : https).request(requestOptions)
request.once('connect', (response, socket, head) => {
request.removeAllListeners()
socket.removeAllListeners()
if (response.statusCode === 200) {
callback(null, socket)
} else {
callback(new Error(`Bad response: ${response.statusCode}`), null)
}
})

request.once('timeout', () => {
request.destroy(new Error('Proxy timeout'))
})

request.once('error', err => {
request.removeAllListeners()
callback(err, null)
})

request.end()
}
}

class HttpsProxyAgent extends https.Agent {
constructor (options) {
const { proxy, ...opts } = options
super(opts)
this.proxy = typeof proxy === 'string'
? new URL(proxy)
: proxy
}

createConnection (options, callback) {
const requestOptions = {
method: 'CONNECT',
host: this.proxy.hostname,
port: this.proxy.port,
path: `${options.host}:${options.port}`,
setHost: false,
headers: { connection: this.keepAlive ? 'keep-alive' : 'close', host: `${options.host}:${options.port}` },
agent: false,
timeout: options.timeout || 0
}

if (this.proxy.username || this.proxy.password) {
const base64 = Buffer.from(`${decodeURIComponent(this.proxy.username || '')}:${decodeURIComponent(this.proxy.password || '')}`).toString('base64')
requestOptions.headers['proxy-authorization'] = `Basic ${base64}`
}

// Necessary for the TLS check with the proxy to succeed.
if (this.proxy.protocol === 'https:') {
requestOptions.servername = this.proxy.hostname
}

const request = (this.proxy.protocol === 'http:' ? http : https).request(requestOptions)
request.once('connect', (response, socket, head) => {
request.removeAllListeners()
socket.removeAllListeners()
if (response.statusCode === 200) {
const secureSocket = super.createConnection({ ...options, socket })
callback(null, secureSocket)
} else {
callback(new Error(`Bad response: ${response.statusCode}`), null)
}
})

request.once('timeout', () => {
request.destroy(new Error('Proxy timeout'))
})

request.once('error', err => {
request.removeAllListeners()
callback(err, null)
})

request.end()
}
}

module.exports = {
HttpProxyAgent,
HttpsProxyAgent
}


/***/ }),

/***/ 738:
Expand Down Expand Up @@ -16438,6 +16566,7 @@ exports.default = createRejection;
const core = __webpack_require__(470);
const command = __webpack_require__(431);
const got = __webpack_require__(77).default;
const { HttpProxyAgent, HttpsProxyAgent } = __webpack_require__(737);
const jsonata = __webpack_require__(350);
const { auth: { retrieveToken }, secrets: { getSecrets } } = __webpack_require__(676);

Expand All @@ -16463,6 +16592,7 @@ async function exportSecrets() {
prefixUrl: vaultUrl,
headers: {},
https: {},
agent: {},
retry: {
statusCodes: [
...got.defaults.options.retry.statusCodes,
Expand All @@ -16473,6 +16603,26 @@ async function exportSecrets() {
}
}

if (process.env['http_proxy'] || process.env['HTTP_PROXY']) {
defaultOptions.agent.http = new HttpProxyAgent({
keepAlive: true,
keepAliveMsecs: 1000,
maxSockets: 256,
maxFreeSockets: 256,
proxy: process.env['http_proxy'] || process.env['HTTP_PROXY']
})
}

if (process.env['https_proxy'] || process.env['HTTPS_PROXY']) {
defaultOptions.agent.https = new HttpsProxyAgent({
keepAlive: true,
keepAliveMsecs: 1000,
maxSockets: 256,
maxFreeSockets: 256,
proxy: process.env['https_proxy'] || process.env['HTTPS_PROXY']
})
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generate the dist/index when releasing new versions its fine to not commit it if you prefer. It often leads to annoying merge conflicts to resolve.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about that part... whether it needed to be added to the commit or not. I can definitely remove it.

const tlsSkipVerify = (core.getInput('tlsSkipVerify', { required: false }) || 'false').toLowerCase() != 'false';
if (tlsSkipVerify === true) {
defaultOptions.https.rejectUnauthorized = false;
Expand Down
14 changes: 14 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"homepage": "https://github.com/hashicorp/vault-action#readme",
"dependencies": {
"got": "^11.8.5",
"hpagent": "^1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the latest version is 1.2.0 if we want to start with an up-to-date new dependency

"jsonata": "^1.8.6",
"jsrsasign": "^10.5.25"
},
Expand Down
22 changes: 22 additions & 0 deletions src/action.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const core = require('@actions/core');
const command = require('@actions/core/lib/command');
const got = require('got').default;
const { HttpProxyAgent, HttpsProxyAgent } = require('hpagent');
const jsonata = require('jsonata');
const { auth: { retrieveToken }, secrets: { getSecrets } } = require('./index');

Expand All @@ -27,6 +28,7 @@ async function exportSecrets() {
prefixUrl: vaultUrl,
headers: {},
https: {},
agent: {},
retry: {
statusCodes: [
...got.defaults.options.retry.statusCodes,
Expand All @@ -37,6 +39,26 @@ async function exportSecrets() {
}
}

if (process.env['http_proxy'] || process.env['HTTP_PROXY']) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add this setting in the list of inputs supported by the action so it is explicit and documented.

Unless these environment variables have a special meaning and are required for the agent to work properly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are pretty standard environment variables that, if needed, could be added to the .env file for a self-hosted runner, negating the need for them to be explicitly provided in the action inputs.

When using this action on github.com, these variables would not be set and would not be needed. They are only necessary in a corporate environment where network "compartments" might require a proxy to be configured in order to reach them.

That's why I did not add them as inputs, but rather just made the decision to pick them up from the runner's environment.

Copy link
Contributor

@maxcoulombe maxcoulombe Mar 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it make sense since they are standard, not vault-specific and the use case is somewhat niche. It's probably worth a mention in the README though, otherwise people would need to read the source code to know proxy servers are supported by the action.

defaultOptions.agent.http = new HttpProxyAgent({
keepAlive: true,
keepAliveMsecs: 1000,
maxSockets: 256,
maxFreeSockets: 256,
proxy: process.env['http_proxy'] || process.env['HTTP_PROXY']
})
}

if (process.env['https_proxy'] || process.env['HTTPS_PROXY']) {
defaultOptions.agent.https = new HttpsProxyAgent({
keepAlive: true,
keepAliveMsecs: 1000,
maxSockets: 256,
maxFreeSockets: 256,
proxy: process.env['https_proxy'] || process.env['HTTPS_PROXY']
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this might be very difficult to test functionally but can we add a small UT that at least checks the agent is set or unset depending of the config received?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will see what I can do!


const tlsSkipVerify = (core.getInput('tlsSkipVerify', { required: false }) || 'false').toLowerCase() != 'false';
if (tlsSkipVerify === true) {
defaultOptions.https.rejectUnauthorized = false;
Expand Down