Skip to content

Commit 45dfd7c

Browse files
chore: Use safe types already in the CRD
1 parent 752136e commit 45dfd7c

10 files changed

Lines changed: 223 additions & 72 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ built = { version = "0.8.0", features = ["chrono", "git2"] }
1616
clap = "4.5"
1717
futures = { version = "0.3", features = ["compat"] }
1818
pretty_assertions = "1.4"
19-
regex = "1.11.3"
19+
regex = "1.11"
2020
rstest = "0.26"
2121
schemars = { version = "1.0.0", features = ["url2"] } # same as in operator-rs
2222
serde = { version = "1.0", features = ["derive"] }

deploy/helm/opensearch-operator/crds/crds.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ spec:
3838
It must contain the key `ADDRESS` with the address of the Vector aggregator.
3939
Follow the [logging tutorial](https://docs.stackable.tech/home/nightly/tutorials/logging-vector-aggregator)
4040
to learn how to configure log aggregation with Vector.
41+
maxLength: 253
42+
minLength: 1
4143
nullable: true
4244
type: string
4345
type: object
@@ -183,6 +185,8 @@ spec:
183185
type: string
184186
listenerClass:
185187
description: This field controls which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) is used to expose the HTTP communication.
188+
maxLength: 253
189+
minLength: 1
186190
nullable: true
187191
type: string
188192
logging:
@@ -526,6 +530,8 @@ spec:
526530
type: string
527531
listenerClass:
528532
description: This field controls which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) is used to expose the HTTP communication.
533+
maxLength: 253
534+
minLength: 1
529535
nullable: true
530536
type: string
531537
logging:

