From 99bffd3a86cb85f4ca20a22599e64be6abf7efcb Mon Sep 17 00:00:00 2001 From: chris48s Date: Thu, 25 Nov 2021 17:11:49 +0000 Subject: [PATCH] Send better user-agent values (and got config changes) (#7309) * expose fetchLimitBytes/userAgent in got-config module * export a function not a factory * send better user-agent values - add userAgentBase setting - send short SHA in user agent on heroku - set a version (tag or short SHA) in Dockefile and use it to report server version in UA for docker users * add a comment explaining fileSize Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> --- .github/workflows/build-docker-image.yml | 2 ++ .github/workflows/create-release.yml | 2 ++ .github/workflows/publish-docker-next.yml | 2 ++ Dockerfile | 4 +++ config/custom-environment-variables.yml | 1 + config/default.yml | 1 + core/base-service/base.js | 8 +++--- core/base-service/got-config.js | 26 ++++++++++++++++++ core/base-service/got-config.spec.js | 27 +++++++++++++++++++ core/base-service/got.js | 13 ++++++--- core/base-service/got.spec.js | 14 +++++----- core/legacy/regular-update.js | 5 ++-- core/server/server.js | 7 +++-- services/github/auth/acceptor.js | 9 ++----- .../github/github-api-provider.integration.js | 11 +++----- services/github/github-api-provider.js | 4 ++- services/github/github-auth-service.spec.js | 2 +- .../librariesio/librariesio-api-provider.js | 4 ++- services/php-version.js | 7 ++--- services/suggest.js | 5 ++-- services/test-helpers.js | 7 ++--- services/validators.js | 6 +++++ 22 files changed, 113 insertions(+), 54 deletions(-) create mode 100644 core/base-service/got-config.js create mode 100644 core/base-service/got-config.spec.js diff --git a/.github/workflows/build-docker-image.yml b/.github/workflows/build-docker-image.yml index 61294ccc9a347..820b6b56685bd 100644 --- a/.github/workflows/build-docker-image.yml +++ b/.github/workflows/build-docker-image.yml @@ -18,3 +18,5 @@ jobs: context: . push: false tags: shieldsio/shields:pr-validation + build-args: | + version=${GITHUB_SHA::7} diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 90f2bec944878..cae2e59595921 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -45,3 +45,5 @@ jobs: context: . push: true tags: shieldsio/shields:server-${{ steps.date.outputs.date }} + build-args: | + version=server-${{ steps.date.outputs.date }} diff --git a/.github/workflows/publish-docker-next.yml b/.github/workflows/publish-docker-next.yml index 859e91ab86ae7..905779557fb7e 100644 --- a/.github/workflows/publish-docker-next.yml +++ b/.github/workflows/publish-docker-next.yml @@ -26,3 +26,5 @@ jobs: context: . push: true tags: shieldsio/shields:next + build-args: | + version=${GITHUB_SHA::7} diff --git a/Dockerfile b/Dockerfile index 81465fcb11a0d..a1291e828dfeb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -20,6 +20,10 @@ RUN npm cache clean --force # Use multi-stage build to reduce size FROM node:16-alpine + +ARG version=dev +ENV DOCKER_SHIELDS_VERSION=$version + # Run the server using production configs. ENV NODE_ENV production diff --git a/config/custom-environment-variables.yml b/config/custom-environment-variables.yml index d39951a8d5236..a78e9d3d93376 100644 --- a/config/custom-environment-variables.yml +++ b/config/custom-environment-variables.yml @@ -64,6 +64,7 @@ public: defaultCacheLengthSeconds: 'BADGE_MAX_AGE_SECONDS' fetchLimit: 'FETCH_LIMIT' + userAgentBase: 'USER_AGENT_BASE' requestTimeoutSeconds: 'REQUEST_TIMEOUT_SECONDS' requestTimeoutMaxAgeSeconds: 'REQUEST_TIMEOUT_MAX_AGE_SECONDS' diff --git a/config/default.yml b/config/default.yml index 41dc14e5f319c..0a7d5f1743e24 100644 --- a/config/default.yml +++ b/config/default.yml @@ -34,6 +34,7 @@ public: handleInternalErrors: true fetchLimit: '10MB' + userAgentBase: 'shields (self-hosted)' requestTimeoutSeconds: 120 requestTimeoutMaxAgeSeconds: 30 diff --git a/core/base-service/base.js b/core/base-service/base.js index fb2ecaa516b5f..52d87087d8275 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -20,7 +20,7 @@ import { Deprecated, } from './errors.js' import { validateExample, transformExample } from './examples.js' -import { fetchFactory } from './got.js' +import { fetch } from './got.js' import { makeFullUrl, assertValidRoute, @@ -432,7 +432,7 @@ class BaseService { }, serviceConfig ) { - const { cacheHeaders: cacheHeaderConfig, fetchLimitBytes } = serviceConfig + const { cacheHeaders: cacheHeaderConfig } = serviceConfig const { regex, captureNames } = prepareRoute(this.route) const queryParams = getQueryParamNames(this.route) @@ -441,8 +441,6 @@ class BaseService { ServiceClass: this, }) - const fetcher = fetchFactory(fetchLimitBytes) - camp.route( regex, handleRequest(cacheHeaderConfig, { @@ -453,7 +451,7 @@ class BaseService { const namedParams = namedParamsForMatch(captureNames, match, this) const serviceData = await this.invoke( { - requestFetcher: fetcher, + requestFetcher: fetch, githubApiProvider, librariesIoApiProvider, metricHelper, diff --git a/core/base-service/got-config.js b/core/base-service/got-config.js new file mode 100644 index 0000000000000..f369d5ea610eb --- /dev/null +++ b/core/base-service/got-config.js @@ -0,0 +1,26 @@ +import bytes from 'bytes' +import configModule from 'config' +import Joi from 'joi' +import { fileSize } from '../../services/validators.js' + +const schema = Joi.object({ + fetchLimit: fileSize, + userAgentBase: Joi.string().required(), +}).required() +const config = configModule.util.toObject() +const publicConfig = Joi.attempt(config.public, schema, { allowUnknown: true }) + +const fetchLimitBytes = bytes(publicConfig.fetchLimit) + +function getUserAgent(userAgentBase = publicConfig.userAgentBase) { + let version = 'dev' + if (process.env.DOCKER_SHIELDS_VERSION) { + version = process.env.DOCKER_SHIELDS_VERSION + } + if (process.env.HEROKU_SLUG_COMMIT) { + version = process.env.HEROKU_SLUG_COMMIT.substring(0, 7) + } + return `${userAgentBase}/${version}` +} + +export { fetchLimitBytes, getUserAgent } diff --git a/core/base-service/got-config.spec.js b/core/base-service/got-config.spec.js new file mode 100644 index 0000000000000..670b76381905e --- /dev/null +++ b/core/base-service/got-config.spec.js @@ -0,0 +1,27 @@ +import { expect } from 'chai' +import { getUserAgent } from './got-config.js' + +describe('getUserAgent function', function () { + afterEach(function () { + delete process.env.HEROKU_SLUG_COMMIT + delete process.env.DOCKER_SHIELDS_VERSION + }) + + it('uses the default userAgentBase', function () { + expect(getUserAgent()).to.equal('shields (self-hosted)/dev') + }) + + it('applies custom userAgentBase', function () { + expect(getUserAgent('custom')).to.equal('custom/dev') + }) + + it('uses short commit SHA from HEROKU_SLUG_COMMIT if available', function () { + process.env.HEROKU_SLUG_COMMIT = '92090bd44742a5fac03bcb117002088fc7485834' + expect(getUserAgent('custom')).to.equal('custom/92090bd') + }) + + it('uses short commit SHA from DOCKER_SHIELDS_VERSION if available', function () { + process.env.DOCKER_SHIELDS_VERSION = 'server-2021-11-22' + expect(getUserAgent('custom')).to.equal('custom/server-2021-11-22') + }) +}) diff --git a/core/base-service/got.js b/core/base-service/got.js index b62fa24502b10..4930a15938172 100644 --- a/core/base-service/got.js +++ b/core/base-service/got.js @@ -1,7 +1,11 @@ import got from 'got' import { Inaccessible, InvalidResponse } from './errors.js' +import { + fetchLimitBytes as fetchLimitBytesDefault, + getUserAgent, +} from './got-config.js' -const userAgent = 'Shields.io/2003a' +const userAgent = getUserAgent() async function sendRequest(gotWrapper, url, options) { const gotOptions = Object.assign({}, options) @@ -22,8 +26,7 @@ async function sendRequest(gotWrapper, url, options) { } } -const TEN_MB = 10485760 -function fetchFactory(fetchLimitBytes = TEN_MB) { +function _fetchFactory(fetchLimitBytes = fetchLimitBytesDefault) { const gotWithLimit = got.extend({ handlers: [ (options, next) => { @@ -52,4 +55,6 @@ function fetchFactory(fetchLimitBytes = TEN_MB) { return sendRequest.bind(sendRequest, gotWithLimit) } -export { fetchFactory, userAgent } +const fetch = _fetchFactory() + +export { fetch, _fetchFactory } diff --git a/core/base-service/got.spec.js b/core/base-service/got.spec.js index e592ed675cb00..004ee667a1bca 100644 --- a/core/base-service/got.spec.js +++ b/core/base-service/got.spec.js @@ -1,6 +1,6 @@ import { expect } from 'chai' import nock from 'nock' -import { fetchFactory } from './got.js' +import { _fetchFactory } from './got.js' import { Inaccessible, InvalidResponse } from './errors.js' describe('got wrapper', function () { @@ -9,7 +9,7 @@ describe('got wrapper', function () { .get('/foo/bar') .once() .reply(200, 'x'.repeat(100)) - const sendRequest = fetchFactory(100) + const sendRequest = _fetchFactory(100) const { res } = await sendRequest('https://www.google.com/foo/bar') expect(res.statusCode).to.equal(200) }) @@ -19,7 +19,7 @@ describe('got wrapper', function () { .get('/foo/bar') .once() .reply(200, 'x'.repeat(101)) - const sendRequest = fetchFactory(100) + const sendRequest = _fetchFactory(100) return expect( sendRequest('https://www.google.com/foo/bar') ).to.be.rejectedWith(InvalidResponse, 'Maximum response size exceeded') @@ -27,7 +27,7 @@ describe('got wrapper', function () { it('should throw an Inaccessible error if the request throws a (non-HTTP) error', async function () { nock('https://www.google.com').get('/foo/bar').replyWithError('oh no') - const sendRequest = fetchFactory(1024) + const sendRequest = _fetchFactory(1024) return expect( sendRequest('https://www.google.com/foo/bar') ).to.be.rejectedWith(Inaccessible, 'oh no') @@ -36,7 +36,7 @@ describe('got wrapper', function () { it('should throw an Inaccessible error if the host can not be accessed', async function () { this.timeout(5000) nock.disableNetConnect() - const sendRequest = fetchFactory(1024) + const sendRequest = _fetchFactory(1024) return expect( sendRequest('https://www.google.com/foo/bar') ).to.be.rejectedWith( @@ -49,14 +49,14 @@ describe('got wrapper', function () { nock('https://www.google.com', { reqheaders: { 'user-agent': function (agent) { - return agent.startsWith('Shields.io') + return agent.startsWith('shields (self-hosted)') }, }, }) .get('/foo/bar') .once() .reply(200) - const sendRequest = fetchFactory(1024) + const sendRequest = _fetchFactory(1024) await sendRequest('https://www.google.com/foo/bar') }) diff --git a/core/legacy/regular-update.js b/core/legacy/regular-update.js index 0fad3489c86c1..4b77cb0152158 100644 --- a/core/legacy/regular-update.js +++ b/core/legacy/regular-update.js @@ -3,9 +3,8 @@ */ import { InvalidResponse } from '../base-service/errors.js' -import { fetchFactory } from '../../core/base-service/got.js' +import { fetch } from '../../core/base-service/got.js' import checkErrorResponse from '../../core/base-service/check-error-response.js' -const fetcher = fetchFactory() // Map from URL to { timestamp: last fetch time, data: data }. let regularUpdateCache = Object.create(null) @@ -28,7 +27,7 @@ async function regularUpdate({ json = true, scraper = buffer => buffer, options = {}, - requestFetcher = fetcher, + requestFetcher = fetch, }) { const timestamp = Date.now() const cached = regularUpdateCache[url] diff --git a/core/server/server.js b/core/server/server.js index fea2bca5c76ab..defe835f3ed72 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -6,7 +6,6 @@ import path from 'path' import url, { fileURLToPath } from 'url' import { bootstrap } from 'global-agent' import cloudflareMiddleware from 'cloudflare-middleware' -import bytes from 'bytes' import Camp from '@shields_io/camp' import originalJoi from 'joi' import makeBadge from '../../badge-maker/lib/make-badge.js' @@ -18,7 +17,7 @@ import { makeSend } from '../base-service/legacy-result-sender.js' import { handleRequest } from '../base-service/legacy-request-handler.js' import { clearRegularUpdateCache } from '../legacy/regular-update.js' import { rasterRedirectUrl } from '../badge-urls/make-badge-url.js' -import { nonNegativeInteger } from '../../services/validators.js' +import { fileSize, nonNegativeInteger } from '../../services/validators.js' import log from './log.js' import PrometheusMetrics from './prometheus-metrics.js' import InfluxMetrics from './influx-metrics.js' @@ -143,7 +142,8 @@ const publicConfigSchema = Joi.object({ }).required(), cacheHeaders: { defaultCacheLengthSeconds: nonNegativeInteger }, handleInternalErrors: Joi.boolean().required(), - fetchLimit: Joi.string().regex(/^[0-9]+(b|kb|mb|gb|tb)$/i), + fetchLimit: fileSize, + userAgentBase: Joi.string().required(), requestTimeoutSeconds: nonNegativeInteger, requestTimeoutMaxAgeSeconds: nonNegativeInteger, documentRoot: Joi.string().default( @@ -433,7 +433,6 @@ class Server { { handleInternalErrors: config.public.handleInternalErrors, cacheHeaders: config.public.cacheHeaders, - fetchLimitBytes: bytes(config.public.fetchLimit), rasterUrl: config.public.rasterUrl, private: config.private, public: config.public, diff --git a/services/github/auth/acceptor.js b/services/github/auth/acceptor.js index 3671f75cc89ba..adebd89581bb4 100644 --- a/services/github/auth/acceptor.js +++ b/services/github/auth/acceptor.js @@ -1,9 +1,7 @@ import queryString from 'query-string' -import { fetchFactory } from '../../../core/base-service/got.js' +import { fetch } from '../../../core/base-service/got.js' import log from '../../../core/server/log.js' -const requestFetcher = fetchFactory() - function setRoutes({ server, authHelper, onTokenAccepted }) { const baseUrl = process.env.GATSBY_BASE_URL || 'https://img.shields.io' @@ -48,10 +46,7 @@ function setRoutes({ server, authHelper, onTokenAccepted }) { let resp try { - resp = await requestFetcher( - 'https://github.com/login/oauth/access_token', - options - ) + resp = await fetch('https://github.com/login/oauth/access_token', options) } catch (e) { return end('The connection to GitHub failed.') } diff --git a/services/github/github-api-provider.integration.js b/services/github/github-api-provider.integration.js index 920fd26089d17..9c3435ce5da9a 100644 --- a/services/github/github-api-provider.integration.js +++ b/services/github/github-api-provider.integration.js @@ -1,8 +1,7 @@ import { expect } from 'chai' import config from 'config' -import { fetchFactory } from '../../core/base-service/got.js' +import { fetch } from '../../core/base-service/got.js' import GithubApiProvider from './github-api-provider.js' -const requestFetcher = fetchFactory() describe('Github API provider', function () { const baseUrl = process.env.GITHUB_URL || 'https://api.github.com' @@ -31,11 +30,7 @@ describe('Github API provider', function () { it('should be able to run 10 requests', async function () { this.timeout('20s') for (let i = 0; i < 10; ++i) { - await githubApiProvider.fetch( - requestFetcher, - '/repos/rust-lang/rust', - {} - ) + await githubApiProvider.fetch(fetch, '/repos/rust-lang/rust', {}) } }) }) @@ -54,7 +49,7 @@ describe('Github API provider', function () { const headers = [] async function performOneRequest() { const { res } = await githubApiProvider.fetch( - requestFetcher, + fetch, '/repos/rust-lang/rust', {} ) diff --git a/services/github/github-api-provider.js b/services/github/github-api-provider.js index aed1994e65c18..0cad3e920a84d 100644 --- a/services/github/github-api-provider.js +++ b/services/github/github-api-provider.js @@ -1,10 +1,12 @@ import Joi from 'joi' import log from '../../core/server/log.js' import { TokenPool } from '../../core/token-pooling/token-pool.js' -import { userAgent } from '../../core/base-service/got.js' +import { getUserAgent } from '../../core/base-service/got-config.js' import { nonNegativeInteger } from '../validators.js' import { ImproperlyConfigured } from '../index.js' +const userAgent = getUserAgent() + const headerSchema = Joi.object({ 'x-ratelimit-limit': nonNegativeInteger, 'x-ratelimit-remaining': nonNegativeInteger, diff --git a/services/github/github-auth-service.spec.js b/services/github/github-auth-service.spec.js index 2e1eba4406e43..3569fba848e9e 100644 --- a/services/github/github-auth-service.spec.js +++ b/services/github/github-auth-service.spec.js @@ -54,7 +54,7 @@ describe('GithubAuthV3Service', function () { 'https://github-api.example.com/repos/badges/shields/check-runs', { headers: { - 'User-Agent': 'Shields.io/2003a', + 'User-Agent': 'shields (self-hosted)/dev', Accept: 'application/vnd.github.antiope-preview+json', Authorization: 'token undefined', }, diff --git a/services/librariesio/librariesio-api-provider.js b/services/librariesio/librariesio-api-provider.js index 07e140a767d18..4eaf5f54badd1 100644 --- a/services/librariesio/librariesio-api-provider.js +++ b/services/librariesio/librariesio-api-provider.js @@ -1,7 +1,9 @@ import { ImproperlyConfigured } from '../index.js' import log from '../../core/server/log.js' import { TokenPool } from '../../core/token-pooling/token-pool.js' -import { userAgent } from '../../core/base-service/got.js' +import { getUserAgent } from '../../core/base-service/got-config.js' + +const userAgent = getUserAgent() // Provides an interface to the Libraries.io API. export default class LibrariesIoApiProvider { diff --git a/services/php-version.js b/services/php-version.js index 20401ff5ca041..06c3485d2e627 100644 --- a/services/php-version.js +++ b/services/php-version.js @@ -3,7 +3,7 @@ * using the algorithm followed by Composer (see * https://getcomposer.org/doc/04-schema.md#version). */ -import { fetchFactory } from '../core/base-service/got.js' +import { fetch } from '../core/base-service/got.js' import { regularUpdate } from '../core/legacy/regular-update.js' import { listCompare } from './version.js' import { omitv } from './text-formatters.js' @@ -232,10 +232,7 @@ async function getPhpReleases(githubApiProvider) { .map(tag => tag.ref.match(/^refs\/tags\/php-(\d+\.\d+)\.\d+$/)[1]) ) ), - requestFetcher: githubApiProvider.fetch.bind( - githubApiProvider, - fetchFactory() - ), + requestFetcher: githubApiProvider.fetch.bind(githubApiProvider, fetch), }) } diff --git a/services/suggest.js b/services/suggest.js index b0a610b8173c5..6e2936265b729 100644 --- a/services/suggest.js +++ b/services/suggest.js @@ -5,8 +5,7 @@ // This endpoint is called from frontend/components/suggestion-and-search.js. import { URL } from 'url' -import { fetchFactory } from '../core/base-service/got.js' -const requestFetcher = fetchFactory() +import { fetch } from '../core/base-service/got.js' function twitterPage(url) { if (url.protocol === null) { @@ -77,7 +76,7 @@ async function githubLicense(githubApiProvider, user, repo) { let link = `https://github.com/${repoSlug}` const { buffer } = await githubApiProvider.fetch( - requestFetcher, + fetch, `/repos/${repoSlug}/license` ) try { diff --git a/services/test-helpers.js b/services/test-helpers.js index 1a76c74a77872..2976c339816ce 100644 --- a/services/test-helpers.js +++ b/services/test-helpers.js @@ -1,7 +1,6 @@ -import bytes from 'bytes' import nock from 'nock' import config from 'config' -import { fetchFactory } from '../core/base-service/got.js' +import { fetch } from '../core/base-service/got.js' const runnerConfig = config.util.toObject() function cleanUpNockAfterEach() { @@ -31,8 +30,6 @@ function noToken(serviceClass) { } } -const requestFetcher = fetchFactory(bytes(runnerConfig.public.fetchLimit)) - -const defaultContext = { requestFetcher } +const defaultContext = { requestFetcher: fetch } export { cleanUpNockAfterEach, noToken, defaultContext } diff --git a/services/validators.js b/services/validators.js index 42b89cd1d10e2..4586f08a3716c 100644 --- a/services/validators.js +++ b/services/validators.js @@ -17,3 +17,9 @@ export const optionalDottedVersionNClausesWithOptionalSuffix = // TODO This accepts URLs with query strings and fragments, which for some // purposes should be rejected. export const optionalUrl = Joi.string().uri({ scheme: ['http', 'https'] }) + +// validator for a file size we are going to pass to bytes.parse +// see https://github.com/visionmedia/bytes.js#bytesparsestringnumber-value-numbernull +export const fileSize = Joi.string() + .regex(/^[0-9]+(b|kb|mb|gb|tb)$/i) + .required()