diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 16ac6c48..c58ce5a7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -11,6 +11,7 @@ env: ARTIFACTS_PROJECT_ID: cartodb-on-gcp-main-artifacts NODE_VERSION: '12.18.3' _DOCKER_IMAGE_TO_REPLACE: 'sql-api' + COMPOSE_PROJECT_NAME: 'carto' jobs: build-and-test: @@ -99,7 +100,10 @@ jobs: BRANCH_NAME=${GITHUB_HEAD_REF#refs/heads/} BRANCH_NAME=${BRANCH_NAME//\//-} echo "Building sql-api image from branch: $BRANCH_NAME, commit: ${GITHUB_SHA::7}..." - docker build -f private/Dockerfile --label="org.opencontainers.image.created=$(date --rfc-3339=seconds)" --label=org.opencontainers.image.revision=${GITHUB_SHA} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${BRANCH_NAME} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_SHA::7} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${BRANCH_NAME}--${GITHUB_SHA::7} . + docker build -f private/Dockerfile --label="org.opencontainers.image.created=$(date --rfc-3339=seconds)" --label=org.opencontainers.image.revision=${GITHUB_SHA} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:latest -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${BRANCH_NAME} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_SHA::7} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${BRANCH_NAME}--${GITHUB_SHA::7} . + + echo "Building sql-api-onprem image from branch: $BRANCH_NAME, commit: ${GITHUB_SHA::7}..." + docker build -f private/Dockerfile.onprem --label="org.opencontainers.image.created=$(date --rfc-3339=seconds)" --label=org.opencontainers.image.revision=${GITHUB_SHA} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${BRANCH_NAME} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_SHA::7} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${BRANCH_NAME}--${GITHUB_SHA::7} . - name: Set up gcloud I uses: google-github-actions/setup-gcloud@master @@ -116,13 +120,15 @@ jobs: run: | ./scripts/init.sh - - name: Test networking + - name: Run all tests run: | - docker-compose -f docker-compose.yml -f docker-compose.test.yml run --rm test-networking - - - name: Test DB seed - run: | - docker-compose -f docker-compose.yml -f docker-compose.test.yml run --rm test-db + docker run --rm --name tavern-tester \ + --env CARTO_USER=carto \ + --env CARTO_DOMAIN=localhost.lan \ + --network host \ + --env-file \ + .env gcr.io/cartodb-on-gcp-main-artifacts/tavern-tester:latest \ + all dev - name: Push image run: | @@ -134,4 +140,8 @@ jobs: docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_SHA::7} docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${BRANCH_NAME}--${GITHUB_SHA::7} - + echo 'Pushing onprem images to the registry...' + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${BRANCH_NAME} + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_SHA::7} + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${BRANCH_NAME}--${GITHUB_SHA::7} + diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index 880e5e49..7fc3c3f9 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -28,6 +28,7 @@ jobs: echo ${GITHUB_SHA::7} echo ${GITHUB_REF##*/} docker build -f private/Dockerfile --label="org.opencontainers.image.created=$(date --rfc-3339=seconds)" --label=org.opencontainers.image.revision=${GITHUB_SHA} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:latest -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_REF##*/} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_SHA::7} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_REF##*/}--${GITHUB_SHA::7} . + docker build -f private/Dockerfile.onprem --label="org.opencontainers.image.created=$(date --rfc-3339=seconds)" --label=org.opencontainers.image.revision=${GITHUB_SHA} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:latest -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_REF##*/} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_SHA::7} -t gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_REF##*/}--${GITHUB_SHA::7} . - name: Setup gcloud authentication uses: google-github-actions/setup-gcloud@master @@ -42,5 +43,10 @@ jobs: run: | docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_REF##*/} docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_SHA::7} + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:${GITHUB_REF##*/}--${GITHUB_SHA::7} docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api:latest - \ No newline at end of file + + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_REF##*/} + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_SHA::7} + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:${GITHUB_REF##*/}--${GITHUB_SHA::7} + docker push gcr.io/$ARTIFACTS_PROJECT_ID/sql-api-onprem:latest diff --git a/README.md b/README.md index 38306211..f6fd046f 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,7 @@ $ npm test You can try to run the tests against the dependencies from the `dev-env`. To do so, you need to build the test docker image: ```shell +$ docker build -t sqlapi -f private/Dockerfile . $ docker-compose -f private/docker-compose.yml build ``` diff --git a/config/environments/config.js b/config/environments/config.js index ab3e4982..219c84e3 100644 --- a/config/environments/config.js +++ b/config/environments/config.js @@ -17,20 +17,17 @@ module.exports.routes = { ], // Optional: attach middlewares at the begining of the router // to perform custom operations. - middlewares: [ - function noop () { - return function noopMiddleware (req, res, next) { - next(); - } - } - ], + // This must be a **comma-separated string** with the list of paths to the middlewares. Pe: + // CARTO_SQL_PREROUTING_MIDDLEWARES=/usr/src/lib/mw1.js,/usr/src/lib/mw2.js + // Note: The list order is kept when loading the middlewares! + middlewares: process.env.CARTO_SQL_PREROUTING_MIDDLEWARES || '', sql: [{ // Required paths: [ '/sql' ], // Optional - middlewares: [] + middlewares: process.env.CARTO_SQL_SQLROUTING_MIDDLEWARES || '' }] }] }; diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 3870d064..c64df857 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -14,20 +14,17 @@ module.exports.routes = { ], // Optional: attach middlewares at the begining of the router // to perform custom operations. - middlewares: [ - function noop () { - return function noopMiddleware (req, res, next) { - next(); - } - } - ], + // This must be a **comma-separated string** with the list of paths to the middlewares. Pe: + // CARTO_SQL_PREROUTING_MIDDLEWARES=/usr/src/lib/mw1.js,/usr/src/lib/mw2.js + // Note: The list order is kept when loading the middlewares! + middlewares: process.env.CARTO_SQL_PREROUTING_MIDDLEWARES || '', sql: [{ // Required paths: [ '/sql' ], // Optional - middlewares: [] + middlewares: process.env.CARTO_SQL_SQLROUTING_MIDDLEWARES || '' }] }] }; diff --git a/config/environments/production.js.example b/config/environments/production.js.example index cd5e5c74..56579c9c 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -14,20 +14,17 @@ module.exports.routes = { ], // Optional: attach middlewares at the begining of the router // to perform custom operations. - middlewares: [ - function noop () { - return function noopMiddleware (req, res, next) { - next(); - } - } - ], + // This must be a **comma-separated string** with the list of paths to the middlewares. Pe: + // CARTO_SQL_PREROUTING_MIDDLEWARES=/usr/src/lib/mw1.js,/usr/src/lib/mw2.js + // Note: The list order is kept when loading the middlewares! + middlewares: process.env.CARTO_SQL_PREROUTING_MIDDLEWARES || '', sql: [{ // Required paths: [ '/sql' ], // Optional - middlewares: [] + middlewares: process.env.CARTO_SQL_SQLROUTING_MIDDLEWARES || '' }] }] }; diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 0b6b4835..91e38e9f 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -14,20 +14,17 @@ module.exports.routes = { ], // Optional: attach middlewares at the begining of the router // to perform custom operations. - middlewares: [ - function noop () { - return function noopMiddleware (req, res, next) { - next(); - } - } - ], + // This must be a **comma-separated string** with the list of paths to the middlewares. Pe: + // CARTO_SQL_PREROUTING_MIDDLEWARES=/usr/src/lib/mw1.js,/usr/src/lib/mw2.js + // Note: The list order is kept when loading the middlewares! + middlewares: process.env.CARTO_SQL_PREROUTING_MIDDLEWARES || '', sql: [{ // Required paths: [ '/sql' ], // Optional - middlewares: [] + middlewares: process.env.CARTO_SQL_SQLROUTING_MIDDLEWARES || '' }] }] }; diff --git a/config/environments/test.js.example b/config/environments/test.js.example index d490030b..35456103 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -14,20 +14,17 @@ module.exports.routes = { ], // Optional: attach middlewares at the begining of the router // to perform custom operations. - middlewares: [ - function noop () { - return function noopMiddleware (req, res, next) { - next(); - } - } - ], + // This must be a **comma-separated string** with the list of paths to the middlewares. Pe: + // CARTO_SQL_PREROUTING_MIDDLEWARES=/usr/src/lib/mw1.js,/usr/src/lib/mw2.js + // Note: The list order is kept when loading the middlewares! + middlewares: process.env.CARTO_SQL_PREROUTING_MIDDLEWARES || '', sql: [{ // Required paths: [ '/sql' ], // Optional - middlewares: [] + middlewares: process.env.CARTO_SQL_SQLROUTING_MIDDLEWARES || '' }] }] }; diff --git a/lib/api/api-router.js b/lib/api/api-router.js index 73fb41c1..74caa6dd 100644 --- a/lib/api/api-router.js +++ b/lib/api/api-router.js @@ -29,6 +29,7 @@ module.exports = class ApiRouter { this.versionController = new VersionController(); this.jobsWipController = new JobsWipController({ jobService }); this.pubSubMetricsService = PubSubMetricsService.build(); + this.logger = logger; this.sqlRouter = new SqlRouter({ metadataBackend, @@ -41,11 +42,23 @@ module.exports = class ApiRouter { route (app, routes) { routes.forEach(route => { const apiRouter = router({ mergeParams: true }); - const paths = route.paths; - const middlewares = route.middlewares || []; + const preRoutingMiddlewares = route.middlewares; - middlewares.forEach(middleware => apiRouter.use(middleware())); + if (preRoutingMiddlewares !== undefined && preRoutingMiddlewares.trim()) { + preRoutingMiddlewares.split(',').forEach(middlewarePath => { + try { + const middleware = require(middlewarePath).middlewares; + if (Array.isArray(middleware)) { + middleware.forEach(m => apiRouter.use(m())); + } else { + apiRouter.use(middleware()); + } + } catch (e) { + this.logger.error({ exception: e, path: middlewarePath }, 'Prerouting middleware not found, skipping'); + } + }); + } // FIXME: version controller should be attached to the main entry point: "/" // instead of "/api/:version" or "/user/:user/api/:version" @@ -61,7 +74,7 @@ module.exports = class ApiRouter { paths.forEach(path => app.use(path, apiRouter)); - apiRouter.use(error()); + apiRouter.use(error({ logger: this.logger })); apiRouter.use(pubSubMetrics(this.pubSubMetricsService)); }); } diff --git a/lib/api/jobs-wip-controller.js b/lib/api/jobs-wip-controller.js index 58d3090f..5370ccee 100644 --- a/lib/api/jobs-wip-controller.js +++ b/lib/api/jobs-wip-controller.js @@ -13,7 +13,7 @@ module.exports = class JobsWipController { bodyParser(), listWorkInProgressJobs(this.jobService), sendResponse(), - error() + error({ logger: null }) ]); } }; diff --git a/lib/api/middlewares/error.js b/lib/api/middlewares/error.js index 108dd2cc..288b098e 100644 --- a/lib/api/middlewares/error.js +++ b/lib/api/middlewares/error.js @@ -2,13 +2,13 @@ const errorHandlerFactory = require('../../services/error-handler-factory'); -module.exports = function error () { +module.exports = function error ({ logger }) { return function errorMiddleware (err, req, res, next) { - const { logger } = res.locals; const errorHandler = errorHandlerFactory(err); const errorResponse = errorHandler.getResponse(); + const errorLogger = res.locals.logger || logger; - logger.error({ exception: err }, 'Error while handling the request'); + errorLogger.error({ exception: err }, 'Error while handling the request'); // Force inline content disposition res.header('Content-Disposition', 'inline'); diff --git a/lib/api/sql/sql-router.js b/lib/api/sql/sql-router.js index 7eec6b30..2b1f5809 100644 --- a/lib/api/sql/sql-router.js +++ b/lib/api/sql/sql-router.js @@ -24,6 +24,7 @@ module.exports = class SqlRouter { const userDatabaseService = new UserDatabaseService(metadataBackend); const userLimitsService = new UserLimitsService(metadataBackend, userLimitsServiceOptions); + this.logger = logger; this.queryController = new QueryController({ metadataBackend, userDatabaseService, @@ -55,9 +56,22 @@ module.exports = class SqlRouter { const sqlRouter = router({ mergeParams: true }); const paths = route.paths; - const middlewares = route.middlewares || []; + const preRoutingMiddlewares = route.middlewares; - middlewares.forEach(middleware => sqlRouter.use(middleware())); + if (preRoutingMiddlewares !== undefined && preRoutingMiddlewares.trim()) { + preRoutingMiddlewares.split(',').forEach(middlewarePath => { + try { + const middleware = require(middlewarePath).middlewares; + if (Array.isArray(middleware)) { + middleware.forEach(m => apiRouter.use(m())); + } else { + apiRouter.use(middleware()); + } + } catch (e) { + this.logger.error({ exception: e, path: middlewarePath }, 'SQL prerouting middleware not found, skipping'); + } + }); + } sqlRouter.use(socketTimeout()); sqlRouter.use(cors()); diff --git a/private b/private index d5079899..cb59185b 160000 --- a/private +++ b/private @@ -1 +1 @@ -Subproject commit d50798990e04a12e0e84b08c53e025e2f56cfd08 +Subproject commit cb59185b1ad0654b0e805741aaf29d41ec688357 diff --git a/test/acceptance/custom-middlewares-test.js b/test/acceptance/custom-middlewares-test.js index 2b9892ea..97961b0b 100644 --- a/test/acceptance/custom-middlewares-test.js +++ b/test/acceptance/custom-middlewares-test.js @@ -16,12 +16,6 @@ describe('custom middlewares', function () { statusCode: 418 }; - const customMiddleware = function teapot () { - return function teapotMiddleware (req, res) { - res.status(418).send('I\'m a teapot'); - }; - }; - describe('wired in /api/v1/', function () { before(function () { this.backupRoutes = global.settings.routes; @@ -32,9 +26,7 @@ describe('custom middlewares', function () { '/api/:version', '/user/:user/api/:version' ], - middlewares: [ - customMiddleware - ], + middlewares: '../../test/support/middlewares/teapot-headers.js,../../test/support/middlewares/teapot-response.js', sql: [{ paths: [ '/sql' @@ -59,6 +51,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -73,6 +67,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -87,6 +83,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -101,6 +99,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -122,6 +122,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -136,6 +138,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -156,6 +160,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -175,6 +181,8 @@ describe('custom middlewares', function () { return done(err); } + assert.strictEqual(res.headers['x-what-am-i'], 'I\'m a teapot'); + assert.strictEqual(res.headers['x-again-what-am-i'], 'I\'m a teapot'); assert.strictEqual(res.body, 'I\'m a teapot'); done(); }); @@ -195,7 +203,7 @@ describe('custom middlewares', function () { paths: [ '/sql' ], - middlewares: [customMiddleware] + middlewares: '../../../test/support/middlewares/teapot-response.js' }] }] }; diff --git a/test/support/middlewares/teapot-headers.js b/test/support/middlewares/teapot-headers.js new file mode 100644 index 00000000..487cda81 --- /dev/null +++ b/test/support/middlewares/teapot-headers.js @@ -0,0 +1,14 @@ +exports.middlewares = [ + function () { + return function teapotHeaderMiddleware (req, res, next) { + res.header('X-What-Am-I', 'I\'m a teapot'); + return next(); + }; + }, + function () { + return function teapotAnotherHeaderMiddleware (req, res, next) { + res.header('X-Again-What-Am-I', 'I\'m a teapot'); + return next(); + }; + } +]; diff --git a/test/support/middlewares/teapot-response.js b/test/support/middlewares/teapot-response.js new file mode 100644 index 00000000..d7cb93d8 --- /dev/null +++ b/test/support/middlewares/teapot-response.js @@ -0,0 +1,5 @@ +exports.middlewares = function () { + return function teapotMiddleware (req, res) { + res.status(418).send('I\'m a teapot'); + }; +};