diff --git a/index.d.ts b/index.d.ts index f9ee49d5c..02d6f6b25 100644 --- a/index.d.ts +++ b/index.d.ts @@ -512,6 +512,11 @@ declare module 'snowflake-sdk' { * The option to pass passcode from DUO. */ passcode?: string; + + /** + * The option to override the environment proxy value (HTTPS_PROXY) if the connection proxy is configured in GCS + */ + overrideEnvProxy?: string; } export interface Connection { diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 91cace8ad..a02aa3455 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -515,12 +515,12 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) { passcode = options.passcode; } - let overwriteEnvProxy = true; - if (Util.exists(options.overwriteEnvProxy)) { - Errors.checkArgumentValid(Util.isBoolean(options.overwriteEnvProxy), - ErrorCodes.ERR_CONN_CREATE_INVALID_OVERWRITE_ENV_PROXY); + let overrideEnvProxy = true; + if (Util.exists(options.overrideEnvProxy)) { + Errors.checkArgumentValid(Util.isBoolean(options.overrideEnvProxy), + ErrorCodes.ERR_CONN_CREATE_INVALID_OVERRIDE_ENV_PROXY); - overwriteEnvProxy = options.overwriteEnvProxy; + overrideEnvProxy = options.overrideEnvProxy; } if (validateDefaultParameters) { @@ -845,8 +845,8 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) { return passcode; }; - this.getOverwriteEnvProxy = function () { - return overwriteEnvProxy; + this.getOverrideEnvProxy = function () { + return overrideEnvProxy; }; /** diff --git a/lib/constants/error_messages.js b/lib/constants/error_messages.js index fc4c972df..b9aea896a 100644 --- a/lib/constants/error_messages.js +++ b/lib/constants/error_messages.js @@ -84,7 +84,7 @@ exports[404053] = 'A host must be specified.'; exports[404054] = 'Invalid host. The specified value must be a string.'; exports[404055] = 'Invalid passcodeInPassword. The specified value must be a boolean'; exports[404056] = 'Invalid passcode. The specified value must be a string'; -exports[404057] = 'Invalid overwriteEnvProxy. The specified value must be a boolean'; +exports[404057] = 'Invalid overrideEnvProxy. The specified value must be a boolean'; // 405001 exports[405001] = 'Invalid callback. The specified value must be a function.'; diff --git a/lib/errors.js b/lib/errors.js index 996326e6c..d33fad04d 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -88,7 +88,7 @@ codes.ERR_CONN_CREATE_MISSING_HOST = 404053; codes.ERR_CONN_CREATE_INVALID_HOST = 404054; codes.ERR_CONN_CREATE_INVALID_PASSCODE_IN_PASSWORD = 404055; codes.ERR_CONN_CREATE_INVALID_PASSCODE = 404056; -codes.ERR_CONN_CREATE_INVALID_OVERWRITE_ENV_PROXY = 404057; +codes.ERR_CONN_CREATE_INVALID_OVERRIDE_ENV_PROXY = 404057; // 405001 codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001; diff --git a/lib/file_transfer_agent/gcs_util.js b/lib/file_transfer_agent/gcs_util.js index f80fc377c..575f9eece 100644 --- a/lib/file_transfer_agent/gcs_util.js +++ b/lib/file_transfer_agent/gcs_util.js @@ -26,6 +26,7 @@ const resultStatus = require('./file_util').resultStatus; const HTTPS_PROXY = 'HTTPS_PROXY'; const { Storage } = require('@google-cloud/storage'); +const { isBypassProxy } = require('../http/node'); const EXPIRED_TOKEN = 'ExpiredToken'; @@ -53,19 +54,11 @@ function GCSUtil(connectionConfig, httpClient, fileStream) { const fs = typeof fileStream !== 'undefined' ? fileStream : require('fs'); const connectionProxy = connectionConfig.getProxy(); - let isOverrideEnvProxyEnabled = false; + //Todo Think about Bypass when NOproxy is configured. const originalHttpsProxy = process.env[HTTPS_PROXY]; + let isEnvProxyOverridden = false; let proxyString = null; - if (Util.exists(connectionProxy) && connectionConfig.getOverrideEnvProxy()) { - const compareAndLogEnvAndAgentProxies = ProxyUtil.getCompareAndLogEnvAndAgentProxies(connectionProxy); - Logger.getInstance().debug('proxy settings used in GCS Util: %s', compareAndLogEnvAndAgentProxies.messages); - if ((compareAndLogEnvAndAgentProxies.warnings).length !== 0) { - Logger.getInstance().warn('GCS proxy wanring: %s', compareAndLogEnvAndAgentProxies.warnings); - Logger.getInstance().warn(`The ${HTTPS_PROXY} value will be overwritten during the Storage HTTP calls`); - proxyString = ProxyUtil.convertProxyToString(connectionProxy); - isOverrideEnvProxyEnabled = true; - } - } + /** * Retrieve the GCS token from the stage info metadata. * @@ -101,6 +94,24 @@ function GCSUtil(connectionConfig, httpClient, fileStream) { client = null; } + const isByPassed = isBypassProxy(connectionProxy, endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`); + if (Util.isEmptyObject(connectionProxy) || isByPassed) { + //do nothing + } else if (!Util.exists(originalHttpsProxy)) { + proxyString = ProxyUtil.stringifyProxy(connectionProxy); + isEnvProxyOverridden = true; + } else if (connectionConfig.getOverrideEnvProxy()) { + const compareAndLogEnvAndAgentProxies = ProxyUtil.getCompareAndLogEnvAndAgentProxies(connectionProxy); + Logger.getInstance().debug('proxy settings used in GCS Util: %s', compareAndLogEnvAndAgentProxies.messages); + if (Util.validateEmptyString(compareAndLogEnvAndAgentProxies.warnings) ) { + Logger.getInstance().warn('GCS proxy wanring: %s', compareAndLogEnvAndAgentProxies.warnings); + Logger.getInstance().warn(`The environment variable ${HTTPS_PROXY} value will be overwritten during the Storage HTTP calls`); + proxyString = ProxyUtil.stringifyProxy(connectionProxy); + isEnvProxyOverridden = true; + } + } + + if (typeof httpClient === 'undefined') { const proxyAgent = getProxyAgent(connectionProxy, new URL(connectionConfig.accessUrl), endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com` ); axios = require('axios').create({ @@ -176,7 +187,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); let metadata; - if (isOverrideEnvProxyEnabled) { + if (isEnvProxyOverridden) { process.env[HTTPS_PROXY] = proxyString; metadata = await meta['client'].gcsClient .bucket(gcsLocation.bucketName) @@ -324,7 +335,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) { try { if (shouldPerformGCPBucket(accessToken)) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); - if (isOverrideEnvProxyEnabled){ + if (isEnvProxyOverridden){ process.env[HTTPS_PROXY] = proxyString; await meta['client'].gcsClient .bucket(gcsLocation.bucketName) @@ -415,7 +426,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) { if (shouldPerformGCPBucket(accessToken)) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); let metadata; - if (isOverrideEnvProxyEnabled){ + if (isEnvProxyOverridden){ process.env[HTTPS_PROXY] = proxyString; await meta['client'].gcsClient .bucket(gcsLocation.bucketName) @@ -524,6 +535,15 @@ function GCSUtil(connectionConfig, httpClient, fileStream) { return link; }; + //getProxyString and getIsEnvProxyOverridden are for testing purpose. + this.getProxyString = function () { + return proxyString; + }; + + this.getIsEnvProxyOverridden = function () { + return isEnvProxyOverridden; + }; + /** * Left strip the specified character from a string. * diff --git a/lib/proxy_util.js b/lib/proxy_util.js index 01f02d34f..a696d2560 100644 --- a/lib/proxy_util.js +++ b/lib/proxy_util.js @@ -55,18 +55,21 @@ exports.getCompareAndLogEnvAndAgentProxies = function (agentOptions) { ' protocol=' + agentOptions.protocol + ' proxy=' + proxyHostAndPort : ' proxy=' + proxyHostAndPort; const proxyUsername = agentOptions.user ? ' user=' + agentOptions.user : ''; + const proxyString = `${Util.exists(proxyUsername) ? `${agentOptions.user}:${agentOptions.password}@` : ''}${proxyHostAndPort}`.toLowerCase(); logMessages.messages = logMessages.messages + ` // Proxy configured in Agent:${proxyProtocolHostAndPort}${proxyUsername}`; // check if both the PROXY envvars and Connection proxy config is set // generate warnings if they are, and are also different if (envProxy.httpProxy && - this.removeScheme(envProxy.httpProxy).toLowerCase() !== this.removeScheme(proxyHostAndPort).toLowerCase()) { + this.removeScheme(envProxy.httpProxy).toLowerCase() !== + this.removeScheme(proxyString)) { logMessages.warnings = logMessages.warnings + ` Using both the HTTP_PROXY (${envProxy.httpProxy})` + ` and the proxyHost:proxyPort (${proxyHostAndPort}) settings to connect, but with different values.` + ' If you experience connectivity issues, try unsetting one of them.'; } if (envProxy.httpsProxy && - this.removeScheme(envProxy.httpsProxy).toLowerCase() !== this.removeScheme(proxyHostAndPort).toLowerCase()) { + this.removeScheme(envProxy.httpsProxy).toLowerCase() !== + this.removeScheme(proxyString)) { logMessages.warnings = logMessages.warnings + ` Using both the HTTPS_PROXY (${envProxy.httpsProxy})` + ` and the proxyHost:proxyPort (${proxyHostAndPort}) settings to connect, but with different values.` + ' If you experience connectivity issues, try unsetting one of them.'; @@ -261,4 +264,10 @@ exports.describeProxy = function (proxy) { return `proxyHost: ${proxy.host}, proxyPort: ${proxy.port}, proxyUser: ${proxy.user}, ` + `proxyPassword is ${LoggingUtil.describePresence(proxy.password)}, ` + `proxyProtocol: ${proxy.protocol}, noProxy: ${proxy.noProxy}`; +}; + +exports.stringifyProxy = function (proxy) { + return `${(proxy.protocol).startsWith('https') ? 'https' : 'http'}://` + + `${Util.exists(proxy.user) ? `${proxy.user}:${proxy.password}@` : ''}` + + `${proxy.host}:${proxy.port}`; }; \ No newline at end of file diff --git a/lib/util.js b/lib/util.js index 7144baccc..f8439c448 100644 --- a/lib/util.js +++ b/lib/util.js @@ -755,6 +755,13 @@ exports.isNotEmptyAsString = function (variable) { return exports.exists(variable); }; +exports.isEmptyObject = (object) => { + if (typeof object !== 'object' || object instanceof Array) { + return false; + } + return Object.keys(object).length === 0; +}; + exports.isWindows = function () { return os.platform() === 'win32'; }; \ No newline at end of file diff --git a/test/unit/connection/connection_config_test.js b/test/unit/connection/connection_config_test.js index abfc4cd2f..26c60cf55 100644 --- a/test/unit/connection/connection_config_test.js +++ b/test/unit/connection/connection_config_test.js @@ -776,15 +776,15 @@ describe('ConnectionConfig: basic', function () { errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_PASSCODE }, { - name: 'invalid overwriteEnvProxy', + name: 'invalid overrideEnvProxy', options: { account: 'account', username: 'username', password: 'password', - overwriteEnvProxy: 123456 + overrideEnvProxy: 123456 }, - errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_OVERWRITE_ENV_PROXY + errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_OVERRIDE_ENV_PROXY }, ]; @@ -1678,13 +1678,13 @@ describe('ConnectionConfig: basic', function () { getter: 'getPasscode', }, { - name: 'overwriteEnvProxy', + name: 'overrideEnvProxy', input: { ...mandatoryOption, - overwriteEnvProxy: false, + overrideEnvProxy: false, }, result: false, - getter: 'getOverwriteEnvProxy', + getter: 'getOverrideEnvProxy', }, ]; diff --git a/test/unit/file_transfer_agent/gcs_test.js b/test/unit/file_transfer_agent/gcs_test.js index 9737e9804..feb34375e 100644 --- a/test/unit/file_transfer_agent/gcs_test.js +++ b/test/unit/file_transfer_agent/gcs_test.js @@ -5,6 +5,7 @@ const assert = require('assert'); const mock = require('mock-require'); const SnowflakeGCSUtil = require('./../../../lib/file_transfer_agent/gcs_util'); +const GCSUtil = require('./../../../lib/file_transfer_agent/gcs_util'); const resultStatus = require('./../../../lib/file_transfer_agent/file_util').resultStatus; describe('GCS client', function () { @@ -19,10 +20,11 @@ describe('GCS client', function () { const mockMatDesc = 'mockMatDesc'; const mockPresignedUrl = 'mockPresignedUrl'; const connectionConfig = { + proxy: {}, getProxy: function () { - return null; + return this.proxy; }, - getOverwriteEnvProxy: function () { + getOverrideEnvProxy: function () { return true; }, accessUrl: 'http://fakeaccount.snowflakecomputing.com', @@ -43,7 +45,11 @@ describe('GCS client', function () { meta = { stageInfo: { location: mockLocation, - path: mockTable + '/' + mockPath + '/' + region: '', + path: mockTable + '/' + mockPath + '/', + creds: { + GCS_ACCESS_TOKEN: 'mockToken' + } }, presignedUrl: mockPresignedUrl, dstFileName: mockPresignedUrl, @@ -142,7 +148,7 @@ describe('GCS client', function () { testCases.forEach(({ name, stageInfo, result }) => { it(name, () => { - const client = GCS.createClient({ ...stageInfo, ...meta.stageInfo, creds: { GCS_ACCESS_TOKEN: 'mockToken' } }); + const client = GCS.createClient({ ...meta.stageInfo, ...stageInfo }); assert.strictEqual(client.gcsClient.apiEndpoint, result); } ); @@ -361,4 +367,124 @@ describe('GCS client', function () { await GCS.uploadFile(dataFile, meta, encryptionMetadata); assert.strictEqual(meta['resultStatus'], resultStatus.RENEW_TOKEN); }); -}); + + describe('GCS proxy configuration test', async function () { + let originalHttpsProxy; + before(() => { + originalHttpsProxy = process.env.HTTPS_PROXY; + }); + + after(() => { + originalHttpsProxy ? process.env.HTTPS_PROXY = originalHttpsProxy : delete process.env.HTTPS_PROXY; + }); + const testCases = [ + { + name: 'when both the connection proxy and HTTPS_PROXY are not configured', + connectionConfig: connectionConfig, + HTTPS_PROXY: null, + isOverwriteEnvProxy: false, + proxyString: null, + }, + { + name: 'when HTTPS_PROXY only exists', + connectionConfig: connectionConfig, + HTTPS_PROXY: 'https://abc:dfg@snowflake.test.com:2345', + isOverwriteEnvProxy: false, + proxyString: null, + }, + { + name: 'when the connectionProxy is different from the Env Proxy(HTTPS_PROXY)', + connectionConfig: { ...connectionConfig, + proxy: { + host: 'myproxy.server.com', + user: 'user', + password: 'pass', + port: 1234, + protocol: 'https:', + noProxy: undefined, + }, + }, + HTTPS_PROXY: 'https://abc:dfg@snowflake.test.com:2345', + isOverwriteEnvProxy: true, + proxyString: 'https://user:pass@myproxy.server.com:1234', + }, + { + name: 'when the connectionProxy and HTTPS_PROXY is the same.', + connectionConfig: { ...connectionConfig, + proxy: { + host: 'myproxy.server.com', + user: 'user', + password: 'pass', + port: 1234, + protocol: 'https:', + noProxy: undefined, + }, + }, + HTTPS_PROXY: 'https://user:pass@myproxy.server.com:1234', + isOverwriteEnvProxy: false, + proxyString: null, + }, + { + name: 'when the connectionProxy and HTTPS_PROXY are different, but overrideEnvProxy is false.', + connectionConfig: { ...connectionConfig, + getOverrideEnvProxy: function () { + return false; + }, + proxy: { + host: 'myproxy.server.com', + user: 'user', + password: 'pass', + port: 1234, + protocol: 'https:', + noProxy: undefined, + }, + }, + HTTPS_PROXY: 'https://abc:dfg@snowflake.test.com:2345', + isOverwriteEnvProxy: false, + proxyString: null, + }, + { + name: 'when no HTTPS_PROXY, but the connection Proxy exists.', + connectionConfig: { ...connectionConfig, + proxy: { + host: 'myproxy.server.com', + user: 'user', + password: 'pass', + port: 1234, + protocol: 'https:', + noProxy: undefined, + }, + }, + HTTPS_PROXY: null, + isOverwriteEnvProxy: true, + proxyString: 'https://user:pass@myproxy.server.com:1234', + }, + { + name: 'when the connectionProxy exists, but the noProxy is set with google storage destination.', + connectionConfig: { ...connectionConfig, + proxy: { + host: 'myproxy.server.com', + user: 'user', + password: 'pass', + port: 1234, + protocol: 'https:', + noProxy: 'storage.*.rep.googleapis.com', + }, + }, + HTTPS_PROXY: null, + isOverwriteEnvProxy: false, + proxyString: null, + }, + ]; + + testCases.forEach(({ name, connectionConfig, HTTPS_PROXY, isOverwriteEnvProxy, proxyString }) => { + it(name, () => { + HTTPS_PROXY !== null ? process.env.HTTPS_PROXY = HTTPS_PROXY : delete process.env.HTTPS_PROXY; + const GCS = new GCSUtil(connectionConfig); + GCS.createClient(meta.stageInfo); + assert.strictEqual(isOverwriteEnvProxy, GCS.getIsEnvProxyOverridden()); + assert.strictEqual(proxyString, GCS.getProxyString()); + }); + }); + }); +}); \ No newline at end of file diff --git a/test/unit/util_test.js b/test/unit/util_test.js index f8c538007..0addf373e 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -1215,4 +1215,72 @@ describe('Util', function () { } }); + describe('restoreEnvVar function test', function () { + const testCases = [ + { + name: 'snowflake_restore_env_test', + value: 'mock_value2', + }, + { + name: 'SNOWFLAKE_RESTORE_ENV_TEST', + value: 'MOCK_VALUE2', + }, + ]; + + for (const { name, value, } of testCases) { + it(name, function () { + process.env[name] = 'wrong value'; + Util.restoreEnvVar(name, value); + assert.strictEqual(Util.getEnvVar(name), value); + delete process.env[name]; + }); + } + }); + + describe('isEmptyObject function test', function () { + const testCases = [ + { + name: 'JSON is not empty', + value: { 'hello': 'a' }, + result: false + }, + { + name: 'JSON is empty', + value: {}, + result: true + }, + { + name: 'non object(string)', + value: 'hello world', + result: false, + }, + { + name: 'non object(int)"', + value: 123, + result: false, + }, + { + name: 'non object(int)"', + value: 123, + result: false, + }, + { + name: 'array"', + value: [1, 2, 3], + result: false, + }, + { + name: 'empty array"', + value: [], + result: false, + }, + ]; + + testCases.forEach(({ name, value, result }) => { + it(name, function () { + assert.strictEqual(Util.isEmptyObject(value), result); + }); + }); + }); + });