Skip to content

Allow specifying configuration params in env vars #83

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.

Sign up for GitHub

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

Merged
djmitche merged 2 commits into from
Feb 3, 2025
Merged

Allow specifying configuration params in env vars #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7dec5fe
Allow specifying configuration params in env vars
djmitche Feb 1, 2025
3988641
Suggest using CLIENT_ID instead of --allow-client-id
djmitche Feb 3, 2025
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
10 changes: 10 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ thiserror = "2.0"
futures = "^0.3.25"
serde_json = "^1.0"
serde = { version = "^1.0.147", features = ["derive"] }
clap = { version = "^4.5.6", features = ["string"] }
clap = { version = "^4.5.6", features = ["string", "env"] }
log = "^0.4.17"
env_logger = "^0.11.5"
rusqlite = { version = "0.32", features = ["bundled"] }
chrono = { version = "^0.4.38", features = ["serde"] }
actix-rt = "2"
tempfile = "3"
pretty_assertions = "1"
temp-env = "0.3"
12 changes: 9 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,18 @@ The server is configured with command-line options. See

The `--listen` option specifies the interface and port the server listens on.
It must contain an IP-Address or a DNS name and a port number. This option is
mandatory, but can be repeated to specify multiple interfaces or ports.
mandatory, but can be repeated to specify multiple interfaces or ports. This
value can be specified in environment variable `LISTEN`, as a comma-separated
list of values.

The `--data-dir` option specifies where the server should store its data.
The `--data-dir` option specifies where the server should store its data. This
value can be specified in the environment variable `DATA_DIR`.

By default, the server allows all client IDs. To limit the accepted client IDs,
such as when running a personal server, use `--allow-client-id <client-id>`.
specify them in the environment variable `CLIENT_ID`, as a comma-separated list
of UUIDs. Client IDs can be specified with `--allow-client-id`, but this should
not be used on shared systems, as command line arguments are visible to all
users on the system.

