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 35 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
5 changes: 5 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
sfc-gh-dszmolka marked this conversation as resolved.
Show resolved Hide resolved
}

export interface Connection {
Expand Down
12 changes: 12 additions & 0 deletions lib/connection/connection_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const DEFAULT_PARAMS =
'credentialCacheDir',
'passcodeInPassword',
'passcode',
'overrideEnvProxy'
];

function consolidateHostAndAccount(options) {
Expand Down Expand Up @@ -514,6 +515,13 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) {
passcode = options.passcode;
}

let overrideEnvProxy = true;
if (Util.exists(options.overrideEnvProxy)) {
Errors.checkArgumentValid(Util.isBoolean(options.overrideEnvProxy),
ErrorCodes.ERR_CONN_CREATE_INVALID_OVERRIDE_ENV_PROXY);

overrideEnvProxy = options.overrideEnvProxy;
}

if (validateDefaultParameters) {
for (const [key] of Object.entries(options)) {
Expand Down Expand Up @@ -837,6 +845,10 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) {
return passcode;
};

this.getOverrideEnvProxy = function () {
return overrideEnvProxy;
};

/**
* Returns attributes of Connection Config object that can be used to identify
* the connection, when ID is not available in the scope. This is not sufficient set,
Expand Down
1 change: 1 addition & 0 deletions lib/constants/error_messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +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 overrideEnvProxy. The specified value must be a boolean';

// 405001
exports[405001] = 'Invalid callback. The specified value must be a function.';
Expand Down
1 change: 1 addition & 0 deletions lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +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_OVERRIDE_ENV_PROXY = 404057;

// 405001
codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001;
Expand Down
172 changes: 126 additions & 46 deletions lib/file_transfer_agent/gcs_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

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 Logger = require('../logger');
const { shouldPerformGCPBucket } = require('../util');

const GCS_METADATA_PREFIX = 'x-goog-meta-';
Expand All @@ -19,8 +23,10 @@

const HTTP_HEADER_CONTENT_ENCODING = 'Content-Encoding';
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';

Expand All @@ -37,15 +43,19 @@
/**
* Creates an GCS utility object.
*
* @param {module} httpclient
* @param {module} filestream
* @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');
const connectionProxy = connectionConfig.getProxy();
const originalHttpsProxy = process.env[HTTPS_PROXY];
sfc-gh-pmotacki marked this conversation as resolved.
Show resolved Hide resolved
const proxyString = ProxyUtil.stringifyProxy(connectionProxy);
let isEnvProxyOverridden = false;

/**
* Retrieve the GCS token from the stage info metadata.
Expand All @@ -57,7 +67,15 @@
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,21 +86,38 @@
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;
}

const isByPassed = isBypassProxy(connectionProxy, endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`);
if (Util.isEmptyObject(connectionProxy) || isByPassed) {
isEnvProxyOverridden = false;
} else if (!Util.exists(originalHttpsProxy)) {
isEnvProxyOverridden = true;
} else if (connectionConfig.getOverrideEnvProxy()) {
if (proxyString !== originalHttpsProxy) {
Logger.getInstance().debug(`The destination host is: ${ProxyUtil.getHostFromURL(endPoint || `https://storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`)} and the proxy host is: ${connectionProxy.host}`);
Logger.getInstance().warn(`The environment variable ${HTTPS_PROXY} value will be overwritten during the Storage HTTP calls`);
isEnvProxyOverridden = true;
}
}

if (isEnvProxyOverridden) {
Logger.getInstance().trace(`Initializing the proxy information for the GCS Client: ${ProxyUtil.describeProxy(connectionProxy)}`);
}

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.

const proxyAgent = getProxyAgent(connectionProxy, new URL(connectionConfig.accessUrl), endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com` );
axios = require('axios').create({
proxy: false,
httpAgent: proxyAgent,
httpsAgent: proxyAgent,
});
}

return client;
};

Expand Down Expand Up @@ -148,10 +183,20 @@
if (shouldPerformGCPBucket(accessToken)) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);

const metadata = await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + filename)
.getMetadata();
let metadata;
if (isEnvProxyOverridden) {
process.env[HTTPS_PROXY] = proxyString;
metadata = await meta['client'].gcsClient

Check warning on line 189 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L188-L189

Added lines #L188 - L189 were not covered by tests
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + filename)
.getMetadata();
Util.restoreEnvVar(HTTPS_PROXY, originalHttpsProxy);

Check warning on line 193 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L193

Added line #L193 was not covered by tests
} else {
metadata = await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + filename)
.getMetadata();
}

digest = metadata[0].metadata[SFC_DIGEST];
contentLength = metadata[0].size;
Expand Down Expand Up @@ -287,20 +332,37 @@
try {
if (shouldPerformGCPBucket(accessToken)) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);

await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['dstFileName'])
.save(fileStream, {
resumable: false,
metadata: {
if (isEnvProxyOverridden){
process.env[HTTPS_PROXY] = proxyString;
await meta['client'].gcsClient

Check warning on line 337 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L336-L337

Added lines #L336 - L337 were not covered by tests
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['dstFileName'])
.save(fileStream, {
resumable: false,
metadata: {
[ENCRYPTIONDATAPROP]: gcsHeaders[GCS_METADATA_ENCRYPTIONDATAPROP],
[MATDESC_KEY]: gcsHeaders[GCS_METADATA_MATDESC_KEY],
[SFC_DIGEST]: gcsHeaders[GCS_METADATA_SFC_DIGEST]
metadata: {
[ENCRYPTIONDATAPROP]: gcsHeaders[GCS_METADATA_ENCRYPTIONDATAPROP],
[MATDESC_KEY]: gcsHeaders[GCS_METADATA_MATDESC_KEY],
[SFC_DIGEST]: gcsHeaders[GCS_METADATA_SFC_DIGEST]
}
}
}
});
});
Util.restoreEnvVar(HTTPS_PROXY, originalHttpsProxy);

Check warning on line 350 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L350

Added line #L350 was not covered by tests
} else {
await meta['client'].gcsClient
sfc-gh-pmotacki marked this conversation as resolved.
Show resolved Hide resolved
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['dstFileName'])
.save(fileStream, {
resumable: false,
metadata: {
metadata: {
[ENCRYPTIONDATAPROP]: gcsHeaders[GCS_METADATA_ENCRYPTIONDATAPROP],
[MATDESC_KEY]: gcsHeaders[GCS_METADATA_MATDESC_KEY],
[SFC_DIGEST]: gcsHeaders[GCS_METADATA_SFC_DIGEST]
}
}
});
}
} else {
// Set maxBodyLength to allow large file uploading
await axios.put(uploadUrl, fileStream, { maxBodyLength: Infinity, headers: gcsHeaders });
Expand Down Expand Up @@ -360,28 +422,41 @@
try {
if (shouldPerformGCPBucket(accessToken)) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);
let metadata;
if (isEnvProxyOverridden){
process.env[HTTPS_PROXY] = proxyString;
await meta['client'].gcsClient

Check warning on line 428 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L427-L428

Added lines #L427 - L428 were not covered by tests
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['srcFileName'])
.download({
destination: fullDstPath
});

await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['srcFileName'])
.download({
destination: fullDstPath
});

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

Check warning on line 435 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L435

Added line #L435 was not covered by tests
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['srcFileName'])
.getMetadata();
Util.restoreEnvVar(HTTPS_PROXY, originalHttpsProxy);

Check warning on line 439 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L439

Added line #L439 was not covered by tests
} else {
await meta['client'].gcsClient
sfc-gh-pmotacki marked this conversation as resolved.
Show resolved Hide resolved
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['srcFileName'])
.download({
destination: fullDstPath
});

metadata = await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['srcFileName'])
.getMetadata();
}
encryptionDataprop = metadata[0].metadata[ENCRYPTIONDATAPROP];
matDescKey = metadata[0].metadata[MATDESC_KEY];
sfcDigest = metadata[0].metadata[SFC_DIGEST];
size = metadata[0].size;
} else {
let response;
await axios({
method: 'get',
url: downloadUrl,
await axios.get(downloadUrl, {

Check warning on line 459 in lib/file_transfer_agent/gcs_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_transfer_agent/gcs_util.js#L459

Added line #L459 was not covered by tests
headers: gcsHeaders,
responseType: 'stream'
}).then(async (res) => {
Expand Down Expand Up @@ -457,6 +532,11 @@
return link;
};

//getIsEnvProxyOverridden are for testing purpose
this.getIsEnvProxyOverridden = function () {
return isEnvProxyOverridden;
};

/**
* Left strip the specified character from a string.
*
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