feat: Add generic database connection mechanism by sbernauer · Pull Request #1163 · stackabletech/operator-rs

sbernauer · 2026-03-02T10:21:05Z

Description

Part of stackabletech/issues#238

Definition of Done Checklist

Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
Please make sure all these things are done and tick the boxes

Author

Changes are OpenShift compatible
CRD changes approved
CRD documentation for all fields, following the style guide.
Integration tests passed (for non trivial changes)
Changes need to be "offline" compatible

Reviewer

Code contains useful comments
Code contains useful logging statements
(Integration-)Test cases added
Documentation added or updated. Follows the style guide.
Changelog updated
Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

Feature Tracker has been updated
Proper release label has been added

This reverts commit 8e05224. It was actually mot needed :)

sbernauer · 2026-03-04T14:37:29Z

crates/stackable-operator/src/databases/drivers/celery.rs

+#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct GenericCeleryDatabaseConnection {
+    /// The name of the Secret that contains an `uri` key with the complete SQLAlchemy URI.


Copied over from https://github.com/stackabletech/airflow-operator/pull/743/changes#r2786318061:

I feel URI is very generic. I think connectionString is a better description?

crates/stackable-operator/src/databases/drivers/jdbc.rs

Techassi

Only a partial review, but I left a bunch of comments with my concern and ideas/suggestions

Techassi · 2026-03-23T13:56:26Z

crates/stackable-operator/src/builder/pod/container.rs

+    {
+        self.env
+            .get_or_insert_with(Vec::new)
+            .extend(env_vars.into_iter().map(|e| e.borrow().clone()));


note: This makes zero sense. We make it look like we take an iterator of items which can be borrowed as EnvVar, but they are not actually not only borrowed, but cloned under the hood.

I would go as far as to call this an anti-pattern.

Techassi · 2026-03-23T13:58:43Z

crates/stackable-operator/src/builder/pod/env.rs

@@ -0,0 +1,20 @@
+use k8s_openapi::api::core::v1::{EnvVar, EnvVarSource, SecretKeySelector};
+
+pub fn env_var_from_secret(


note: I think this is not an appropriate name. We are not constructing an env var from a Kubernetes secret (which would need to be looked up), but we instead prepare the env var in such a way that it will source its value from a Secret (which the Kubernetes apiserver or the kubelet is responsible for).

Techassi · 2026-03-23T14:09:02Z

crates/stackable-operator/src/databases/databases/mysql.rs

+    /// against the MySQL server.
+    pub credentials_secret: String,
+
+    /// Additional map of JDBC connection parameters to append to the connection URL. The given


note: I would avoid mentioning JDBC explicitly, because these parameters are not unique to JDBC. Most database connection URI schemas support these extra parameters in some shape or form.

Techassi · 2026-03-23T14:18:49Z

crates/stackable-operator/src/databases/databases/mysql.rs

+
+    /// Name of a Secret containing the `username` and `password` keys used to authenticate
+    /// against the MySQL server.
+    pub credentials_secret: String,


note: I would like to follow the Kubernetes convention here and call this credentials_secret_name/credentialsSecretName instead.

Techassi · 2026-03-23T14:25:58Z

crates/stackable-operator/src/databases/databases/derby.rs

+    pub location: Option<String>,
+}
+
+impl JDBCDatabaseConnection for DerbyConnection {


note: This should be named JdbcDatabaseConnection instead.

Techassi · 2026-03-23T15:10:51Z

crates/stackable-operator/src/databases/drivers/jdbc.rs

+    pub fn add_to_container(&self, cb: &mut ContainerBuilder) {
+        let env_vars = self.username_env.iter().chain(self.password_env.iter());
+        cb.add_env_vars(env_vars);
+    }


note: This function name doesn't clearly convey what is being added to the container.

Putting this very bluntly: Add the connection details? What does this even mean? Do we write a file, do we spawn a command which does something, do we add env vars?

Of course I can see what the function does when I look at the body, but just looking at the name doesn't tell me anything. A better name could be add_env_vars_to_container.

Techassi · 2026-03-23T15:12:19Z

crates/stackable-operator/src/databases/helpers.rs

+    )
+}
+
+/// Returns


question: Returns... what? Seems like half of the sentence is missing here.

Techassi · 2026-03-23T15:16:23Z

