From 8e63b4deebf79d0f2b324f8cbdbd404c466774c7 Mon Sep 17 00:00:00 2001 From: Trygve Amundsen Date: Fri, 2 Jun 2023 16:01:28 +0200 Subject: [PATCH] feat: add config as params BREAKING_CHANGE: receive config in Service constructor in stead of reading it from config file. --- lib/config.js | 209 ++++++++++------------------ lib/main.js | 61 ++++---- package.json | 1 + test/404.js | 4 +- test/alias.map.js | 2 +- test/alias.npm.js | 2 +- test/alias.pkg.js | 2 +- test/auth.js | 4 +- test/config.js | 68 +++++++++ test/http.cache.control.js | 2 +- test/http.etag.js | 48 ++----- test/http.override.cache.control.js | 2 +- test/http.query.params.js | 2 +- test/map.js | 2 +- test/npm.js | 2 +- test/pkg-put-write-integrity.js | 8 +- test/pkg.js | 2 +- 17 files changed, 199 insertions(+), 222 deletions(-) create mode 100644 test/config.js diff --git a/lib/config.js b/lib/config.js index 8198657e..9154b7f3 100644 --- a/lib/config.js +++ b/lib/config.js @@ -1,9 +1,38 @@ -import convict from 'convict'; -import yaml from 'js-yaml'; -import pino from 'pino'; import path, { join } from 'path'; import fs from 'fs'; import os from 'os'; +import Sink from '@eik/sink'; + +/** + * Configuration object + * @typedef {import('@eik/sink')} Sink + * @typedef Config + * @type {object} + * @property {string} name - Name of the application + * @property {('development' | 'production')} env - Applicaton environments + * @property {boolean} metrics - Enable metrics + * @property {object} log - Log configuration + * @property {('trace' | 'debug' | 'info' | 'warn' | 'error' | 'fatal')} log.level - Log level to log at + * @property {object} http - Http configuration + * @property {boolean} http.http2 - Enable http2 for the server + * @property {string} http.address - The address the http server should bind to + * @property {number} http.port - The port the http server should bind to + * @property {object} compression - Compression configuration + * @property {boolean} compression.global - Enable global compression for all http routes + * @property {object} jwt - JWT configuration + * @property {string} jwt.secret - Secret used for JWT signing + * @property {object} basicAuth - Basic auth configuration + * @property {('key' | 'disabled')} basicAuth.type - Type of basic auth to use + * @property {string} basicAuth.key - Key used for basic authorization + * @property {object} organization - Organization configuration + * @property {string} organization.name - Organization name - Used as a folder name in the storage of files + * @property {Array.} organization.hostnames - Hostnames the organization maps to + * @property {object | Sink} sink - Sink configuration + * @property {('fs' | 'mem' | 'test')} sink.type - Type of sink to use + * @property {string} sink.path - Absolute path to store files in when using the "fs" sink + * @property {string} notFoundCacheControl - Cache control header value for 404 responses + * @property {string} aliasCacheControl - Cache control header value for alias responses + */ const CWD = process.cwd(); @@ -14,156 +43,58 @@ try { /* empty */ } -convict.addParser({ extension: ['yml', 'yaml'], parse: yaml.load }); +/** + * @param {Config} config + * @returns {Config} + */ +const withDefaults = (config) => ({ + name: pack.name, + env: 'development', + metrics: true, + notFoundCacheControl: 'public, max-age=5', + aliasCacheControl: '', -convict.addFormat({ - name: 'secret-string', - validate: (value) => { - if (typeof value !== 'string') { - throw new Error('Value must be a String'); - } - }, - coerce: (value) => { - if (path.isAbsolute(value)) { - try { - const file = fs.readFileSync(value); - return file.toString(); - } catch (error) { - throw new Error(`Config could not load secret from path: ${value}`); - } - } - return value; - } -}); + ...config, -const conf = convict({ - name: { - doc: 'Name of the apllication', - default: pack.name, - format: String, - }, - env: { - doc: 'Applicaton environments', - format: ['development', 'production'], - default: 'development', - env: 'NODE_ENV', - arg: 'node-env', - }, - metrics: { - format: Boolean, - default: true, - env: 'METRICS', - }, log: { - level: { - doc: 'Log level to log at', - format: ['trace', 'debug', 'info', 'warn', 'error', 'fatal'], - default: 'info', - env: 'LOG_LEVEL', - arg: 'log-level', - }, + level: 'info', + ...config.log, }, http: { - http2: { - doc: 'Enable http2 for the server', - format: Boolean, - default: false, - env: 'HTTP_HTTP2', - }, - address: { - doc: 'The address the http server should bind to', - format: String, - default: 'localhost', - env: 'HTTP_ADDRESS', - }, - port: { - doc: 'The port the http server should bind to', - format: 'port', - default: 4001, - env: 'HTTP_PORT', - }, + http2: false, + address: 'localhost', + port: 4001, + ...config.http, }, compression: { - global: { - doc: 'Enable global compression for all http routes', - format: Boolean, - default: true, - env: 'COMPRESSION_GLOBAL', - }, + global: true, + ...config.compression, }, jwt: { - secret: { - doc: 'Secret used for JWT signing', - format: 'secret-string', - default: 'change_me', - env: 'AUTH_JWT_SECRET', - sensitive: true, - }, - expire: { - doc: 'Expire time for JWT', - format: String, - default: '60d', - env: 'AUTH_JWT_EXPIRE', - }, + secret: 'change_me', + expire: '60d', + ...config.jwt, }, basicAuth: { - type: { - doc: 'Type of basic auth to use', - format: ['key', 'disabled'], - default: 'key', - env: 'BASIC_AUTH_TYPE', - }, - key: { - doc: 'Key used for basic authorization', - format: 'secret-string', - default: 'change_me', - env: 'BASIC_AUTH_KEY', - sensitive: true, - }, + type: 'key', + key: 'change_me', + ...config.basicAuth, }, organization: { - name: { - doc: 'Organization name - Used as a folder name in the storage of files', - format: String, - default: 'local', - env: 'ORG_NAME', - }, - hostnames: { - doc: 'Hostnames the organization maps to', - format: Array, - default: ['localhost', '127.0.0.1'], - env: 'ORG_HOSTNAMES', - }, + name: 'local', + hostnames: ['localhost', '127.0.0.1'], + ...config.organization, }, - sink: { - type: { - doc: 'Type of sink to use', - format: ['fs', 'mem', 'test'], - default: 'fs', - env: 'SINK_TYPE', - }, - path: { - doc: 'Absolute path to store files in when using the "fs" sink', - format: String, - default: path.join(os.tmpdir(), '/eik'), - env: 'SINK_PATH', - }, - } + sink: + config.sink instanceof Sink + ? config.sink + : { + type: 'fs', + path: path.join(os.tmpdir(), '/eik'), + ...config.sink, + }, }); -const env = conf.get('env'); - -const logger = pino({ - level: conf.get('log.level'), - name: conf.get('name'), -}); - -try { - conf.loadFile(path.join(CWD, `/config/${env}.yaml`)); -} catch (error) { - logger.error(error); -} - -conf.validate(); +const DefaultConfig = withDefaults({}); -export default conf; +export { DefaultConfig, withDefaults }; diff --git a/lib/main.js b/lib/main.js index dd3850b9..158a3ee1 100644 --- a/lib/main.js +++ b/lib/main.js @@ -5,40 +5,43 @@ import pino from 'pino'; import cors from '@fastify/cors'; import jwt from '@fastify/jwt'; import eik from '@eik/core'; +import Sink from '@eik/sink'; -import config from './config.js'; +import { DefaultConfig, withDefaults } from './config.js'; import * as utils from './utils.js'; const EikService = class EikService { - constructor({ customSink, notFoundCacheControl, aliasCacheControl } = {}) { - this._notFoundCacheControl = - notFoundCacheControl || 'public, max-age=5'; + /** + * @param {Config} config + */ + constructor(config) { + const cfg = withDefaults(config); + const logger = pino({ - level: config.get('log.level'), - name: config.get('name'), + level: cfg.log?.level, + name: cfg.name, }); let sink; - if (customSink) { - sink = customSink; - } else if (config.get('sink.type') === 'mem') { + if (cfg.sink instanceof Sink) { + sink = cfg.sink; + } else if (cfg.sink?.type === 'mem') { logger.info( `Server is running with a in memory sink. Uploaded files will be lost on restart!`, ); sink = new eik.sink.MEM(); } else { logger.info( - `Server is running with the file system sink. Uploaded files will be stored under "${config.get( - 'sink.path', - )}"`, + `Server is running with the file system sink. Uploaded files will be stored under "${cfg.sink?.path}"`, ); sink = new eik.sink.FS(); } // Transform organization config - const organizations = config - .get('organization.hostnames') - .map((hostname) => [hostname, config.get('organization.name')]); + const organizations = cfg.organization?.hostnames?.map((hostname) => [ + hostname, + cfg.organization?.name, + ]); this._versionsGet = new eik.http.VersionsGet({ organizations, @@ -55,13 +58,13 @@ const EikService = class EikService { organizations, sink, logger, - cacheControl: aliasCacheControl, + cacheControl: cfg.aliasCacheControl, }); this._aliasPut = new eik.http.AliasPut({ organizations, sink, logger }); this._authPost = new eik.http.AuthPost({ organizations, logger, - authKey: config.get('basicAuth.key'), + authKey: cfg.basicAuth?.key, }); this._pkgLog = new eik.http.PkgLog({ organizations, sink, logger }); this._pkgGet = new eik.http.PkgGet({ organizations, sink, logger }); @@ -112,22 +115,22 @@ const EikService = class EikService { }); this.metrics = metrics; - this.config = config; + this.config = cfg; this.logger = logger; this.sink = sink; // Print warnings if ( - config.get('basicAuth.type') === 'key' && - config.get('basicAuth.key') === config.default('basicAuth.key') + cfg.basicAuth?.type === 'key' && + cfg.basicAuth.key === DefaultConfig.basicAuth.key ) { logger.warn( 'Server is running with default basic authorization key configured! For security purposes, it is highly recommended to set a custom value!', ); } - if (config.get('jwt.secret') === config.default('jwt.secret')) { + if (cfg.jwt?.secret === DefaultConfig.jwt.secret) { logger.warn( 'Server is running with default jwt secret configured! For security purposes, it is highly recommended to set a custom value!', ); @@ -135,11 +138,9 @@ const EikService = class EikService { // Print info - const hosts = config.get('organization.hostnames').join(', '); + const hosts = cfg.organization?.hostnames?.join(', '); logger.info( - `Files for "${hosts}" will be stored in the "${config.get( - 'organization.name', - )}" organization space`, + `Files for "${hosts}" will be stored in the "${cfg.organization?.name}" organization space`, ); } @@ -163,7 +164,7 @@ const EikService = class EikService { // Authentication app.register(jwt, { - secret: config.get('jwt.secret'), + secret: this.config.jwt?.secret, messages: { badRequestErrorMessage: 'Autorization header is malformatted. Format is "Authorization: Bearer [token]"', @@ -198,12 +199,12 @@ const EikService = class EikService { // Compression app.register(compression, { - global: config.get('compression.global'), + global: this.config.compression?.global, }); // 404 handling app.setNotFoundHandler((request, reply) => { - reply.header('cache-control', this._notFoundCacheControl); + reply.header('cache-control', this.config.notFoundCacheControl); reply.type('text/plain'); reply.code(404); reply.send('Not found'); @@ -220,7 +221,7 @@ const EikService = class EikService { if (error.statusCode === 404) { reply.header( 'cache-control', - this._notFoundCacheControl, + this.config.notFoundCacheControl, ); } reply.send(error); @@ -241,7 +242,7 @@ const EikService = class EikService { const body = JSON.parse(JSON.stringify(outgoing.body)); const token = app.jwt.sign(body, { - expiresIn: config.get('jwt.expire'), + expiresIn: this.config.jwt?.expire, }); reply.header('cache-control', outgoing.cacheControl); diff --git a/package.json b/package.json index 1861c3cf..5f3ceb4e 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "homepage": "https://github.com/eik-lib/service#readme", "dependencies": { "@eik/core": "1.3.26", + "@eik/sink": "1.2.1", "convict": "6.2.4", "fastify": "4.17.0", "@fastify/compress": "6.4.0", diff --git a/test/404.js b/test/404.js index 56d6f2b6..d9547c35 100644 --- a/test/404.js +++ b/test/404.js @@ -8,7 +8,7 @@ import Server from '../lib/main.js'; tap.test('404 - POST request to non existing pathname', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -38,7 +38,7 @@ tap.test('404 - POST request to non existing pathname', async (t) => { tap.test('404 - GET request to non existing pathname', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/alias.map.js b/test/alias.map.js index a0d32daf..cff1a719 100644 --- a/test/alias.map.js +++ b/test/alias.map.js @@ -16,7 +16,7 @@ const FIXTURE_MAP_B = path.resolve(__dirname, '../fixtures/import-map-b.json'); tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/alias.npm.js b/test/alias.npm.js index f113a034..4d89b8a8 100644 --- a/test/alias.npm.js +++ b/test/alias.npm.js @@ -21,7 +21,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/alias.pkg.js b/test/alias.pkg.js index eaf5d21c..d096f2c0 100644 --- a/test/alias.pkg.js +++ b/test/alias.pkg.js @@ -21,7 +21,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/auth.js b/test/auth.js index 4f54ede4..1d3ad9dd 100644 --- a/test/auth.js +++ b/test/auth.js @@ -8,7 +8,7 @@ import Server from '../lib/main.js'; tap.test('auth - authenticate - legal "key" value', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -43,7 +43,7 @@ tap.test('auth - authenticate - legal "key" value', async (t) => { tap.test('auth - authenticate - illegal "key" value', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink, port: 0, logger: false }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/config.js b/test/config.js new file mode 100644 index 00000000..45a8a67b --- /dev/null +++ b/test/config.js @@ -0,0 +1,68 @@ +import tap from 'tap'; +import eik from '@eik/core'; +import { withDefaults } from '../lib/config.js'; + +tap.test('config - use default simple values', async (t) => { + const config = withDefaults({}); + t.equal(config.env, 'development', 'default env should be "development"'); +}); + +tap.test('config - provided config should override defaults', async (t) => { + const config = withDefaults({ + name: 'overridden', + }); + t.equal(config.name, 'overridden', 'provided name should override default'); +}); + +tap.test('config - use default object values', async (t) => { + const config = withDefaults({}); + t.same(config.log, { level: 'info' }, 'default log level should be "info"'); +}); + +tap.test( + 'config - provided object values should override default', + async (t) => { + const config = withDefaults({ + log: { + level: 'debug', + other: 'value', + }, + }); + t.same( + config.log, + { level: 'debug', other: 'value' }, + 'default log level should be "info"', + ); + }, +); + +tap.test( + 'config - default object values should not override other object content', + async (t) => { + const config = withDefaults({ + log: { + other: 'value', + }, + }); + t.same( + config.log, + { level: 'info', other: 'value' }, + 'default log level should be "info"', + ); + }, +); + +tap.test( + "config - don't apply default value on sink when providing a custom Sink", + async (t) => { + const customSink = new eik.sink.MEM(); + const config = withDefaults({ + sink: customSink, + }); + t.equal( + config.sink, + customSink, + 'customSink should not be overridden by default value', + ); + }, +); diff --git a/test/http.cache.control.js b/test/http.cache.control.js index afa20389..cf1c47e3 100644 --- a/test/http.cache.control.js +++ b/test/http.cache.control.js @@ -22,7 +22,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/http.etag.js b/test/http.etag.js index 37945f7f..f5ca5570 100644 --- a/test/http.etag.js +++ b/test/http.etag.js @@ -11,7 +11,7 @@ import Server from '../lib/main.js'; tap.test('ETag - pkg:get - ETag and "If-None-Match" is matching', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -63,11 +63,7 @@ tap.test( 'ETag - pkg:get - ETag and "If-None-Match" is NOT matching', async (t) => { const sink = new Sink(); - const service = new Server({ - customSink: sink, - port: 0, - logger: false, - }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -124,11 +120,7 @@ tap.test( 'ETag - pkg:get - "If-None-Match" is NOT set on request', async (t) => { const sink = new Sink(); - const service = new Server({ - customSink: sink, - port: 0, - logger: false, - }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -180,7 +172,7 @@ tap.test( /* tap.test('ETag - pkg:get - ETags is configured to not be set', async t => { const sink = new Sink(); - const service = new Server({ customSink: sink, port: 0, config: { etag: false }, logger: false }); + const service = new Server({ sink, etag: false }); const address = await service.start(); const url = `${address}/pkg/fuzz/8.4.1/main/index.js`; @@ -217,7 +209,7 @@ tap.test('ETag - pkg:get - ETags is configured to not be set', async t => { tap.test('ETag - pkg:log - ETag and "If-None-Match" is matching', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink, port: 0, logger: false }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -269,11 +261,7 @@ tap.test( 'ETag - pkg:log - ETag and "If-None-Match" is NOT matching', async (t) => { const sink = new Sink(); - const service = new Server({ - customSink: sink, - port: 0, - logger: false, - }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -330,11 +318,7 @@ tap.test( 'ETag - pkg:log - "If-None-Match" is NOT set on request', async (t) => { const sink = new Sink(); - const service = new Server({ - customSink: sink, - port: 0, - logger: false, - }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -386,7 +370,7 @@ tap.test( /* tap.test('ETag - pkg:log - ETags is configured to not be set', async t => { const sink = new Sink(); - const service = new Server({ customSink: sink, port: 0, config: { etag: false }, logger: false }); + const service = new Server({ sink, etag: false }); const address = await service.start(); const url = `${address}/pkg/fuzz/8.4.1`; @@ -423,7 +407,7 @@ tap.test('ETag - pkg:log - ETags is configured to not be set', async t => { tap.test('ETag - map:get - ETag and "If-None-Match" is matching', async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink, port: 0, logger: false }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -475,11 +459,7 @@ tap.test( 'ETag - map:get - ETag and "If-None-Match" is NOT matching', async (t) => { const sink = new Sink(); - const service = new Server({ - customSink: sink, - port: 0, - logger: false, - }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -536,11 +516,7 @@ tap.test( 'ETag - map:get - "If-None-Match" is NOT set on request', async (t) => { const sink = new Sink(); - const service = new Server({ - customSink: sink, - port: 0, - logger: false, - }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -592,7 +568,7 @@ tap.test( /* tap.test('ETag - map:get - ETags is configured to not be set', async t => { const sink = new Sink(); - const service = new Server({ customSink: sink, port: 0, config: { etag: false }, logger: false }); + const service = new Server({ sink, etag: false }); const address = await service.start(); const url = `${address}/map/buzz/4.2.2`; diff --git a/test/http.override.cache.control.js b/test/http.override.cache.control.js index 00290071..1b5ed277 100644 --- a/test/http.override.cache.control.js +++ b/test/http.override.cache.control.js @@ -22,7 +22,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); const service = new Server({ - customSink: sink, + sink, aliasCacheControl: 'public, max-age=600', }); diff --git a/test/http.query.params.js b/test/http.query.params.js index 7aac700f..d490ca95 100644 --- a/test/http.query.params.js +++ b/test/http.query.params.js @@ -22,7 +22,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/map.js b/test/map.js index 4012323f..52329e2d 100644 --- a/test/map.js +++ b/test/map.js @@ -15,7 +15,7 @@ const FIXTURE_MAP = path.resolve(__dirname, '../fixtures/import-map.json'); tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/npm.js b/test/npm.js index c15d2196..9a54aa4e 100644 --- a/test/npm.js +++ b/test/npm.js @@ -21,7 +21,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/pkg-put-write-integrity.js b/test/pkg-put-write-integrity.js index e32e7641..20c7e479 100644 --- a/test/pkg-put-write-integrity.js +++ b/test/pkg-put-write-integrity.js @@ -44,7 +44,7 @@ tap.test( return Math.floor(Math.random() * max) + min; }; - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -92,7 +92,7 @@ tap.test( return Math.floor(Math.random() * max) + min; }; - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -140,7 +140,7 @@ tap.test( return Math.floor(Math.random() * max) + min; }; - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, @@ -188,7 +188,7 @@ tap.test( return Math.floor(Math.random() * max) + min; }; - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true, diff --git a/test/pkg.js b/test/pkg.js index 86b125a1..39fd0799 100644 --- a/test/pkg.js +++ b/test/pkg.js @@ -21,7 +21,7 @@ tap.cleanSnapshot = (s) => { tap.beforeEach(async (t) => { const sink = new Sink(); - const service = new Server({ customSink: sink }); + const service = new Server({ sink }); const app = Fastify({ ignoreTrailingSlash: true,