Skip to content

Commit 7f2f59c

Browse files
committed
fix: improve handling of boolan flags
Bool flags seems to have been broken by a clap upgrade, meaning they required an argument to be provided in order to work. Moreover, this PR removes the `--enable-verification` flag, which was no longer used by the code. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
1 parent 56d9ad7 commit 7f2f59c

File tree

2 files changed

+26
-17
lines changed

2 files changed

+26
-17
lines changed

src/cli.rs

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clap::builder::PossibleValue;
2-
use clap::{crate_authors, crate_description, crate_name, crate_version, Arg, Command};
2+
use clap::{crate_authors, crate_description, crate_name, crate_version, Arg, ArgAction, Command};
33
use itertools::Itertools;
44
use lazy_static::lazy_static;
55
use policy_evaluator::burrego;
@@ -49,7 +49,7 @@ pub(crate) fn build_cli() -> Command {
4949
Arg::new("log-no-color")
5050
.long("log-no-color")
5151
.env("NO_COLOR")
52-
.required(false)
52+
.action(ArgAction::SetTrue)
5353
.help("Disable colored output for logs"),
5454
Arg::new("address")
5555
.long("addr")
@@ -116,13 +116,8 @@ pub(crate) fn build_cli() -> Command {
116116
Arg::new("enable-metrics")
117117
.long("enable-metrics")
118118
.env("KUBEWARDEN_ENABLE_METRICS")
119-
.required(false)
119+
.action(ArgAction::SetTrue)
120120
.help("Enable metrics"),
121-
Arg::new("enable-verification")
122-
.long("enable-verification")
123-
.env("KUBEWARDEN_ENABLE_VERIFICATION")
124-
.required(false)
125-
.help("Enable Sigstore verification"),
126121
Arg::new("always-accept-admission-reviews-on-namespace")
127122
.long("always-accept-admission-reviews-on-namespace")
128123
.value_name("NAMESPACE")
@@ -131,8 +126,8 @@ pub(crate) fn build_cli() -> Command {
131126
.help("Always accept AdmissionReviews that target the given namespace"),
132127
Arg::new("disable-timeout-protection")
133128
.long("disable-timeout-protection")
129+
.action(ArgAction::SetTrue)
134130
.env("KUBEWARDEN_DISABLE_TIMEOUT_PROTECTION")
135-
.required(false)
136131
.help("Disable policy timeout protection"),
137132
Arg::new("policy-timeout")
138133
.long("policy-timeout")
@@ -143,7 +138,7 @@ pub(crate) fn build_cli() -> Command {
143138
Arg::new("daemon")
144139
.long("daemon")
145140
.env("KUBEWARDEN_DAEMON")
146-
.required(false)
141+
.action(ArgAction::SetTrue)
147142
.help("If set, runs policy-server in detached mode as a daemon"),
148143
Arg::new("daemon-pid-file")
149144
.long("daemon-pid-file")
@@ -163,7 +158,7 @@ pub(crate) fn build_cli() -> Command {
163158
Arg::new("ignore-kubernetes-connection-failure")
164159
.long("ignore-kubernetes-connection-failure")
165160
.env("KUBEWARDEN_IGNORE_KUBERNETES_CONNECTION_FAILURE")
166-
.required(false)
161+
.action(ArgAction::SetTrue)
167162
.help("Do not exit with an error if the Kubernetes connection fails. This will cause context aware policies to break when there's no connection with Kubernetes."),
168163
];
169164
args.sort_by(|a, b| a.get_id().cmp(b.get_id()));

src/config.rs

+20-6
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ impl Config {
6161
.get_one::<String>("policies-download-dir")
6262
.map(PathBuf::from)
6363
.expect("This should not happen, there's a default value for policies-download-dir");
64-
let policy_evaluation_limit_seconds = if matches.contains_id("disable-timeout-protection") {
64+
let policy_evaluation_limit_seconds = if *matches
65+
.get_one::<bool>("disable-timeout-protection")
66+
.expect("clap should have set a default value")
67+
{
6568
None
6669
} else {
6770
Some(
@@ -82,16 +85,24 @@ impl Config {
8285
.get_one::<String>("always-accept-admission-reviews-on-namespace")
8386
.map(|s| s.to_owned());
8487

85-
let metrics_enabled = matches.contains_id("enable-metrics");
86-
let ignore_kubernetes_connection_failure =
87-
matches.contains_id("ignore-kubernetes-connection-failure");
88+
let metrics_enabled = matches
89+
.get_one::<bool>("enable-metrics")
90+
.expect("clap should have set a default value")
91+
.to_owned();
92+
let ignore_kubernetes_connection_failure = matches
93+
.get_one::<bool>("ignore-kubernetes-connection-failure")
94+
.expect("clap should have set a default value")
95+
.to_owned();
8896
let verification_config = verification_config(matches)?;
8997
let sigstore_cache_dir = matches
9098
.get_one::<String>("sigstore-cache-dir")
9199
.map(PathBuf::from)
92100
.expect("This should not happen, there's a default value for sigstore-cache-dir");
93101

94-
let daemon = matches.contains_id("daemon");
102+
let daemon = matches
103+
.get_one::<bool>("daemon")
104+
.expect("clap should have set a default value")
105+
.to_owned();
95106
let daemon_pid_file = matches
96107
.get_one::<String>("daemon-pid-file")
97108
.expect("This should not happen, there's a default value for daemon-pid-file")
@@ -107,7 +118,10 @@ impl Config {
107118
.get_one::<String>("log-fmt")
108119
.expect("This should not happen, there's a default value for log-fmt")
109120
.to_owned();
110-
let log_no_color = matches.contains_id("log-no-color");
121+
let log_no_color = matches
122+
.get_one::<bool>("log-no-color")
123+
.expect("clap should have assigned a default value")
124+
.to_owned();
111125
let (cert_file, key_file) = tls_files(matches)?;
112126
let tls_config = if cert_file.is_empty() {
113127
None

0 commit comments

Comments
 (0)