crates/stackable-operator/src/databases/mod.rs

+
+#[derive(Debug, Snafu)]
+pub enum Error {
+    #[snafu(context(false), display("PostgreSQL error"))]


note: I don't see a reason why we would want/need context(false). If there is a good reason, it should be explained as a dev comment.

note: The error messages seem a little bland. We could at least say what happened, like "failed to construct PostgreSQL connection string/uri".

Techassi · 2026-03-23T15:17:19Z

crates/stackable-operator/src/databases/mod.rs

+}
+
+#[derive(Copy, Clone, Debug, Default)]
+pub enum TemplatingMechanism {


question: Do we really need both mechanims? If so, we should describe that in a dev comment.

Techassi · 2026-03-23T15:20:56Z

crates/xtask/src/crd/dummy.rs

+    pub enum DummyDatabaseConnection {
+        Postgresql(PostgresqlConnection),
+        Mysql(MysqlConnection),
+        Derby(DerbyConnection),
+        Redis(RedisConnection),
+        GenericJDBC(GenericJDBCDatabaseConnection),
+        GenericSQLAlchemy(GenericSQLAlchemyDatabaseConnection),
+        GenericCelery(GenericCeleryDatabaseConnection),
+    }


note: Again, I have a whole couple of issues with this:

I think this enum should be provided by the framework, just with the right variants: postgresql, mysql, redis (new), and generic.

All the Generic* variants are misplaced in my opinion. As per ADR, they should be covered by a single generic variant.

The enum itself could provide helper functions to downstream users to construct/get the connection details.

sbernauer added 13 commits February 11, 2026 15:41

WIP

df6084b

postgres -> postgresql

f8a0b62

Store username and password envs separately

1dc73a1

Better name modules

c581a5b

Remove leftover code

32c0ec9

Switch to config-utils templating

b9a16a9

Add TODO marker

b51f227

Add MySQL

159faac

Improve Derby JDBC driver

9bb1e95

empty commit

c2799fa

Auto-create Derby database

69fbb2a

Merge remote-tracking branch 'origin/main' into spike/generic-databases

675dcc8

Update module structure

f457c20

sbernauer mentioned this pull request Mar 2, 2026

Implement generic database connection stackabletech/hive-operator#677

Open

sbernauer added 7 commits March 2, 2026 12:52

clippy

7c193c4

cargo fmt

9e31680

Implement CeleryConnection for Postgresql

33ae519

Support specifying the templating mechanism

cfa085d

fix: Use $VAR instead of ${VAR} (for Airflow)

8e05224

Revert "fix: Use $VAR instead of ${VAR} (for Airflow)"

49f8ba0

This reverts commit 8e05224. It was actually mot needed :)

Add lots of docs

3e391ff

sbernauer mentioned this pull request Mar 4, 2026

tracking: Generic Database connection rollout stackabletech/issues#238

Open

19 tasks

Merge branch 'main' into spike/generic-databases

a21b8bd

sbernauer marked this pull request as ready for review March 4, 2026 14:23

sbernauer commented Mar 4, 2026

View reviewed changes

crates/stackable-operator/src/databases/drivers/jdbc.rs Show resolved Hide resolved

sbernauer and others added 2 commits March 19, 2026 09:26

Merge branch 'main' into spike/generic-databases

51cfff6

Improve docs

43730e2

sbernauer requested review from Techassi and removed request for Techassi March 19, 2026 11:45

sbernauer requested review from a team and maltesander March 19, 2026 11:45

sbernauer self-assigned this Mar 19, 2026

sbernauer added this to Stackable Engineering Mar 19, 2026

sbernauer moved this to Development: Waiting for Review in Stackable Engineering Mar 19, 2026

Techassi self-requested a review March 19, 2026 11:45

maltesander moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Mar 23, 2026

Merge branch 'main' into spike/generic-databases

75efe83

Techassi requested changes Mar 23, 2026

View reviewed changes

		@@ -0,0 +1,20 @@
		use k8s_openapi::api::core::v1::{EnvVar, EnvVarSource, SecretKeySelector};

		pub fn env_var_from_secret(

Uh oh!

Conversation

sbernauer commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Definition of Done Checklist

Author

Reviewer

Acceptance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Techassi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sbernauer commented Mar 2, 2026 •

edited

Loading