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

SNOW-1820372: (from 947) when proxy is set in Connection, driver does send traffic through the proxy to S3, but not to Azure blob / GCS bucket (only Snowflake). Works with proxy envvar (GCP) #982

Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
02444e3
add destiantion in the cache key
sfc-gh-ext-simba-jy Nov 5, 2024
6731437
add testing cases for the agentcache
sfc-gh-ext-simba-jy Nov 6, 2024
57fb674
fix
sfc-gh-ext-simba-jy Nov 6, 2024
c6941e3
fix
sfc-gh-ext-simba-jy Nov 6, 2024
661d3db
fix testing
sfc-gh-ext-simba-jy Nov 6, 2024
a31e911
fix
sfc-gh-ext-simba-jy Nov 6, 2024
1ce6b99
add proxy settings in Azure_util
sfc-gh-ext-simba-jy Nov 18, 2024
43cb685
Merge branch 'master' into SNOW-1781419
sfc-gh-ext-simba-jy Nov 18, 2024
c7533f2
fix azure testing
sfc-gh-ext-simba-jy Nov 18, 2024
799c53f
move proxy functions to proxy_util.js and made proxy_util_test.js
sfc-gh-ext-simba-jy Nov 19, 2024
233643a
add proxy_util.js and proxy_util_test.js
sfc-gh-ext-simba-jy Nov 19, 2024
18e1065
add copyrights
sfc-gh-ext-simba-jy Nov 19, 2024
90b2ff3
fix the bug
sfc-gh-ext-simba-jy Nov 20, 2024
4ba8394
fix
sfc-gh-ext-simba-jy Nov 20, 2024
a0ed255
add logs
sfc-gh-ext-simba-jy Nov 20, 2024
443b16b
fix
sfc-gh-ext-simba-jy Nov 20, 2024
f1f5701
Merge branch 'master' into SNOW-1781419
sfc-gh-ext-simba-jy Nov 22, 2024
8baf884
updated logs
sfc-gh-ext-simba-jy Nov 22, 2024
2cdaa20
Merge branch 'SNOW-1781419' of https://github.com/snowflakedb/snowfla…
sfc-gh-ext-simba-jy Nov 22, 2024
1cea678
fix lint
sfc-gh-ext-simba-jy Nov 22, 2024
ea8534b
Merge branch 'master' into SNOW-1781419
sfc-gh-ext-simba-jy Nov 26, 2024
f5c992b
Merge branch 'master' into SNOW-1781419
sfc-gh-ext-simba-jy Dec 3, 2024
2bb0e5f
fix
sfc-gh-ext-simba-jy Dec 3, 2024
ee58566
save
sfc-gh-ext-simba-jy Dec 4, 2024
3cc9544
save
sfc-gh-ext-simba-jy Dec 6, 2024
e634a2c
Merge branch 'SNOW-1820372' of https://github.com/snowflakedb/snowfla…
sfc-gh-ext-simba-jy Dec 6, 2024
d7e022c
add testing
sfc-gh-ext-simba-jy Dec 7, 2024
4233eb8
add connectionConfig test and remove unnecessary codes
sfc-gh-ext-simba-jy Dec 7, 2024
cb839a6
added more testing and refactored the code
sfc-gh-ext-simba-jy Dec 10, 2024
9628eb0
fix
sfc-gh-ext-simba-jy Dec 10, 2024
d8d46a7
fix
sfc-gh-ext-simba-jy Dec 10, 2024
18ad7ab
Merge branch 'master' into SNOW-1820372
sfc-gh-ext-simba-jy Dec 10, 2024
703d9fd
add more testings and fix typos
sfc-gh-ext-simba-jy Dec 10, 2024
c531d0f
Merge branch 'SNOW-1820372' of https://github.com/snowflakedb/snowfla…
sfc-gh-ext-simba-jy Dec 10, 2024
51b0702
fix bug
sfc-gh-ext-simba-jy Dec 10, 2024
5c3328a
fix errors
sfc-gh-ext-simba-jy Dec 11, 2024
f12f124
Merge branch 'master' into SNOW-1820372
sfc-gh-ext-simba-jy Dec 11, 2024
7bf600a
Merge branch 'master' into SNOW-1820372
sfc-gh-ext-simba-jy Dec 12, 2024
0c5c2d0
fix GCS erros and update PR based on the comments
sfc-gh-ext-simba-jy Dec 13, 2024
8899472
fix
sfc-gh-ext-simba-jy Dec 13, 2024
47089a5
Merge branch 'master' into SNOW-1820372
sfc-gh-ext-simba-jy Dec 13, 2024
f97e83c
add comments
sfc-gh-ext-simba-jy Dec 16, 2024
35fc4b0
fix
sfc-gh-ext-simba-jy Dec 18, 2024
0ac468d
fix
sfc-gh-ext-simba-jy Dec 18, 2024
5e44c43
refactor
sfc-gh-ext-simba-jy Dec 18, 2024
6d75374
Merge branch 'master' into SNOW-1820372
sfc-gh-ext-simba-jy Dec 20, 2024
696255d
fix
sfc-gh-ext-simba-jy Dec 20, 2024
b4a89fd
Merge branch 'SNOW-1820372' of https://github.com/snowflakedb/snowfla…
sfc-gh-ext-simba-jy Dec 20, 2024
bf4eb28
Merge branch 'master' into SNOW-1820372
sfc-gh-ext-simba-jy Dec 20, 2024
a969e08
fix errors from merging
sfc-gh-ext-simba-jy Dec 20, 2024
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
18 changes: 1 addition & 17 deletions lib/connection/connection_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const levenshtein = require('fastest-levenshtein');
const RowMode = require('./../constants/row_mode');
const DataTypes = require('./result/data_types');
const Logger = require('../logger');
const LoggingUtil = require('../logger/logging_util');
const WAIT_FOR_BROWSER_ACTION_TIMEOUT = 120000;
const DEFAULT_PARAMS =
[
Expand Down Expand Up @@ -513,7 +512,6 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) {

passcode = options.passcode;
}


