-
Notifications
You must be signed in to change notification settings - Fork 44
Fix/esbuild #179
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
Fix/esbuild #179
Changes from all commits
013efb0
d5a085c
e999ae5
1834ced
8ce2c5b
d702b4d
c3f6b88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,14 @@ | ||
#!/bin/sh | ||
|
||
cat >build/cjs/package.json <<!EOF | ||
{ | ||
"type": "commonjs" | ||
} | ||
!EOF | ||
package_version=`npm show . version` | ||
|
||
cat >build/esm/package.json <<!EOF | ||
for target in esm cjs; do | ||
cp -R certs build/$target/src/ | ||
[[ $target == "esm" ]] && type="module" || type="commonjs" | ||
cat >build/$target/package.json <<!EOF | ||
{ | ||
"type": "module" | ||
"version": "$package_version", | ||
"type": "$type" | ||
} | ||
!EOF | ||
done |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,30 @@ | ||
// noinspection ES6PreferShortImport | ||
import {Logger} from './logging'; | ||
import { Logger } from './logging'; | ||
import fs from 'fs'; | ||
import path from 'path'; | ||
import {getRelTopLevelPath} from "./version"; | ||
|
||
const FALLBACK_INTERNAL_ROOT_CERTS = path.join(__dirname, getRelTopLevelPath(), 'certs/internal.pem'); | ||
const FALLBACK_SYSTEM_ROOT_CERTS = path.join(__dirname, getRelTopLevelPath(), 'certs/system.pem'); | ||
const CERTIFICATES_FOLDER = 'certs' | ||
const RELATIVE_PATH = process.env.TEST_ENVIRONMENT ? '../' : './' | ||
const RESOLVED_PATH = path.join(__dirname, RELATIVE_PATH, CERTIFICATES_FOLDER) | ||
const FALLBACK_INTERNAL_ROOT_CERTS = path.join(RESOLVED_PATH, 'internal.pem'); | ||
const FALLBACK_SYSTEM_ROOT_CERTS = path.join(RESOLVED_PATH, 'system.pem'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Почему бы содержимое сертификатов не положить в текстовом виде в переменную в какой-нибудь js модуль и не реквайрить их как обычно? Кажется это решит все проблемы (в том числе с бандлерами). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Я вполне с этим согласен -- это предлагалось, однако по уму их надо бандлить вебпком во время компиляции из файлов\по урлу. Если есть консенсус по этому вопросу могу реализовать как через бандлер, так и переменной в .js There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А расскажите, в какой ситуации здесь появляются бандлеры? Ведь ydb-nodejs-sdk - это нодовая библиотека и, кажется, нет особого смысла бандлить/минифицировать код, который выполняется на ноде и не доезжает до клиента. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Смысл еще как есть. Кроме того, что код собирается в один файлик (что при прочих равных удобно) так еще и размер артефакта значительно меньше (учитывая что все node_modules не надо тащить). В контексте serverless функции очень значимый момент. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Хотелось бы понять можно ли всё-таки добавить webpack\esbuild для бандлинга cert или вставить сертификаты руками в код как предлагал @DavyJohnes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Я правильно понимаю, что бандлер и зависимости ydb-sdk тоже в бандл положит? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Да, он соберёт всё в один файл |
||
|
||
function makeInternalRootCertificates() { | ||
const internalRootCertificates = fs.readFileSync(FALLBACK_INTERNAL_ROOT_CERTS); | ||
if (!fs.existsSync(FALLBACK_INTERNAL_ROOT_CERTS) | ||
|| !fs.existsSync(FALLBACK_SYSTEM_ROOT_CERTS)) { | ||
throw new Error(certificateNotFoundMessage) | ||
} | ||
|
||
const internalRootCertificates = fs.readFileSync(FALLBACK_INTERNAL_ROOT_CERTS) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @esseswann @tsufiev а почему решили оставить чтение с fs? Это тоже потребует дополнительной настройки бандлера. Может таки положим в виде строки в js модуль? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Я не против, но меня немного смущает размер system.pem, возможно стоит обдумать как это можно сделать конфигурируемо There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Так а какая разница? Он тут так или иначе в памяти окажется, читаешь ты его с fs или реквайришь как js модуль. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Там есть кейс при котором его чтение необязательно, следовательно мы просто разжираем размер бандла. Это правда не меняет ситуацию относительно текущей, но теоретически можно было бы этого избежать |
||
const fallbackSystemRootCertificates = fs.readFileSync(FALLBACK_SYSTEM_ROOT_CERTS) | ||
|
||
let systemRootCertificates; | ||
let systemRootCertificates: Buffer; | ||
const tls = require('tls'); | ||
const nodeRootCertificates = tls.rootCertificates as string[] | undefined; | ||
if (nodeRootCertificates && nodeRootCertificates.length > 0) { | ||
systemRootCertificates = Buffer.from(nodeRootCertificates.join('\n')); | ||
} else { | ||
systemRootCertificates = fs.readFileSync(FALLBACK_SYSTEM_ROOT_CERTS); | ||
systemRootCertificates = fallbackSystemRootCertificates; | ||
} | ||
|
||
return Buffer.concat([internalRootCertificates, systemRootCertificates]); | ||
|
@@ -47,6 +55,12 @@ export function makeSslCredentials(endpoint: string, logger: Logger, sslCredenti | |
return makeDefaultSslCredentials(); | ||
} | ||
|
||
const certificateNotFoundMessage = `No certificate found | ||
It seems that you are using grpcs (secure) endpoint in a bundled environment. | ||
Either provide YDB_SSL_ROOT_CERTIFICATES_FILE environment variable | ||
or copy contents of ydb-nodejs-sdk/certs to ./certs path relative to the bundled file | ||
` | ||
|
||
export interface ISslCredentials { | ||
rootCertificates?: Buffer, | ||
clientPrivateKey?: Buffer, | ||
|
Uh oh!
There was an error while loading. Please reload this page.