MindFlavor / prometheus_wireguard_exporter

A Prometheus exporter for WireGuard, written in Rust.
https://mindflavor.github.io/prometheus_wireguard_exporter
MIT License
462 stars 49 forks source link

error: The argument '--prepend_sudo <prepend_sudo>' requires a value but none was supplied #99

Open andrewlow opened 1 year ago

andrewlow commented 1 year ago

Previously reported in https://github.com/MindFlavor/prometheus_wireguard_exporter/issues/89

I suspect that was closed because the original issue was dealt with - but this same issue was called out.

The Dockerfile has this line https://github.com/MindFlavor/prometheus_wireguard_exporter/blob/master/Dockerfile#L133

Which appears to make -a the default command line. Unfortunately - the -a command line is invalid - it requires a parameter which is exactly the error that is being shown.

Reproduce this using

$ docker run --rm -it mindflavor/prometheus-wireguard-exporter 
error: The argument '--prepend_sudo <prepend_sudo>' requires a value but none was supplied

USAGE:
    prometheus_wireguard_exporter [OPTIONS]

For more information try --help

granted, that's not a useful invocation of the container - but it does show the problem.

Adding a command line option fixes this

$ docker run --rm -it mindflavor/prometheus-wireguard-exporter -a true
[2022-11-09T14:40:44Z INFO  prometheus_wireguard_exporter] prometheus_wireguard_exporter v3.6.3 starting...
[2022-11-09T14:40:44Z INFO  prometheus_wireguard_exporter] using options: Options { verbose: false, prepend_sudo: true, separate_allowed_ips: false, extract_names_config_files: None, interfaces: None, export_remote_ip_and_port: false }
[2022-11-09T14:40:44Z INFO  prometheus_wireguard_exporter] starting exporter on http://0.0.0.0:9586/metrics
[2022-11-09T14:40:44Z INFO  prometheus_exporter_base] Listening on http://0.0.0.0:9586/metrics

Alternatively - any valid command line will fix this

$ docker run --rm -it mindflavor/prometheus-wireguard-exporter -v true
[2022-11-09T14:41:23Z INFO  prometheus_wireguard_exporter] prometheus_wireguard_exporter v3.6.3 starting...
[2022-11-09T14:41:23Z INFO  prometheus_wireguard_exporter] using options: Options { verbose: true, prepend_sudo: false, separate_allowed_ips: false, extract_names_config_files: None, interfaces: None, export_remote_ip_and_port: false }
[2022-11-09T14:41:23Z INFO  prometheus_wireguard_exporter] starting exporter on http://0.0.0.0:9586/metrics
[2022-11-09T14:41:23Z INFO  prometheus_exporter_base] Listening on http://0.0.0.0:9586/metrics

I suspect all active installations are using some version of the command line flags .. so they don't run into this default behaviour

Should be easy to fix by modifying the Dockerfile

garbelini commented 1 year ago

Just bumped into this. Seems to be related to a recent change that was not reflected in the docker image: https://github.com/MindFlavor/prometheus_wireguard_exporter/blob/master/README.md?plain=1#L110

andrewlow commented 1 year ago

The README.md change is this PR https://github.com/MindFlavor/prometheus_wireguard_exporter/pull/93

However, that is only the comment / documentation fix to address a previous change in behaviour. I think that this was back in this PR https://github.com/MindFlavor/prometheus_wireguard_exporter/pull/83

It was documented as a breaking change as well. The specific change that impacted the use of -a appears to be here: https://github.com/MindFlavor/prometheus_wireguard_exporter/blame/72d3f7393e166fb3846c1b1cf2d50ff8dbea71d6/src/main.rs#L153

        .arg(
            Arg::with_name("prepend_sudo")
                .short("a")
                .long("prepend_sudo")
                .env("PROMETHEUS_WIREGUARD_EXPORTER_PREPEND_SUDO_ENABLED")
                .help("Prepend sudo to the wg show commands")
                .default_value("false")
                .takes_value(true),

The use of .takes_value(true) I believe means that this command line argument MUST have a value.

This appears to be the clap library - https://docs.rs/clap/2.33.0/clap/index.html

While I've written some Rust - I'm not an expert. There may be a way to recode this to permit an optional value vs. forcing the use of a value - but it seems clear from the doc that takes_value puts things into the situation where a value must be supplied.

kramrm commented 1 year ago

I'm having this same issue with trying to start a container.

hatamiarash7 commented 1 year ago

@garbelini @kramrm

Just run the container with that argument like this:

docker run -d --net=host --cap-add=NET_ADMIN mindflavor/prometheus-wireguard-exporter -a true
kramrm commented 1 year ago

How would I do this via a docker-compose? The following doesn't seem to work.

---
version: '3'

services:
  wg_metrics:
    image: mindflavor/prometheus-wireguard-exporter
    container_name: wg_metrics
    environment:
      TZ: 'America/Los_Angeles'
      EXPORT_LATEST_HANDSHAKE_DELAY: 'true'
      PROMETHEUS_WIREGUARD_EXPORTER_CONFIG_FILE_NAMES: '/etc/wireguard/wg0.conf'
      PROMETHEUS_WIREGUARD_EXPORTER_PREPEND_SUDO_ENABLED: 'true'
    cap_add:
      - NET_ADMIN
    network_mode: host
hatamiarash7 commented 1 year ago

Try this:

version: '3'

services:
  wg_metrics:
    image: mindflavor/prometheus-wireguard-exporter
    container_name: wg_metrics
    command: -a true
    cap_add:
      - NET_ADMIN
    network_mode: host
kramrm commented 1 year ago

Thanks @hatamiarash7 that works!

@MindFlavor Could the above be added to the readme as an example of using docker compose, in addition to the docker run?

greigm commented 2 months ago

While the workaround is useful (fixed it for me!), I still think it's a bug that it seems to ignore the environment variable and only work when a command is specified.