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

Tweaking config instructions and initialization - resolves #365 #369

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
21 changes: 19 additions & 2 deletions docs/setup_selfbuild.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,26 @@ cd mjolnir
yarn install
yarn build

# Copy and edit the config. It *is* recommended to change the data path.
# Edit the config.
# You probably should change `dataPath`.
nano config/default.yaml

node lib/index.js
```

Or, if you wish to use a different configuration file, e.g. `development.yaml`

```bash
git clone https://github.com/matrix-org/mjolnir.git
cd mjolnir

yarn install
yarn build

# Edit the config.
# You probably should change `dataPath`.
cp config/default.yaml config/development.yaml
nano config/development.yaml

node lib/index.js
NODE_ENV=development node lib/index.js
```
32 changes: 28 additions & 4 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,36 @@ const defaultConfig: IConfig = {
},
};

export function read(): IConfig {
export function read(checkConfigured = true): IConfig {
const config_dir = process.env.NODE_CONFIG_DIR || "./config";
const config_file = `${process.env.NODE_ENV || "default"}.yaml`

const content = fs.readFileSync(path.join(config_dir, config_file), "utf8");
let config_path;
for (let key of [
process.env.NODE_ENV,
"production",
"default"
]) {
if (!key) {
continue;
}
const candidate_path = path.join(config_dir, `${key}.yaml`);
if (!fs.existsSync(candidate_path)) {
continue;
}
config_path = candidate_path;
break;
}
if (!config_path) {
throw new Error("Could not find any valid configuration file");
}
const content = fs.readFileSync(config_path, "utf8");
const parsed = load(content);
const config = {...defaultConfig, ...(parsed as object)} as IConfig;
if (checkConfigured && config.accessToken === "YOUR_TOKEN_HERE") {
// A small check to simplify the error message in case
// the configuration file hasn't been modified.
throw new Error(`Invalid access token in configuration file ${config_path}. `
+ "This usually indicates that Mjölnir's configuration file was not setup "
+ "or that Mjölnir is using the wrong configuration file.");
Comment on lines +200 to +202
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not work if they are using pantalaimon. It will say this every time.

It also is confusing because yes the token is invalid but that's not because of something the user has done, it's because they haven't done something (made a config available) or we have silently loaded a default config instead of exploding.

}
return config;
}
2 changes: 1 addition & 1 deletion test/commands/UnbanBanCommandTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { read as configRead } from "../../src/config";
import { RULE_ROOM, RULE_SERVER, RULE_USER } from "../../src/models/ListRule";

function createTestMjolnir(defaultShortcode: string|null = null): Mjolnir {
const config = configRead();
const config = configRead(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not confident in this, since this is really trying to be an option whether to load a default if the "original" wasn't found. But we would need to change this in a few more places now that the appservice exists.

const client = {
// Mock `MatrixClient.getAccountData` .
getAccountData: (eventType: string): Promise<any> => {
Expand Down