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..7d1d1c51 100644 --- a/docs/modules/trino/pages/usage-guide/catalogs/index.adoc +++ b/docs/modules/trino/pages/usage-guide/catalogs/index.adoc @@ -105,6 +105,29 @@ 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 there 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 using `_` in catalog names. + +In order to replace `-` with `_`, use the setting `name.inferred.replaceHyphensWithUnderscores`. +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..7cfeb75a 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 there can only be one TrinoCatalog with a + given name. + properties: + replaceHyphensWithUnderscores: + default: false + description: |- + Whether 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. This mechanism allows + you to use valid Kubernetes names, but keeps the convenience of using `_` in + catalog names. + type: boolean + type: object + type: object required: - connector type: object diff --git a/rust/operator-binary/Cargo.toml b/rust/operator-binary/Cargo.toml index 6474db02..e53118ed 100644 --- a/rust/operator-binary/Cargo.toml +++ b/rust/operator-binary/Cargo.toml @@ -9,6 +9,12 @@ repository.workspace = true publish = false build = "build.rs" +[features] +# The macro attributed_string_type is used in this operator. It produces test +# code if the feature "test-support" is set. This feature is defined here to +# suppress a Clippy warning. +test-support = [] + [dependencies] stackable-operator = { workspace = true, features = ["test-support"] } diff --git a/rust/operator-binary/src/catalog/black_hole.rs b/rust/operator-binary/src/catalog/black_hole.rs index 82ab529b..12cb44af 100644 --- a/rust/operator-binary/src/catalog/black_hole.rs +++ b/rust/operator-binary/src/catalog/black_hole.rs @@ -1,8 +1,10 @@ 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::{ + catalog::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{TrinoCatalogName, 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..9078014e 100644 --- a/rust/operator-binary/src/catalog/commons.rs +++ b/rust/operator-binary/src/catalog/commons.rs @@ -8,20 +8,23 @@ use stackable_operator::{ v2::types::kubernetes::NamespaceName, }; -use super::{ - ExtendCatalogConfig, FromTrinoCatalogError, - config::CatalogConfig, - from_trino_catalog_error::{ - ConfigureS3Snafu, FailedToGetDiscoveryConfigMapDataKeySnafu, - FailedToGetDiscoveryConfigMapDataSnafu, FailedToGetDiscoveryConfigMapSnafu, - S3TlsNoVerificationNotSupportedSnafu, S3TlsRequiredSnafu, - }, -}; use crate::{ + catalog::{ + ExtendCatalogConfig, FromTrinoCatalogError, + config::CatalogConfig, + from_trino_catalog_error::{ + ConfigureS3Snafu, FailedToGetDiscoveryConfigMapDataKeySnafu, + FailedToGetDiscoveryConfigMapDataSnafu, FailedToGetDiscoveryConfigMapSnafu, + S3TlsNoVerificationNotSupportedSnafu, S3TlsRequiredSnafu, + }, + }, config, crd::{ CONFIG_DIR_NAME, - catalog::commons::{HdfsConnection, MetastoreConnection}, + catalog::{ + TrinoCatalogName, + commons::{HdfsConnection, MetastoreConnection}, + }, }, }; @@ -30,7 +33,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 +75,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 +120,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, diff --git a/rust/operator-binary/src/catalog/config.rs b/rust/operator-binary/src/catalog/config.rs index 86c9c42b..34e0ecd5 100644 --- a/rust/operator-binary/src/catalog/config.rs +++ b/rust/operator-binary/src/catalog/config.rs @@ -5,17 +5,18 @@ 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::{ + catalog::{FromTrinoCatalogError, ToCatalogConfig}, + crd::catalog::{TrinoCatalogConnector, TrinoCatalogName, 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 +40,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 +106,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 +124,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 +134,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 +145,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..11f65ab5 100644 --- a/rust/operator-binary/src/catalog/delta_lake.rs +++ b/rust/operator-binary/src/catalog/delta_lake.rs @@ -1,8 +1,10 @@ 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::{ + catalog::{ExtendCatalogConfig, FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{TrinoCatalogName, 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..a91ca578 100644 --- a/rust/operator-binary/src/catalog/generic.rs +++ b/rust/operator-binary/src/catalog/generic.rs @@ -1,20 +1,25 @@ 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::{ + catalog::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{ + TrinoCatalogName, + 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..f8c14c31 100644 --- a/rust/operator-binary/src/catalog/google_sheet.rs +++ b/rust/operator-binary/src/catalog/google_sheet.rs @@ -5,8 +5,13 @@ use stackable_operator::{ v2::types::kubernetes::NamespaceName, }; -use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; -use crate::crd::{CONFIG_DIR_NAME, catalog::google_sheet::GoogleSheetConnector}; +use crate::{ + catalog::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::{ + CONFIG_DIR_NAME, + catalog::{TrinoCatalogName, google_sheet::GoogleSheetConnector}, + }, +}; pub const CONNECTOR_NAME: &str = "gsheets"; @@ -14,16 +19,16 @@ 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 volume_name = format!("{catalog_name}-sheets-credentials"); let google_sheets_credentials_dir = - format!("{CONFIG_DIR_NAME}/catalog/{catalog_name}/google-sheets-credentials/"); + format!("{CONFIG_DIR_NAME}/catalog/{catalog_name}/sheets-credentials/"); config.volumes.push( VolumeBuilder::new(&volume_name) diff --git a/rust/operator-binary/src/catalog/hive.rs b/rust/operator-binary/src/catalog/hive.rs index 96ffdf19..79357a39 100644 --- a/rust/operator-binary/src/catalog/hive.rs +++ b/rust/operator-binary/src/catalog/hive.rs @@ -1,8 +1,10 @@ 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::{ + catalog::{ExtendCatalogConfig, FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{TrinoCatalogName, hive::HiveConnector}, +}; pub const CONNECTOR_NAME: &str = "hive"; @@ -10,12 +12,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..694fcca0 100644 --- a/rust/operator-binary/src/catalog/iceberg.rs +++ b/rust/operator-binary/src/catalog/iceberg.rs @@ -1,8 +1,10 @@ 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::{ + catalog::{ExtendCatalogConfig, FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{TrinoCatalogName, iceberg::IcebergConnector}, +}; pub const CONNECTOR_NAME: &str = "iceberg"; @@ -10,12 +12,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..1ee1d3e4 100644 --- a/rust/operator-binary/src/catalog/mod.rs +++ b/rust/operator-binary/src/catalog/mod.rs @@ -15,6 +15,7 @@ use snafu::Snafu; use stackable_operator::{client::Client, v2::types::kubernetes::NamespaceName}; use self::config::CatalogConfig; +use crate::crd::catalog::TrinoCatalogName; #[derive(Debug, Snafu)] #[snafu(module)] @@ -62,7 +63,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 +75,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..9d0e8abb 100644 --- a/rust/operator-binary/src/catalog/postgresql.rs +++ b/rust/operator-binary/src/catalog/postgresql.rs @@ -5,10 +5,12 @@ use stackable_operator::{ v2::types::kubernetes::NamespaceName, }; -use super::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}; use crate::{ - catalog::from_trino_catalog_error::GetPostgresConnectionDetailsSnafu, - crd::catalog::postgresql::PostgresqlConnector, + catalog::{ + FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig, + from_trino_catalog_error::GetPostgresConnectionDetailsSnafu, + }, + crd::catalog::{TrinoCatalogName, postgresql::PostgresqlConnector}, }; pub const CONNECTOR_NAME: &str = "postgresql"; @@ -17,16 +19,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..791f3676 100644 --- a/rust/operator-binary/src/catalog/tpcds.rs +++ b/rust/operator-binary/src/catalog/tpcds.rs @@ -1,8 +1,10 @@ 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::{ + catalog::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{TrinoCatalogName, tpcds::TpcdsConnector}, +}; pub const CONNECTOR_NAME: &str = "tpcds"; @@ -10,12 +12,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..1ff5bc41 100644 --- a/rust/operator-binary/src/catalog/tpch.rs +++ b/rust/operator-binary/src/catalog/tpch.rs @@ -1,8 +1,10 @@ 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::{ + catalog::{FromTrinoCatalogError, ToCatalogConfig, config::CatalogConfig}, + crd::catalog::{TrinoCatalogName, tpch::TpchConnector}, +}; pub const CONNECTOR_NAME: &str = "tpch"; @@ -10,12 +12,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..f2121fd9 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, @@ -15,14 +17,14 @@ use crate::{ CONFIG_DIR_NAME, Container, RW_CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, - TrinoRole, + TrinoRole, catalog::TrinoCatalogName, }, trino_controller::{STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR}, }; pub fn container_prepare_args( cluster: &ValidatedCluster, - catalogs: &[CatalogConfig], + catalogs: &BTreeMap, merged_config: &ValidatedTrinoConfig, resolved_fte_config: &Option, resolved_spooling_config: &Option, @@ -63,7 +65,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 +84,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 +106,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..93a8d050 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::{ @@ -19,7 +21,8 @@ use crate::{ }, crd::{ authentication::{ResolvedAuthenticationClassRef, resolve_authentication_classes}, - catalog, v1alpha1, + catalog::{self, TrinoCatalogName}, + v1alpha1, }, }; @@ -30,6 +33,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, @@ -64,6 +70,12 @@ pub enum Error { InvalidOpaConfig { source: stackable_operator::commons::opa::Error, }, + + #[snafu(display("invalid trino catalog name"))] + InvalidTrinoCatalogName { + source: stackable_operator::v2::macros::attributed_string_type::Error, + catalog_name: String, + }, } type Result = std::result::Result; @@ -106,12 +118,21 @@ 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, - })?; + let catalog_name = determine_catalog_name( + &catalog.spec.name, + &catalog.name().context(ObjectHasNoNameSnafu)?, + )?; + let catalog_config = CatalogConfig::from_catalog( + &catalog_name, + catalog, + client, + &namespace, + product_version, + ) + .await + .context(ParseCatalogSnafu { + catalog: catalog_ref, + })?; catalogs.push(catalog_config); } @@ -159,3 +180,46 @@ pub async fn dereference( resolved_client_protocol_config, }) } + +/// Determines the Trino catalog name based on user settings and the Kubernetes TrinoCatalog object +/// name. +fn determine_catalog_name( + catalog_name_spec: &catalog::v1alpha1::TrinoCatalogNameSpec, + catalog_object_name: &str, +) -> Result { + // A `match` is used because we might support other ways of naming (e.g. custom) later. + match catalog_name_spec { + catalog::v1alpha1::TrinoCatalogNameSpec::Inferred { + replace_hyphens_with_underscores, + } => { + let mut catalog_name = catalog_object_name.to_owned(); + if *replace_hyphens_with_underscores { + catalog_name = catalog_name.replace('-', "_"); + } + TrinoCatalogName::from_str(&catalog_name) + .with_context(|_| InvalidTrinoCatalogNameSnafu { catalog_name }) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn determine_catalog_name_replaces_hyphens() { + let inferred = |replace_hyphens_with_underscores| { + determine_catalog_name( + &catalog::v1alpha1::TrinoCatalogNameSpec::Inferred { + replace_hyphens_with_underscores, + }, + "my-postgres", + ) + .expect("catalog name should be valid") + .to_string() + }; + + assert_eq!(inferred(false), "my-postgres"); + assert_eq!(inferred(true), "my_postgres"); + } +} diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index a9080d93..8f1482bb 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -32,7 +32,7 @@ use crate::{ client_protocol::ResolvedClientProtocolConfig, fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig, }, - crd::{APP_NAME, TrinoRole, discovery::TrinoPodRef, v1alpha1}, + crd::{APP_NAME, TrinoRole, catalog::TrinoCatalogName, discovery::TrinoPodRef, v1alpha1}, trino_controller::{CONTROLLER_NAME, OPERATOR_NAME}, }; @@ -57,7 +57,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..21d82b90 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -26,14 +26,13 @@ use stackable_operator::{ }; use strum::{EnumDiscriminants, IntoEnumIterator, IntoStaticStr}; -use super::{ - ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig, ValidatedTls, - ValidatedTrinoConfig, -}; use crate::{ authentication::{self, TrinoAuthenticationConfig, TrinoAuthenticationTypes}, - controller::dereference::DereferencedObjects, - crd::{Container, TrinoRole, v1alpha1}, + controller::{ + ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig, ValidatedTls, + ValidatedTrinoConfig, dereference::DereferencedObjects, + }, + crd::{Container, TrinoRole, catalog::TrinoCatalogName, v1alpha1}, }; #[derive(Snafu, Debug, EnumDiscriminants)] @@ -109,6 +108,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 +275,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 +299,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..49a51107 100644 --- a/rust/operator-binary/src/crd/catalog/mod.rs +++ b/rust/operator-binary/src/crd/catalog/mod.rs @@ -9,7 +9,7 @@ pub mod postgresql; pub mod tpcds; pub mod tpch; -use std::collections::HashMap; +use std::{collections::HashMap, str::FromStr}; use black_hole::BlackHoleConnector; use generic::GenericConnector; @@ -18,6 +18,7 @@ use hive::HiveConnector; use iceberg::IcebergConnector; use serde::{Deserialize, Serialize}; use stackable_operator::{ + attributed_string_type, kube::CustomResource, schemars::{self, JsonSchema}, versioned::versioned, @@ -48,6 +49,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 +70,36 @@ pub mod versioned { #[serde(default, rename = "experimentalConfigRemovals")] pub config_removals: Vec, } + + // We might implement more variants in the future. See the CRD decision in + // https://github.com/stackabletech/trino-operator/issues/891 for details. + #[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 there can only be one TrinoCatalog with a + /// given name. + #[serde(rename_all = "camelCase")] + Inferred { + /// Whether 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. This mechanism allows + /// you to use valid Kubernetes names, but keeps the convenience of using `_` in + /// catalog names. + #[serde(default)] + replace_hyphens_with_underscores: bool, + }, + } +} + +impl Default for v1alpha1::TrinoCatalogNameSpec { + fn default() -> Self { + Self::Inferred { + replace_hyphens_with_underscores: false, + } + } } #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] @@ -98,6 +133,19 @@ pub enum TrinoCatalogConnector { Tpch(TpchConnector), } +attributed_string_type! { + TrinoCatalogName, + "The name of a TrinoCluster", + "lakehouse", + // Suffixes are added to produce resource/volume names. + // + // 40 characters should be sufficient and still allow the operators to append custom suffixes. + // As of 2026-07 the longest suffix is for a volume name (63 characters limit) and is + // "-sheets-credentials" (19 characters). + (max_length = 40), + is_valid_label_value +} + #[cfg(test)] mod tests { use stackable_operator::versioned::test_utils::RoundtripTestData; @@ -172,7 +220,10 @@ mod tests { secretClass: minio-credentials - connector: tpcds: {} - - connector: + - name: + inferred: + replaceHyphensWithUnderscores: true + connector: tpch: {} "}) .expect("Failed to parse TrinoCatalogSpec YAML")