rust/operator-binary/src/controller.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ use crate::{
3333
v1alpha1::{self},
3434
},
3535
framework::{
36-
ClusterName, ControllerName, HasName, HasUid, NameIsValidLabelValue, NamespaceName,
37-
OperatorName, ProductName, ProductVersion, RoleGroupName, RoleName, Uid,
36+
ClusterName, ControllerName, HasName, HasUid, ListenerClassName, NameIsValidLabelValue,
37+
NamespaceName, OperatorName, ProductName, ProductVersion, RoleGroupName, RoleName, Uid,
3838
product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig},
3939
role_utils::{GenericProductSpecificCommonConfig, RoleGroupConfig},
4040
},
@@ -128,7 +128,7 @@ type OpenSearchNodeResources =
128128
#[derive(Clone, Debug, PartialEq)]
129129
pub struct ValidatedOpenSearchConfig {
130130
pub affinity: StackableAffinity,
131-
pub listener_class: String,
131+
pub listener_class: ListenerClassName,
132132
pub logging: ValidatedLogging,
133133
pub node_roles: NodeRoles,
134134
pub resources: OpenSearchNodeResources,
@@ -379,8 +379,8 @@ mod tests {
379379
controller::{OpenSearchNodeResources, ValidatedOpenSearchConfig},
380380
crd::{NodeRoles, v1alpha1},
381381
framework::{
382-
ClusterName, NamespaceName, OperatorName, ProductVersion, RoleGroupName,
383-
builder::pod::container::EnvVarSet,
382+
ClusterName, ListenerClassName, NamespaceName, OperatorName, ProductVersion,
383+
RoleGroupName, builder::pod::container::EnvVarSet,
384384
product_logging::framework::ValidatedContainerLogConfigChoice,
385385
role_utils::GenericProductSpecificCommonConfig,
386386
},
@@ -504,7 +504,7 @@ mod tests {
504504
replicas,
505505
config: ValidatedOpenSearchConfig {
506506
affinity: StackableAffinity::default(),
507-
listener_class: "external-stable".to_owned(),
507+
listener_class: ListenerClassName::from_str_unsafe("external-stable"),
508508
logging: ValidatedLogging {
509509
opensearch_container: ValidatedContainerLogConfigChoice::Automatic(
510510
AutomaticContainerLogConfig::default(),

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ mod tests {
7979
},
8080
crd::{NodeRoles, v1alpha1},
8181
framework::{
82-
ClusterName, ControllerName, NamespaceName, OperatorName, ProductName, ProductVersion,
83-
RoleGroupName, builder::pod::container::EnvVarSet,
82+
ClusterName, ControllerName, ListenerClassName, NamespaceName, OperatorName,
83+
ProductName, ProductVersion, RoleGroupName, builder::pod::container::EnvVarSet,
8484
role_utils::GenericProductSpecificCommonConfig,
8585
},
8686
};
@@ -202,7 +202,7 @@ mod tests {
202202
replicas,
203203
config: ValidatedOpenSearchConfig {
204204
affinity: StackableAffinity::default(),
205-
listener_class: "external-stable".to_owned(),
205+
listener_class: ListenerClassName::from_str_unsafe("external-stable"),
206206
logging: ValidatedLogging {
207207
opensearch_container: ValidatedContainerLogConfigChoice::Automatic(
208208
AutomaticContainerLogConfig::default(),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ mod tests {
285285
controller::{ValidatedLogging, ValidatedOpenSearchConfig},
286286
crd::NodeRoles,
287287
framework::{
288-
ClusterName, NamespaceName, ProductVersion, RoleGroupName,
288+
ClusterName, ListenerClassName, NamespaceName, ProductVersion, RoleGroupName,
289289
product_logging::framework::ValidatedContainerLogConfigChoice,
290290
role_utils::GenericProductSpecificCommonConfig,
291291
},
@@ -315,7 +315,7 @@ mod tests {
315315
replicas: test_config.replicas,
316316
config: ValidatedOpenSearchConfig {
317317
affinity: StackableAffinity::default(),
318-
listener_class: "cluster-internal".to_string(),
318+
listener_class: ListenerClassName::from_str_unsafe("cluster-internal"),
319319
logging: ValidatedLogging {
320320
opensearch_container: ValidatedContainerLogConfigChoice::Automatic(
321321
AutomaticContainerLogConfig::default(),

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ mod tests {
241241
},
242242
crd::{NodeRoles, v1alpha1},
243243
framework::{
244-
ClusterName, ControllerName, NamespaceName, OperatorName, ProductName, ProductVersion,
245-
RoleGroupName, builder::pod::container::EnvVarSet,
244+
ClusterName, ControllerName, ListenerClassName, NamespaceName, OperatorName,
245+
ProductName, ProductVersion, RoleGroupName, builder::pod::container::EnvVarSet,
246246
role_utils::GenericProductSpecificCommonConfig,
247247
},
248248
};
@@ -263,7 +263,7 @@ mod tests {
263263
replicas: 1,
264264
config: ValidatedOpenSearchConfig {
265265
affinity: StackableAffinity::default(),
266-
listener_class: "cluster-internal".to_string(),
266+
listener_class: ListenerClassName::from_str_unsafe("cluster-internal"),
267267
logging: ValidatedLogging {
268268
opensearch_container: ValidatedContainerLogConfigChoice::Automatic(
269269
AutomaticContainerLogConfig::default(),

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ impl<'a> RoleGroupBuilder<'a> {
569569
listener::v1alpha1::Listener {
570570
metadata,
571571
spec: listener::v1alpha1::ListenerSpec {
572-
class_name: Some(listener_class),
572+
class_name: Some(listener_class.to_string()),
573573
ports: Some(ports.to_vec()),
574574
..listener::v1alpha1::ListenerSpec::default()
575575
},
@@ -653,9 +653,9 @@ mod tests {
653653
},
654654
crd::{NodeRoles, v1alpha1},
655655
framework::{
656-
ClusterName, ConfigMapName, ControllerName, NamespaceName, OperatorName, ProductName,
657-
ProductVersion, RoleGroupName, ServiceAccountName, ServiceName,
658-
builder::pod::container::EnvVarSet,
656+
ClusterName, ConfigMapName, ControllerName, ListenerClassName, NamespaceName,
657+
OperatorName, ProductName, ProductVersion, RoleGroupName, ServiceAccountName,
658+
ServiceName, builder::pod::container::EnvVarSet,
659659
product_logging::framework::VectorContainerLogConfig,
660660
role_utils::GenericProductSpecificCommonConfig,
661661
},
@@ -693,7 +693,7 @@ mod tests {
693693
replicas: 1,
694694
config: ValidatedOpenSearchConfig {
695695
affinity: StackableAffinity::default(),
696-
listener_class: "cluster-internal".to_string(),
696+
listener_class: ListenerClassName::from_str_unsafe("cluster-internal"),
697697
logging: ValidatedLogging {
698698
opensearch_container: ValidatedContainerLogConfigChoice::Automatic(
699699
AutomaticContainerLogConfig::default(),

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

Lines changed: 22 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,6 @@ pub enum Error {
6464
#[snafu(display("failed to set role-group name"))]
6565
ParseRoleGroupName { source: crate::framework::Error },
6666

67-
#[snafu(display("failed to set vectorAggregatorConfigMapName"))]
68-
ParseVectorAggregatorConfigMapName { source: crate::framework::Error },
69-
7067
#[snafu(display("failed to resolve product image"))]
7168
ResolveProductImage {
7269
source: stackable_operator::commons::product_image_selection::Error,
@@ -166,22 +163,12 @@ fn validate_role_group_config(
166163
)
167164
.context(ValidateOpenSearchConfigSnafu)?;
168165

169-
let vector_aggregator_config_map_name = if let Some(config_map_name) = &cluster
170-
.spec
171-
.cluster_config
172-
.vector_aggregator_config_map_name
173-
{
174-
Some(
175-
ConfigMapName::from_str(config_map_name)
176-
.context(ParseVectorAggregatorConfigMapNameSnafu)?,
177-
)
178-
} else {
179-
None
180-
};
181-
182166
let logging = validate_logging_configuration(
183167
&merged_role_group.config.config.logging,
184-
vector_aggregator_config_map_name,
168+
&cluster
169+
.spec
170+
.cluster_config
171+
.vector_aggregator_config_map_name,
185172
)?;
186173

187174
let graceful_shutdown_timeout = merged_role_group.config.config.graceful_shutdown_timeout;
@@ -223,15 +210,16 @@ fn validate_role_group_config(
223210

224211
fn validate_logging_configuration(
225212
logging: &Logging<v1alpha1::Container>,
226-
vector_aggregator_config_map_name: Option<ConfigMapName>,
213+
vector_aggregator_config_map_name: &Option<ConfigMapName>,
227214
) -> Result<ValidatedLogging> {
228215
let opensearch_container =
229216
validate_logging_configuration_for_container(logging, v1alpha1::Container::OpenSearch)
230217
.context(ValidateLoggingConfigSnafu)?;
231218

232219
let vector_container = if logging.enable_vector_agent {
233-
let vector_aggregator_config_map_name =
234-
vector_aggregator_config_map_name.context(GetVectorAggregatorConfigMapNameSnafu)?;
220+
let vector_aggregator_config_map_name = vector_aggregator_config_map_name
221+
.clone()
222+
.context(GetVectorAggregatorConfigMapNameSnafu)?;
235223
Some(VectorContainerLogConfig {
236224
log_config: validate_logging_configuration_for_container(
237225
logging,
@@ -288,8 +276,8 @@ mod tests {
288276
v1alpha1::{self},
289277
},
290278
framework::{
291-
ClusterName, ConfigMapName, ControllerName, NamespaceName, OperatorName, ProductName,
292-
ProductVersion, RoleGroupName,
279+
ClusterName, ConfigMapName, ControllerName, ListenerClassName, NamespaceName,
280+
OperatorName, ProductName, ProductVersion, RoleGroupName,
293281
builder::pod::container::{EnvVarName, EnvVarSet},
294282
product_logging::framework::{
295283
ValidatedContainerLogConfigChoice, VectorContainerLogConfig,
@@ -360,7 +348,9 @@ mod tests {
360348
}),
361349
..StackableAffinity::default()
362350
},
363-
listener_class: "listener-class-from-role-group-level".to_owned(),
351+
listener_class: ListenerClassName::from_str_unsafe(
352+
"listener-class-from-role-group-level"
353+
),
364354
logging: ValidatedLogging {
365355
opensearch_container: ValidatedContainerLogConfigChoice::Automatic(
366356
AutomaticContainerLogConfig {
@@ -580,19 +570,6 @@ mod tests {
580570
);
581571
}
582572

583-
#[test]
584-
fn test_validate_err_parse_vector_aggregator_config_map_name() {
585-
test_validate_err(
586-
|cluster| {
587-
cluster
588-
.spec
589-
.cluster_config
590-
.vector_aggregator_config_map_name = Some("invalid ConfigMap name".to_owned())
591-
},
592-
ErrorDiscriminants::ParseVectorAggregatorConfigMapName,
593-
);
594-
}
595-
596573
#[test]
597574
fn test_validate_err_validate_logging_config() {
598575
test_validate_err(
@@ -685,14 +662,18 @@ mod tests {
685662
image: serde_json::from_str(r#"{"productVersion": "3.1.0"}"#)
686663
.expect("should be a valid ProductImage structure"),
687664
cluster_config: v1alpha1::OpenSearchClusterConfig {
688-
vector_aggregator_config_map_name: Some("vector-aggregator".to_owned()),
665+
vector_aggregator_config_map_name: Some(ConfigMapName::from_str_unsafe(
666+
"vector-aggregator",
667+
)),
689668
},
690669
cluster_operation: ClusterOperation::default(),
691670
nodes: Role {
692671
config: CommonConfiguration {
693672
config: v1alpha1::OpenSearchConfigFragment {
694673
graceful_shutdown_timeout: Some(Duration::from_minutes_unchecked(5)),
695-
listener_class: Some("listener-class-from-role-level".to_owned()),
674+
listener_class: Some(ListenerClassName::from_str_unsafe(
675+
"listener-class-from-role-level",
676+
)),
696677
logging: LoggingFragment {
697678
enable_vector_agent: Some(true),
698679
containers: BTreeMap::default(),
@@ -740,9 +721,9 @@ mod tests {
740721
RoleGroup {
741722
config: CommonConfiguration {
742723
config: v1alpha1::OpenSearchConfigFragment {
743-
listener_class: Some(
744-
"listener-class-from-role-group-level".to_owned(),
745-
),
724+
listener_class: Some(ListenerClassName::from_str_unsafe(
725+
"listener-class-from-role-group-level",
726+
)),
746727
..v1alpha1::OpenSearchConfigFragment::default()
747728
},
748729
config_overrides: [(

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,15 @@ use stackable_operator::{
2626
};
2727
use strum::{Display, EnumIter};
2828

29-
use crate::framework::{
30-
ClusterName, ContainerName, NameIsValidLabelValue, ProductName, RoleName,
31-
role_utils::GenericProductSpecificCommonConfig,
29+
use crate::{
30+
constant,
31+
framework::{
32+
ClusterName, ConfigMapName, ContainerName, ListenerClassName, NameIsValidLabelValue,
33+
ProductName, RoleName, role_utils::GenericProductSpecificCommonConfig,
34+
},
3235
};
3336

34-
const DEFAULT_LISTENER_CLASS: &str = "cluster-internal";
37+
constant!(DEFAULT_LISTENER_CLASS: ListenerClassName = "cluster-internal");
3538

3639
#[versioned(
3740
version(name = "v1alpha1"),
@@ -82,7 +85,7 @@ pub mod versioned {
8285
/// Follow the [logging tutorial](DOCS_BASE_URL_PLACEHOLDER/tutorials/logging-vector-aggregator)
8386
/// to learn how to configure log aggregation with Vector.
8487
#[serde(skip_serializing_if = "Option::is_none")]
85-
pub vector_aggregator_config_map_name: Option<String>,
88+
pub vector_aggregator_config_map_name: Option<ConfigMapName>,
8689
}
8790

8891
// The possible node roles are by default the built-in roles and the search role, see
@@ -151,7 +154,7 @@ pub mod versioned {
151154

152155
/// This field controls which [ListenerClass](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass.html) is used to expose the HTTP communication.
153156
#[fragment_attrs(serde(default))]
154-
pub listener_class: String,
157+
pub listener_class: ListenerClassName,
155158

156159
#[fragment_attrs(serde(default))]
157160
pub logging: Logging<Container>,
@@ -257,7 +260,7 @@ impl v1alpha1::OpenSearchConfig {
257260
),
258261
// Defaults taken from the Helm chart, see
259262
// https://github.com/opensearch-project/helm-charts/blob/opensearch-3.0.0/charts/opensearch/values.yaml#L16-L20
260-
listener_class: Some(DEFAULT_LISTENER_CLASS.to_string()),
263+
listener_class: Some(DEFAULT_LISTENER_CLASS.to_owned()),
261264
logging: product_logging::spec::default_logging(),
262265
node_roles: Some(NodeRoles(vec![
263266
v1alpha1::NodeRole::ClusterManager,

0 commit comments

Comments
 (0)