Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: set default config as node.config.json #57171

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ doc: $(NODE_EXE) doc-only ## Build Node.js, and then build the documentation wit

out/doc:
mkdir -p $@
cp doc/node_config_json_schema.json $@
cp doc/node-config-schema.json $@

# If it's a source tarball, doc/api already contains the generated docs.
# Just copy everything under doc/api over.
Expand Down
31 changes: 22 additions & 9 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -911,29 +911,30 @@ added: v23.6.0

Enable experimental import support for `.node` addons.

### `--experimental-config-file`
### `--experimental-config-file=config`

<!-- YAML
added: REPLACEME
-->

> Stability: 1.0 - Early development

Use this flag to specify a configuration file that will be loaded and parsed
before the application starts.
If present, Node.js will look for a
configuration file at the specified path.
Node.js will read the configuration file and apply the settings.
The configuration file should be a JSON file
with the following structure:

> \[!NOTE]
> Replace `vX.Y.Z` in the `$schema` with the version of Node.js you are using.

```json
{
"$schema": "https://nodejs.org/dist/REPLACEME/docs/node_config_json_schema.json",
"$schema": "https://nodejs.org/dist/vX.Y.Z/docs/node-config-schema.json",
"nodeOptions": {
"experimental-transform-types": true,
"import": [
"amaro/transform"
"amaro/strip"
],
"disable-warning": "ExperimentalWarning",
"watch-path": "src",
"watch-preserve-output": true
}
Expand All @@ -944,7 +945,7 @@ In the `nodeOptions` field, only flags that are allowed in [`NODE_OPTIONS`][] ar
No-op flags are not supported.
Not all V8 flags are currently supported.

It is possible to use the [official JSON schema](../node_config_json_schema.json)
It is possible to use the [official JSON schema](../node-config-schema.json)
to validate the configuration file, which may vary depending on the Node.js version.
Each key in the configuration file corresponds to a flag that can be passed
as a command-line argument. The value of the key is the value that would be
Expand All @@ -954,7 +955,7 @@ For example, the configuration file above is equivalent to
the following command-line arguments:

```bash
node --experimental-transform-types --import amaro/transform --disable-warning=ExperimentalWarning --watch-path=src --watch-preserve-output
node --import amaro/strip --watch-path=src --watch-preserve-output
```

The priority in configuration is as follows:
Expand All @@ -976,6 +977,18 @@ unknown keys or keys that cannot used in `NODE_OPTIONS`.
Node.js will not sanitize or perform validation on the user-provided configuration,
so **NEVER** use untrusted configuration files.

### `--experimental-default-config-file`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the day comes that we load node.config.json without needing any flags, I assume we’ll need a way to disable that behavior and not just as a temporary “while still experimental” measure, because I assume that some people will consider it a security hazard that we might load configuration implicitly. (Though we already do so via NODE_OPTIONS, I guess, so if you think I’m wrong about this assumption I’m very open to be persuaded that I’m wrong.) So when this feature is stable and if we do need a permanent way to prevent the automatic loading, I think the options are either --no-default-config-file or --disable-default-config-file. We have a bunch of --no-* flags and also a bunch of --disable-* flags, and I’m not sure if there’s any reasoning behind one versus the other.

Personally I think disable makes more sense for this particular feature from an English language perspective of “disable the loading of the default config file”, as opposed to “no default config file” being a bit ambiguous (am I telling it not to load a default config file, or that Node should error if there is no default config file, or something else?). But I defer to the crowd on this one.


<!-- YAML
added: REPLACEME
-->

> Stability: 1.0 - Early development

If the `--experimental-default-config-file` flag is present, Node.js will look for a
`node.config.json` file in the current working directory and load it as a
as configuration file.

### `--experimental-eventsource`

<!-- YAML
Expand Down
File renamed without changes.
5 changes: 4 additions & 1 deletion doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ Interpret the entry point as a URL.
Enable experimental addon module support.
.
.It Fl -experimental-config-file
Enable support for experimental config file
Specifies the configuration file to load.
.
.It Fl -experimental-default-config-file
Enable support for automatically loading node.config.json.
.
.It Fl -experimental-import-meta-resolve
Enable experimental ES modules support for import.meta.resolve().
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ function setupSQLite() {
}

function initializeConfigFileSupport() {
if (getOptionValue('--experimental-config-file')) {
if (getOptionValue('--experimental-default-config-file') ||
getOptionValue('--experimental-config-file')) {
emitExperimentalWarning('--experimental-config-file');
}
}
Expand Down
20 changes: 15 additions & 5 deletions src/node_config_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,32 @@ namespace node {

std::optional<std::string_view> ConfigReader::GetDataFromArgs(
const std::vector<std::string>& args) {
constexpr std::string_view flag = "--experimental-config-file";
constexpr std::string_view flag_path = "--experimental-config-file";
constexpr std::string_view default_file =
"--experimental-default-config-file";

bool has_default_config_file = false;

for (auto it = args.begin(); it != args.end(); ++it) {
if (*it == flag) {
if (*it == flag_path) {
// Case: "--experimental-config-file foo"
if (auto next = std::next(it); next != args.end()) {
return *next;
}
} else if (it->starts_with(flag)) {
} else if (it->starts_with(flag_path)) {
// Case: "--experimental-config-file=foo"
if (it->size() > flag.size() && (*it)[flag.size()] == '=') {
return it->substr(flag.size() + 1);
if (it->size() > flag_path.size() && (*it)[flag_path.size()] == '=') {
return it->substr(flag_path.size() + 1);
}
} else if (*it == default_file || it->starts_with(default_file)) {
has_default_config_file = true;
}
}

if (has_default_config_file) {
return "node.config.json";
}

return std::nullopt;
}

Expand Down
5 changes: 4 additions & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
Implies("--env-file-if-exists", "[has_env_file_string]");
AddOption("--experimental-config-file",
"set config file from supplied file",
&EnvironmentOptions::experimental_config_file);
&EnvironmentOptions::experimental_config_file_path);
AddOption("--experimental-default-config-file",
"set config file from default config file",
&EnvironmentOptions::experimental_default_config_file);
AddOption("--test",
"launch test runner on startup",
&EnvironmentOptions::test_runner);
Expand Down
3 changes: 2 additions & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ class EnvironmentOptions : public Options {

bool report_exclude_env = false;
bool report_exclude_network = false;
std::string experimental_config_file;
std::string experimental_config_file_path;
bool experimental_default_config_file = false;

inline DebugOptions* get_debug_options() { return &debug_options_; }
inline const DebugOptions& debug_options() const { return debug_options_; }
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/rc/default/node.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"nodeOptions": {
"max-http-header-size": 10
}
}
5 changes: 5 additions & 0 deletions test/fixtures/rc/default/override.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"nodeOptions": {
"max-http-header-size": 20
}
}
5 changes: 5 additions & 0 deletions test/fixtures/rc/non-readable/node.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"nodeOptions": {
"max-http-header-size": 10
}
}
44 changes: 44 additions & 0 deletions test/parallel/test-config-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { spawnPromisified } = require('../common');
const fixtures = require('../common/fixtures');
const { match, strictEqual } = require('node:assert');
const { test } = require('node:test');
const { chmodSync } = require('node:fs');

test('should handle non existing json', async () => {
const result = await spawnPromisified(process.execPath, [
Expand Down Expand Up @@ -304,3 +305,46 @@ test('broken value in node_options', async () => {
strictEqual(result.stdout, '');
strictEqual(result.code, 9);
});

test('should use node.config.json as default', async () => {
const result = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-default-config-file',
'-p', 'http.maxHeaderSize',
], {
cwd: fixtures.path('rc/default'),
});
strictEqual(result.stderr, '');
strictEqual(result.stdout, '10\n');
strictEqual(result.code, 0);
});

test('should override node.config.json when specificied', async () => {
const result = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-default-config-file',
'--experimental-config-file',
fixtures.path('rc/default/override.json'),
'-p', 'http.maxHeaderSize',
], {
cwd: fixtures.path('rc/default'),
});
strictEqual(result.stderr, '');
strictEqual(result.stdout, '20\n');
strictEqual(result.code, 0);
});

test('should throw an error when the file is non readable', async () => {
chmodSync(fixtures.path('rc/non-readable/node.config.json'), '000');
const result = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-default-config-file',
'-p', 'http.maxHeaderSize',
], {
cwd: fixtures.path('rc/non-readable'),
});
match(result.stderr, /Cannot read configuration from node\.config\.json: permission denied/);
strictEqual(result.stdout, '');
strictEqual(result.code, 9);
chmodSync(fixtures.path('rc/non-readable/node.config.json'), '777');
});
4 changes: 2 additions & 2 deletions test/parallel/test-config-json-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ if (!common.hasIntl) {
const {
generateConfigJsonSchema,
} = require('internal/options');
const schemaInDoc = require('../../doc/node_config_json_schema.json');
const schemaInDoc = require('../../doc/node-config-schema.json');
const assert = require('assert');

const schema = generateConfigJsonSchema();
Expand All @@ -35,6 +35,6 @@ const schema = generateConfigJsonSchema();
// current JSON schema.
// To regenerate the JSON schema, run:
// out/Release/node --expose-internals tools/doc/generate-json-schema.mjs
// And then run make doc to update the out/doc/node_config_json_schema.json file.
// And then run make doc to update the out/doc/node-config-schema.json file.
assert.strictEqual(JSON.stringify(schema), JSON.stringify(schemaInDoc), 'JSON schema is outdated.' +
'Run `out/Release/node --expose-internals tools/doc/generate-json-schema.mjs` to update it.');
2 changes: 1 addition & 1 deletion tools/doc/generate-json-schema.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ import internal from 'internal/options';
import { writeFileSync } from 'fs';

const schema = internal.generateConfigJsonSchema();
writeFileSync('doc/node_config_json_schema.json', `${JSON.stringify(schema, null, 2)}\n`);
writeFileSync('doc/node-config-schema.json', `${JSON.stringify(schema, null, 2)}\n`);
Loading