if (validateDefaultParameters) {
for (const [key] of Object.entries(options)) {
Expand Down Expand Up @@ -847,21 +845,7 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) {
this.describeIdentityAttributes = function () {
return `host: ${this.host}, account: ${this.account}, accessUrl: ${this.accessUrl}, `
+ `user: ${this.username}, role: ${this.getRole()}, database: ${this.getDatabase()}, `
+ `schema: ${this.getSchema()}, warehouse: ${this.getWarehouse()}, ` + this.describeProxy();
};

/**
* @returns {string}
*/
this.describeProxy = function () {
const proxy = this.getProxy();
if (Util.exists(proxy)) {
return `proxyHost: ${proxy.host}, proxyPort: ${proxy.port}, proxyUser: ${proxy.user}, `
+ `proxyPassword is ${LoggingUtil.describePresence(proxy.password)}, `
+ `proxyProtocol: ${proxy.protocol}, noProxy: ${proxy.noProxy}`;
} else {
return 'proxy was not configured';
}
+ `schema: ${this.getSchema()}, warehouse: ${this.getWarehouse()}, ` + ProxyUtil.describeProxy(this.getProxy());
};

// save config options
Expand Down
1 change: 0 additions & 1 deletion lib/file_transfer_agent/azure_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ function AzureUtil(connectionConfig, azure, filestream) {
Logger.getInstance().trace(`Initializing the proxy information for the Azure Client: ${ProxyUtil.describeProxy(proxy)}`);

proxy = ProxyUtil.getAzureProxy(proxy);
Logger.getInstance().trace(connectionConfig.describe);
}
ProxyUtil.hideEnvironmentProxy();
const blobServiceClient = new AZURE.BlobServiceClient(
Expand Down
73 changes: 44 additions & 29 deletions lib/file_transfer_agent/gcs_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

const EncryptionMetadata = require('./encrypt_util').EncryptionMetadata;
const FileHeader = require('./file_util').FileHeader;
const getProxyAgent = require('../http/node').getProxyAgent;
const ProxyUtil = require('../proxy_util');
const Util = require('../util');
const { shouldPerformGCPBucket } = require('../util');

const GCS_METADATA_PREFIX = 'x-goog-meta-';
Expand All @@ -19,7 +22,6 @@ const GCS_FILE_HEADER_ENCRYPTION_METADATA = 'gcs-file-header-encryption-metadata

const HTTP_HEADER_CONTENT_ENCODING = 'Content-Encoding';
const resultStatus = require('./file_util').resultStatus;

const { Storage } = require('@google-cloud/storage');

const EXPIRED_TOKEN = 'ExpiredToken';
Expand All @@ -36,17 +38,18 @@ function GCSLocation(bucketName, path) {

/**
* Creates an GCS utility object.
*
* @param {module} httpclient
* @param {module} filestream
* @param {module} connectionConfig
* @param {module} httpClient
* @param {module} fileStream
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
*
* @returns {Object}
* @constructor
*/
function GCSUtil(httpclient, filestream) {
const axios = typeof httpclient !== 'undefined' ? httpclient : require('axios');
const fs = typeof filestream !== 'undefined' ? filestream : require('fs');

function GCSUtil(connectionConfig, httpClient, fileStream) {
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
let axios = httpClient;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
const fs = typeof fileStream !== 'undefined' ? fileStream : require('fs');
let isProxyEnabled = false;

/**
* Retrieve the GCS token from the stage info metadata.
*
Expand All @@ -57,7 +60,15 @@ function GCSUtil(httpclient, filestream) {
this.createClient = function (stageInfo) {
const stageCredentials = stageInfo['creds'];
const gcsToken = stageCredentials['GCS_ACCESS_TOKEN'];

//TODO: SNOW-1789759 the value is hardcoded now, but it should be server driven
const isRegionalUrlEnabled = (stageInfo.region).toLowerCase() === 'me-central2' || stageInfo.useRegionalUrl;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
let endPoint = null;
if (stageInfo['endPoint']) {
endPoint = stageInfo['endPoint'];
} else if (isRegionalUrlEnabled) {
endPoint = `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`;
}

let client;
if (gcsToken) {
const interceptors = [];
Expand All @@ -68,20 +79,27 @@ function GCSUtil(httpclient, filestream) {
return requestConfig;
}
});

//TODO: SNOW-1789759 hardcoded region will be replaced in the future
const isRegionalUrlEnabled = (stageInfo.region).toLowerCase() === 'me-central2' || stageInfo.useRegionalUrl;
let endPoint = null;
if (stageInfo['endPoint']) {
endPoint = stageInfo['endPoint'];
} else if (isRegionalUrlEnabled) {
endPoint = `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`;
}
const storage = endPoint ? new Storage({ interceptors_: interceptors, apiEndpoint: endPoint }) : new Storage({ interceptors_: interceptors });
const storage = Util.exists(endPoint) ? new Storage({ interceptors_: interceptors, apiEndpoint: endPoint }) : new Storage({ interceptors_: interceptors });
client = { gcsToken: gcsToken, gcsClient: storage };
} else {
client = null;
}

if (typeof httpClient === 'undefined') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code of setting up axios could be extracted to the separated method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

axios = require('axios');
Copy link
Collaborator

Choose a reason for hiding this comment

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

The line could be a part of if ... else when we confirm whether the proxy is set or not.

if (proxy || Util.getEnvVar('http_proxy')) {
...
} else { 
     axios = require('axios');
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

const proxy = ProxyUtil.getProxy(connectionConfig.getProxy(), 'GCS Util');

//When http_proxy is enabled, the driver should use Axios for HTTPS requests to avoid relying on HTTP_PROXY in GCS.
if (proxy || Util.getEnvVar('http_proxy')) {
isProxyEnabled = true;
const proxyAgent = getProxyAgent(proxy, new URL(connectionConfig.accessUrl), endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com` );
axios = require('axios').create({
proxy: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it correct to set here proxy = false?

Copy link
Collaborator Author

@sfc-gh-ext-simba-jy sfc-gh-ext-simba-jy Dec 20, 2024

Choose a reason for hiding this comment

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

Yes, we need to set proxy: false if we want to use https-proxy-agent in Axios. If not, the driver will face the 'too many direction' error. The same implementation is here: https://github.com/snowflakedb/snowflake-connector-nodejs/blob/master/lib/http/base.js#L289

httpAgent: proxyAgent,
httpsAgent: proxyAgent,
});
}
}

return client;
};
Expand Down Expand Up @@ -145,14 +163,14 @@ function GCSUtil(httpclient, filestream) {
let matDescKey;

try {
if (shouldPerformGCPBucket(accessToken)) {
if (shouldPerformGCPBucket(accessToken) && !isProxyEnabled) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);

const metadata = await meta['client'].gcsClient
const metadata = await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + filename)
.getMetadata();


digest = metadata[0].metadata[SFC_DIGEST];
contentLength = metadata[0].size;
encryptionDataProp = metadata[0].metadata[ENCRYPTIONDATAPROP];
Expand Down Expand Up @@ -285,9 +303,9 @@ function GCSUtil(httpclient, filestream) {
}

try {
if (shouldPerformGCPBucket(accessToken)) {
if (shouldPerformGCPBucket(accessToken) && !isProxyEnabled) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);

await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['dstFileName'])
Expand Down Expand Up @@ -358,9 +376,8 @@ function GCSUtil(httpclient, filestream) {
let size;

try {
if (shouldPerformGCPBucket(accessToken)) {
if (shouldPerformGCPBucket(accessToken) && !isProxyEnabled) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);

await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['srcFileName'])
Expand All @@ -379,9 +396,7 @@ function GCSUtil(httpclient, filestream) {
size = metadata[0].size;
} else {
let response;
await axios({
method: 'get',
url: downloadUrl,
await axios.get(downloadUrl, {
headers: gcsHeaders,
responseType: 'stream'
}).then(async (res) => {
Expand Down
14 changes: 9 additions & 5 deletions lib/file_transfer_agent/remote_storage_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ exports.SnowflakeFileEncryptionMaterial = SnowflakeFileEncryptionMaterial;
* @constructor
*/
function RemoteStorageUtil(connectionConfig) {
let client = null;

/**
* Get storage type based on location type.
*
Expand All @@ -41,15 +43,17 @@ function RemoteStorageUtil(connectionConfig) {
* @returns {Object}
*/
this.getForStorageType = function (type) {
if (client) {
return client;
}
if (type === 'S3') {
return new SnowflakeS3Util(connectionConfig);
client = new SnowflakeS3Util(connectionConfig);
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
} else if (type === 'AZURE') {
return new SnowflakeAzureUtil(connectionConfig);
client = new SnowflakeAzureUtil(connectionConfig);
} else if (type === 'GCS') {
return new SnowflakeGCSUtil();
} else {
return null;
client = new SnowflakeGCSUtil(connectionConfig);
}
return client;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
};

/**
Expand Down
Loading
Loading