Make the Datastore Schema (DB Name or etcd Prefix) configurable

[SimonKienzler]

I envision a feature that allows Kamaji users to control the database name resp. the etcd prefix used by a specific TCP upon creation. Currently, this value is auto-generated based on the Namespace and Name metadata of the TCP.

Current Behavior

Currently, when creating a TCP, the DataStoreStorageConfig resource is responsible for creating a Secret that contains (among other connection credentials) the so-called DB_SCHEMA key. The value of this key is subsequently used by the DatastoreSetup resource to bootstrap the configured Datastore. Depending on the type of datastore, the DB_SCHEMA value is used differently:

• for etcd, the value becomes the key prefix under which all resources of the TCP are stored,
• for SQL-based datastores, the value is used as the name for the dedicated database.

In the DataStoreStorageConfig resource, the coalesceFn() is responsible for either reading the existing schema name from the TCP status or create a new one:

kamaji/internal/resources/datastore/datastore_storage_config.go

Lines 124 to 131 in 28a098a

	coalesceFn := func(fromStatus string) []byte {
	if len(fromStatus) > 0 {
	return []byte(fromStatus)
	}
	// The dash character (-) must be replaced with an underscore, PostgreSQL is complaining about it:
	// https://github.com/clastix/kamaji/issues/328
	return []byte(strings.ReplaceAll(fmt.Sprintf("%s_%s", tenantControlPlane.GetNamespace(), tenantControlPlane.GetName()), "-", "_"))
	}

Envisioned Behavior

I think the easiest way to make the schema name configurable would be an additional field in the TCP spec, e.g. called DatastoreSchemaName. We would then implement an additional "fallback behavior" in the coalesceFn that checks if this field ist set:

coalesceFn := func(fromStatus string) []byte {
	if len(fromStatus) > 0 {
		return []byte(fromStatus)
	}

	if tenantControlPlane.Spec.DataStoreSchemaName != nil && *tenantControlPlane.Spec.DataStoreSchemaName == "" {
		return []byte(*tenantControlPlane.Spec.DataStoreSchemaName)
	}

	return []byte(strings.ReplaceAll(fmt.Sprintf("%s_%s", tenantControlPlane.GetNamespace(), tenantControlPlane.GetName()), "-", "_"))
}

Benefits

• Allows better control over the created prefixes/databases
• Especially handy when the TCP name is long and devoid of semantic meaning, as is the case when using the CAPICPPK External Cluster Reference Alpha Feature
• Allows existing Controlplane data to be adopted by a Kamaji TCP. (Think of an existing etcd cluster already hosting several Controlplanes, you could create a Datastore CR for it and configure the correct (existing) prefixes.)
• In the future, this could be used to offer not only migrations between different Datastores, but also migrations within the same Datastore.

Challenges

• The spec field would have to be set to the auto-generated default value if the user has not specified a custom name upon creation.
• The spec field would need to be immutable, migrating within the same datastore is out of scope (for now).
• Is it the user's responsibility to make sure TCPs do not use the same schema name within the same datastore? Otherwise this feature would become much more complex, as Kamaji would have to ensure that schema names are unique. With the current approach (namespace + name), we get uniqueness for free.

Alternatives

• You can of course control the schema name by carefully selecting the namespace and name of a TCP, but depending on the setup, you e.g. can't control the namespace name. And you will always have at least one underscore in the schema name.

Related Discussions

• Use existing ETCD and Kubernetes CA certificates #463

Happy about any feedback regarding this idea or counter-proposals!

[prometherion]

@SimonKienzler LGTM, it's a feature request raised also by other adopters in stealth mode.

Are you up to work on this? I think that we could extend the TenantControlPlane specification with an additional key, such as:

apiGroup: kamaji.clastix.io/v1alpha1
kind: TenantControlPlane
metadata:
  ...
spec:
  dataStore: default  # name of the Kamaji DataStore 
  dataStorePrefix: override  # or default with the mutating webhook we already have in place to `${NAMESPACE_NAME}_${TCP_NAME}`

[SimonKienzler]

I will give this a shot, yes. Expect a PR soon-ish.

Does the name dataStorePrefix make sense when using some datastore other than etcd? In that case, it's the database name, right?

[prometherion]

Rather than prefix, we could name it schema, even tho it's nonsense considering the etcd or NATS flavours, but it's already done that way.

[SimonKienzler]

@prometherion What do you think regarding

Is it the user's responsibility to make sure TCPs do not use the same schema name within the same datastore?

[prometherion]

In the future, we could make it easier to avoid collisions with some safety guardrails, right now let's focus on getting this done.