Skip to content

Commit

Permalink
config: Remove hardcoded default for GROUPS_DATA_FILE
Browse files Browse the repository at this point in the history
Require it is set via env or config file.

The hardcoded default to production vs. testing paths was sort of the
odd one out at this point, and as @victorlin said in his good suggestion
to remove the default¹,

> This would be consistent with treating those config files as a central
> source of all non-secret environment-specific configuration.

Well put!

¹ <#733 (comment)>
  • Loading branch information
tsibley committed Oct 25, 2023
1 parent 911dbf5 commit 672180b
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 24 deletions.
6 changes: 3 additions & 3 deletions docs/infrastructure.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ Deploys to the production app are performed by manually [promoting](https://devc
- `CONFIG_FILE` is the path to a JSON file defining the defaults for the required variables below.
If not provided, the checked-in files `env/production/config.json` and `env/testing/config.json` are used (depending on the value of `NODE_ENV`).

- `GROUPS_DATA_FILE` is the path to a JSON file defining the known Groups.
If not provided, the checked-in files `env/production/groups.json` and `env/testing/groups.json` are used (depending on the value of `NODE_ENV`).

Several variables are required but obtain defaults from a config file (e.g. `env/production/config.json`):

- `COGNITO_USER_POOL_ID` must be set to the id of the Cognito user pool to use for authentication.
Expand All @@ -90,6 +87,9 @@ Several variables are required but obtain defaults from a config file (e.g. `env
- `OIDC_GROUPS_CLAIM` must be set to the field in the id token claims which contains the list of group names for a user.
For Cognito, this is `cognito:groups`.

- `GROUPS_DATA_FILE` is the path to a JSON file defining the known Groups.
Our config files point to the checked-in files `env/production/groups.json` and `env/testing/groups.json`.

Variables in the environment override defaults from the config file.

### Redis add-on
Expand Down
4 changes: 4 additions & 0 deletions env/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ output "OIDC_USERNAME_CLAIM" {
output "OIDC_GROUPS_CLAIM" {
value = "cognito:groups"
}

output "GROUPS_DATA_FILE" {
value = "groups.json"
}
3 changes: 2 additions & 1 deletion env/production/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
"COGNITO_USER_POOL_ID": "us-east-1_Cg5rcTged",
"OIDC_USERNAME_CLAIM": "cognito:username",
"OIDC_GROUPS_CLAIM": "cognito:groups",
"SESSION_COOKIE_DOMAIN": "nextstrain.org"
"SESSION_COOKIE_DOMAIN": "nextstrain.org",
"GROUPS_DATA_FILE": "groups.json"
}
3 changes: 2 additions & 1 deletion env/testing/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
"OAUTH2_LOGOUT_URL": "https://nextstrain-testing.auth.us-east-1.amazoncognito.com/logout",
"COGNITO_USER_POOL_ID": "us-east-1_zqpCrjM7I",
"OIDC_USERNAME_CLAIM": "cognito:username",
"OIDC_GROUPS_CLAIM": "cognito:groups"
"OIDC_GROUPS_CLAIM": "cognito:groups",
"GROUPS_DATA_FILE": "groups.json"
}
49 changes: 30 additions & 19 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,23 @@ const configFile = CONFIG_FILE
/**
* Obtain a configuration variable from the environment or the config file.
*
* Values obtained from the environment will be deserialized from JSON if
* possible.
* By default, values obtained from the environment will be deserialized from
* JSON if possible.
*
* Optional conversion functions may be called on undefined values when there
* is no value present in the environment or config file.
*
* @param {string} name - Variable name, e.g. "COGNITO_USER_POOL_ID"
* @param {any} default - Final fallback value
* @param {object} options
* @param {function} options.fromEnv - conversion function to apply to values obtained from the environment; defaults to {@link maybeJSON}
* @param {function} options.fromConfig - conversion function to apply to values obtained from the config file; defaults to the identity function
* @throws {Error} if no value is found and default is undefined
*/
const fromEnvOrConfig = (name, default_) => {
const fromEnvOrConfig = (name, default_, {fromEnv = maybeJSON, fromConfig = x => x} = {}) => {
const value =
maybeJSON(process.env[name])
|| configFile?.[name];
fromEnv(process.env[name])
|| fromConfig(configFile?.[name]);

if (!value && default_ === undefined) {
throw new Error(`${name} is required (because default is undefined) but it was not found in the environment or config file (${CONFIG_FILE})`);
Expand All @@ -103,6 +109,23 @@ function maybeJSON(x) {
return x;
}

/**
* Resolve a value as a filesystem path relative to {@link CONFIG_FILE}.
*
* Values which are null or undefined are passed thru.
*
* @param {?string} value - relative or absolute path
* @returns {?string} absolute path
* @throws {Error} if {@link CONFIG_FILE} is not set
*/
function configPath(value) {
if (!CONFIG_FILE)
throw new Error(`configPath() called without CONFIG_FILE set`);
if (value === null || value === undefined)
return value;
return path.resolve(path.dirname(CONFIG_FILE), value);
}


/**
* Id of our Cognito user pool.
Expand Down Expand Up @@ -290,23 +313,11 @@ export const SESSION_MAX_AGE = fromEnvOrConfig("SESSION_MAX_AGE", 30 * 24 * 60 *
* If sourced from the config file, relative paths are resolved relative to the
* directory containing the config file.
*
* Defaults to env/production/groups.json if {@link PRODUCTION} or
* env/testing/groups.json otherwise.
* Required.
*
* @type {string}
*/
export const GROUPS_DATA_FILE =
process.env.GROUPS_DATA_FILE
?? configPath(configFile?.GROUPS_DATA_FILE)
?? path.join(__basedir, "env", (PRODUCTION ? "production" : "testing"), "groups.json");

function configPath(value) {
if (!CONFIG_FILE)
throw new Error(`configPath() called without CONFIG_FILE set`);
if (value === null || value === undefined)
return value;
return path.resolve(path.dirname(CONFIG_FILE), value);
}
export const GROUPS_DATA_FILE = fromEnvOrConfig("GROUPS_DATA_FILE", undefined, {fromConfig: configPath});


/**
Expand Down

0 comments on commit 672180b

Please sign in to comment.