address feedback on PR

Author: labrenbe

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

@@ -40,6 +40,8 @@ spec:
                         properties:
                           key:
                             description: Key in the OpenSearch keystore
+                            minLength: 1
+                            pattern: ^[A-Za-z0-9_\-.]+$
                             type: string
                           secretKeyRef:
                             description: Reference to the Secret containing the value which will be stored in the OpenSearch keystore

docs/modules/opensearch/pages/usage-guide/keystore.adoc

@@ -2,7 +2,7 @@
 :description: Add entries to the OpenSearch Keystore
 
 The OpenSearch keystore provides secure storage for sensitive configuration settings such as credentials and API keys.
-You can populate the keystore by referencing Secrets from in your OpenSearch configuration.
+You can populate the keystore by referencing Secrets within your OpenSearch configuration.
 
 [source,yaml]
 ----

rust/operator-binary/src/controller.rs

@@ -28,10 +28,7 @@ use update_status::update_status;
 use validate::validate;
 
 use crate::{
-    crd::{
-        NodeRoles,
-        v1alpha1::{self, OpenSearchKeystore},
-    },
+    crd::{NodeRoles, v1alpha1},
     framework::{
         HasName, HasUid, NameIsValidLabelValue,
         product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig},
@@ -172,7 +169,7 @@ pub struct ValidatedCluster {
     pub uid: Uid,
     pub role_config: GenericRoleConfig,
     pub role_group_configs: BTreeMap<RoleGroupName, OpenSearchRoleGroupConfig>,
-    pub keystores: Vec<OpenSearchKeystore>,
+    pub keystores: Vec<v1alpha1::OpenSearchKeystore>,
 }
 
 impl ValidatedCluster {
@@ -185,7 +182,7 @@ impl ValidatedCluster {
         uid: impl Into<Uid>,
         role_config: GenericRoleConfig,
         role_group_configs: BTreeMap<RoleGroupName, OpenSearchRoleGroupConfig>,
-        keystores: Vec<OpenSearchKeystore>,
+        keystores: Vec<v1alpha1::OpenSearchKeystore>,
     ) -> Self {
         let uid = uid.into();
         ValidatedCluster {
@@ -389,7 +386,7 @@ mod tests {
     use crate::{
         controller::{OpenSearchNodeResources, ValidatedOpenSearchConfig},
         crd::{
-            NodeRoles,
+            NodeRoles, OpenSearchKeystoreKey,
             v1alpha1::{self, OpenSearchKeystore, SecretKeyRef},
         },
         framework::{
@@ -511,7 +508,7 @@ mod tests {
             ]
             .into(),
             vec![OpenSearchKeystore {
-                key: "Keystore1".to_string(),
+                key: OpenSearchKeystoreKey::from_str_unsafe("Keystore1"),
                 secret_key_ref: SecretKeyRef {
                     name: SecretName::from_str_unsafe("my-keystore-secret"),
                     key: SecretKey::from_str_unsafe("my-keystore-file"),

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

@@ -77,10 +77,7 @@ mod tests {
             ContextNames, OpenSearchNodeResources, OpenSearchRoleGroupConfig, ValidatedCluster,
             ValidatedContainerLogConfigChoice, ValidatedLogging, ValidatedOpenSearchConfig,
         },
-        crd::{
-            NodeRoles,
-            v1alpha1::{self, OpenSearchKeystore, SecretKeyRef},
-        },
+        crd::{NodeRoles, OpenSearchKeystoreKey, v1alpha1},
         framework::{
             builder::pod::container::EnvVarSet,
             role_utils::GenericProductSpecificCommonConfig,
@@ -200,9 +197,9 @@ mod tests {
                 ),
             ]
             .into(),
-            vec![OpenSearchKeystore {
-                key: "Keystore1".to_string(),
-                secret_key_ref: SecretKeyRef {
+            vec![v1alpha1::OpenSearchKeystore {
+                key: OpenSearchKeystoreKey::from_str_unsafe("Keystore1"),
+                secret_key_ref: v1alpha1::SecretKeyRef {
                     name: SecretName::from_str_unsafe("my-keystore-secret"),
                     key: SecretKey::from_str_unsafe("my-keystore-file"),
                 },

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

@@ -757,10 +757,7 @@ mod tests {
             ContextNames, OpenSearchRoleGroupConfig, ValidatedCluster,
             ValidatedContainerLogConfigChoice, ValidatedLogging, ValidatedOpenSearchConfig,
         },
-        crd::{
-            NodeRoles,
-            v1alpha1::{self, OpenSearchKeystore, SecretKeyRef},
-        },
+        crd::{NodeRoles, OpenSearchKeystoreKey, v1alpha1},
         framework::{
             builder::pod::container::EnvVarSet,
             product_logging::framework::VectorContainerLogConfig,
@@ -852,9 +849,9 @@ mod tests {
                 role_group_config.clone(),
             )]
             .into(),
-            vec![OpenSearchKeystore {
-                key: "Keystore1".to_string(),
-                secret_key_ref: SecretKeyRef {
+            vec![v1alpha1::OpenSearchKeystore {
+                key: OpenSearchKeystoreKey::from_str_unsafe("Keystore1"),
+                secret_key_ref: v1alpha1::SecretKeyRef {
                     name: SecretName::from_str_unsafe("my-keystore-secret"),
                     key: SecretKey::from_str_unsafe("my-keystore-file"),
                 },

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

@@ -286,10 +286,7 @@ mod tests {
     use crate::{
         built_info,
         controller::{ContextNames, ValidatedCluster, ValidatedLogging, ValidatedOpenSearchConfig},
-        crd::{
-            NodeRoles,
-            v1alpha1::{self, OpenSearchKeystore, SecretKeyRef},
-        },
+        crd::{NodeRoles, OpenSearchKeystoreKey, v1alpha1},
         framework::{
             builder::pod::container::{EnvVarName, EnvVarSet},
             product_logging::framework::{
@@ -513,9 +510,9 @@ mod tests {
                     }
                 )]
                 .into(),
-                vec![OpenSearchKeystore {
-                    key: "Keystore1".to_string(),
-                    secret_key_ref: SecretKeyRef {
+                vec![v1alpha1::OpenSearchKeystore {
+                    key: OpenSearchKeystoreKey::from_str_unsafe("Keystore1"),
+                    secret_key_ref: v1alpha1::SecretKeyRef {
                         name: SecretName::from_str_unsafe("my-keystore-secret"),
                         key: SecretKey::from_str_unsafe("my-keystore-file")
                     }
@@ -697,9 +694,9 @@ mod tests {
                 image: serde_json::from_str(r#"{"productVersion": "3.1.0"}"#)
                     .expect("should be a valid ProductImage structure"),
                 cluster_config: v1alpha1::OpenSearchClusterConfig {
-                    keystore: vec![OpenSearchKeystore {
-                        key: "Keystore1".to_string(),
-                        secret_key_ref: SecretKeyRef {
+                    keystore: vec![v1alpha1::OpenSearchKeystore {
+                        key: OpenSearchKeystoreKey::from_str_unsafe("Keystore1"),
+                        secret_key_ref: v1alpha1::SecretKeyRef {
                             name: SecretName::from_str_unsafe("my-keystore-secret"),
                             key: SecretKey::from_str_unsafe("my-keystore-file"),
                         },

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

@@ -27,7 +27,7 @@ use stackable_operator::{
 use strum::{Display, EnumIter};
 
 use crate::{
-    constant,
+    attributed_string_type, constant,
     framework::{
         NameIsValidLabelValue,
         role_utils::GenericProductSpecificCommonConfig,
@@ -87,6 +87,7 @@ pub mod versioned {
         /// Entries to add to the OpenSearch keystore.
         #[serde(default)]
         pub keystore: Vec<OpenSearchKeystore>,
+
         /// Name of the Vector aggregator [discovery ConfigMap](DOCS_BASE_URL_PLACEHOLDER/concepts/service_discovery).
         /// It must contain the key `ADDRESS` with the address of the Vector aggregator.
         /// Follow the [logging tutorial](DOCS_BASE_URL_PLACEHOLDER/tutorials/logging-vector-aggregator)
@@ -99,7 +100,7 @@ pub mod versioned {
     #[serde(rename_all = "camelCase")]
     pub struct OpenSearchKeystore {
         /// Key in the OpenSearch keystore
-        pub key: String,
+        pub key: OpenSearchKeystoreKey,
 
         /// Reference to the Secret containing the value which will be stored in the OpenSearch keystore
         pub secret_key_ref: SecretKeyRef,
@@ -357,6 +358,15 @@ impl v1alpha1::Container {
     }
 }
 
+// See https://github.com/opensearch-project/OpenSearch/blob/8ff7c6ee924a49f0f59f80a6e1c73073c8904214/server/src/main/java/org/opensearch/common/settings/KeyStoreWrapper.java#L125
+attributed_string_type! {
+    OpenSearchKeystoreKey,
+    "Key in an OpenSearch keystore",
+    "s3.client.default.access_key",
+    (min_length = 1),
+    (regex = "[A-Za-z0-9_\\-.]+")
+}
+
 #[cfg(test)]
 mod tests {
     use strum::IntoEnumIterator;

tests/templates/kuttl/backup-restore/10-install-s3-credentials-secret.yaml

@@ -4,5 +4,5 @@ kind: Secret
 metadata:
   name: s3-credentials
 stringData:
-  s3.client.default.access_key: openSearchAccessKey
-  s3.client.default.secret_key: openSearchSecretKey
+  ACCESS_KEY: openSearchAccessKey
+  SECRET_KEY: openSearchSecretKey

tests/templates/kuttl/backup-restore/21-install-opensearch-1.yaml.j2

@@ -15,11 +15,11 @@ spec:
     - key: s3.client.default.access_key
       secretKeyRef:
         name: s3-credentials
-        key: s3.client.default.access_key
+        key: ACCESS_KEY
     - key: s3.client.default.secret_key
       secretKeyRef:
         name: s3-credentials
-        key: s3.client.default.secret_key
+        key: SECRET_KEY
 {% if lookup('env', 'VECTOR_AGGREGATOR') %}
     vectorAggregatorConfigMapName: vector-aggregator-discovery
 {% endif %}