fix: cleanup, visibility, constants

Author: maltesander

rust/operator-binary/src/authentication/ldap.rs

@@ -83,7 +83,7 @@ fn add_authorizer_config(config: &mut BTreeMap<String, String>) {
     );
 }
 
-pub fn generate_runtime_properties_config(
+pub(super) fn generate_runtime_properties_config(
     provider: &ldap::v1alpha1::AuthenticationProvider,
     config: &mut BTreeMap<String, String>,
 ) -> Result<(), Error> {
@@ -93,7 +93,7 @@ pub fn generate_runtime_properties_config(
     Ok(())
 }
 
-pub fn prepare_container_commands(
+pub(super) fn prepare_container_commands(
     provider: &ldap::v1alpha1::AuthenticationProvider,
     command: &mut Vec<String>,
 ) {
@@ -106,7 +106,7 @@ pub fn prepare_container_commands(
     }
 }
 
-pub fn add_volumes_and_mounts(
+pub(super) fn add_volumes_and_mounts(
     provider: &ldap::v1alpha1::AuthenticationProvider,
     pb: &mut PodBuilder,
     cb_druid: &mut ContainerBuilder,

rust/operator-binary/src/authentication/mod.rs

@@ -11,7 +11,7 @@ use crate::{
     crd::{
         DruidRole,
         authentication::{AuthenticationClassResolved, AuthenticationClassesResolved},
-        security::{ESCALATOR_INTERNAL_CLIENT_PASSWORD_ENV, INTERNAL_INITIAL_CLIENT_PASSWORD_ENV},
+        security::INTERNAL_INITIAL_CLIENT_PASSWORD_ENV,
         v1alpha1,
     },
     internal_secret::{build_shared_internal_secret_name, env_var_from_secret},
@@ -20,6 +20,9 @@ use crate::{
 pub mod ldap;
 pub mod oidc;
 
+// It seems this needs to be the same password for Druid to work, so we re-use the existing env variable.
+const ESCALATOR_INTERNAL_CLIENT_PASSWORD_ENV: &str = INTERNAL_INITIAL_CLIENT_PASSWORD_ENV;
+
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 #[derive(Snafu, Debug)]

rust/operator-binary/src/authentication/oidc.rs

@@ -97,7 +97,7 @@ fn add_authorizer_config(config: &mut BTreeMap<String, String>) {
 /// Creates the OIDC parts of the runtime.properties config file.
 /// OIDC authentication is not configured on middlemanagers, because end users don't interact with them directly using the web console and
 /// turning on OIDC will lead to problems with the communication with coordinators during data ingest.
-pub fn generate_runtime_properties_config(
+pub(super) fn generate_runtime_properties_config(
     provider: &oidc::v1alpha1::AuthenticationProvider,
     oidc: &DruidClientAuthenticationOptions,
     role: &DruidRole,
@@ -118,7 +118,7 @@ pub fn generate_runtime_properties_config(
     Ok(())
 }
 
-pub fn main_container_commands(
+pub(super) fn main_container_commands(
     provider: &oidc::v1alpha1::AuthenticationProvider,
     command: &mut Vec<String>,
 ) {
@@ -129,7 +129,7 @@ pub fn main_container_commands(
 
 /// Mounts the OIDC credentials secret and the auto-generated internal secret containing the cookie passphrase.
 /// Not necessary on middlemanagers, because OIDC is not configured on them.
-pub fn get_env_var_mounts(
+pub(super) fn get_env_var_mounts(
     role: &DruidRole,
     oidc: &DruidClientAuthenticationOptions,
     internal_secret_name: &str,
@@ -153,7 +153,7 @@ pub fn get_env_var_mounts(
     envs
 }
 
-pub fn add_volumes_and_mounts(
+pub(super) fn add_volumes_and_mounts(
     provider: &oidc::v1alpha1::AuthenticationProvider,
     pb: &mut PodBuilder,
     cb_druid: &mut ContainerBuilder,

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

@@ -18,9 +18,6 @@ use crate::{
 
 #[derive(Snafu, Debug)]
 pub enum Error {
-    #[snafu(display("failed to get service FQDN"))]
-    NoServiceFqdn,
-
     #[snafu(display("failed to build ConfigMap"))]
     BuildConfigMap {
         source: stackable_operator::builder::configmap::Error,

rust/operator-binary/src/controller/validate.rs

@@ -88,11 +88,11 @@ pub type RoleGroupName = String;
 #[derive(Clone)]
 pub struct DruidRoleGroupConfig {
     pub merged_config: CommonRoleGroupConfig,
-    /// The runtime.properties "validated config" (compute_files + recommended defaults + merged overrides).
+    /// The runtime.properties config: recommended defaults plus merged overrides.
     pub runtime_config: BTreeMap<String, String>,
     /// The security.properties "validated config".
     pub security_config: BTreeMap<String, String>,
-    /// Merged env overrides (role <- rolegroup). compute_env is empty for druid.
+    /// Merged env overrides (role <- rolegroup). Druid has no computed env vars.
     pub env: BTreeMap<String, String>,
     /// The fully rendered `jvm.config` (operator defaults merged with the role/rolegroup JVM
     /// argument overrides). Precomputed here so the config-map builder no longer needs the raw
@@ -300,8 +300,8 @@ fn build_role_group_config(
 
 const INDEXER_JAVA_OPTS: &str = "druid.indexer.runner.javaOptsArray";
 
-/// The `druid.indexer.runner.javaOptsArray` entry that `MiddleManagerConfigFragment::compute_files`
-/// adds for *every* file (runtime.properties and security.properties).
+/// The `druid.indexer.runner.javaOptsArray` entry that must be present in *every* rendered file
+/// (runtime.properties and security.properties) for MiddleManagers.
 fn middlemanager_indexer_java_opts() -> (String, String) {
     (
         INDEXER_JAVA_OPTS.to_string(),

rust/operator-binary/src/crd/authentication.rs

@@ -13,7 +13,7 @@ use crate::crd::v1alpha1::DruidClusterConfig;
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 // The assumed OIDC provider if no hint is given in the AuthClass
-pub const DEFAULT_OIDC_PROVIDER: oidc::v1alpha1::IdentityProviderHint =
+const DEFAULT_OIDC_PROVIDER: oidc::v1alpha1::IdentityProviderHint =
     oidc::v1alpha1::IdentityProviderHint::Keycloak;
 
 const SUPPORTED_OIDC_PROVIDERS: &[oidc::v1alpha1::IdentityProviderHint] =
@@ -101,7 +101,7 @@ impl AuthenticationClassesResolved {
     }
 
     /// Retrieves all provided `AuthenticationClass` references and checks if the configuration (TLS settings, secret class, OIDC config, etc.) is valid.
-    pub async fn resolve<R>(
+    async fn resolve<R>(
         cluster_config: &DruidClusterConfig,
         resolve_auth_class: impl Fn(
             core::v1alpha1::ClientAuthenticationDetails<

rust/operator-binary/src/crd/database.rs

@@ -11,11 +11,11 @@ use stackable_operator::{
 };
 
 // metadata storage config properties
-pub const METADATA_STORAGE_TYPE: &str = "druid.metadata.storage.type";
-pub const METADATA_STORAGE_CONNECTOR_CONNECT_URI: &str =
+pub(crate) const METADATA_STORAGE_TYPE: &str = "druid.metadata.storage.type";
+pub(crate) const METADATA_STORAGE_CONNECTOR_CONNECT_URI: &str =
     "druid.metadata.storage.connector.connectURI";
-pub const METADATA_STORAGE_USER: &str = "druid.metadata.storage.connector.user";
-pub const METADATA_STORAGE_PASSWORD: &str = "druid.metadata.storage.connector.password";
+pub(crate) const METADATA_STORAGE_USER: &str = "druid.metadata.storage.connector.user";
+pub(crate) const METADATA_STORAGE_PASSWORD: &str = "druid.metadata.storage.connector.password";
 
 #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]

rust/operator-binary/src/crd/memory.rs

@@ -21,7 +21,7 @@ pub static RESERVED_OS_MEMORY: LazyLock<MemoryQuantity> =
 
 /// Max size for direct access buffers. This is defined in Druid to be 2GB:
 /// <https://druid.apache.org/docs/latest/configuration/index.html#processing-1>
-pub static MAX_DIRECT_BUFFER_SIZE: LazyLock<MemoryQuantity> =
+static MAX_DIRECT_BUFFER_SIZE: LazyLock<MemoryQuantity> =
     LazyLock::new(|| MemoryQuantity::from_gibi(2.));
 
 #[derive(Snafu, Debug)]
@@ -55,7 +55,7 @@ pub struct HistoricalDerivedSettings {
 }
 
 impl HistoricalDerivedSettings {
-    pub fn new(total_memory: MemoryQuantity, cpu_millis: CpuQuantity) -> Self {
+    fn new(total_memory: MemoryQuantity, cpu_millis: CpuQuantity) -> Self {
         Self {
             total_memory,
             cpu_millis,

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

@@ -59,7 +59,6 @@ pub mod tls;
 
 pub const APP_NAME: &str = "druid";
 pub const OPERATOR_NAME: &str = "druid.stackable.tech";
-pub const FIELD_MANAGER: &str = "druid-operator";
 
 // config directories
 pub const DRUID_CONFIG_DIRECTORY: &str = "/stackable/config";
@@ -73,22 +72,21 @@ pub const STACKABLE_TRUST_STORE_PASSWORD: &str = "changeit";
 pub const STACKABLE_LOG_DIR: &str = "/stackable/log";
 
 pub const PROP_SEGMENT_CACHE_LOCATIONS: &str = "druid.segmentCache.locations";
-pub const PATH_SEGMENT_CACHE: &str = "/stackable/var/druid/segment-cache";
 
 /////////////////////////////
 //    CONFIG PROPERTIES    //
 /////////////////////////////
 // deep storage
-pub const DS_TYPE: &str = "druid.storage.type";
-pub const DS_DIRECTORY: &str = "druid.storage.storageDirectory";
-pub const DS_BASE_KEY: &str = "druid.storage.baseKey";
+const DS_TYPE: &str = "druid.storage.type";
+const DS_DIRECTORY: &str = "druid.storage.storageDirectory";
+const DS_BASE_KEY: &str = "druid.storage.baseKey";
 // OPA
-pub const AUTH_AUTHORIZERS: &str = "druid.auth.authorizers";
-pub const AUTH_AUTHORIZERS_VALUE: &str = "[\"OpaAuthorizer\"]";
-pub const AUTH_AUTHORIZER_OPA_TYPE: &str = "druid.auth.authorizer.OpaAuthorizer.type";
-pub const AUTH_AUTHORIZER_OPA_TYPE_VALUE: &str = "opa";
+const AUTH_AUTHORIZERS: &str = "druid.auth.authorizers";
+const AUTH_AUTHORIZERS_VALUE: &str = "[\"OpaAuthorizer\"]";
+const AUTH_AUTHORIZER_OPA_TYPE: &str = "druid.auth.authorizer.OpaAuthorizer.type";
+const AUTH_AUTHORIZER_OPA_TYPE_VALUE: &str = "opa";
 // metrics
-pub const PROMETHEUS_PORT: &str = "druid.emitter.prometheus.port";
+const PROMETHEUS_PORT: &str = "druid.emitter.prometheus.port";
 pub const METRICS_PORT_NAME: &str = "metrics";
 pub const METRICS_PORT: u16 = 9090;
 
@@ -546,7 +544,7 @@ impl v1alpha1::DruidCluster {
     }
 
     /// Merges and validates the given role group, role, and default configurations
-    pub fn merged_rolegroup_config<T>(
+    pub(crate) fn merged_rolegroup_config<T>(
         rolegroup_config: &T::Fragment,
         role_config: &T::Fragment,
         default_config: &T::Fragment,
@@ -900,7 +898,7 @@ impl DruidRole {
     }
 
     /// Return the default graceful shutdown timeout
-    pub fn default_graceful_shutdown_timeout(&self) -> Duration {
+    fn default_graceful_shutdown_timeout(&self) -> Duration {
         match &self {
             DruidRole::Coordinator => DEFAULT_COORDINATOR_GRACEFUL_SHUTDOWN_TIMEOUT,
             DruidRole::Broker => DEFAULT_BROKER_GRACEFUL_SHUTDOWN_TIMEOUT,
@@ -1003,11 +1001,7 @@ pub enum DeepStorageSpec {
 }
 
 impl DeepStorageSpec {
-    pub fn is_hdfs(&self) -> bool {
-        matches!(self, DeepStorageSpec::Hdfs(_))
-    }
-
-    pub fn is_s3(&self) -> bool {
+    fn is_s3(&self) -> bool {
         matches!(self, DeepStorageSpec::S3(_))
     }
 }

rust/operator-binary/src/crd/resource.rs

@@ -17,11 +17,13 @@ use stackable_operator::{
 use strum::{EnumDiscriminants, IntoStaticStr};
 
 use crate::crd::{
-    DruidRole, PATH_SEGMENT_CACHE, PROP_SEGMENT_CACHE_LOCATIONS,
+    DruidRole, PROP_SEGMENT_CACHE_LOCATIONS,
     memory::{HistoricalDerivedSettings, RESERVED_OS_MEMORY},
     storage::{self, default_free_percentage_empty_dir_fragment},
 };
 
+const PATH_SEGMENT_CACHE: &str = "/stackable/var/druid/segment-cache";
+
 // volume names
 const SEGMENT_CACHE_VOLUME_NAME: &str = "segment-cache";
 
@@ -172,7 +174,7 @@ impl RoleResource {
     }
 }
 
-pub static MIDDLE_MANAGER_RESOURCES: LazyLock<
+pub(crate) static MIDDLE_MANAGER_RESOURCES: LazyLock<
     ResourcesFragment<storage::DruidStorage, NoRuntimeLimits>,
 > = LazyLock::new(|| ResourcesFragment {
     cpu: CpuLimitsFragment {
@@ -186,20 +188,21 @@ pub static MIDDLE_MANAGER_RESOURCES: LazyLock<
     storage: storage::DruidStorageFragment {},
 });
 
-pub static BROKER_RESOURCES: LazyLock<ResourcesFragment<storage::DruidStorage, NoRuntimeLimits>> =
-    LazyLock::new(|| ResourcesFragment {
-        cpu: CpuLimitsFragment {
-            min: Some(Quantity("300m".to_owned())),
-            max: Some(Quantity("1200m".to_owned())),
-        },
-        memory: MemoryLimitsFragment {
-            limit: Some(Quantity("1500Mi".to_owned())),
-            runtime_limits: NoRuntimeLimitsFragment {},
-        },
-        storage: storage::DruidStorageFragment {},
-    });
+pub(crate) static BROKER_RESOURCES: LazyLock<
+    ResourcesFragment<storage::DruidStorage, NoRuntimeLimits>,
+> = LazyLock::new(|| ResourcesFragment {
+    cpu: CpuLimitsFragment {
+        min: Some(Quantity("300m".to_owned())),
+        max: Some(Quantity("1200m".to_owned())),
+    },
+    memory: MemoryLimitsFragment {
+        limit: Some(Quantity("1500Mi".to_owned())),
+        runtime_limits: NoRuntimeLimitsFragment {},
+    },
+    storage: storage::DruidStorageFragment {},
+});
 
-pub static HISTORICAL_RESOURCES: LazyLock<
+pub(crate) static HISTORICAL_RESOURCES: LazyLock<
     ResourcesFragment<storage::HistoricalStorage, NoRuntimeLimits>,
 > = LazyLock::new(|| ResourcesFragment {
     cpu: CpuLimitsFragment {
@@ -215,7 +218,7 @@ pub static HISTORICAL_RESOURCES: LazyLock<
     },
 });
 
-pub static COORDINATOR_RESOURCES: LazyLock<
+pub(crate) static COORDINATOR_RESOURCES: LazyLock<
     ResourcesFragment<storage::DruidStorage, NoRuntimeLimits>,
 > = LazyLock::new(|| ResourcesFragment {
     cpu: CpuLimitsFragment {
@@ -229,18 +232,19 @@ pub static COORDINATOR_RESOURCES: LazyLock<
     storage: storage::DruidStorageFragment {},
 });
 
-pub static ROUTER_RESOURCES: LazyLock<ResourcesFragment<storage::DruidStorage, NoRuntimeLimits>> =
-    LazyLock::new(|| ResourcesFragment {
-        cpu: CpuLimitsFragment {
-            min: Some(Quantity("300m".to_owned())),
-            max: Some(Quantity("1200m".to_owned())),
-        },
-        memory: MemoryLimitsFragment {
-            limit: Some(Quantity("512Mi".to_owned())),
-            runtime_limits: NoRuntimeLimitsFragment {},
-        },
-        storage: storage::DruidStorageFragment {},
-    });
+pub(crate) static ROUTER_RESOURCES: LazyLock<
+    ResourcesFragment<storage::DruidStorage, NoRuntimeLimits>,
+> = LazyLock::new(|| ResourcesFragment {
+    cpu: CpuLimitsFragment {
+        min: Some(Quantity("300m".to_owned())),
+        max: Some(Quantity("1200m".to_owned())),
+    },
+    memory: MemoryLimitsFragment {
+        limit: Some(Quantity("512Mi".to_owned())),
+        runtime_limits: NoRuntimeLimitsFragment {},
+    },
+    storage: storage::DruidStorageFragment {},
+});
 
 #[cfg(test)]
 mod test {