fix: Correctly encode user given content, such as passwords (#627)

* fix: Correctly encode user given content, such as passwords * changelog * fix bcrypt problems * Update rust/operator-binary/src/security/authentication.rs Co-authored-by: Malte Sander <malte.sander.it@gmail.com> --------- Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
stackabletech · Jun 21, 2024 · f99b1ae · f99b1ae
1 parent fbc2852
commit f99b1ae
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 95 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Fixed
+
+- Use [config-utils](https://github.com/stackabletech/config-utils/) for text-replacement of variables in configs.
+  This fixes escaping problems, especially when you have special characters in your password ([#627]).
+
 ### Added
 
 - Support specifying the SecretClass that is used to obtain TLS certificates ([#622]).
@@ -19,6 +24,7 @@ All notable changes to this project will be documented in this file.
 
 [#616]: https://github.com/stackabletech/nifi-operator/pull/616
 [#622]: https://github.com/stackabletech/nifi-operator/pull/622
+[#627]: https://github.com/stackabletech/nifi-operator/pull/627
 [#628]: https://github.com/stackabletech/nifi-operator/pull/628
 
 ## [24.3.0] - 2024-03-20

diff --git a/rust/crd/src/authentication.rs b/rust/crd/src/authentication.rs
@@ -16,15 +16,19 @@ pub enum Error {
         source: stackable_operator::client::Error,
         authentication_class: ObjectRef<AuthenticationClass>,
     },
+
     #[snafu(display("The nifi-operator does not support running Nifi without any authentication. Please provide a AuthenticationClass to use."))]
     NoAuthenticationNotSupported {},
+
     #[snafu(display("The nifi-operator does not support multiple AuthenticationClasses simultaneously. Please provide a single AuthenticationClass to use."))]
     MultipleAuthenticationClassesNotSupported {},
+
     #[snafu(display("The nifi-operator does not support the AuthenticationClass provider [{authentication_class_provider}] from AuthenticationClass [{authentication_class}]."))]
     AuthenticationClassProviderNotSupported {
         authentication_class_provider: String,
         authentication_class: ObjectRef<AuthenticationClass>,
     },
+
     #[snafu(display("Nifi doesn't support skipping the LDAP TLS verification of the AuthenticationClass {authentication_class}"))]
     NoLdapTlsVerificationNotSupported {
         authentication_class: ObjectRef<AuthenticationClass>,

diff --git a/rust/operator-binary/src/config.rs b/rust/operator-binary/src/config.rs
@@ -474,10 +474,10 @@ pub fn build_nifi_properties(
     );
 
     //#############################################
-
-    // This will be replaced in the init container, so 0.0.0.0 acts mainly as
-    // marker
-    properties.insert("nifi.web.https.host".to_string(), "0.0.0.0".to_string());
+    properties.insert(
+        "nifi.web.https.host".to_string(),
+        "${env:NODE_ADDRESS}".to_string(),
+    );
     properties.insert("nifi.web.https.port".to_string(), HTTPS_PORT.to_string());
     properties.insert(
         "nifi.web.https.network.interface.default".to_string(),
@@ -492,9 +492,10 @@ pub fn build_nifi_properties(
     properties.insert("nifi.web.proxy.context.path".to_string(), "".to_string());
     properties.insert("nifi.web.proxy.host".to_string(), proxy_hosts.to_string());
 
-    // security properties
-    // this property is later set from a secret, so can remain empty here
-    properties.insert("nifi.sensitive.props.key".to_string(), "".to_string());
+    properties.insert(
+        "nifi.sensitive.props.key".to_string(),
+        "${file:UTF-8:/stackable/sensitiveproperty/nifiSensitivePropsKey}".to_string(),
+    );
     properties.insert(
         "nifi.sensitive.props.key.protected".to_string(),
         "".to_string(),
@@ -562,8 +563,10 @@ pub fn build_nifi_properties(
     );
     // cluster node properties (only configure for cluster nodes)
     properties.insert("nifi.cluster.is.node".to_string(), "true".to_string());
-    // this will be overwritten to the correct FQDN in the container start command
-    properties.insert("nifi.cluster.node.address".to_string(), "".to_string());
+    properties.insert(
+        "nifi.cluster.node.address".to_string(),
+        "${env:NODE_ADDRESS}".to_string(),
+    );
     properties.insert(
         "nifi.cluster.node.protocol.port".to_string(),
         PROTOCOL_PORT.to_string(),
@@ -581,10 +584,13 @@ pub fn build_nifi_properties(
     // this will be replaced via a container command script
     properties.insert(
         "nifi.zookeeper.connect.string".to_string(),
-        "xxxxxx".to_string(),
+        "${env:ZOOKEEPER_HOSTS}".to_string(),
     );
     // this will be replaced via a container command script
-    properties.insert("nifi.zookeeper.root.node".to_string(), "xxxxxx".to_string());
+    properties.insert(
+        "nifi.zookeeper.root.node".to_string(),
+        "${env:ZOOKEEPER_CHROOT}".to_string(),
+    );
 
     // override with config overrides
     properties.extend(overrides);
@@ -593,8 +599,6 @@ pub fn build_nifi_properties(
 }
 
 pub fn build_state_management_xml() -> String {
-    // The "xxxxxx" Connect String is a placeholder and will be replaced via container
-    // command script
     format!(
         "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>
         <stateManagement>
@@ -609,8 +613,8 @@ pub fn build_state_management_xml() -> String {
           <cluster-provider>
             <id>zk-provider</id>
             <class>org.apache.nifi.controller.state.providers.zookeeper.ZooKeeperStateProvider</class>
-            <property name=\"Connect String\">xxxxxx</property>
-            <property name=\"Root Node\">yyyyyy</property>
+            <property name=\"Connect String\">${{env:ZOOKEEPER_HOSTS}}</property>
+            <property name=\"Root Node\">${{env:ZOOKEEPER_CHROOT}}</property>
             <property name=\"Session Timeout\">10 seconds</property>
             <property name=\"Access Control\">Open</property>
           </cluster-provider>

diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs
@@ -697,7 +697,9 @@ async fn build_node_rolegroup_config_map(
 ) -> Result<ConfigMap> {
     tracing::debug!("building rolegroup configmaps");
 
-    let (login_identity_provider_xml, authorizers_xml) = nifi_auth_config.get_auth_config();
+    let (login_identity_provider_xml, authorizers_xml) = nifi_auth_config
+        .get_auth_config()
+        .context(InvalidNifiAuthenticationConfigSnafu)?;
 
     let jvm_sec_props: BTreeMap<String, Option<String>> = rolegroup_config
         .get(&PropertyNameKind::File(
@@ -863,9 +865,6 @@ async fn build_node_rolegroup_statefulset(
     tracing::debug!("Building statefulset");
     let role_group = role.role_groups.get(&rolegroup_ref.role_group);
 
-    let zookeeper_host = "ZOOKEEPER_HOSTS";
-    let zookeeper_chroot = "ZOOKEEPER_CHROOT";
-
     // get env vars and env overrides
     let mut env_vars: Vec<EnvVar> = rolegroup_config
         .get(&PropertyNameKind::Env)
@@ -894,12 +893,12 @@ async fn build_node_rolegroup_statefulset(
     });
 
     env_vars.push(zookeeper_env_var(
-        zookeeper_host,
+        "ZOOKEEPER_HOSTS",
         &nifi.spec.cluster_config.zookeeper_config_map_name,
     ));
 
     env_vars.push(zookeeper_env_var(
-        zookeeper_chroot,
+        "ZOOKEEPER_CHROOT",
         &nifi.spec.cluster_config.zookeeper_config_map_name,
     ));
 
@@ -911,7 +910,7 @@ async fn build_node_rolegroup_statefulset(
             .metadata
             .namespace
             .as_ref()
-            .with_context(|| ObjectHasNoNamespaceSnafu {})?
+            .context(ObjectHasNoNamespaceSnafu)?
     );
 
     let sensitive_key_secret = &nifi.spec.cluster_config.sensitive_properties.key_secret;
@@ -943,24 +942,21 @@ async fn build_node_rolegroup_statefulset(
         "echo Replacing config directory".to_string(),
         "cp /conf/* /stackable/nifi/conf".to_string(),
         "ln -sf /stackable/log_config/logback.xml /stackable/nifi/conf/logback.xml".to_string(),
-        "echo Replacing nifi.cluster.node.address in nifi.properties".to_string(),
-        format!("sed -i \"s/nifi.cluster.node.address=/nifi.cluster.node.address={}/g\" /stackable/nifi/conf/nifi.properties", node_address),
-        "echo Replacing nifi.web.https.host in nifi.properties".to_string(),
-        format!("sed -i \"s/nifi.web.https.host=0.0.0.0/nifi.web.https.host={}/g\" /stackable/nifi/conf/nifi.properties", node_address),
-        "echo Replacing nifi.sensitive.props.key in nifi.properties".to_string(),
-        "sed -i \"s|nifi.sensitive.props.key=|nifi.sensitive.props.key=$(cat /stackable/sensitiveproperty/nifiSensitivePropsKey)|g\" /stackable/nifi/conf/nifi.properties".to_string(),
-        "echo Replacing 'nifi.zookeeper.connect.string=xxxxxx' in /stackable/nifi/conf/nifi.properties".to_string(),
-        format!("sed -i \"s|nifi.zookeeper.connect.string=xxxxxx|nifi.zookeeper.connect.string=${{{}}}|g\" /stackable/nifi/conf/nifi.properties", zookeeper_host),
-        "echo Replacing 'nifi.zookeeper.root.node=xxxxxx' in /stackable/nifi/conf/nifi.properties".to_string(),
-        format!("sed -i \"s|nifi.zookeeper.root.node=xxxxxx|nifi.zookeeper.root.node=${{{}}}|g\" /stackable/nifi/conf/nifi.properties", zookeeper_chroot),
-        "echo Replacing connect string 'xxxxxx' in /stackable/nifi/conf/state-management.xml".to_string(),
-        format!("sed -i \"s|xxxxxx|${{{}}}|g\" /stackable/nifi/conf/state-management.xml", zookeeper_host),
-        "echo Replacing root node 'yyyyyy' in /stackable/nifi/conf/state-management.xml".to_string(),
-        format!("sed -i \"s|yyyyyy|${{{}}}|g\" /stackable/nifi/conf/state-management.xml",zookeeper_chroot)
+        format!("export NODE_ADDRESS=\"{node_address}\""),
     ]);
 
+    // This commands needs to go first, as they might set env variables needed by the templating
     prepare_args.extend_from_slice(nifi_auth_config.get_additional_container_args().as_slice());
 
+    prepare_args.extend(vec![
+        "echo Templating config files".to_string(),
+        "config-utils template /stackable/nifi/conf/nifi.properties".to_string(),
+        "config-utils template /stackable/nifi/conf/state-management.xml".to_string(),
+        "config-utils template /stackable/nifi/conf/login-identity-providers.xml".to_string(),
+        "config-utils template /stackable/nifi/conf/authorizers.xml".to_string(),
+        "config-utils template /stackable/nifi/conf/security.properties".to_string(),
+    ]);
+
     let mut container_prepare =
         ContainerBuilder::new(&prepare_container_name).with_context(|_| {
             IllegalContainerNameSnafu {
@@ -1404,7 +1400,7 @@ async fn get_proxy_hosts(
                         .addresses
                         .unwrap_or_default()
                 })
-                .map(|node_address| format!("{}:{}", node_address.address, external_port)),
+                .map(|node_address| format!("{}:{external_port}", node_address.address)),
         );
     }
 

diff --git a/rust/operator-binary/src/reporting_task/mod.rs b/rust/operator-binary/src/reporting_task/mod.rs
@@ -46,7 +46,7 @@ use stackable_operator::{
 };
 
 use crate::security::{
-    authentication::{NifiAuthenticationConfig, STACKABLE_ADMIN_USER_NAME},
+    authentication::{NifiAuthenticationConfig, STACKABLE_ADMIN_USERNAME},
     build_tls_volume,
 };
 
@@ -249,7 +249,7 @@ fn build_reporting_task_job(
 
     let user_name_command = if admin_username_file.is_empty() {
         // In case of the username being simple (e.g admin for SingleUser) just use it as is
-        format!("-u {STACKABLE_ADMIN_USER_NAME}")
+        format!("-u {STACKABLE_ADMIN_USERNAME}")
     } else {
         // If the username is a bind dn (e.g. cn=integrationtest,ou=my users,dc=example,dc=org) we have to extract the cn/dn/uid (in this case integrationtest)
         format!(