The server only logs errors by default. To add additional logging output, set
environment variable `RUST_LOG` to `info` to get a log message for every
Expand Down
5 changes: 3 additions & 2 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ services:
volume:
nocopy: true
subpath: tss
command: --data-dir /tss/taskchampion-sync-server --listen 0.0.0.0:8080
environment:
- RUST_LOG=info
- "RUST_LOG=info"
- "DATA_DIR=/tss/taskchampion-sync-server"
- "LISTEN=0.0.0.0:8080"
depends_on:
mkdir:
condition: service_completed_successfully
1 change: 1 addition & 0 deletions server/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ chrono.workspace = true
actix-rt.workspace = true
tempfile.workspace = true
pretty_assertions.workspace = true
temp-env.workspace = true
204 changes: 166 additions & 38 deletions server/src/bin/taskchampion-sync-server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,36 @@ fn command() -> Command {
.arg(
arg!(-l --listen <ADDRESS>)
.help("Address and Port on which to listen on. Can be an IP Address or a DNS name followed by a colon and a port e.g. localhost:8080")
.value_delimiter(',')
.value_parser(ValueParser::string())
.env("LISTEN")
.action(ArgAction::Append)
.required(true),
)
.arg(
arg!(-d --"data-dir" <DIR> "Directory in which to store data")
.value_parser(ValueParser::os_string())
.env("DATA_DIR")
.default_value("/var/lib/taskchampion-sync-server"),
)
.arg(
arg!(-C --"allow-client-id" <CLIENT_ID> "Client IDs to allow (can be repeated; if not specified, all clients are allowed)")
.value_delimiter(',')
.value_parser(value_parser!(Uuid))
.env("CLIENT_ID")
.action(ArgAction::Append)
.required(false),
)
.arg(
arg!(--"snapshot-versions" <NUM> "Target number of versions between snapshots")
.value_parser(value_parser!(u32))
.env("SNAPSHOT_VERSIONS")
.default_value(default_snapshot_versions),
)
.arg(
arg!(--"snapshot-days" <NUM> "Target number of days between snapshots")
.value_parser(value_parser!(i64))
.env("SNAPSHOT_DAYS")
.default_value(default_snapshot_days),
)
}
Expand Down Expand Up @@ -95,6 +102,7 @@ mod test {
use actix_web::{self, App};
use clap::ArgMatches;
use taskchampion_sync_server_core::InMemoryStorage;
use temp_env::{with_var, with_var_unset, with_vars, with_vars_unset};

/// Get the list of allowed client IDs
fn allowed(matches: &ArgMatches) -> Option<Vec<Uuid>> {
Expand All @@ -103,60 +111,180 @@ mod test {
.map(|ids| ids.copied().collect::<Vec<_>>())
}

#[test]
fn command_listen_two() {
with_var_unset("LISTEN", || {
let matches = command().get_matches_from([
"tss",
"--listen",
"localhost:8080",
"--listen",
"otherhost:9090",
]);
assert_eq!(
matches
.get_many::<String>("listen")
.unwrap()
.cloned()
.collect::<Vec<String>>(),
vec!["localhost:8080".to_string(), "otherhost:9090".to_string()]
);
});
}

#[test]
fn command_listen_two_env() {
with_var("LISTEN", Some("localhost:8080,otherhost:9090"), || {
let matches = command().get_matches_from(["tss"]);
assert_eq!(
matches
.get_many::<String>("listen")
.unwrap()
.cloned()
.collect::<Vec<String>>(),
vec!["localhost:8080".to_string(), "otherhost:9090".to_string()]
);
});
}

#[test]
fn command_allowed_client_ids_none() {
let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
assert_eq!(allowed(&matches), None);
with_var_unset("CLIENT_ID", || {
let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
assert_eq!(allowed(&matches), None);
});
}

#[test]
fn command_allowed_client_ids_one() {
let matches = command().get_matches_from([
"tss",
"--listen",
"localhost:8080",
"-C",
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
]);
assert_eq!(
allowed(&matches),
Some(vec![Uuid::parse_str(
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
)
.unwrap()])
with_var_unset("CLIENT_ID", || {
let matches = command().get_matches_from([
"tss",
"--listen",
"localhost:8080",
"-C",
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
]);
assert_eq!(
allowed(&matches),
Some(vec![Uuid::parse_str(
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
)
.unwrap()])
);
});
}

#[test]
fn command_allowed_client_ids_one_env() {
with_var(
"CLIENT_ID",
Some("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"),
|| {
let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
assert_eq!(
allowed(&matches),
Some(vec![Uuid::parse_str(
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0"
)
.unwrap()])
);
},
);
}

#[test]
fn command_allowed_client_ids_two() {
let matches = command().get_matches_from([
"tss",
"--listen",
"localhost:8080",
"-C",
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
"-C",
"bbaf4b61-344a-4a39-a19e-8caa0669b353",
]);
assert_eq!(
allowed(&matches),
Some(vec![
Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
])
with_var_unset("CLIENT_ID", || {
let matches = command().get_matches_from([
"tss",
"--listen",
"localhost:8080",
"-C",
"711d5cf3-0cf0-4eb8-9eca-6f7f220638c0",
"-C",
"bbaf4b61-344a-4a39-a19e-8caa0669b353",
]);
assert_eq!(
allowed(&matches),
Some(vec![
Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
])
);
});
}

#[test]
fn command_allowed_client_ids_two_env() {
with_var(
"CLIENT_ID",
Some("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0,bbaf4b61-344a-4a39-a19e-8caa0669b353"),
|| {
let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
assert_eq!(
allowed(&matches),
Some(vec![
Uuid::parse_str("711d5cf3-0cf0-4eb8-9eca-6f7f220638c0").unwrap(),
Uuid::parse_str("bbaf4b61-344a-4a39-a19e-8caa0669b353").unwrap()
])
);
},
);
}

#[test]
fn command_data_dir() {
let matches = command().get_matches_from([
"tss",
"--data-dir",
"/foo/bar",
"--listen",
"localhost:8080",
]);
assert_eq!(matches.get_one::<OsString>("data-dir").unwrap(), "/foo/bar");
with_var_unset("DATA_DIR", || {
let matches = command().get_matches_from([
"tss",
"--data-dir",
"/foo/bar",
"--listen",
"localhost:8080",
]);
assert_eq!(matches.get_one::<OsString>("data-dir").unwrap(), "/foo/bar");
});
}

#[test]
fn command_data_dir_env() {
with_var("DATA_DIR", Some("/foo/bar"), || {
let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
assert_eq!(matches.get_one::<OsString>("data-dir").unwrap(), "/foo/bar");
});
}

#[test]
fn command_snapshot() {
with_vars_unset(["SNAPSHOT_DAYS", "SNAPSHOT_VERSIONS"], || {
let matches = command().get_matches_from([
"tss",
"--listen",
"localhost:8080",
"--snapshot-days",
"13",
"--snapshot-versions",
"20",
]);
assert_eq!(*matches.get_one::<i64>("snapshot-days").unwrap(), 13i64);
assert_eq!(*matches.get_one::<u32>("snapshot-versions").unwrap(), 20u32);
Comment on lines +270 to +271
Copy link
Contributor

@ryneeverett ryneeverett Feb 3, 2025

Choose a reason for hiding this comment

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

Seems odd that these end up with such different types.

Copy link
Collaborator Author

@djmitche djmitche Feb 3, 2025

Choose a reason for hiding this comment

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

I don't remember if I had a reason for that, but I don't think it's too important.

});
}

#[test]
fn command_snapshot_env() {
with_vars(
[
("SNAPSHOT_DAYS", Some("13")),
("SNAPSHOT_VERSIONS", Some("20")),
],
|| {
let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]);
assert_eq!(*matches.get_one::<i64>("snapshot-days").unwrap(), 13i64);
assert_eq!(*matches.get_one::<u32>("snapshot-versions").unwrap(), 20u32);
},
);
}

#[actix_rt::test]
Expand Down
Loading