Skip to content

Commit

Permalink
FE-6181 validate profile when no config is found (#481)
Browse files Browse the repository at this point in the history
  • Loading branch information
ptpaterson authored Dec 5, 2024
1 parent a7227a3 commit 593d754
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 17 deletions.
9 changes: 3 additions & 6 deletions src/cli.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ export let container;
/** @type {import('yargs').Argv} */
export let builtYargs;

export let argvInput;

const BUG_REPORT_MESSAGE = `If you believe this is a bug, please report this issue on GitHub: https://github.com/fauna/fauna-shell/issues`;

/**
Expand All @@ -37,7 +35,7 @@ const BUG_REPORT_MESSAGE = `If you believe this is a bug, please report this iss
*/
export async function run(_argvInput, _container) {
container = _container;
argvInput = _argvInput;
const argvInput = _argvInput;
const logger = container.resolve("logger");
const parseYargs = container.resolve("parseYargs");
if (process.env.NODE_ENV === "production") {
Expand Down Expand Up @@ -140,7 +138,7 @@ function buildYargs(argvInput) {
return yargsInstance
.scriptName("fauna")
.env("FAUNA")
.config("config", configParser)
.config("config", configParser.bind(null, argvInput))
.middleware([checkForUpdates, logArgv], true)
.middleware([applyLocalArg, fixPaths, buildCredentials], false)
.command(queryCommand)
Expand Down Expand Up @@ -171,8 +169,7 @@ function buildYargs(argvInput) {
alias: "p",
type: "string",
description:
"Profile from the CLI config file to use. Each profile specifies a set of CLI settings.",
default: "default",
"Profile from the CLI config file to use. Each profile specifies a set of CLI settings. Defaults to the 'default' profile when a config file is provided.",
group: "Config:",
},
json: {
Expand Down
69 changes: 59 additions & 10 deletions src/lib/config/config.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import yaml from "yaml";
import yargsParser from "yargs-parser";

import { argvInput, container } from "../../cli.mjs";
import { container } from "../../cli.mjs";
import { ValidationError } from "../command-helpers.mjs";

export const validDefaultConfigNames = [
Expand All @@ -13,7 +13,13 @@ export const validDefaultConfigNames = [
".fauna.config.json",
];

/** @type {yaml.Document.Parsed<Record<string, any>>} */
/**
* Parses a config file at the given path.
*
* @param {string} path
* @throws {ValidationError} If the config file does not exist.
* @return {yaml.Document.Parsed<Record<string, any>>}
*/
export function getConfig(path) {
const fs = container.resolve("fs");
let fileBody;
Expand All @@ -29,6 +35,13 @@ export function getConfig(path) {
return yaml.parseDocument(fileBody);
}

/**
* Checks the specified directory for default configuration files.
*
* @param {string} path - The path to the directory to search for config files.
* @throws {ValidationError} If multiple config files with valid default names are found.
* @returns {string|undefined} - The name of the default config file if found, otherwise undefined.
*/
function checkForDefaultConfig(path) {
const logger = container.resolve("logger");
const fs = container.resolve("fs");
Expand All @@ -54,10 +67,19 @@ function checkForDefaultConfig(path) {
}
}

/**
* Validate the config profile
* Checks that the specified profile name exists in the config file.
*
* @param {string} profileName - The name of the profile to validate.
* @param {Record<string, any>} profileBody - The parsed config profile JSON.
* @param {string} configPath - The path to the config file.
* @throws {ValidationError} If the profile name is not found in the config file.
*/
function validateConfig(profileName, profileBody, configPath) {
if (profileName === "default" && !profileBody) {
throw new ValidationError(
`No "default" profile found in config file at ${configPath}. Either specify a profile with "--profile NAME" or add a "default" profile.`,
`No "default" profile found in the config file at "${configPath}". Either specify a profile with "--profile NAME" or add a "default" profile.`,
);
}

Expand All @@ -68,18 +90,45 @@ function validateConfig(profileName, profileBody, configPath) {
}
}

export function configParser(path) {
/**
* A parser to convert config files into appropriate command line arguments
* given existing arguments.
*
* @param {string|string[]} argvInput - The raw command line arguments.
* @param {string} path
* @returns {object} - The yargs parser
*/
export function configParser(argvInput, path) {
let parsedPath = path;
let parsedProfile;
const logger = container.resolve("logger");

if (path === process.cwd()) {
path = checkForDefaultConfig(process.cwd());
parsedPath = checkForDefaultConfig(process.cwd());
}

if (!path) return {};
if (!parsedPath) {
// if there no config file, we need to assert that no either no profile is
// specified or that the profile is "default"
const preConfigArgv = yargsParser(argvInput, {
alias: {
profile: ["p"],
},
string: ["profile"],
});
if (preConfigArgv.profile && preConfigArgv.profile !== "default") {
throw new ValidationError(
`Profile "${preConfigArgv.profile}" cannot be specified because there was no config file found at "${path}". ` +
`Remove the profile, or provide a config file.`,
);
}
return {};
}

logger.debug(`Reading config from ${parsedPath}.`, "config");
const config = getConfig(parsedPath);

logger.debug(`Reading config from ${path}.`, "config");
const config = getConfig(path);
// The "default" profile will be injected here
const argv = yargsParser(argvInput, {
alias: {
profile: ["p"],
Expand All @@ -89,10 +138,10 @@ export function configParser(path) {
},
string: ["profile"],
});

logger.debug(`Using profile ${argv.profile}...`, "config");
parsedProfile = config.toJSON()[argv.profile];
validateConfig(argv.profile, parsedProfile, path);

validateConfig(argv.profile, parsedProfile, parsedPath);

logger.debug(
`Applying config: ${JSON.stringify(parsedProfile, null, 4)}`,
Expand Down
13 changes: 12 additions & 1 deletion test/config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ describe("configuration file", function () {
configToReturn: noDefaultConfig,
});
} catch (e) {}
const errorText = `No "default" profile found in config file at ${path.join(__dirname, "../dev.yaml")}. Either specify a profile with "--profile NAME" or add a "default" profile.`;
const errorText = `No "default" profile found in the config file at "${path.join(__dirname, "../dev.yaml")}". Either specify a profile with "--profile NAME" or add a "default" profile.`;
const message = `${await builtYargs.getHelp()}\n\n${errorText}\n`;
expect(stdout.getWritten()).to.equal("");
expect(stripAnsi(stderr.getWritten())).to.equal(message);
Expand All @@ -385,6 +385,17 @@ describe("configuration file", function () {
expect(stripAnsi(stderr.getWritten())).to.equal(message);
});

it("exits with an error if a profile is specified and there is no config", async function () {
try {
await run(`eval "Database.all()" --profile nonexistent`, container);
} catch (e) {}

const errorText = `Profile "nonexistent" cannot be specified because there was no config file found at "${path.join(__dirname, "..")}". Remove the profile, or provide a config file.`;
const message = `${await builtYargs.getHelp()}\n\n${errorText}\n`;
expect(stdout.getWritten()).to.equal("");
expect(stripAnsi(stderr.getWritten())).to.equal(message);
});

it.skip("preserves comments in the config file", async function () {});
});

Expand Down

0 comments on commit 593d754

Please sign in to comment.