Skip to content

Commit 7754cd4

Browse files
maltesandermaltesander
committed
fix: remove hardcoded filenames for ConfigFileName enum, remove hardcoded constants
1 parent 4f982b5 commit 7754cd4

6 files changed

Lines changed: 47 additions & 37 deletions

File tree

‎rust/operator-binary/src/controller/build.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
44
use std::str::FromStr;
55

6+
use stackable_operator::v2::types::common::Port;
7+
68
use crate::controller::RoleGroupName;
79

810
pub mod properties;
911
pub mod resource;
1012

13+
/// The port the bundle-builder sidecar listens on, and which OPA connects to for bundle polling.
14+
pub(crate) const BUNDLE_BUILDER_PORT: Port = Port(3030);
15+
1116
// Placeholder role-group name for the recommended labels of the role-level `Service`, which is not
1217
// bound to a single role group. `global` matches the historical `app.kubernetes.io/role-group`
1318
// value.

‎rust/operator-binary/src/controller/build/properties/config_json.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use stackable_operator::{
1212

1313
use super::ConfigFileName;
1414
use crate::{
15-
controller::ValidatedOpaConfig,
15+
controller::{ValidatedOpaConfig, build::BUNDLE_BUILDER_PORT},
1616
crd::{Container, OpaConfigOverrides},
1717
opa_controller::OPA_STACKABLE_SERVICE_NAME,
1818
};
@@ -21,6 +21,10 @@ use crate::{
2121
/// level other than `NONE`.
2222
const DEFAULT_DECISION_LOGGING_ENABLED: bool = false;
2323

24+
/// Default delays OPA uses when long-polling the bundle-builder for new bundles.
25+
const DEFAULT_BUNDLE_POLLING_MIN_DELAY_SECONDS: i32 = 10;
26+
const DEFAULT_BUNDLE_POLLING_MAX_DELAY_SECONDS: i32 = 20;
27+
2428
#[derive(Snafu, Debug)]
2529
pub enum Error {
2630
#[snafu(display("failed to serialize config file {file:?}"))]
@@ -84,16 +88,16 @@ impl OpaClusterConfigFile {
8488
Self {
8589
services: vec![OpaClusterConfigService {
8690
name: OPA_STACKABLE_SERVICE_NAME.to_owned(),
87-
url: "http://localhost:3030/opa/v1".to_owned(),
91+
url: format!("http://localhost:{BUNDLE_BUILDER_PORT}/opa/v1"),
8892
}],
8993
bundles: OpaClusterBundle {
9094
stackable: OpaClusterBundleConfig {
9195
service: OPA_STACKABLE_SERVICE_NAME.to_owned(),
9296
resource: "opa/bundle.tar.gz".to_owned(),
9397
persist: true,
9498
polling: OpaClusterBundleConfigPolling {
95-
min_delay_seconds: 10,
96-
max_delay_seconds: 20,
99+
min_delay_seconds: DEFAULT_BUNDLE_POLLING_MIN_DELAY_SECONDS,
100+
max_delay_seconds: DEFAULT_BUNDLE_POLLING_MAX_DELAY_SECONDS,
97101
},
98102
},
99103
},

‎rust/operator-binary/src/controller/build/resource/daemonset.rs‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use stackable_operator::{
3030
apps::v1::{DaemonSet, DaemonSetSpec, DaemonSetUpdateStrategy, RollingUpdateDaemonSet},
3131
core::v1::{
3232
EmptyDirVolumeSource, EnvVarSource, HTTPGetAction, ObjectFieldSelector, Probe,
33-
SecretVolumeSource, ServiceAccount,
33+
ResourceRequirements, SecretVolumeSource, ServiceAccount,
3434
},
3535
},
3636
apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString},
@@ -48,10 +48,7 @@ use stackable_operator::{
4848
product_logging::framework::{
4949
STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container,
5050
},
51-
types::{
52-
common::Port,
53-
kubernetes::{ContainerName, VolumeName},
54-
},
51+
types::kubernetes::{ContainerName, VolumeName},
5552
},
5653
};
5754

@@ -62,12 +59,9 @@ use crate::{
6259
operations::graceful_shutdown::add_graceful_shutdown_config,
6360
};
6461

65-
pub const CONFIG_FILE: &str = "config.json";
66-
6762
pub const BUNDLES_ACTIVE_DIR: &str = "/bundles/active";
6863
pub const BUNDLES_INCOMING_DIR: &str = "/bundles/incoming";
6964
pub const BUNDLES_TMP_DIR: &str = "/bundles/tmp";
70-
pub const BUNDLE_BUILDER_PORT: Port = Port(3030);
7165

7266
stackable_operator::constant!(CONFIG_VOLUME_NAME: VolumeName = "config");
7367
const CONFIG_DIR: &str = "/stackable/config";
@@ -181,6 +175,16 @@ fn container_name(container: &Container) -> ContainerName {
181175
.expect("Container enum variants are valid container names")
182176
}
183177

178+
/// The CPU and memory requests/limits shared by the bundle-builder and user-info-fetcher sidecars.
179+
fn sidecar_resource_requirements() -> ResourceRequirements {
180+
ResourceRequirementsBuilder::new()
181+
.with_cpu_request("100m")
182+
.with_cpu_limit("200m")
183+
.with_memory_request("128Mi")
184+
.with_memory_limit("128Mi")
185+
.build()
186+
}
187+
184188
/// The strict-mode `bash` entrypoint shared by the prepare, bundle-builder, and OPA containers.
185189
/// The actual script is passed via `.args(...)`.
186190
fn bash_entrypoint_command() -> Vec<String> {
@@ -281,22 +285,15 @@ pub fn build_server_rolegroup_daemonset(
281285
.context(AddVolumeMountSnafu)?
282286
.add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR)
283287
.context(AddVolumeMountSnafu)?
284-
.resources(
285-
ResourceRequirementsBuilder::new()
286-
.with_cpu_request("100m")
287-
.with_cpu_limit("200m")
288-
.with_memory_request("128Mi")
289-
.with_memory_limit("128Mi")
290-
.build(),
291-
)
288+
.resources(sidecar_resource_requirements())
292289
.readiness_probe(http_readiness_probe(
293290
BUNDLE_BUILDER_PROBE_PATH,
294-
IntOrString::Int(BUNDLE_BUILDER_PORT.into()),
291+
IntOrString::Int(build::BUNDLE_BUILDER_PORT.into()),
295292
None,
296293
))
297294
.liveness_probe(http_liveness_probe(
298295
BUNDLE_BUILDER_PROBE_PATH,
299-
IntOrString::Int(BUNDLE_BUILDER_PORT.into()),
296+
IntOrString::Int(build::BUNDLE_BUILDER_PORT.into()),
300297
None,
301298
));
302299
add_stackable_rust_cli_env_vars(
@@ -444,18 +441,17 @@ pub fn build_server_rolegroup_daemonset(
444441
.image_from_product_image(resolved_product_image) // inherit the pull policy and pull secrets, and then...
445442
.image(user_info_fetcher_image) // ...override the image
446443
.command(vec!["stackable-opa-user-info-fetcher".to_string()])
447-
.add_env_var("CONFIG", format!("{CONFIG_DIR}/user-info-fetcher.json"))
444+
.add_env_var(
445+
"CONFIG",
446+
format!(
447+
"{CONFIG_DIR}/{file}",
448+
file = build::properties::ConfigFileName::UserInfoFetcher
449+
),
450+
)
448451
.add_env_var("CREDENTIALS_DIR", USER_INFO_FETCHER_CREDENTIALS_DIR)
449452
.add_volume_mount(CONFIG_VOLUME_NAME.as_ref(), CONFIG_DIR)
450453
.context(AddVolumeMountSnafu)?
451-
.resources(
452-
ResourceRequirementsBuilder::new()
453-
.with_cpu_request("100m")
454-
.with_cpu_limit("200m")
455-
.with_memory_request("128Mi")
456-
.with_memory_limit("128Mi")
457-
.build(),
458-
);
454+
.resources(sidecar_resource_requirements());
459455
add_stackable_rust_cli_env_vars(
460456
&mut cb_user_info_fetcher,
461457
cluster_info,
@@ -722,13 +718,15 @@ fn build_opa_start_command(
722718
.collect::<Vec<_>>()
723719
.join(" ");
724720

721+
let config_file = build::properties::ConfigFileName::ConfigJson;
722+
725723
// TODO: Think about adding --shutdown-wait-period, as suggested by https://github.com/open-policy-agent/opa/issues/2764
726724
formatdoc! {"
727725
{COMMON_BASH_TRAP_FUNCTIONS}
728726
{remove_vector_shutdown_file_command}
729727
prepare_signal_handlers
730728
containerdebug --output={STACKABLE_LOG_DIR}/containerdebug-state.json --loop &
731-
opa run -s -a 0.0.0.0:{bind_port} -c {CONFIG_DIR}/{CONFIG_FILE} -l {opa_log_level} --shutdown-grace-period {shutdown_grace_period_s} --disable-telemetry {tls_flags} {extra_cli_args} {logging_redirects} &
729+
opa run -s -a 0.0.0.0:{bind_port} -c {CONFIG_DIR}/{config_file} -l {opa_log_level} --shutdown-grace-period {shutdown_grace_period_s} --disable-telemetry {tls_flags} {extra_cli_args} {logging_redirects} &
732730
wait_for_termination $!
733731
{create_vector_shutdown_file_command}
734732
",

‎rust/operator-binary/src/crd/user_info_fetcher/mod.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,11 @@ pub mod versioned {
218218
}
219219
}
220220

221+
/// Default time-to-live for cached user metadata.
222+
pub(crate) const DEFAULT_CACHE_ENTRY_TIME_TO_LIVE: Duration = Duration::from_minutes_unchecked(1);
223+
221224
const fn default_entry_time_to_live() -> Duration {
222-
Duration::from_minutes_unchecked(1)
225+
DEFAULT_CACHE_ENTRY_TIME_TO_LIVE
223226
}
224227

225228
fn default_root_path() -> String {

‎rust/operator-binary/src/crd/user_info_fetcher/v1alpha1_impl.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use stackable_operator::shared::time::Duration;
22

3-
use crate::crd::user_info_fetcher::v1alpha1;
3+
use crate::crd::user_info_fetcher::{DEFAULT_CACHE_ENTRY_TIME_TO_LIVE, v1alpha1};
44

55
// TODO (@Techassi): Most of these impls are the exact same across v1alpha1 and v1alpha2. Explore
66
// and design a more elegant solution for it.
@@ -20,6 +20,6 @@ impl Default for v1alpha1::Cache {
2020

2121
impl v1alpha1::Cache {
2222
pub const fn default_entry_time_to_live() -> Duration {
23-
Duration::from_minutes_unchecked(1)
23+
DEFAULT_CACHE_ENTRY_TIME_TO_LIVE
2424
}
2525
}

‎rust/operator-binary/src/crd/user_info_fetcher/v1alpha2_impl.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use stackable_operator::{crd::authentication::ldap, shared::time::Duration};
22

3-
use crate::crd::user_info_fetcher::v1alpha2;
3+
use crate::crd::user_info_fetcher::{DEFAULT_CACHE_ENTRY_TIME_TO_LIVE, v1alpha2};
44

55
// TODO (@Techassi): Most of these impls are the exact same across v1alpha1 and v1alpha2. Explore
66
// and design a more elegant solution for it.
@@ -20,7 +20,7 @@ impl Default for v1alpha2::Cache {
2020

2121
impl v1alpha2::Cache {
2222
pub const fn default_entry_time_to_live() -> Duration {
23-
Duration::from_minutes_unchecked(1)
23+
DEFAULT_CACHE_ENTRY_TIME_TO_LIVE
2424
}
2525
}
2626

0 commit comments

Comments
 (0)