Skip to content

Commit e691312

Browse files
maltesanderadwk67
andauthored
refactor: extract dereference/validate pipeline from reconcile (#935)
* feat: add validate and dereference steps * fix: update crate hashes * feat: add cluster snapshot tests * docs: adapted changelog * fix: add zookeeperless dimension to smoke snapshot * Update rust/operator-binary/src/security/authentication.rs Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com> * fix: move compute_proxy_hosts to validate as no async call required * fix: adjust proxy host comment --------- Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com>
1 parent 9b8f1ad commit e691312

12 files changed

Lines changed: 965 additions & 339 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ All notable changes to this project will be documented in this file.
1616
- BREAKING: `configOverrides` now only accepts `bootstrap.conf`, `nifi.properties` and `security.properties`.
1717
Previously, arbitrary keys were silently accepted but ignored ([#921]).
1818
- Bump `stackable-operator` to 0.110.1 and `kube` to 3.1.0 ([#921]).
19+
- Internal operator refactoring: introduce dereference() and validate() steps in the reconciler ([#935]).
1920

2021
### Fixed
2122

@@ -27,6 +28,7 @@ All notable changes to this project will be documented in this file.
2728
[#922]: https://github.com/stackabletech/nifi-operator/pull/922
2829
[#924]: https://github.com/stackabletech/nifi-operator/pull/924
2930
[#928]: https://github.com/stackabletech/nifi-operator/pull/928
31+
[#935]: https://github.com/stackabletech/nifi-operator/pull/935
3032

3133
## [26.3.0] - 2026-03-16
3234

Cargo.nix

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate-hashes.json

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/operator-binary/src/controller.rs

Lines changed: 36 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use std::{
44
borrow::Cow,
5-
collections::{BTreeMap, HashMap, HashSet},
5+
collections::{BTreeMap, HashMap},
66
sync::Arc,
77
};
88

@@ -27,10 +27,7 @@ use stackable_operator::{
2727
cli::OperatorEnvironmentOptions,
2828
client::Client,
2929
cluster_resources::{ClusterResourceApplyStrategy, ClusterResources},
30-
commons::{
31-
product_image_selection::{self, ResolvedProductImage},
32-
rbac::build_rbac_resources,
33-
},
30+
commons::{product_image_selection::ResolvedProductImage, rbac::build_rbac_resources},
3431
constants::RESTART_CONTROLLER_ENABLED_LABEL,
3532
crd::{authentication::oidc::v1alpha1::AuthenticationProvider, git_sync},
3633
k8s_openapi::{
@@ -75,20 +72,21 @@ use stackable_operator::{
7572
use strum::{EnumDiscriminants, IntoStaticStr};
7673
use tracing::Instrument;
7774

75+
mod dereference;
76+
mod validate;
77+
7878
use crate::{
7979
OPERATOR_NAME,
8080
config::{
8181
self, JVM_SECURITY_PROPERTIES_FILE, NIFI_BOOTSTRAP_CONF, NIFI_CONFIG_DIRECTORY,
8282
NIFI_PROPERTIES, NIFI_PYTHON_WORKING_DIRECTORY, NIFI_STATE_MANAGEMENT_XML, NifiRepository,
8383
build_bootstrap_conf, build_nifi_properties, build_state_management_xml,
84-
validated_product_config,
8584
},
8685
crd::{
8786
APP_NAME, BALANCE_PORT, BALANCE_PORT_NAME, Container, HTTPS_PORT, HTTPS_PORT_NAME,
8887
METRICS_PORT, METRICS_PORT_NAME, NifiConfig, NifiNodeRoleConfig, NifiRole, NifiRoleType,
8988
NifiStatus, PROTOCOL_PORT, PROTOCOL_PORT_NAME, STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR,
90-
authentication::AuthenticationClassResolved, authorization::NifiAccessPolicyProvider,
91-
v1alpha1,
89+
authorization::NifiAccessPolicyProvider, v1alpha1,
9290
},
9391
listener::{
9492
LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener, build_group_listener_pvc,
@@ -100,7 +98,7 @@ use crate::{
10098
upgrade::{self, ClusterVersionUpdateState},
10199
},
102100
product_logging::extend_role_group_config_map,
103-
reporting_task::{self, build_maybe_reporting_task, build_reporting_task_service_name},
101+
reporting_task::{build_maybe_reporting_task, build_reporting_task_service_name},
104102
security::{
105103
authentication::{
106104
AUTHORIZERS_XML_FILE_NAME, LOGIN_IDENTITY_PROVIDERS_XML_FILE_NAME,
@@ -143,6 +141,12 @@ pub enum Error {
143141
#[snafu(display("object defines no namespace"))]
144142
ObjectHasNoNamespace,
145143

144+
#[snafu(display("failed to dereference resources"))]
145+
Dereference { source: dereference::Error },
146+
147+
#[snafu(display("failed to validate cluster"))]
148+
ValidateCluster { source: validate::Error },
149+
146150
#[snafu(display("failed to create cluster resources"))]
147151
CreateClusterResources {
148152
source: stackable_operator::cluster_resources::Error,
@@ -200,12 +204,6 @@ pub enum Error {
200204
source: stackable_operator::builder::meta::Error,
201205
},
202206

203-
#[snafu(display("Failed to load product config"))]
204-
ProductConfigLoadFailed {
205-
#[snafu(source(from(config::Error, Box::new)))]
206-
source: Box<config::Error>,
207-
},
208-
209207
#[snafu(display("Failed to find information about file [{}] in product config", kind))]
210208
ProductConfigKindNotSpecified { kind: String },
211209

@@ -277,11 +275,6 @@ pub enum Error {
277275
source: crate::security::authorization::Error,
278276
},
279277

280-
#[snafu(display("Failed to resolve NiFi Authentication Configuration"))]
281-
FailedResolveNifiAuthenticationConfig {
282-
source: crate::crd::authentication::Error,
283-
},
284-
285278
#[snafu(display("failed to create PodDisruptionBudget"))]
286279
FailedToCreatePdb {
287280
source: crate::operations::pdb::Error,
@@ -348,11 +341,6 @@ pub enum Error {
348341

349342
#[snafu(display("failed to build authorization configuration"))]
350343
AuthorizationConfiguration { source: authorization::Error },
351-
352-
#[snafu(display("failed to resolve product image"))]
353-
ResolveProductImage {
354-
source: product_image_selection::Error,
355-
},
356344
}
357345

358346
type Result<T, E = Error> = std::result::Result<T, E>;
@@ -376,15 +364,26 @@ pub async fn reconcile_nifi(
376364

377365
let client = &ctx.client;
378366

379-
let resolved_product_image = nifi
380-
.spec
381-
.image
382-
.resolve(
383-
CONTAINER_IMAGE_BASE_NAME,
384-
&ctx.operator_environment.image_repository,
385-
crate::built_info::PKG_VERSION,
386-
)
387-
.context(ResolveProductImageSnafu)?;
367+
// dereference (client required)
368+
let dereferenced_objects = dereference::dereference(client, nifi)
369+
.await
370+
.context(DereferenceSnafu)?;
371+
372+
// validate (no Kubernetes API calls required)
373+
let validated = validate::validate(
374+
nifi,
375+
&dereferenced_objects,
376+
&ctx.operator_environment,
377+
&ctx.product_config,
378+
&client.kubernetes_cluster_info,
379+
)
380+
.context(ValidateClusterSnafu)?;
381+
382+
let resolved_product_image = validated.image;
383+
let authentication_config = validated.authentication_config;
384+
let authorization_config = validated.authorization_config;
385+
let validated_config = validated.validated_role_config;
386+
let proxy_hosts = validated.proxy_hosts;
388387

389388
tracing::info!("Checking for sensitive key configuration");
390389
check_or_generate_sensitive_key(client, nifi)
@@ -419,14 +418,6 @@ pub async fn reconcile_nifi(
419418
}
420419
// end todo
421420

422-
let validated_config = validated_product_config(
423-
nifi,
424-
&resolved_product_image.product_version,
425-
nifi.spec.nodes.as_ref().context(NoNodesDefinedSnafu)?,
426-
&ctx.product_config,
427-
)
428-
.context(ProductConfigLoadFailedSnafu)?;
429-
430421
let mut cluster_resources = ClusterResources::new(
431422
APP_NAME,
432423
OPERATOR_NAME,
@@ -442,30 +433,12 @@ pub async fn reconcile_nifi(
442433
.map(Cow::Borrowed)
443434
.unwrap_or_default();
444435

445-
let authentication_config = NifiAuthenticationConfig::try_from(
446-
AuthenticationClassResolved::from(nifi, client)
447-
.await
448-
.context(FailedResolveNifiAuthenticationConfigSnafu)?,
449-
)
450-
.context(InvalidNifiAuthenticationConfigSnafu)?;
451-
452436
if let NifiAuthenticationConfig::Oidc { .. } = authentication_config {
453437
check_or_generate_oidc_admin_password(client, nifi)
454438
.await
455439
.context(SecuritySnafu)?;
456440
}
457441

458-
let authorization_config = ResolvedNifiAuthorizationConfig::from(
459-
&nifi.spec.cluster_config.authorization,
460-
client,
461-
nifi.metadata
462-
.namespace
463-
.as_deref()
464-
.context(ObjectHasNoNamespaceSnafu)?,
465-
)
466-
.await
467-
.context(InvalidNifiAuthorizationConfigSnafu)?;
468-
469442
let (rbac_sa, rbac_rolebinding) = build_rbac_resources(
470443
nifi,
471444
APP_NAME,
@@ -530,13 +503,9 @@ pub async fn reconcile_nifi(
530503

531504
let role = nifi.spec.nodes.as_ref().context(NoNodesDefinedSnafu)?;
532505

533-
// This is due to the fact that users might access NiFi via these addresses, if they try to
534-
// connect from an external machine (not inside the k8s overlay network).
535-
// Since we cannot predict which of the addresses a user might decide to use we will simply
536-
// add all of them to the setting for now.
506+
// The proxy hosts allow-list lets external users access NiFi via addresses we cannot
507+
// predict, so all of them are added to the setting.
537508
// For more information see <https://nifi.apache.org/docs/nifi-docs/html/administration-guide.html#proxy_configuration>
538-
let proxy_hosts = get_proxy_hosts(client, nifi, &resolved_product_image).await?;
539-
540509
let rg_configmap = build_node_rolegroup_config_map(
541510
nifi,
542511
&resolved_product_image,
@@ -738,9 +707,7 @@ async fn build_node_rolegroup_config_map(
738707
.get_authentication_config()
739708
.context(InvalidNifiAuthenticationConfigSnafu)?;
740709

741-
let authorizers_xml = authorization_config
742-
.get_authorizers_config(nifi)
743-
.context(InvalidNifiAuthorizationConfigSnafu)?;
710+
let authorizers_xml = authorization_config.get_authorizers_config(nifi);
744711

745712
let jvm_sec_props: BTreeMap<String, Option<String>> = rolegroup_config
746713
.get(&PropertyNameKind::File(
@@ -1534,48 +1501,6 @@ fn get_volume_claim_templates(
15341501
Ok(pvcs)
15351502
}
15361503

1537-
async fn get_proxy_hosts(
1538-
client: &Client,
1539-
nifi: &v1alpha1::NifiCluster,
1540-
resolved_product_image: &ResolvedProductImage,
1541-
) -> Result<String> {
1542-
let host_header_check = nifi.spec.cluster_config.host_header_check.clone();
1543-
1544-
if host_header_check.allow_all {
1545-
tracing::info!(
1546-
"spec.clusterConfig.hostHeaderCheck.allowAll is set to true. All proxy hosts will be allowed."
1547-
);
1548-
if !host_header_check.additional_allowed_hosts.is_empty() {
1549-
tracing::info!(
1550-
"spec.clusterConfig.hostHeaderCheck.additionalAllowedHosts is ignored and only '*' is added to the allow-list."
1551-
)
1552-
}
1553-
return Ok("*".to_string());
1554-
}
1555-
1556-
// Address and port are injected from the listener volume during the prepare container
1557-
let mut proxy_hosts = HashSet::from([
1558-
"${env:LISTENER_DEFAULT_ADDRESS}:${env:LISTENER_DEFAULT_PORT_HTTPS}".to_string(),
1559-
]);
1560-
proxy_hosts.extend(host_header_check.additional_allowed_hosts);
1561-
1562-
// Reporting task only exists for NiFi 1.x
1563-
if resolved_product_image.product_version.starts_with("1.") {
1564-
let reporting_task_service_name = reporting_task::build_reporting_task_fqdn_service_name(
1565-
nifi,
1566-
&client.kubernetes_cluster_info,
1567-
)
1568-
.context(ReportingTaskSnafu)?;
1569-
1570-
proxy_hosts.insert(format!("{reporting_task_service_name}:{HTTPS_PORT}"));
1571-
}
1572-
1573-
let mut proxy_hosts = Vec::from_iter(proxy_hosts);
1574-
proxy_hosts.sort();
1575-
1576-
Ok(proxy_hosts.join(","))
1577-
}
1578-
15791504
pub fn error_policy(
15801505
_obj: Arc<DeserializeGuard<v1alpha1::NifiCluster>>,
15811506
error: &Error,

0 commit comments

Comments
 (0)