Skip to content

Commit ca2fe91

Browse files
siegfriedwebersiegfriedweber
committed
chore: Revise the code and fix the tests
1 parent 0f97bf2 commit ca2fe91

20 files changed

Lines changed: 376 additions & 290 deletions

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ use crate::{
4040
crd::v1alpha1,
4141
framework::{
4242
HasName, HasUid, NameIsValidLabelValue,
43+
config_overrides::JsonConfigOverrides,
4344
product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig},
4445
role_utils::{GenericCommonConfig, RoleGroupConfig},
4546
types::{
@@ -149,7 +150,7 @@ impl ReconcilerError for Error {
149150
type OpenSearchRoleGroupConfig = RoleGroupConfig<
150151
ValidatedOpenSearchConfig,
151152
GenericCommonConfig,
152-
v1alpha1::OpenSearchConfigOverrides,
153+
ValidatedOpenSearchConfigOverrides,
153154
>;
154155

155156
type OpenSearchNodeResources =
@@ -173,6 +174,11 @@ pub struct ValidatedOpenSearchConfig {
173174
pub termination_grace_period_seconds: i64,
174175
}
175176

177+
#[derive(Clone, Debug, Default, PartialEq)]
178+
pub struct ValidatedOpenSearchConfigOverrides {
179+
pub opensearch_yml: JsonConfigOverrides,
180+
}
181+
176182
/// Validated log configuration per container
177183
#[derive(Clone, Debug, PartialEq)]
178184
pub struct ValidatedLogging {
@@ -505,7 +511,7 @@ mod tests {
505511
use crate::{
506512
controller::{
507513
OpenSearchNodeResources, ValidatedNodeRole, ValidatedNodeRoles,
508-
ValidatedOpenSearchConfig, ValidatedSecurity,
514+
ValidatedOpenSearchConfig, ValidatedOpenSearchConfigOverrides, ValidatedSecurity,
509515
},
510516
crd::v1alpha1,
511517
framework::{
@@ -661,7 +667,7 @@ mod tests {
661667
resources: OpenSearchNodeResources::default(),
662668
termination_grace_period_seconds: 120,
663669
},
664-
config_overrides: v1alpha1::OpenSearchConfigOverrides::default(),
670+
config_overrides: ValidatedOpenSearchConfigOverrides::default(),
665671
env_overrides: EnvVarSet::default(),
666672
cli_overrides: BTreeMap::default(),
667673
pod_overrides: PodTemplateSpec::default(),

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ mod tests {
8282
controller::{
8383
ContextNames, OpenSearchNodeResources, OpenSearchRoleGroupConfig, ValidatedCluster,
8484
ValidatedContainerLogConfigChoice, ValidatedDiscoveryEndpoint, ValidatedLogging,
85-
ValidatedNodeRole, ValidatedNodeRoles, ValidatedOpenSearchConfig, ValidatedSecurity,
85+
ValidatedNodeRole, ValidatedNodeRoles, ValidatedOpenSearchConfig,
86+
ValidatedOpenSearchConfigOverrides, ValidatedSecurity,
8687
},
8788
crd::v1alpha1,
8889
framework::{
@@ -245,7 +246,7 @@ mod tests {
245246
resources: OpenSearchNodeResources::default(),
246247
termination_grace_period_seconds: 120,
247248
},
248-
config_overrides: v1alpha1::OpenSearchConfigOverrides::default(),
249+
config_overrides: ValidatedOpenSearchConfigOverrides::default(),
249250
env_overrides: EnvVarSet::default(),
250251
cli_overrides: BTreeMap::default(),
251252
pod_overrides: PodTemplateSpec::default(),

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

Lines changed: 54 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -176,85 +176,64 @@ impl NodeConfig {
176176
/// Creates the main OpenSearch configuration file in YAML format
177177
pub fn opensearch_config_file_content(&self) -> String {
178178
serde_yaml::to_string(&self.opensearch_config())
179-
.expect("serde_json::Value should always be serializable")
179+
.expect("serde_json::Value should always be serializable as a string of YAML")
180180
}
181181

182182
pub fn opensearch_config(&self) -> serde_json::Value {
183183
let mut config = self.static_opensearch_config();
184184

185185
config.merge_from(self.tls_config());
186186

187-
let json_config_overrides: JsonConfigOverrides = self
187+
let overrides: JsonConfigOverrides = self
188188
.role_group_config
189189
.config_overrides
190190
.opensearch_yml
191-
.clone()
192-
.into();
191+
.clone();
193192

194-
json_config_overrides.apply(&config).into_owned()
193+
overrides.apply(&config).into_owned()
195194
}
196195

197196
/// Creates the main OpenSearch configuration file as JSON map
198197
///
199198
/// The file should only contain cluster-wide configuration options. Node-specific options
200199
/// should be defined as environment variables.
201200
pub fn static_opensearch_config(&self) -> serde_json::Value {
202-
let mut config = serde_json::Map::new();
203-
204-
config.insert(
205-
CONFIG_OPTION_CLUSTER_NAME.to_owned(),
206-
json!(self.cluster.name.to_string()),
207-
);
208-
config.insert(
209-
CONFIG_OPTION_NETWORK_HOST.to_owned(),
201+
let mut config = json!({
202+
CONFIG_OPTION_CLUSTER_NAME: self.cluster.name,
210203
// Bind to all interfaces because the IP address is not known in advance.
211-
json!("0.0.0.0".to_owned()),
212-
);
213-
config.insert(
214-
CONFIG_OPTION_DISCOVERY_TYPE.to_owned(),
215-
json!(self.discovery_type()),
216-
);
217-
config.insert
218-
// Accept certificates generated by the secret-operator
219-
(
220-
CONFIG_OPTION_PLUGINS_SECURITY_NODES_DN.to_owned(),
221-
json!(["CN=generated certificate for pod".to_owned()]),
222-
);
223-
config.insert(
224-
CONFIG_OPTION_NODE_ATTR_ROLE_GROUP.to_owned(),
225-
json!(self.role_group_name),
226-
);
227-
config.insert(
228-
CONFIG_OPTION_PATH_LOGS.to_owned(),
229-
json!(format!(
204+
CONFIG_OPTION_NETWORK_HOST: "0.0.0.0",
205+
CONFIG_OPTION_DISCOVERY_TYPE: self.discovery_type(),
206+
// Accept certificates generated by the secret-operator
207+
CONFIG_OPTION_PLUGINS_SECURITY_NODES_DN: [
208+
"CN=generated certificate for pod"
209+
],
210+
CONFIG_OPTION_NODE_ATTR_ROLE_GROUP: self.role_group_name,
211+
CONFIG_OPTION_PATH_LOGS: format!(
230212
"{STACKABLE_LOG_DIR}/{container}",
231213
container = v1alpha1::Container::OpenSearch.to_container_name()
232-
)),
233-
);
214+
),
215+
});
234216

235-
match self.role_group_security_mode {
217+
config.merge_from(match self.role_group_security_mode {
236218
RoleGroupSecurityMode::Initializing { .. } => {
237-
config.insert(
238-
CONFIG_OPTION_PLUGINS_SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX.to_owned(),
239-
json!(true),
240-
);
219+
json!({
220+
CONFIG_OPTION_PLUGINS_SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX: true
221+
})
241222
}
242223
RoleGroupSecurityMode::Managing { .. }
243224
| RoleGroupSecurityMode::Participating { .. } => {
244-
config.insert(
245-
CONFIG_OPTION_PLUGINS_SECURITY_AUTHCZ_ADMIN_DN.to_owned(),
246-
json!(self.super_admin_dn()),
247-
);
225+
json!({
226+
CONFIG_OPTION_PLUGINS_SECURITY_AUTHCZ_ADMIN_DN: self.super_admin_dn()
227+
})
248228
}
249229
RoleGroupSecurityMode::Disabled => {
250-
config.insert(
251-
CONFIG_OPTION_PLUGINS_SECURITY_DISABLED.to_owned(),
252-
json!(true),
253-
);
230+
json!({
231+
CONFIG_OPTION_PLUGINS_SECURITY_DISABLED: true
232+
})
254233
}
255-
};
234+
});
256235

257-
json!(config)
236+
config
258237
}
259238

260239
/// Distinguished name (DN) of the super admin certificate
@@ -264,7 +243,7 @@ impl NodeConfig {
264243
}
265244

266245
pub fn tls_config(&self) -> serde_json::Value {
267-
let mut config = serde_json::Map::new();
246+
let mut config = json!({});
268247

269248
let opensearch_path_conf = self.opensearch_path_conf();
270249

@@ -273,53 +252,32 @@ impl NodeConfig {
273252
.tls_internal_secret_class()
274253
.is_some()
275254
{
276-
config.insert(
277-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_ENABLED.to_owned(),
278-
json!(true),
279-
);
280-
config.insert(
281-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_PEMCERT_FILEPATH.to_owned(),
282-
json!(format!("{opensearch_path_conf}/tls/internal/tls.crt")),
283-
);
284-
config.insert(
285-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_PEMKEY_FILEPATH.to_owned(),
286-
json!(format!("{opensearch_path_conf}/tls/internal/tls.key")),
287-
);
288-
config.insert(
289-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_PEMTRUSTEDCAS_FILEPATH.to_owned(),
290-
json!(format!("{opensearch_path_conf}/tls/internal/ca.crt")),
291-
);
255+
config.merge_from(json!({
256+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_ENABLED: true,
257+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_PEMCERT_FILEPATH: format!("{opensearch_path_conf}/tls/internal/tls.crt"),
258+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_PEMKEY_FILEPATH: format!("{opensearch_path_conf}/tls/internal/tls.key"),
259+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_TRANSPORT_PEMTRUSTEDCAS_FILEPATH: format!("{opensearch_path_conf}/tls/internal/ca.crt"),
260+
}));
292261
}
293262

294263
if self
295264
.role_group_security_mode
296265
.tls_server_secret_class()
297266
.is_some()
298267
{
299-
config.insert(
300-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_ENABLED.to_owned(),
301-
json!(true),
302-
);
303-
config.insert(
304-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_PEMCERT_FILEPATH.to_owned(),
305-
json!(format!("{opensearch_path_conf}/tls/server/tls.crt")),
306-
);
307-
config.insert(
308-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_PEMKEY_FILEPATH.to_owned(),
309-
json!(format!("{opensearch_path_conf}/tls/server/tls.key")),
310-
);
311-
config.insert(
312-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_PEMTRUSTEDCAS_FILEPATH.to_owned(),
313-
json!(format!("{opensearch_path_conf}/tls/server/ca.crt")),
314-
);
268+
config.merge_from(json!({
269+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_ENABLED: true,
270+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_PEMCERT_FILEPATH: format!("{opensearch_path_conf}/tls/server/tls.crt"),
271+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_PEMKEY_FILEPATH: format!("{opensearch_path_conf}/tls/server/tls.key"),
272+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_PEMTRUSTEDCAS_FILEPATH: format!("{opensearch_path_conf}/tls/server/ca.crt"),
273+
}));
315274
} else {
316-
config.insert(
317-
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_ENABLED.to_owned(),
318-
json!(false),
319-
);
275+
config.merge_from(json!({
276+
CONFIG_OPTION_PLUGINS_SECURITY_SSL_HTTP_ENABLED: false
277+
}));
320278
}
321279

322-
json!(config)
280+
config
323281
}
324282

325283
/// Creates environment variables for the OpenSearch configurations
@@ -542,10 +500,12 @@ mod tests {
542500

543501
use super::*;
544502
use crate::{
545-
controller::{ValidatedLogging, ValidatedOpenSearchConfig, ValidatedSecurity},
503+
controller::{
504+
ValidatedLogging, ValidatedOpenSearchConfig, ValidatedOpenSearchConfigOverrides,
505+
ValidatedSecurity,
506+
},
546507
crd::v1alpha1,
547508
framework::{
548-
config_overrides::KeyValueConfigOverrides,
549509
product_logging::framework::ValidatedContainerLogConfigChoice,
550510
role_utils::GenericCommonConfig,
551511
types::{
@@ -559,15 +519,15 @@ mod tests {
559519

560520
struct TestConfig {
561521
replicas: u16,
562-
config_settings: &'static [(&'static str, &'static str)],
522+
config_settings: serde_json::Value,
563523
env_vars: &'static [(&'static str, &'static str)],
564524
}
565525

566526
impl Default for TestConfig {
567527
fn default() -> Self {
568528
Self {
569529
replicas: 3,
570-
config_settings: &[],
530+
config_settings: json!({}),
571531
env_vars: &[],
572532
}
573533
}
@@ -603,16 +563,8 @@ mod tests {
603563
resources: Resources::default(),
604564
termination_grace_period_seconds: 30,
605565
},
606-
config_overrides: v1alpha1::OpenSearchConfigOverrides {
607-
opensearch_yml: v1alpha1::ConfigOverridesChoice::KeyValue(
608-
KeyValueConfigOverrides {
609-
overrides: test_config
610-
.config_settings
611-
.iter()
612-
.map(|(k, v)| (k.to_string(), Some(v.to_string())))
613-
.collect(),
614-
},
615-
),
566+
config_overrides: ValidatedOpenSearchConfigOverrides {
567+
opensearch_yml: JsonConfigOverrides::JsonMergePatch(test_config.config_settings),
616568
},
617569
env_overrides: EnvVarSet::new().with_values(
618570
test_config
@@ -693,7 +645,7 @@ mod tests {
693645
#[test]
694646
pub fn test_static_opensearch_config_file() {
695647
let node_config = node_config(TestConfig {
696-
config_settings: &[("test", "value")],
648+
config_settings: json!({"test": "value"}),
697649
..TestConfig::default()
698650
});
699651

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,9 @@ impl<'a> RoleBuilder<'a> {
225225
{
226226
data.insert(
227227
file_type.filename.to_owned(),
228-
serde_yaml::to_string(value)
229-
.expect("serde_json::Value should be serializable"),
228+
serde_yaml::to_string(value).expect(
229+
"serde_json::Value should always be serializable as a string of YAML",
230+
),
230231
);
231232
}
232233
}
@@ -386,7 +387,8 @@ mod tests {
386387
controller::{
387388
ContextNames, OpenSearchRoleGroupConfig, ValidatedCluster,
388389
ValidatedContainerLogConfigChoice, ValidatedDiscoveryEndpoint, ValidatedLogging,
389-
ValidatedNodeRole, ValidatedOpenSearchConfig, ValidatedSecurity,
390+
ValidatedNodeRole, ValidatedOpenSearchConfig, ValidatedOpenSearchConfigOverrides,
391+
ValidatedSecurity,
390392
build::role_builder::{
391393
discovery_config_map_name, discovery_service_listener_name, seed_nodes_service_name,
392394
},
@@ -447,7 +449,7 @@ mod tests {
447449
resources: Resources::default(),
448450
termination_grace_period_seconds: 30,
449451
},
450-
config_overrides: v1alpha1::OpenSearchConfigOverrides::default(),
452+
config_overrides: ValidatedOpenSearchConfigOverrides::default(),
451453
env_overrides: EnvVarSet::default(),
452454
cli_overrides: BTreeMap::default(),
453455
pod_overrides: PodTemplateSpec::default(),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,7 +1465,7 @@ mod tests {
14651465
controller::{
14661466
ContextNames, OpenSearchRoleGroupConfig, ValidatedCluster,
14671467
ValidatedContainerLogConfigChoice, ValidatedLogging, ValidatedNodeRole,
1468-
ValidatedOpenSearchConfig, ValidatedSecurity,
1468+
ValidatedOpenSearchConfig, ValidatedOpenSearchConfigOverrides, ValidatedSecurity,
14691469
build::role_group_builder::{
14701470
DISCOVERY_SERVICE_LISTENER_VOLUME_NAME, OPENSEARCH_KEYSTORE_VOLUME_NAME,
14711471
TLS_INTERNAL_VOLUME_NAME, TLS_SERVER_CA_VOLUME_NAME, TLS_SERVER_VOLUME_NAME,
@@ -1583,7 +1583,7 @@ mod tests {
15831583
resources: Resources::default(),
15841584
termination_grace_period_seconds: 30,
15851585
},
1586-
config_overrides: v1alpha1::OpenSearchConfigOverrides::default(),
1586+
config_overrides: ValidatedOpenSearchConfigOverrides::default(),
15871587
env_overrides: EnvVarSet::default(),
15881588
cli_overrides: BTreeMap::default(),
15891589
pod_overrides: PodTemplateSpec::default(),

0 commit comments

Comments
 (0)