diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ea7d892..f134e8c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file. - Added support for the [PostgreSQL connector](https://trino.io/docs/current/connector/postgresql.html) using the new generic database connection mechanism. Previously, users had to use the `generic` connector ([#883]). - Added support for Trino 481 ([#900]). +- Add a new `.spec.name.inferred.replaceHyphensWithUnderscores` field on TrinoCatalog, which allows tweaking the catalog name in Trino ([#903]). ### Changed @@ -43,6 +44,7 @@ All notable changes to this project will be documented in this file. [#895]: https://github.com/stackabletech/trino-operator/pull/895 [#897]: https://github.com/stackabletech/trino-operator/pull/897 [#900]: https://github.com/stackabletech/trino-operator/pull/900 +[#903]: https://github.com/stackabletech/trino-operator/pull/903 ## [26.3.0] - 2026-03-16 diff --git a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc index a1946c1c..6a0a6123 100644 --- a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc +++ b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc @@ -105,6 +105,28 @@ In this case the `hive` and `iceberg` catalogs will be used as they both match t A `TrinoCluster` can, once created, detect and use new catalogs that have been subsequently created with a matching label. This also means that it is possible to reuse a `TrinoCatalog` within multiple `TrinoClusters`. +=== Catalog name tweaking + +By default the name of the catalog in Trino is inferred from the `.metadata.name` of the TrinoCatalog object. +This ensures that no catalog names clash, as their can only be one TrinoCatalog with a given name. + +One inconvenience is that you need to quote catalogs (or schemas and tables for that matter) containing `-` in Trino, while `_` is fine. +As Kubernetes doesn't allow `_` in the object names, we offer a feature to replace `-` with `_`, which allows you to use valid Kubernetes names, but keeps the convenience of `_` in catalog names. + +You can enable it like this, the Trino catalog will be called `my_postgres`: + +[source,yaml] +---- +kind: TrinoCatalog +metadata: + name: my-postgres +spec: + name: + inferred: + replaceHyphensWithUnderscores: true +# ... +---- + === Generic fallback connector Trino supports lots of different connectors and we can not cover all the available connectors. diff --git a/extra/crds.yaml b/extra/crds.yaml index db68d3d0..21103d92 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -4227,6 +4227,34 @@ spec: items: type: string type: array + name: + default: + inferred: + replaceHyphensWithUnderscores: false + description: The name of the catalog + oneOf: + - required: + - inferred + properties: + inferred: + description: |- + Infer the catalog name from the `.metadata.name` of the TrinoCatalog resource. + + This ensures that no catalog names clash, as their can only be one TrinoCatalog with a + given name. + properties: + replaceHyphensWithUnderscores: + default: false + description: |- + Wether hyphens (`-`) in the name of the catalog should be replaced by underscores (`_`). + + This is recommended because Kubernetes only allows `a-z` and `-`, while Trino + requires quoting for catalogs containing `-` characters, but not for `_`. This mechanism + allows you to use valid Kubernetes names, but keeps the convenience of `_` in catalog + names. + type: boolean + type: object + type: object required: - connector type: object diff --git a/rust/operator-binary/src/catalog/black_hole.rs b/rust/operator-binary/src/catalog/black_hole.rs index 82ab529b..2668c936 100644 --- a/rust/operator-binary/src/catalog/black_hole.rs +++ b/rust/operator-binary/src/catalog/black_hole.rs @@ -2,7 +2,9 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::black_hole::BlackHoleConnector; +use crate::{ + controller::dereference::TrinoCatalogName, crd::catalog::black_hole::BlackHoleConnector, +}; pub const CONNECTOR_NAME: &str = "blackhole"; @@ -10,12 +12,12 @@ pub const CONNECTOR_NAME: &str = "blackhole"; impl ToCatalogConfig for BlackHoleConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, ) -> Result { // No additional properties needed - Ok(CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME)) + Ok(CatalogConfig::new(catalog_name, CONNECTOR_NAME)) } } diff --git a/rust/operator-binary/src/catalog/commons.rs b/rust/operator-binary/src/catalog/commons.rs index ded15dc0..275703dd 100644 --- a/rust/operator-binary/src/catalog/commons.rs +++ b/rust/operator-binary/src/catalog/commons.rs @@ -19,6 +19,7 @@ use super::{ }; use crate::{ config, + controller::dereference::TrinoCatalogName, crd::{ CONFIG_DIR_NAME, catalog::commons::{HdfsConnection, MetastoreConnection}, @@ -30,7 +31,7 @@ impl ExtendCatalogConfig for MetastoreConnection { async fn extend_catalog_config( &self, catalog_config: &mut CatalogConfig, - catalog_name: &str, + catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, _trino_version: u16, @@ -72,7 +73,7 @@ impl ExtendCatalogConfig for s3::v1alpha1::InlineConnectionOrReference { async fn extend_catalog_config( &self, catalog_config: &mut CatalogConfig, - _catalog_name: &str, + _catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, _trino_version: u16, @@ -117,7 +118,7 @@ impl ExtendCatalogConfig for HdfsConnection { async fn extend_catalog_config( &self, catalog_config: &mut CatalogConfig, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, @@ -125,7 +126,7 @@ impl ExtendCatalogConfig for HdfsConnection { // Since Trino 458, fs.hadoop.enabled defaults to false. catalog_config.add_property("fs.hadoop.enabled", "true"); - let hdfs_site_dir = format!("{CONFIG_DIR_NAME}/catalog/{catalog_name}/hdfs-config"); + let hdfs_site_dir = format!("{CONFIG_DIR_NAME}/catalog/{catalog_name}/hdfs-config",); catalog_config.add_property( "hive.config.resources", format!("{hdfs_site_dir}/core-site.xml,{hdfs_site_dir}/hdfs-site.xml"), diff --git a/rust/operator-binary/src/catalog/config.rs b/rust/operator-binary/src/catalog/config.rs index 86c9c42b..0bd42c27 100644 --- a/rust/operator-binary/src/catalog/config.rs +++ b/rust/operator-binary/src/catalog/config.rs @@ -5,17 +5,19 @@ use stackable_operator::{ k8s_openapi::api::core::v1::{ ConfigMapKeySelector, EnvVar, EnvVarSource, SecretKeySelector, Volume, VolumeMount, }, - kube::Resource, v2::types::kubernetes::NamespaceName, }; use super::{FromTrinoCatalogError, ToCatalogConfig}; -use crate::crd::catalog::{TrinoCatalogConnector, v1alpha1}; +use crate::{ + controller::dereference::TrinoCatalogName, + crd::catalog::{TrinoCatalogConnector, v1alpha1}, +}; #[derive(Clone, Debug)] pub struct CatalogConfig { /// Name of the catalog - pub name: String, + pub name: TrinoCatalogName, /// Properties of the catalog pub properties: BTreeMap, @@ -39,9 +41,9 @@ pub struct CatalogConfig { } impl CatalogConfig { - pub fn new(name: impl Into, connector_name: impl Into) -> Self { + pub fn new(name: &TrinoCatalogName, connector_name: impl Into) -> Self { let mut config = CatalogConfig { - name: name.into(), + name: name.clone(), properties: BTreeMap::new(), env_bindings: Vec::new(), load_env_from_files: BTreeMap::new(), @@ -105,17 +107,12 @@ impl CatalogConfig { } pub async fn from_catalog( + catalog_name: &TrinoCatalogName, catalog: &v1alpha1::TrinoCatalog, client: &Client, catalog_namespace: &NamespaceName, trino_version: u16, ) -> Result { - let catalog_name = catalog - .meta() - .name - .clone() - .ok_or(FromTrinoCatalogError::InvalidCatalogSpec)?; - let to_catalog_config: &dyn ToCatalogConfig = match &catalog.spec.connector { TrinoCatalogConnector::BlackHole(black_hole_connector) => black_hole_connector, TrinoCatalogConnector::DeltaLake(delta_lake_connector) => delta_lake_connector, @@ -128,7 +125,7 @@ impl CatalogConfig { TrinoCatalogConnector::Tpch(tpch_connector) => tpch_connector, }; let mut catalog_config = to_catalog_config - .to_catalog_config(&catalog_name, catalog_namespace, client, trino_version) + .to_catalog_config(catalog_name, catalog_namespace, client, trino_version) .await?; catalog_config @@ -138,7 +135,7 @@ impl CatalogConfig { for removal in &catalog.spec.config_removals { if catalog_config.properties.remove(removal).is_none() { tracing::warn!( - catalog.name = catalog_name, + catalog.name = %catalog_name, property = removal, "You asked to remove a non-existing config property from a catalog" ); @@ -149,8 +146,8 @@ impl CatalogConfig { } } -fn calculate_env_name(catalog: impl Into, property: impl Into) -> String { - let catalog = catalog.into().replace(['.', '-'], "_"); +fn calculate_env_name(catalog_name: &TrinoCatalogName, property: impl Into) -> String { + let catalog = catalog_name.to_string().replace(['.', '-'], "_"); let property = property.into().replace(['.', '-'], "_"); format!("CATALOG_{catalog}_{property}").to_uppercase() } diff --git a/rust/operator-binary/src/catalog/delta_lake.rs b/rust/operator-binary/src/catalog/delta_lake.rs index a2c089e9..1cd8b483 100644 --- a/rust/operator-binary/src/catalog/delta_lake.rs +++ b/rust/operator-binary/src/catalog/delta_lake.rs @@ -2,7 +2,9 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{ExtendCatalogConfig, FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::delta_lake::DeltaLakeConnector; +use crate::{ + controller::dereference::TrinoCatalogName, crd::catalog::delta_lake::DeltaLakeConnector, +}; pub const CONNECTOR_NAME: &str = "delta_lake"; @@ -10,12 +12,12 @@ pub const CONNECTOR_NAME: &str = "delta_lake"; impl ToCatalogConfig for DeltaLakeConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, trino_version: u16, ) -> Result { - let mut config = CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME); + let mut config = CatalogConfig::new(catalog_name, CONNECTOR_NAME); // No authorization checks are enforced at the catalog level. // We don't want the delta connector to prevent users from dropping tables. diff --git a/rust/operator-binary/src/catalog/generic.rs b/rust/operator-binary/src/catalog/generic.rs index af3de842..6de60ec6 100644 --- a/rust/operator-binary/src/catalog/generic.rs +++ b/rust/operator-binary/src/catalog/generic.rs @@ -2,19 +2,22 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::generic::{GenericConnector, Property}; +use crate::{ + controller::dereference::TrinoCatalogName, + crd::catalog::generic::{GenericConnector, Property}, +}; #[async_trait] impl ToCatalogConfig for GenericConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, ) -> Result { let connector_name = &self.connector_name; - let mut config = CatalogConfig::new(catalog_name.to_string(), connector_name); + let mut config = CatalogConfig::new(catalog_name, connector_name); for (property_name, property) in &self.properties { match property { diff --git a/rust/operator-binary/src/catalog/google_sheet.rs b/rust/operator-binary/src/catalog/google_sheet.rs index 90396be2..b5accc00 100644 --- a/rust/operator-binary/src/catalog/google_sheet.rs +++ b/rust/operator-binary/src/catalog/google_sheet.rs @@ -6,7 +6,10 @@ use stackable_operator::{ }; use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::{CONFIG_DIR_NAME, catalog::google_sheet::GoogleSheetConnector}; +use crate::{ + controller::dereference::TrinoCatalogName, + crd::{CONFIG_DIR_NAME, catalog::google_sheet::GoogleSheetConnector}, +}; pub const CONNECTOR_NAME: &str = "gsheets"; @@ -14,12 +17,12 @@ pub const CONNECTOR_NAME: &str = "gsheets"; impl ToCatalogConfig for GoogleSheetConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, ) -> Result { - let mut config = CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME); + let mut config = CatalogConfig::new(catalog_name, CONNECTOR_NAME); let volume_name = format!("{catalog_name}-google-sheets-credentials"); let google_sheets_credentials_dir = diff --git a/rust/operator-binary/src/catalog/hive.rs b/rust/operator-binary/src/catalog/hive.rs index 96ffdf19..8623ad97 100644 --- a/rust/operator-binary/src/catalog/hive.rs +++ b/rust/operator-binary/src/catalog/hive.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{ExtendCatalogConfig, FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::hive::HiveConnector; +use crate::{controller::dereference::TrinoCatalogName, crd::catalog::hive::HiveConnector}; pub const CONNECTOR_NAME: &str = "hive"; @@ -10,12 +10,12 @@ pub const CONNECTOR_NAME: &str = "hive"; impl ToCatalogConfig for HiveConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, trino_version: u16, ) -> Result { - let mut config = CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME); + let mut config = CatalogConfig::new(catalog_name, CONNECTOR_NAME); // No authorization checks are enforced at the catalog level. // We don't want the hive connector to prevent users from dropping tables. diff --git a/rust/operator-binary/src/catalog/iceberg.rs b/rust/operator-binary/src/catalog/iceberg.rs index 65b6f41f..faf3518d 100644 --- a/rust/operator-binary/src/catalog/iceberg.rs +++ b/rust/operator-binary/src/catalog/iceberg.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{ExtendCatalogConfig, FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::iceberg::IcebergConnector; +use crate::{controller::dereference::TrinoCatalogName, crd::catalog::iceberg::IcebergConnector}; pub const CONNECTOR_NAME: &str = "iceberg"; @@ -10,12 +10,12 @@ pub const CONNECTOR_NAME: &str = "iceberg"; impl ToCatalogConfig for IcebergConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, trino_version: u16, ) -> Result { - let mut config = CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME); + let mut config = CatalogConfig::new(catalog_name, CONNECTOR_NAME); // No authorization checks are enforced at the catalog level. // We don't want the iceberg connector to prevent users from dropping tables. diff --git a/rust/operator-binary/src/catalog/mod.rs b/rust/operator-binary/src/catalog/mod.rs index 6a2f8ae3..0760dcea 100644 --- a/rust/operator-binary/src/catalog/mod.rs +++ b/rust/operator-binary/src/catalog/mod.rs @@ -14,6 +14,8 @@ use async_trait::async_trait; use snafu::Snafu; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; +use crate::controller::dereference::TrinoCatalogName; + use self::config::CatalogConfig; #[derive(Debug, Snafu)] @@ -62,7 +64,7 @@ pub enum FromTrinoCatalogError { pub trait ToCatalogConfig { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, trino_version: u16, @@ -74,7 +76,7 @@ pub trait ExtendCatalogConfig { async fn extend_catalog_config( &self, catalog_config: &mut CatalogConfig, - catalog_name: &str, + catalog_name: &TrinoCatalogName, catalog_namespace: &NamespaceName, client: &Client, trino_version: u16, diff --git a/rust/operator-binary/src/catalog/postgresql.rs b/rust/operator-binary/src/catalog/postgresql.rs index 65ef335f..4f0fa5e5 100644 --- a/rust/operator-binary/src/catalog/postgresql.rs +++ b/rust/operator-binary/src/catalog/postgresql.rs @@ -8,7 +8,7 @@ use stackable_operator::{ use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; use crate::{ catalog::from_trino_catalog_error::GetPostgresConnectionDetailsSnafu, - crd::catalog::postgresql::PostgresqlConnector, + controller::dereference::TrinoCatalogName, crd::catalog::postgresql::PostgresqlConnector, }; pub const CONNECTOR_NAME: &str = "postgresql"; @@ -17,16 +17,16 @@ pub const CONNECTOR_NAME: &str = "postgresql"; impl ToCatalogConfig for PostgresqlConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, ) -> Result { - let mut config = CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME); + let mut config = CatalogConfig::new(catalog_name, CONNECTOR_NAME); // SAFETY: `unique_database_name` must only contain uppercase ASCII letters and underscores. let unique_database_name = format!( "POSTGRESQL_{}", - catalog_name.replace('-', "_").to_uppercase() + catalog_name.to_string().replace('-', "_").to_uppercase() ); let jdbc_connection_details = self .inner diff --git a/rust/operator-binary/src/catalog/tpcds.rs b/rust/operator-binary/src/catalog/tpcds.rs index 2da5ceb7..dbd91170 100644 --- a/rust/operator-binary/src/catalog/tpcds.rs +++ b/rust/operator-binary/src/catalog/tpcds.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::tpcds::TpcdsConnector; +use crate::{controller::dereference::TrinoCatalogName, crd::catalog::tpcds::TpcdsConnector}; pub const CONNECTOR_NAME: &str = "tpcds"; @@ -10,12 +10,12 @@ pub const CONNECTOR_NAME: &str = "tpcds"; impl ToCatalogConfig for TpcdsConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, ) -> Result { // No additional properties needed - Ok(CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME)) + Ok(CatalogConfig::new(catalog_name, CONNECTOR_NAME)) } } diff --git a/rust/operator-binary/src/catalog/tpch.rs b/rust/operator-binary/src/catalog/tpch.rs index e355de3c..451a4242 100644 --- a/rust/operator-binary/src/catalog/tpch.rs +++ b/rust/operator-binary/src/catalog/tpch.rs @@ -2,7 +2,7 @@ use async_trait::async_trait; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::catalog::tpch::TpchConnector; +use crate::{controller::dereference::TrinoCatalogName, crd::catalog::tpch::TpchConnector}; pub const CONNECTOR_NAME: &str = "tpch"; @@ -10,12 +10,12 @@ pub const CONNECTOR_NAME: &str = "tpch"; impl ToCatalogConfig for TpchConnector { async fn to_catalog_config( &self, - catalog_name: &str, + catalog_name: &TrinoCatalogName, _catalog_namespace: &NamespaceName, _client: &Client, _trino_version: u16, ) -> Result { // No additional properties needed - Ok(CatalogConfig::new(catalog_name.to_string(), CONNECTOR_NAME)) + Ok(CatalogConfig::new(catalog_name, CONNECTOR_NAME)) } } diff --git a/rust/operator-binary/src/controller/build/command.rs b/rust/operator-binary/src/controller/build/command.rs index 5c20ee65..dd5ede63 100644 --- a/rust/operator-binary/src/controller/build/command.rs +++ b/rust/operator-binary/src/controller/build/command.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use stackable_operator::{ product_logging::framework::{ create_vector_shutdown_file_command, remove_vector_shutdown_file_command, @@ -10,7 +12,10 @@ use crate::{ authentication::TrinoAuthenticationConfig, catalog::config::CatalogConfig, config::{client_protocol, fault_tolerant_execution}, - controller::{ValidatedCluster, ValidatedTrinoConfig, build::properties::ConfigFileName}, + controller::{ + ValidatedCluster, ValidatedTrinoConfig, build::properties::ConfigFileName, + dereference::TrinoCatalogName, + }, crd::{ CONFIG_DIR_NAME, Container, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, @@ -22,7 +27,7 @@ use crate::{ pub fn container_prepare_args( cluster: &ValidatedCluster, - catalogs: &[CatalogConfig], + catalogs: &BTreeMap, merged_config: &ValidatedTrinoConfig, resolved_fte_config: &Option, resolved_spooling_config: &Option, @@ -63,7 +68,7 @@ pub fn container_prepare_args( } // Add the commands that are needed to set up the catalogs - catalogs.iter().for_each(|catalog| { + catalogs.values().for_each(|catalog| { args.extend_from_slice(&catalog.init_container_extra_start_commands); }); @@ -82,7 +87,7 @@ pub fn container_prepare_args( pub fn container_trino_args( authentication_config: &TrinoAuthenticationConfig, - catalogs: &[CatalogConfig], + catalogs: &BTreeMap, ) -> Vec { let mut args = vec![ // copy config files to a writeable empty folder @@ -104,7 +109,7 @@ pub fn container_trino_args( // Add the commands that are needed to set up the catalogs // Don't print secret contents! args.push("set +x".to_string()); - catalogs.iter().for_each(|catalog| { + catalogs.values().for_each(|catalog| { for (env_name, file) in &catalog.load_env_from_files { args.push(format!("export {env_name}=\"$(cat {file})\"")); } diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs index 37bb891e..7476f9f0 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -213,8 +213,8 @@ pub fn build_rolegroup_catalog_config_map( .cluster_config .catalogs .iter() - .map(|catalog| { - let file = format!("{}.properties", catalog.name); + .map(|(catalog_name, catalog)| { + let file = format!("{catalog_name}.properties"); let rendered = to_java_properties_string(catalog.properties.iter()) .with_context(|_| WritePropertiesSnafu { file: file.clone() })?; Ok((file, rendered)) diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 19ed2321..a15fc106 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -145,7 +145,7 @@ pub fn build_rolegroup_statefulset( // so the caller only needs to pass those (plus the applied ServiceAccount name). let resolved_product_image = &cluster.image; let trino_authentication_config = &cluster.cluster_config.authentication; - let catalogs = cluster.cluster_config.catalogs.as_slice(); + let catalogs = &cluster.cluster_config.catalogs; let resolved_fte_config = &cluster.cluster_config.fault_tolerant_execution; let resolved_spooling_config = &cluster.cluster_config.client_protocol; let trino_opa_config = &cluster.cluster_config.authorization; @@ -208,7 +208,7 @@ pub fn build_rolegroup_statefulset( // Add the needed stuff for catalogs env.extend( catalogs - .iter() + .values() .flat_map(|catalog| &catalog.env_bindings) .cloned(), ); @@ -632,7 +632,7 @@ fn tls_volume_mounts( cb_trino: &mut ContainerBuilder, requested_secret_lifetime: &Duration, ) -> Result<()> { - let catalogs = cluster.cluster_config.catalogs.as_slice(); + let catalogs = &cluster.cluster_config.catalogs; let resolved_fte_config = &cluster.cluster_config.fault_tolerant_execution; let resolved_spooling_config = &cluster.cluster_config.client_protocol; let trino_opa_config = &cluster.cluster_config.authorization; @@ -715,7 +715,7 @@ fn tls_volume_mounts( } // catalogs - for catalog in catalogs { + for catalog in catalogs.values() { cb_prepare .add_volume_mounts(catalog.volume_mounts.clone()) .context(AddVolumeMountSnafu)?; diff --git a/rust/operator-binary/src/controller/dereference.rs b/rust/operator-binary/src/controller/dereference.rs index 408657fb..b6e9f881 100644 --- a/rust/operator-binary/src/controller/dereference.rs +++ b/rust/operator-binary/src/controller/dereference.rs @@ -5,9 +5,11 @@ use std::{num::ParseIntError, str::FromStr}; -use snafu::{ResultExt, Snafu}; +use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ - client::Client, kube::runtime::reflector::ObjectRef, v2::controller_utils::get_namespace, + client::Client, + kube::runtime::reflector::{Lookup, ObjectRef}, + v2::controller_utils::get_namespace, }; use crate::{ @@ -30,6 +32,9 @@ pub enum Error { source: stackable_operator::v2::controller_utils::Error, }, + #[snafu(display("object defines no name"))] + ObjectHasNoName, + #[snafu(display("failed to retrieve AuthenticationClass"))] AuthenticationClassRetrieval { source: crate::crd::authentication::Error, @@ -66,6 +71,20 @@ pub enum Error { }, } +/// TODO: Use a typed String from operator-rs similar to [`stackable_operator::v2::types::operator::ClusterName`]. +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub struct TrinoCatalogName(pub String); +impl AsRef for TrinoCatalogName { + fn as_ref(&self) -> &str { + &self.0 + } +} +impl std::fmt::Display for TrinoCatalogName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + type Result = std::result::Result; /// Kubernetes objects referenced from the TrinoCluster spec, fetched from the cluster. @@ -106,12 +125,29 @@ pub async fn dereference( let mut catalogs = Vec::with_capacity(catalog_definitions.len()); for catalog in &catalog_definitions { let catalog_ref = ObjectRef::from_obj(catalog); - let catalog_config = - CatalogConfig::from_catalog(catalog, client, &namespace, product_version) - .await - .context(ParseCatalogSnafu { - catalog: catalog_ref, - })?; + // We are using a match here, as we might support other ways of naming (e.g. custom) later + let catalog_name = match catalog.spec.name { + catalog::v1alpha1::TrinoCatalogNameSpec::Inferred { + replace_hyphens_with_underscores, + } => { + let mut catalog_name = catalog.name().context(ObjectHasNoNameSnafu)?.to_string(); + if replace_hyphens_with_underscores { + catalog_name = catalog_name.replace('-', "_"); + } + TrinoCatalogName(catalog_name) + } + }; + let catalog_config = CatalogConfig::from_catalog( + &catalog_name, + catalog, + client, + &namespace, + product_version, + ) + .await + .context(ParseCatalogSnafu { + catalog: catalog_ref, + })?; catalogs.push(catalog_config); } diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index a9080d93..3e75cd1a 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -32,6 +32,7 @@ use crate::{ client_protocol::ResolvedClientProtocolConfig, fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig, }, + controller::dereference::TrinoCatalogName, crd::{APP_NAME, TrinoRole, discovery::TrinoPodRef, v1alpha1}, trino_controller::{CONTROLLER_NAME, OPERATOR_NAME}, }; @@ -57,7 +58,7 @@ pub struct ValidatedClusterConfig { pub fault_tolerant_execution: Option, pub client_protocol: Option, pub coordinator_pod_refs: Vec, - pub catalogs: Vec, + pub catalogs: BTreeMap, } impl ValidatedClusterConfig { diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 034e036d..f9b08b17 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -32,7 +32,7 @@ use super::{ }; use crate::{ authentication::{self, TrinoAuthenticationConfig, TrinoAuthenticationTypes}, - controller::dereference::DereferencedObjects, + controller::dereference::{DereferencedObjects, TrinoCatalogName}, crd::{Container, TrinoRole, v1alpha1}, }; @@ -109,6 +109,11 @@ pub enum Error { "the Vector aggregator discovery ConfigMap name is required when the Vector agent is enabled" ))] MissingVectorAggregatorConfigMapName, + + #[snafu(display( + "The catalog name {catalog_name:?} clashes (there are multiple catalogs with this name). Please make sure there is exactly one catalog for any given name" + ))] + ClashingCatalogName { catalog_name: TrinoCatalogName }, } type Result = std::result::Result; @@ -271,6 +276,19 @@ pub fn validate( role_group_configs.insert(trino_role, groups); } + let mut catalogs = BTreeMap::new(); + for catalog in &dereferenced_objects.catalogs { + if catalogs + .insert(catalog.name.clone(), catalog.clone()) + .is_some() + { + return ClashingCatalogNameSnafu { + catalog_name: catalog.name.clone(), + } + .fail(); + } + } + let tls = &trino.spec.cluster_config.tls; let cluster_config = ValidatedClusterConfig { tls: ValidatedTls { @@ -282,7 +300,7 @@ pub fn validate( fault_tolerant_execution: dereferenced_objects.resolved_fte_config.clone(), client_protocol: dereferenced_objects.resolved_client_protocol_config.clone(), coordinator_pod_refs: trino.coordinator_pods(&namespace).collect(), - catalogs: dereferenced_objects.catalogs.clone(), + catalogs, }; let name = get_cluster_name(trino).context(GetClusterNameSnafu)?; diff --git a/rust/operator-binary/src/crd/catalog/mod.rs b/rust/operator-binary/src/crd/catalog/mod.rs index 73df79f8..04d4d5c8 100644 --- a/rust/operator-binary/src/crd/catalog/mod.rs +++ b/rust/operator-binary/src/crd/catalog/mod.rs @@ -48,6 +48,10 @@ pub mod versioned { #[derive(Clone, CustomResource, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct TrinoCatalogSpec { + /// The name of the catalog + #[serde(default)] + pub name: TrinoCatalogNameSpec, + /// The `connector` defines which connector is used. pub connector: TrinoCatalogConnector, @@ -65,6 +69,49 @@ pub mod versioned { #[serde(default, rename = "experimentalConfigRemovals")] pub config_removals: Vec, } + + #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] + #[serde(rename_all = "camelCase")] + pub enum TrinoCatalogNameSpec { + /// Infer the catalog name from the `.metadata.name` of the TrinoCatalog resource. + /// + /// This ensures that no catalog names clash, as their can only be one TrinoCatalog with a + /// given name. + #[serde(rename_all = "camelCase")] + Inferred { + /// Wether hyphens (`-`) in the name of the catalog should be replaced by underscores (`_`). + /// + /// This is recommended because Kubernetes only allows `a-z` and `-`, while Trino + /// requires quoting for catalogs containing `-` characters, but not for `_`. This mechanism + /// allows you to use valid Kubernetes names, but keeps the convenience of `_` in catalog + /// names. + // + // /// In case you need complete flexibility over the catalog name, you can use + // /// `name.custom`. + #[serde(default)] + replace_hyphens_with_underscores: bool, + }, + // As requested in https://github.com/stackabletech/trino-operator/issues/891 we are not + // implementing the custom variant yet. Please re-open or create a new decision before + // implementing this. + // + // /// Specify the name of the catalog as it shows up in Trino. + // /// + // /// It is your responsibility to make sure that no catalog names clash, the operator will + // /// raise an error in that case. + // /// + // /// TIP: If you only want to replace `-` with `_` use + // /// `name.inferred.replaceHyphensWithUnderscores` instead. + // Custom(String), + } +} + +impl Default for v1alpha1::TrinoCatalogNameSpec { + fn default() -> Self { + Self::Inferred { + replace_hyphens_with_underscores: false, + } + } } #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] @@ -172,7 +219,10 @@ mod tests { secretClass: minio-credentials - connector: tpcds: {} - - connector: + - name: + inferred: + replaceHyphensWithUnderscores: true + connector: tpch: {} "}) .expect("Failed to parse TrinoCatalogSpec YAML")