From d026eda95d3d053ef14c822bdff42484b9a3e14b Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Thu, 22 Feb 2024 16:18:36 +0100 Subject: [PATCH 1/9] SNOW-1064052 Windows PUT multifiles - fix (#776) * SNOW-1064052 - fix PUT files with wildcards on Windows --- lib/connection/statement.js | 2 +- .../file_transfer_agent.js | 12 ++--- lib/file_transfer_agent/file_util.js | 13 ++++- test/integration/testUtil.js | 13 +++++ .../file_transfer_agent/file_util_test.js | 52 +++++++++++++++++++ 5 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 test/unit/file_transfer_agent/file_util_test.js diff --git a/lib/connection/statement.js b/lib/connection/statement.js index f8a33c15a..e73652483 100644 --- a/lib/connection/statement.js +++ b/lib/connection/statement.js @@ -15,7 +15,7 @@ const Errors = require('../errors'); const ErrorCodes = Errors.codes; const Logger = require('../logger'); const NativeTypes = require('./result/data_types').NativeTypes; -const FileTransferAgent = require('.././file_transfer_agent/file_transfer_agent'); +const FileTransferAgent = require('../file_transfer_agent/file_transfer_agent'); const Bind = require('./bind_uploader'); const RowMode = require('./../constants/row_mode'); diff --git a/lib/file_transfer_agent/file_transfer_agent.js b/lib/file_transfer_agent/file_transfer_agent.js index 719949dad..d2b53aa1f 100644 --- a/lib/file_transfer_agent/file_transfer_agent.js +++ b/lib/file_transfer_agent/file_transfer_agent.js @@ -4,7 +4,6 @@ const binascii = require('binascii'); const crypto = require('crypto'); -const glob = require('glob'); const fs = require('fs'); const os = require('os'); const mime = require('mime-types'); @@ -13,14 +12,15 @@ const path = require('path'); const statement = require('../connection/statement'); const fileCompressionType = require('./file_compression_type'); const expandTilde = require('expand-tilde'); -const SnowflakeFileUtil = new (require('./file_util').FileUtil)(); const SnowflakeRemoteStorageUtil = require('./remote_storage_util').RemoteStorageUtil; +const LocalUtil = require('./local_util').LocalUtil; const SnowflakeFileEncryptionMaterial = require('./remote_storage_util').SnowflakeFileEncryptionMaterial; const SnowflakeS3Util = require('./s3_util'); -const SnowflakeLocalUtil = new (require('./local_util').LocalUtil)(); - +const { FileUtil, getMatchingFilePaths } = require('./file_util'); const resultStatus = require('./file_util').resultStatus; +const SnowflakeFileUtil = new FileUtil(); +const SnowflakeLocalUtil = new LocalUtil(); const S3_FS = 'S3'; const AZURE_FS = 'AZURE'; const GCS_FS = 'GCS'; @@ -379,7 +379,7 @@ function FileTransferAgent(context) { meta['errorDetails'] += ` file=${meta['srcFileName']}, real file=${meta['realSrcFilePath']}`; } finally { // Remove all files inside tmp folder - const matchingFileNames = glob.sync(path.join(meta['tmpDir'], meta['srcFileName'] + '*')); + const matchingFileNames = getMatchingFilePaths(meta['tmpDir'], meta['srcFileName'] + '*'); for (const matchingFileName of matchingFileNames) { await new Promise((resolve, reject) => { fs.unlink(matchingFileName, err => { @@ -618,7 +618,7 @@ function FileTransferAgent(context) { // If file name has a wildcard if (fileName.includes('*')) { // Get all file names that matches the wildcard - const matchingFileNames = glob.sync(path.join(root, fileName)); + const matchingFileNames = getMatchingFilePaths(root, fileName); for (const matchingFileName of matchingFileNames) { initEncryptionMaterial(); diff --git a/lib/file_transfer_agent/file_util.js b/lib/file_transfer_agent/file_util.js index f9e56601e..36ab38218 100644 --- a/lib/file_transfer_agent/file_util.js +++ b/lib/file_transfer_agent/file_util.js @@ -7,6 +7,8 @@ const fs = require('fs'); const path = require('path'); const struct = require('python-struct'); const zlib = require('zlib'); +const os = require('os'); +const glob = require('glob'); const resultStatus = { ERROR: 'ERROR', @@ -132,5 +134,14 @@ function FileUtil() { }; }; } - exports.FileUtil = FileUtil; + +function getMatchingFilePaths(dir, fileName) { + const pathWithWildcard = path.join(dir, fileName); + const pathWithWildcardDependsOnPlatform = os.platform() === 'win32' + ? pathWithWildcard.replace(/\\/g, '/') + : pathWithWildcard; + return glob.sync(pathWithWildcardDependsOnPlatform); +} + +exports.getMatchingFilePaths = getMatchingFilePaths; diff --git a/test/integration/testUtil.js b/test/integration/testUtil.js index 2290afb1b..4e72b4849 100644 --- a/test/integration/testUtil.js +++ b/test/integration/testUtil.js @@ -5,6 +5,7 @@ const snowflake = require('./../../lib/snowflake'); const connOptions = require('./connectionOptions'); const assert = require('assert'); const fs = require('fs'); +const fsPromises = require('fs').promises; const crypto = require('crypto'); const Logger = require('../../lib/logger'); const path = require('path'); @@ -290,6 +291,18 @@ module.exports.createTempFile = function (mainDir, fileName, data = '') { fs.writeFileSync(fullpath, data); return fullpath; }; +/** + * Async version of method to create temp file + * @param mainDir string Main directory for created file + * @param fileName string Created file name + * @param data string Input for created file + * @return string + */ +module.exports.createTempFileAsync = async function (mainDir, fileName, data = '') { + const fullpath = path.join(mainDir, fileName); + await fsPromises.writeFile(fullpath, data); + return fullpath; +}; /** * @param option object diff --git a/test/unit/file_transfer_agent/file_util_test.js b/test/unit/file_transfer_agent/file_util_test.js new file mode 100644 index 000000000..8705001bd --- /dev/null +++ b/test/unit/file_transfer_agent/file_util_test.js @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +const assert = require('assert'); +const testUtil = require('../../integration/testUtil'); +const os = require('os'); +const fsPromises = require('fs').promises; +const crypto = require('crypto'); +const getMatchingFilePaths = require('../../../lib/file_transfer_agent/file_util').getMatchingFilePaths; + + +describe('matching files by wildcard', function () { + const randomName = crypto.randomUUID(); + const excpetedNomberOfMatchedFiles = 3; + + async function createFiles(options) { + for (let i = 0; i < excpetedNomberOfMatchedFiles; i++) { + await testUtil.createTempFileAsync(os.tmpdir(), testUtil.createRandomFileName(options)); + } + } + + after(async function () { + const matchedFiles = getMatchingFilePaths(os.tmpdir(), `${randomName}matched` + '*'); + const notmatchedFiles = getMatchingFilePaths(os.tmpdir(), `${randomName}notmatched` + '*'); + const promises = []; + + for (const filePath of matchedFiles) { + promises.push(fsPromises.rm(filePath)); + } + for (const filePath of notmatchedFiles) { + promises.push(fsPromises.rm(filePath)); + } + await Promise.all(promises); + }); + + it('match paths with prefix', async function () { + await createFiles({ prefix: `${randomName}matched` }); + await createFiles({ prefix: `${randomName}notmatched` }); + const matched = getMatchingFilePaths(os.tmpdir(), `${randomName}matched` + '*'); + assert.strictEqual(matched.length, excpetedNomberOfMatchedFiles); + }); + + it('match paths with prefix and extension', async function () { + await createFiles({ prefix: `${randomName}matched`, extension: '.gz' }); + await createFiles({ prefix: `${randomName}matched`, extension: '.txt' }); + await createFiles({ prefix: `${randomName}notmatched` }); + const matched = getMatchingFilePaths(os.tmpdir(), `${randomName}matched` + '*.gz'); + assert.strictEqual(matched.length, excpetedNomberOfMatchedFiles); + }); + +}); From ea4ef66dab4342c745019f922b47b43ae3791695 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 22 Jan 2024 18:44:25 -0800 Subject: [PATCH 2/9] fix error --- lib/core.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/core.js b/lib/core.js index f2d089550..bda529b07 100644 --- a/lib/core.js +++ b/lib/core.js @@ -72,6 +72,10 @@ function Core(options) { const httpClient = options.httpClient || new options.httpClientClass(connectionConfig); + if (process.env.GOOGLE_APPLICATION_CREDENTIALS) { + delete process.env.GOOGLE_APPLICATION_CREDENTIALS; + } + return new connectionClass( new ConnectionContext(connectionConfig, httpClient, config)); }; From 753ea00c77033e7517426143aebf11ef1fc3b2a7 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 23 Jan 2024 14:39:08 -0800 Subject: [PATCH 3/9] workaround applied --- lib/core.js | 4 ---- lib/file_transfer_agent/gcs_util.js | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/core.js b/lib/core.js index bda529b07..f2d089550 100644 --- a/lib/core.js +++ b/lib/core.js @@ -72,10 +72,6 @@ function Core(options) { const httpClient = options.httpClient || new options.httpClientClass(connectionConfig); - if (process.env.GOOGLE_APPLICATION_CREDENTIALS) { - delete process.env.GOOGLE_APPLICATION_CREDENTIALS; - } - return new connectionClass( new ConnectionContext(connectionConfig, httpClient, config)); }; diff --git a/lib/file_transfer_agent/gcs_util.js b/lib/file_transfer_agent/gcs_util.js index 332c26a1a..57e48efa0 100644 --- a/lib/file_transfer_agent/gcs_util.js +++ b/lib/file_transfer_agent/gcs_util.js @@ -137,7 +137,7 @@ function GCSUtil(httpclient, filestream) { let matDescKey; try { - if (accessToken) { + if (accessToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); const metadata = await meta['client'].gcsClient @@ -277,7 +277,7 @@ function GCSUtil(httpclient, filestream) { } try { - if (accessToken) { + if (accessToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); await meta['client'].gcsClient @@ -350,7 +350,7 @@ function GCSUtil(httpclient, filestream) { let size; try { - if (accessToken) { + if (accessToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); await meta['client'].gcsClient From d783f8ff3e4fa2c05d7894771d26dfafb7de5cd2 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 6 Feb 2024 11:58:11 -0800 Subject: [PATCH 4/9] added option to enable or disable the gcp bucket --- lib/connection/connection_config.js | 10 +++ lib/constants/error_messages.js | 1 + lib/errors.js | 1 + lib/file_transfer_agent/gcs_util.js | 7 +- lib/util.js | 25 ++++--- test/unit/util_test.js | 105 +++++++++++++++++++--------- 6 files changed, 102 insertions(+), 47 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 0603465b6..9c1b0d86a 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -52,6 +52,7 @@ const DEFAULT_PARAMS = 'includeRetryReason', 'disableQueryContextCache', 'retryTimeout', + 'disableGCPTokenUpload' ]; const Logger = require('../logger'); @@ -481,6 +482,15 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) { disableConsoleLogin = options.disableConsoleLogin; } + if (Util.exists(options.disableGCPTokenUpload)) { + Errors.checkArgumentValid(Util.isBoolean(options.disableGCPTokenUpload), + ErrorCodes.ERR_CONN_CREATE_INVALID_DISABLE_GCP_TOKEN_UPLOAD); + + process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD = options.disableGCPTokenUpload; + } else { + process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD = false; + } + /** * Returns an object that contains information about the proxy hostname, port, * etc. for when http requests are made. diff --git a/lib/constants/error_messages.js b/lib/constants/error_messages.js index caa178cac..4e11a32a1 100644 --- a/lib/constants/error_messages.js +++ b/lib/constants/error_messages.js @@ -72,6 +72,7 @@ exports[404044] = 'Invalid retryTimeout value. The specified value must be a num exports[404045] = 'Invalid account. The specified value must be a valid subdomain string.'; exports[404046] = 'Invalid region. The specified value must be a valid subdomain string.'; exports[404047] = 'Invalid disableConsoleLogin. The specified value must be a boolean'; +exports[404048] = 'Invalid disableGCPTokenUpload. 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 66f0abeb4..786367b04 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -77,6 +77,7 @@ codes.ERR_CONN_CREATE_INVALID_RETRY_TIMEOUT = 404044; codes.ERR_CONN_CREATE_INVALID_ACCOUNT_REGEX = 404045; codes.ERR_CONN_CREATE_INVALID_REGION_REGEX = 404046; codes.ERR_CONN_CREATE_INVALID_DISABLE_CONSOLE_LOGIN = 404047; +codes.ERR_CONN_CREATE_INVALID_DISABLE_GCP_TOKEN_UPLOAD = 404048; // 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 57e48efa0..b72d9ef91 100644 --- a/lib/file_transfer_agent/gcs_util.js +++ b/lib/file_transfer_agent/gcs_util.js @@ -4,6 +4,7 @@ const EncryptionMetadata = require('./encrypt_util').EncryptionMetadata; const FileHeader = require('./file_util').FileHeader; +const { shouldPerformGCPBucket } = require('../util'); const GCS_METADATA_PREFIX = 'x-goog-meta-'; const SFC_DIGEST = 'sfc-digest'; @@ -137,7 +138,7 @@ function GCSUtil(httpclient, filestream) { let matDescKey; try { - if (accessToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { + if (shouldPerformGCPBucket(accessToken)) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); const metadata = await meta['client'].gcsClient @@ -277,7 +278,7 @@ function GCSUtil(httpclient, filestream) { } try { - if (accessToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { + if (shouldPerformGCPBucket(accessToken)) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); await meta['client'].gcsClient @@ -350,7 +351,7 @@ function GCSUtil(httpclient, filestream) { let size; try { - if (accessToken && !process.env.GOOGLE_APPLICATION_CREDENTIALS) { + if (shouldPerformGCPBucket(accessToken)) { const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']); await meta['client'].gcsClient diff --git a/lib/util.js b/lib/util.js index 51b4f88f6..74bd2175f 100644 --- a/lib/util.js +++ b/lib/util.js @@ -327,7 +327,7 @@ exports.url = if (option.includeRetryReason) { retryUrl = this.appendParam(retryUrl, 'retryReason', option.retryReason); } - + return retryUrl; } }; @@ -393,8 +393,8 @@ exports.nextSleepTime = function ( /** * Return next sleep time calculated by the jitter rule. * - * @param {Number} numofRetries - * @param {Number} currentSleepTime + * @param {Number} numofRetries + * @param {Number} currentSleepTime * @param {Number} totalElapsedTime * @param {Number} maxRetryTimeout * @returns {JSON} return next sleep Time and totalTime. @@ -409,11 +409,11 @@ exports.getJitteredSleepTime = function (numofRetries, currentSleepTime, totalEl /** * Choose one of the number between two numbers. * - * @param {Number} firstNumber - * @param {Number} secondNumber + * @param {Number} firstNumber + * @param {Number} secondNumber * @returns {Number} return a random number between two numbers. */ -function chooseRandom(firstNumber, secondNumber) { +function chooseRandom(firstNumber, secondNumber) { return Math.random() * (firstNumber - secondNumber) + secondNumber; } @@ -421,8 +421,8 @@ exports.chooseRandom = chooseRandom; /** * return the next sleep Time. - * @param {Number} numofRetries - * @param {Number} currentSleepTime + * @param {Number} numofRetries + * @param {Number} currentSleepTime * @returns {Number} return jitter. */ function getNextSleepTime(numofRetries, currentSleepTime) { @@ -434,7 +434,7 @@ exports.getNextSleepTime = getNextSleepTime; /** * return the jitter value. - * @param {Number} currentSleepTime + * @param {Number} currentSleepTime * @returns {Number} return jitter. */ function getJitter(currentSleepTime) { @@ -697,4 +697,9 @@ exports.isFileNotWritableByGroupOrOthers = async function (configFilePath, fsPro exports.shouldRetryOktaAuth = function ({ maxRetryTimeout, maxRetryCount, numRetries, startTime, remainingTimeout }) { return (maxRetryTimeout === 0 || Date.now() < startTime + remainingTimeout) && numRetries <= maxRetryCount; -}; \ No newline at end of file +}; + +exports.shouldPerformGCPBucket = function (accessToken) { + console.log( process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD); + return !!accessToken && process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD !== 'true'; +}; diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 659b6faa5..b944a7d8c 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -494,17 +494,17 @@ describe('Util', function () { describe('Util.isLoginRequest Test', function () { const baseUrl = 'wwww.test.com'; - const testCases = + const testCases = [ { testName: 'test URL with a right login end point', endPoint: '/v1/login-request', - result: true, + result: true, }, { testName: 'test URL with a wrong login end point', endPoint: '/login-request', - result: false, + result: false, }, { testName: 'test URL with a right authenticator-request point', @@ -561,7 +561,7 @@ describe('Util', function () { isRetryable: true, }, ]; - + const maxRetryTimeout = 300; let currentSleepTime = 1; let retryCount = 0; @@ -573,14 +573,14 @@ describe('Util', function () { currentSleepTime = result.sleep; totalElapsedTime = result.totalElapsedTime; retryCount++; - + assert.strictEqual(Util.isRetryableHttpError(response, true), true); assert.ok(currentSleepTime <= nextSleep + jitter || currentSleepTime >= nextSleep - jitter); } - + assert.strictEqual(retryCount, 6); assert.ok(totalElapsedTime <= maxRetryTimeout); - }); + }); it('test - retryTimeout is 0', function () { const maxRetryTimeout = 0; @@ -732,9 +732,9 @@ describe('Util', function () { [ { name: 'test - default values', - retryOption: { - maxRetryCount: 7, - numRetries: 1, + retryOption: { + maxRetryCount: 7, + numRetries: 1, remainingTimeout: 300000, maxRetryTimeout: 300000 }, @@ -742,9 +742,9 @@ describe('Util', function () { }, { name: 'test - the value of the numRetries is the same as the max retry count', - retryOption: { - maxRetryCount: 7, - numRetries: 7, + retryOption: { + maxRetryCount: 7, + numRetries: 7, remainingTimeout: 300000, maxRetryTimeout: 300000 }, @@ -752,29 +752,29 @@ describe('Util', function () { }, { name: 'test - max retry timout is 0', - retryOption: { - maxRetryCount: 7, - numRetries: 1, + retryOption: { + maxRetryCount: 7, + numRetries: 1, remainingTimeout: 300000, - maxRetryTimeout: 0 + maxRetryTimeout: 0 }, result: true, }, { name: 'test - the max retry timeout is 0 and number of retry is over', - retryOption: { - maxRetryCount: 7, - numRetries: 8, + retryOption: { + maxRetryCount: 7, + numRetries: 8, remainingTimeout: -50, - maxRetryTimeout: 0 + maxRetryTimeout: 0 }, result: false, }, { name: 'test - the retry count is over the max retry count ', - retryOption: { - maxRetryCount: 7, - numRetries: 8, + retryOption: { + maxRetryCount: 7, + numRetries: 8, remainingTimeout: 300000, maxRetryTimeout: 300 }, @@ -782,21 +782,21 @@ describe('Util', function () { }, { name: 'test - the remaining timout is 0', - retryOption: { - maxRetryCount: 7, - numRetries: 8, + retryOption: { + maxRetryCount: 7, + numRetries: 8, remainingTimeout: 0, - maxRetryTimeout: 300 + maxRetryTimeout: 300 }, result: false, }, { name: 'test - the remaining timoue is negative', - retryOption: { - maxRetryCount: 7, - numRetries: 8, + retryOption: { + maxRetryCount: 7, + numRetries: 8, remainingTimeout: -10, - maxRetryTimeout: 300 + maxRetryTimeout: 300 }, result: false, }, @@ -856,13 +856,13 @@ describe('Util', function () { describe('Util Test - handling circular reference in isValidAsync exception handling', () => { const shouldMatchNonCircular = '{"one":1,"two":2}'; const shouldMatchCircular = '{"one":1,"two":2,"myself":"[Circular]"}'; - + it('non-circular reference is handled correctly by JSON.stringify replacer', () => { const a = { 'one': 1, 'two': 2 }; const replacedA = JSON.stringify(a, Util.getCircularReplacer()); assert.deepEqual(replacedA, shouldMatchNonCircular); }); - + it('circular reference is handled correctly by JSON.stringify replacer', () => { const b = { 'one': 1, 'two': 2 }; b.myself = b; @@ -1062,4 +1062,41 @@ describe('Util', function () { }); }); } + + describe('shouldPerformGCPBucket function test', () => { + const testCases = [ + { + name: 'test - default', + accessToken: 'Token', + disableGCPTokenUplaod: false, + result: true, + }, + { + name: 'test - when the disableGCPTokenUplaod is enabled', + accessToken: 'Token', + disableGCPTokenUplaod: true, + result: false, + }, + { + name: 'test - when token is emptry but the disableGCPTokenUplaod is enabled', + accessToken: null, + disableGCPTokenUplaod: true, + result: false, + }, + { + name: 'test - test - when token is emptry but the disableGCPTokenUplaod is disabled', + accessToken: null, + disableGCPTokenUplaod: false, + result: false, + }, + ]; + + testCases.forEach(({ name, accessToken, disableGCPTokenUplaod, result }) => { + it(name, () => { + process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD = disableGCPTokenUplaod; + assert.strictEqual(Util.shouldPerformGCPBucket(accessToken), result); + delete process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD; + }); + }); + }); }); From 95d6651feb89387bc50b224840c8aac845a15fb4 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 6 Feb 2024 12:41:48 -0800 Subject: [PATCH 5/9] added connection config unit test --- test/unit/connection/connection_config_test.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/unit/connection/connection_config_test.js b/test/unit/connection/connection_config_test.js index ea54baf2f..238095bd1 100644 --- a/test/unit/connection/connection_config_test.js +++ b/test/unit/connection/connection_config_test.js @@ -700,6 +700,16 @@ describe('ConnectionConfig: basic', function () { }, errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_DISABLE_CONSOLE_LOGIN }, + { + name: 'invalid disableGCPTokenUpload', + options: { + account: 'account', + username: 'username', + password: 'password', + disableGCPTokenUpload: 'invalud' + }, + errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_DISABLE_GCP_TOKEN_UPLOAD + }, ]; const createNegativeITCallback = function (testCase) { @@ -1242,7 +1252,7 @@ describe('ConnectionConfig: basic', function () { password: 'password', account: 'account' } - } + }, ]; const createItCallback = function (testCase) { From 960ed7c32f995af4f8d29d492e569c32505237b1 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 6 Feb 2024 12:46:22 -0800 Subject: [PATCH 6/9] removed console statement --- lib/util.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/util.js b/lib/util.js index 74bd2175f..7e43ec16a 100644 --- a/lib/util.js +++ b/lib/util.js @@ -661,6 +661,11 @@ exports.removeScheme = function (input) { return input.toString().replace(/(^\w+:|^)\/\//, ''); }; + +exports.shouldPerformGCPBucket = function (accessToken) { + return !!accessToken && process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD !== 'true'; +}; + /** * Checks if the provided file or directory permissions are correct. * @param filePath From 30b0ee87d99b1d61ce064f61d1c78f97e445e5f4 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 6 Feb 2024 13:33:25 -0800 Subject: [PATCH 7/9] fix the option name --- lib/connection/connection_config.js | 12 ++++++------ lib/errors.js | 2 +- lib/util.js | 2 +- test/unit/connection/connection_config_test.js | 4 ++-- test/unit/util_test.js | 14 +++++++------- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 9c1b0d86a..00ce022cc 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -52,7 +52,7 @@ const DEFAULT_PARAMS = 'includeRetryReason', 'disableQueryContextCache', 'retryTimeout', - 'disableGCPTokenUpload' + 'forceGCPUseDownscopedCredential' ]; const Logger = require('../logger'); @@ -482,13 +482,13 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) { disableConsoleLogin = options.disableConsoleLogin; } - if (Util.exists(options.disableGCPTokenUpload)) { - Errors.checkArgumentValid(Util.isBoolean(options.disableGCPTokenUpload), - ErrorCodes.ERR_CONN_CREATE_INVALID_DISABLE_GCP_TOKEN_UPLOAD); + if (Util.exists(options.forceGCPUseDownscopedCredential)) { + Errors.checkArgumentValid(Util.isBoolean(options.forceGCPUseDownscopedCredential), + ErrorCodes.ERR_CONN_CREATE_INVALID_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL); - process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD = options.disableGCPTokenUpload; + process.env.SNOWFLAKE_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL = options.forceGCPUseDownscopedCredential; } else { - process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD = false; + process.env.SNOWFLAKE_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL = false; } /** diff --git a/lib/errors.js b/lib/errors.js index 786367b04..956edbf8a 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -77,7 +77,7 @@ codes.ERR_CONN_CREATE_INVALID_RETRY_TIMEOUT = 404044; codes.ERR_CONN_CREATE_INVALID_ACCOUNT_REGEX = 404045; codes.ERR_CONN_CREATE_INVALID_REGION_REGEX = 404046; codes.ERR_CONN_CREATE_INVALID_DISABLE_CONSOLE_LOGIN = 404047; -codes.ERR_CONN_CREATE_INVALID_DISABLE_GCP_TOKEN_UPLOAD = 404048; +codes.ERR_CONN_CREATE_INVALID_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL = 404048; // 405001 codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001; diff --git a/lib/util.js b/lib/util.js index 7e43ec16a..77cdf2dd2 100644 --- a/lib/util.js +++ b/lib/util.js @@ -663,7 +663,7 @@ exports.removeScheme = function (input) { exports.shouldPerformGCPBucket = function (accessToken) { - return !!accessToken && process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD !== 'true'; + return !!accessToken && process.env.SNOWFLAKE_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL !== 'true'; }; /** diff --git a/test/unit/connection/connection_config_test.js b/test/unit/connection/connection_config_test.js index 238095bd1..62acbc03a 100644 --- a/test/unit/connection/connection_config_test.js +++ b/test/unit/connection/connection_config_test.js @@ -706,9 +706,9 @@ describe('ConnectionConfig: basic', function () { account: 'account', username: 'username', password: 'password', - disableGCPTokenUpload: 'invalud' + forceGCPUseDownscopedCredential: 'invalud' }, - errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_DISABLE_GCP_TOKEN_UPLOAD + errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL }, ]; diff --git a/test/unit/util_test.js b/test/unit/util_test.js index b944a7d8c..a8cfbfe51 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -1068,34 +1068,34 @@ describe('Util', function () { { name: 'test - default', accessToken: 'Token', - disableGCPTokenUplaod: false, + forceGCPUseDownscopedCredential: false, result: true, }, { name: 'test - when the disableGCPTokenUplaod is enabled', accessToken: 'Token', - disableGCPTokenUplaod: true, + forceGCPUseDownscopedCredential: true, result: false, }, { name: 'test - when token is emptry but the disableGCPTokenUplaod is enabled', accessToken: null, - disableGCPTokenUplaod: true, + forceGCPUseDownscopedCredential: true, result: false, }, { name: 'test - test - when token is emptry but the disableGCPTokenUplaod is disabled', accessToken: null, - disableGCPTokenUplaod: false, + forceGCPUseDownscopedCredential: false, result: false, }, ]; - testCases.forEach(({ name, accessToken, disableGCPTokenUplaod, result }) => { + testCases.forEach(({ name, accessToken, forceGCPUseDownscopedCredential, result }) => { it(name, () => { - process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD = disableGCPTokenUplaod; + process.env.SNOWFLAKE_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL = forceGCPUseDownscopedCredential; assert.strictEqual(Util.shouldPerformGCPBucket(accessToken), result); - delete process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD; + delete process.env.SNOWFLAKE_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL; }); }); }); From 195fda8b61fee282dbabb490dd6bdb05dbe2fd17 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 6 Feb 2024 13:38:31 -0800 Subject: [PATCH 8/9] removed some spaces which were created during updating --- lib/util.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/util.js b/lib/util.js index 77cdf2dd2..cb0cc136e 100644 --- a/lib/util.js +++ b/lib/util.js @@ -661,11 +661,6 @@ exports.removeScheme = function (input) { return input.toString().replace(/(^\w+:|^)\/\//, ''); }; - -exports.shouldPerformGCPBucket = function (accessToken) { - return !!accessToken && process.env.SNOWFLAKE_FORCE_GCP_USE_DOWNSCOPED_CREDENTIAL !== 'true'; -}; - /** * Checks if the provided file or directory permissions are correct. * @param filePath @@ -699,12 +694,10 @@ exports.isFileNotWritableByGroupOrOthers = async function (configFilePath, fsPro return (stats.mode & (1 << 4)) === 0 && (stats.mode & (1 << 1)) === 0; }; - exports.shouldRetryOktaAuth = function ({ maxRetryTimeout, maxRetryCount, numRetries, startTime, remainingTimeout }) { return (maxRetryTimeout === 0 || Date.now() < startTime + remainingTimeout) && numRetries <= maxRetryCount; }; exports.shouldPerformGCPBucket = function (accessToken) { - console.log( process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD); return !!accessToken && process.env.SNOWFLAKE_DISABLE_GCP_TOKEN_UPLOAD !== 'true'; }; From 569af67003a6e8fe690126e2848d1a3101f5e944 Mon Sep 17 00:00:00 2001 From: Naira Petrosyan Date: Mon, 12 Feb 2024 18:51:15 +0400 Subject: [PATCH 9/9] fix: when err.code is not numeric or doesnot exist take err.response.status instead --- lib/file_transfer_agent/gcs_util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/file_transfer_agent/gcs_util.js b/lib/file_transfer_agent/gcs_util.js index b72d9ef91..3cf6a0e2b 100644 --- a/lib/file_transfer_agent/gcs_util.js +++ b/lib/file_transfer_agent/gcs_util.js @@ -178,7 +178,7 @@ function GCSUtil(httpclient, filestream) { encryptionMetadata ); } catch (err) { - const errCode = err['code'] ? err['code'] : err.response.status; + const errCode = !isNaN(err['code']) && !isNaN(parseInt(err['code'])) ? err['code'] : err.response.status; if ([403, 408, 429, 500, 503].includes(errCode)) { meta['lastError'] = err;