fix: config loading required vars (#1955)

Author: hugomrdias

.env.tpl

@@ -27,4 +27,19 @@ DATABASE_URL=http://localhost:3000
 DATABASE_TOKEN=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJzdXBhYmFzZSIsImlhdCI6MTYwMzk2ODgzNCwiZXhwIjoyNTUwNjUzNjM0LCJyb2xlIjoic2VydmljZV9yb2xlIn0.necIJaiP7X2T2QjGeV-FhpkizcNTX8HjDDBAxpgQTEI
 
 # Postgres Database
-DATABASE_CONNECTION=postgresql://postgres:postgres@localhost:5432/postgres
+DATABASE_CONNECTION=postgresql://postgres:postgres@localhost:5432/postgres
+
+# Cluster 
+CLUSTER_BASIC_AUTH_TOKEN = dGVzdDp0ZXN0
+CLUSTER_SERVICE = 
+CLUSTER_API_URL = http://localhost:9094
+
+# Maintenance Mode
+MAINTENANCE_MODE = rw
+
+# S3
+S3_ENDPOINT = http://localhost:9095
+S3_REGION = test
+S3_ACCESS_KEY_ID = test
+S3_SECRET_ACCESS_KEY = test
+S3_BUCKET_NAME = test

packages/api/README.md

@@ -76,6 +76,8 @@ wrangler secret put S3_ACCESS_KEY_ID --env production # Get from Amazon S3 (not
 wrangler secret put S3_SECRET_ACCESS_KEY --env production # Get from Amazon S3 (not required for dev)
 wrangler secret put S3_BUCKET_NAME --env production # e.g nft.storage-staging-us-east-2 (not required for dev)
 wrangler secret put PRIVATE_KEY --env production # Get from 1password
+wrangler secret put MAINTENANCE_MODE --env production # default value is "rw"
+
 wrangler publish --env production
 ```
 

packages/api/pw-test.config.cjs

@@ -24,16 +24,23 @@ const nodeBuiltinsPlugin = {
   },
 }
 
+const config = {
+  inject: [
+    path.join(__dirname, './scripts/node-globals.js'),
+    path.join(__dirname, './test/scripts/globals.js'),
+  ],
+  define: {
+    NFT_STORAGE_VERSION: JSON.stringify('0.1.0'),
+    NFT_STORAGE_COMMITHASH: JSON.stringify('322332'),
+    NFT_STORAGE_BRANCH: JSON.stringify('main'),
+  },
+  plugins: [nodeBuiltinsPlugin],
+}
+
 /** @type {import('playwright-test').RunnerOptions} */
 module.exports = {
-  buildConfig: {
-    inject: [path.join(__dirname, './scripts/node-globals.js')],
-    plugins: [nodeBuiltinsPlugin],
-  },
-  buildSWConfig: {
-    inject: [path.join(__dirname, './scripts/node-globals.js')],
-    plugins: [nodeBuiltinsPlugin],
-  },
+  buildConfig: config,
+  buildSWConfig: config,
   beforeTests: async () => {
     const mock = await startMockServer('AWS S3', 9095, 'test/mocks/aws-s3')
 

packages/api/src/bindings.d.ts

@@ -17,13 +17,13 @@ export interface ServiceConfiguration {
   ENV: RuntimeEnvironmentName
 
   /** Semantic version for current build */
-  NFT_STORAGE_VERSION: string
+  VERSION: string
 
   /** Git branch name of current build */
-  NFT_STORAGE_BRANCH: string
+  BRANCH: string
 
   /** Git commit hash of current build */
-  NFT_STORAGE_COMMITHASH: string
+  COMMITHASH: string
 
   /** Current maintenance mode */
   MAINTENANCE_MODE: Mode

packages/api/src/config.js

@@ -8,37 +8,6 @@ import {
  * @typedef {import('./bindings').RuntimeEnvironmentName} RuntimeEnvironmentName
  */
 
-/**
- * Default configuration values to be used in test and dev if no explicit definition is found.
- *
- * @type Record<string, string>
- */
-export const DEFAULT_CONFIG_VALUES = {
-  SALT: 'secret',
-  DEBUG: 'true',
-  DATABASE_URL: 'http://localhost:3000',
-  DATABASE_TOKEN:
-    'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJzdXBhYmFzZSIsImlhdCI6MTYwMzk2ODgzNCwiZXhwIjoyNTUwNjUzNjM0LCJyb2xlIjoic2VydmljZV9yb2xlIn0.necIJaiP7X2T2QjGeV-FhpkizcNTX8HjDDBAxpgQTEI',
-  CLUSTER_API_URL: 'http://localhost:9094',
-  CLUSTER_BASIC_AUTH_TOKEN: 'dGVzdDp0ZXN0', // test:test
-  MAGIC_SECRET_KEY: 'test',
-  ENV: 'test',
-  SENTRY_DSN: 'https://test@test.ingest.sentry.io/0000000',
-  NFT_STORAGE_BRANCH: 'test',
-  NFT_STORAGE_VERSION: 'test',
-  NFT_STORAGE_COMMITHASH: 'test',
-  MAINTENANCE_MODE: 'rw',
-  METAPLEX_AUTH_TOKEN: 'metaplex-test-token',
-  MAILCHIMP_API_KEY: '',
-  LOGTAIL_TOKEN: '',
-  S3_ENDPOINT: 'http://localhost:9095',
-  S3_REGION: 'test',
-  S3_ACCESS_KEY_ID: 'test',
-  S3_SECRET_ACCESS_KEY: 'test',
-  S3_BUCKET_NAME: 'test',
-  PRIVATE_KEY: 'xmbtWjE9eYuAxae9G65lQSkw36HV6H+0LSFq2aKqVwY=',
-}
-
 /**
  * If the CLUSTER_SERVICE variable is set, the service URL will be resolved from here.
  *
@@ -49,12 +18,6 @@ const CLUSTER_SERVICE_URLS = {
   IpfsCluster3: 'https://nft3.storage.ipfscluster.io/api/',
 }
 
-/**
- * @param {RuntimeEnvironmentName} env
- * @returns {boolean} true if the named runtime environment should fallback to values from DEFAULT_CONFIG_VALUES if no config var is present.
- */
-const allowDefaultConfigValues = (env) => env === 'test' || env === 'dev'
-
 /** @type ServiceConfiguration|undefined */
 let _globalConfig
 
@@ -90,14 +53,6 @@ export const overrideServiceConfigForTesting = (config) => {
  *
  * Exported for testing. See {@link getServiceConfig} for main public accessor.
  *
- * Config values are resolved by looking for global variables with the names matching the keys of {@link DEFAULT_CONFIG_VALUES}.
- *
- * If no global value is found for a variable, an error will be thrown if the runtimeEnvironment (ENV variable)
- * is set to a "production like" environment.
- *
- * If {@link allowDefaultConfigValues} returns true for the current environment, the value from {@link DEFAULT_CONFIG_VALUES} will be
- * used if a variable is missing.
- *
  * @returns {ServiceConfiguration}
  */
 export function loadServiceConfig() {
@@ -114,13 +69,14 @@ export function loadServiceConfig() {
  * @returns {ServiceConfiguration}
  */
 export function serviceConfigFromVariables(vars) {
-  let clusterUrl = vars.CLUSTER_API_URL
-  if (vars.CLUSTER_SERVICE) {
-    const serviceUrl = CLUSTER_SERVICE_URLS[vars.CLUSTER_SERVICE]
-    if (!serviceUrl) {
+  let clusterUrl
+  if (!vars.CLUSTER_SERVICE) {
+    clusterUrl = vars.CLUSTER_API_URL
+  } else {
+    clusterUrl = CLUSTER_SERVICE_URLS[vars.CLUSTER_SERVICE]
+    if (!clusterUrl) {
       throw new Error(`unknown cluster service: ${vars.CLUSTER_SERVICE}`)
     }
-    clusterUrl = serviceUrl
   }
 
   return {
@@ -135,9 +91,6 @@ export function serviceConfigFromVariables(vars) {
     CLUSTER_BASIC_AUTH_TOKEN: vars.CLUSTER_BASIC_AUTH_TOKEN,
     MAGIC_SECRET_KEY: vars.MAGIC_SECRET_KEY,
     SENTRY_DSN: vars.SENTRY_DSN,
-    NFT_STORAGE_BRANCH: vars.NFT_STORAGE_BRANCH,
-    NFT_STORAGE_VERSION: vars.NFT_STORAGE_VERSION,
-    NFT_STORAGE_COMMITHASH: vars.NFT_STORAGE_COMMITHASH,
     METAPLEX_AUTH_TOKEN: vars.METAPLEX_AUTH_TOKEN,
     MAILCHIMP_API_KEY: vars.MAILCHIMP_API_KEY,
     LOGTAIL_TOKEN: vars.LOGTAIL_TOKEN,
@@ -147,6 +100,13 @@ export function serviceConfigFromVariables(vars) {
     S3_SECRET_ACCESS_KEY: vars.S3_SECRET_ACCESS_KEY,
     S3_BUCKET_NAME: vars.S3_BUCKET_NAME,
     PRIVATE_KEY: vars.PRIVATE_KEY,
+    // These are injected in esbuild
+    // @ts-ignore
+    BRANCH: NFT_STORAGE_BRANCH,
+    // @ts-ignore
+    VERSION: NFT_STORAGE_VERSION,
+    // @ts-ignore
+    COMMITHASH: NFT_STORAGE_COMMITHASH,
   }
 }
 
@@ -166,29 +126,45 @@ export function loadConfigVariables() {
   /** @type Record<string, unknown> */
   const globals = globalThis
 
-  const notFound = []
-  for (const name of Object.keys(DEFAULT_CONFIG_VALUES)) {
+  const required = [
+    'ENV',
+    'DEBUG',
+    'SALT',
+    'DATABASE_URL',
+    'DATABASE_TOKEN',
+    'MAGIC_SECRET_KEY',
+    'MAILCHIMP_API_KEY',
+    'METAPLEX_AUTH_TOKEN',
+    'LOGTAIL_TOKEN',
+    'PRIVATE_KEY',
+    'SENTRY_DSN',
+    'CLUSTER_BASIC_AUTH_TOKEN',
+    'MAINTENANCE_MODE',
+    'S3_REGION',
+    'S3_ACCESS_KEY_ID',
+    'S3_SECRET_ACCESS_KEY',
+    'S3_BUCKET_NAME',
+  ]
+
+  for (const name of required) {
     const val = globals[name]
     if (typeof val === 'string') {
       vars[name] = val
     } else {
-      notFound.push(name)
-    }
-  }
-
-  if (notFound.length !== 0) {
-    const env = parseRuntimeEnv(vars.ENV)
-    if (!allowDefaultConfigValues(env)) {
       throw new Error(
-        'Missing required config variables: ' + notFound.join(', ')
+        `Missing required config variables: ${name}. Check your .env, testing globals or cloudflare vars.`
       )
     }
-    console.warn(
-      'Using default values for config variables: ',
-      notFound.join(', ')
-    )
-    for (const name of notFound) {
-      vars[name] = DEFAULT_CONFIG_VALUES[name]
+  }
+
+  const optional = ['CLUSTER_SERVICE', 'CLUSTER_API_URL', 'S3_ENDPOINT']
+
+  for (const name of optional) {
+    const val = globals[name]
+    if (typeof val === 'string') {
+      vars[name] = val
+    } else {
+      console.warn(`Missing optional config variables: ${name}`)
     }
   }
 

packages/api/src/index.js

@@ -58,16 +58,11 @@ r.add(
   'get',
   '/version',
   (event) => {
-    const {
-      NFT_STORAGE_VERSION,
-      NFT_STORAGE_COMMITHASH,
-      NFT_STORAGE_BRANCH,
-      MAINTENANCE_MODE,
-    } = getServiceConfig()
+    const { VERSION, COMMITHASH, BRANCH, MAINTENANCE_MODE } = getServiceConfig()
     return new JSONResponse({
-      version: NFT_STORAGE_VERSION,
-      commit: NFT_STORAGE_COMMITHASH,
-      branch: NFT_STORAGE_BRANCH,
+      version: VERSION,
+      commit: COMMITHASH,
+      branch: BRANCH,
       mode: MAINTENANCE_MODE,
     })
   },

packages/api/src/utils/context.js

@@ -32,7 +32,7 @@ const sentryOptions = {
   rewriteFrames: {
     root: '/',
   },
-  release: config.NFT_STORAGE_VERSION,
+  release: config.VERSION,
   pkg,
 }
 

packages/api/src/utils/logs.js

@@ -1,8 +1,7 @@
 import { nanoid } from 'nanoid/non-secure'
 import { getServiceConfig } from '../config'
 
-const { NFT_STORAGE_VERSION, NFT_STORAGE_COMMITHASH, NFT_STORAGE_BRANCH } =
-  getServiceConfig()
+const { VERSION, COMMITHASH, BRANCH } = getServiceConfig()
 const logtailApiURL = 'https://in.logtail.com/'
 
 const buildMetadataFromHeaders = (/** @type {Headers} */ headers) => {
@@ -56,9 +55,9 @@ export class Logging {
         cf: rCf,
       },
       cloudflare_worker: {
-        version: NFT_STORAGE_VERSION,
-        commit: NFT_STORAGE_COMMITHASH,
-        branch: NFT_STORAGE_BRANCH,
+        version: VERSION,
+        commit: COMMITHASH,
+        branch: BRANCH,
         worker_id: nanoid(10),
         worker_started: this.startTs,
       },

packages/api/test/config.spec.js

@@ -1,23 +1,13 @@
-import * as assert from 'assert'
+import assert from 'assert'
 import {
   serviceConfigFromVariables,
-  DEFAULT_CONFIG_VALUES,
   loadConfigVariables,
   loadServiceConfig,
 } from '../src/config.js'
 
 /** @type Record<string, unknown> */
 const globals = globalThis
 
-/**
- * Helper to remove all the config variables we care about from the global context.
- */
-const scrubGlobals = () => {
-  for (const name of Object.keys(DEFAULT_CONFIG_VALUES)) {
-    delete globals[name]
-  }
-}
-
 /**
  * @param {Record<string, string>} vars
  */
@@ -28,17 +18,6 @@ const defineGlobals = (vars) => {
 }
 
 describe('loadServiceConfig', () => {
-  beforeEach(scrubGlobals)
-  afterEach(scrubGlobals)
-
-  it('fails if config vars are missing when ENV == "staging" or "production"', () => {
-    const strictEnvs = ['staging', 'production']
-    for (const env of strictEnvs) {
-      defineGlobals({ ENV: env })
-      assert.throws(() => loadServiceConfig())
-    }
-  })
-
   it('uses default config values for missing vars when ENV == "test" or "dev"', () => {
     const lenientEnvs = ['test', 'dev']
     for (const env of lenientEnvs) {
@@ -51,9 +30,6 @@ describe('loadServiceConfig', () => {
 })
 
 describe('loadConfigVariables', () => {
-  beforeEach(scrubGlobals)
-  afterEach(scrubGlobals)
-
   it('looks up values on the globalThis object', () => {
     globals['SALT'] = 'extra-salty'
     const vars = loadConfigVariables()
@@ -125,9 +101,6 @@ describe('serviceConfigFromVariables', () => {
 
   describe('uses unaltered values for string config variables', () => {
     const stringValuedVars = [
-      'NFT_STORAGE_VERSION',
-      'NFT_STORAGE_BRANCH',
-      'NFT_STORAGE_COMMITHASH',
       'SALT',
       'METAPLEX_AUTH_TOKEN',
       'PRIVATE_KEY',

packages/api/test/scripts/globals.js

@@ -0,0 +1,26 @@
+globalThis.ENV = 'test'
+globalThis.DEBUG = 'true'
+globalThis.SALT = 'secret'
+globalThis.DATABASE_URL = 'http://localhost:3000'
+globalThis.DATABASE_TOKEN =
+  'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJzdXBhYmFzZSIsImlhdCI6MTYwMzk2ODgzNCwiZXhwIjoyNTUwNjUzNjM0LCJyb2xlIjoic2VydmljZV9yb2xlIn0.necIJaiP7X2T2QjGeV-FhpkizcNTX8HjDDBAxpgQTEI'
+
+globalThis.MAGIC_SECRET_KEY = 'test'
+globalThis.MAILCHIMP_API_KEY = ''
+globalThis.METAPLEX_AUTH_TOKEN = 'metaplex-test-token'
+globalThis.LOGTAIL_TOKEN = ''
+globalThis.PRIVATE_KEY = 'xmbtWjE9eYuAxae9G65lQSkw36HV6H+0LSFq2aKqVwY='
+globalThis.SENTRY_DSN = 'https://test@test.ingest.sentry.io/0000000'
+
+globalThis.CLUSTER_API_URL = 'http://localhost:9094'
+// will be used with we can active auth in cluster base64 of test:test
+globalThis.CLUSTER_BASIC_AUTH_TOKEN = 'dGVzdDp0ZXN0'
+globalThis.CLUSTER_SERVICE = ''
+
+globalThis.MAINTENANCE_MODE = 'rw'
+
+globalThis.S3_ENDPOINT = 'http://localhost:9095'
+globalThis.S3_REGION = 'test'
+globalThis.S3_ACCESS_KEY_ID = 'test'
+globalThis.S3_SECRET_ACCESS_KEY = 'test'
+globalThis.S3_BUCKET_NAME = 'test'