feat: Make DataStore Schema (prefix) Configurable

Makefile

@@ -121,8 +121,13 @@ generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and
 golint: golangci-lint ## Linting the code according to the styling guide.
  $(GOLANGCI_LINT) run -c .golangci.yml
 
-test:
- go test ./... -coverprofile cover.out
+.PHONY: test
+test: ## Run unit tests (all tests except E2E).
+ @go test \
+ ./api/... \
+ ./cmd/... \
+ ./internal/... \
+ -coverprofile cover.out
 
 _datastore-mysql:
  $(MAKE) NAME=$(NAME) -C deploy/kine/mysql mariadb

api/v1alpha1/tenantcontrolplane_types.go

@@ -249,12 +249,21 @@ type AddonsSpec struct {
 }
 
 // TenantControlPlaneSpec defines the desired state of TenantControlPlane.
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.dataStore) || has(self.dataStore)", message="unsetting the dataStore is not supported"
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.dataStoreSchema) || has(self.dataStoreSchema)", message="unsetting the dataStoreSchema is not supported"
 type TenantControlPlaneSpec struct {
  // DataStore allows to specify a DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane.
  // This parameter is optional and acts as an override over the default one which is used by the Kamaji Operator.
- // Migration from a different DataStore to another one is not yet supported and the reconciliation will be blocked.
- DataStore string `json:"dataStore,omitempty"`
- ControlPlane ControlPlane `json:"controlPlane"`
+ // Migration from one DataStore to another backed by the same Driver is possible. See: https://kamaji.clastix.io/guides/datastore-migration/
+ // Migration from one DataStore to another backed by a different Driver is not supported.
+ DataStore string `json:"dataStore,omitempty"`
+ // DataStoreSchema allows to specify the name of the database (for relational DataStores) or the key prefix (for etcd). This
+ // value is optional and immutable. Note that Kamaji currently doesn't ensure that DataStoreSchema values are unique. It's up
+ // to the user to avoid clashes between different TenantControlPlanes. If not set upon creation, Kamaji will default the
+ // DataStoreSchema by concatenating the namespace and name of the TenantControlPlane.
+ // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="changing the dataStoreSchema is not supported"
+ DataStoreSchema string `json:"dataStoreSchema,omitempty"`
+ ControlPlane ControlPlane `json:"controlPlane"`
  // Kubernetes specification for tenant control plane
  Kubernetes KubernetesSpec `json:"kubernetes"`
  // NetworkProfile specifies how the network is

charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml

@@ -6415,8 +6415,19 @@ spec:
  description: |-
  DataStore allows to specify a DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane.
  This parameter is optional and acts as an override over the default one which is used by the Kamaji Operator.
- Migration from a different DataStore to another one is not yet supported and the reconciliation will be blocked.
+ Migration from one DataStore to another backed by the same Driver is possible. See: https://kamaji.clastix.io/guides/datastore-migration/
+ Migration from one DataStore to another backed by a different Driver is not supported.
  type: string
+ dataStoreSchema:
+ description: |-
+ DataStoreSchema allows to specify the name of the database (for relational DataStores) or the key prefix (for etcd). This
+ value is optional and immutable. Note that Kamaji currently doesn't ensure that DataStoreSchema values are unique. It's up
+ to the user to avoid clashes between different TenantControlPlanes. If not set upon creation, Kamaji will default the
+ DataStoreSchema by concatenating the namespace and name of the TenantControlPlane.
+ type: string
+ x-kubernetes-validations:
+ - message: changing the dataStoreSchema is not supported
+ rule: self == oldSelf
  kubernetes:
  description: Kubernetes specification for tenant control plane
  properties:
@@ -6563,6 +6574,11 @@ spec:
  - controlPlane
  - kubernetes
  type: object
+ x-kubernetes-validations:
+ - message: unsetting the dataStore is not supported
+ rule: '!has(oldSelf.dataStore) || has(self.dataStore)'
+ - message: unsetting the dataStoreSchema is not supported
+ rule: '!has(oldSelf.dataStoreSchema) || has(self.dataStoreSchema)'
  status:
  description: TenantControlPlaneStatus defines the observed state of TenantControlPlane.
  properties:

docs/content/reference/api.md

@@ -839,7 +839,18 @@ such as the number of Pod replicas, the Service resource, or the Ingress.<br/>
  <td>
  DataStore allows to specify a DataStore that should be used to store the Kubernetes data for the given Tenant Control Plane.
 This parameter is optional and acts as an override over the default one which is used by the Kamaji Operator.
-Migration from a different DataStore to another one is not yet supported and the reconciliation will be blocked.<br/>
+Migration from one DataStore to another backed by the same Driver is possible. See: https://kamaji.clastix.io/guides/datastore-migration/
+Migration from one DataStore to another backed by a different Driver is not supported.<br/>
+ </td>
+ <td>false</td>
+ </tr><tr>
+ <td><b>dataStoreSchema</b></td>
+ <td>string</td>
+ <td>
+ DataStoreSchema allows to specify the name of the database (for relational DataStores) or the key prefix (for etcd). This
+value is optional and immutable. Note that Kamaji currently doesn't ensure that DataStoreSchema values are unique. It's up
+to the user to avoid clashes between different TenantControlPlanes. If not set upon creation, Kamaji will default the
+DataStoreSchema by concatenating the namespace and name of the TenantControlPlane.<br/>
  </td>
  <td>false</td>
  </tr><tr>

internal/resources/datastore/datastore_storage_config.go

@@ -155,9 +155,25 @@ func (r *Config) mutate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.
  username = coalesceFn(tenantControlPlane.Status.Storage.Setup.User)
  }
 
+ var dataStoreSchema string
+ switch {
+ case len(tenantControlPlane.Status.Storage.Setup.Schema) > 0:
+ // for existing TCPs, the dataStoreSchema will be adopted from the status,
+ // as the mutating webhook only takes care of TCP creations, not updates
+ dataStoreSchema = tenantControlPlane.Status.Storage.Setup.Schema
+ tenantControlPlane.Spec.DataStoreSchema = dataStoreSchema
+ case len(tenantControlPlane.Spec.DataStoreSchema) > 0:
+ // for new TCPs, the spec field will have been provided by the user
+ // or defaulted by the defaulting webhook
+ dataStoreSchema = tenantControlPlane.Spec.DataStoreSchema
+ default:
+ // this can only happen on TCP creations when the webhook is not installed
+ return fmt.Errorf("cannot build datastore storage config, schema name must either exist in Spec or Status")
+ }
+
  r.resource.Data = map[string][]byte{
  "DB_CONNECTION_STRING": []byte(r.ConnString),
- "DB_SCHEMA": coalesceFn(tenantControlPlane.Status.Storage.Setup.Schema),
+ "DB_SCHEMA": []byte(dataStoreSchema),
  "DB_USER": username,
  "DB_PASSWORD": password,
  }

internal/resources/datastore/datastore_storage_config_test.go

@@ -0,0 +1,115 @@
+// Copyright 2022 Clastix Labs
+// SPDX-License-Identifier: Apache-2.0
+
+package datastore_test
+
+import (
+ "context"
+
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+ corev1 "k8s.io/api/core/v1"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/fake"
+ "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+ kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1"
+ "github.com/clastix/kamaji/internal/resources"
+ "github.com/clastix/kamaji/internal/resources/datastore"
+)
+
+var _ = Describe("DatastoreStorageConfig", func() {
+ var (
+ ctx context.Context
+ dsc *datastore.Config
+ tcp *kamajiv1alpha1.TenantControlPlane
+ ds *kamajiv1alpha1.DataStore
+ )
+
+ BeforeEach(func() {
+ ctx = context.Background()
+
+ tcp = &kamajiv1alpha1.TenantControlPlane{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "tcp",
+ Namespace: "default",
+ },
+ Spec: kamajiv1alpha1.TenantControlPlaneSpec{},
+ }
+
+ ds = &kamajiv1alpha1.DataStore{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "datastore",
+ Namespace: "default",
+ },
+ }
+
+ Expect(kamajiv1alpha1.AddToScheme(scheme)).To(Succeed())
+ Expect(corev1.AddToScheme(scheme)).To(Succeed())
+ })
+
+ JustBeforeEach(func() {
+ fakeClient = fake.NewClientBuilder().
+ WithScheme(scheme).WithObjects(tcp).WithStatusSubresource(tcp).Build()
+
+ dsc = &datastore.Config{
+ Client: fakeClient,
+ ConnString: "",
+ DataStore: *ds,
+ }
+ })
+
+ When("TCP has no dataStoreSchema defined", func() {
+ It("should return an error", func() {
+ _, err := resources.Handle(ctx, dsc, tcp)
+ Expect(err).To(HaveOccurred())
+ })
+ })
+
+ When("TCP has dataStoreSchema set in spec", func() {
+ BeforeEach(func() {
+ tcp.Spec.DataStoreSchema = "custom-prefix"
+ })
+
+ It("should create the datastore secret with the schema name from the spec", func() {
+ op, err := resources.Handle(ctx, dsc, tcp)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(op).To(Equal(controllerutil.OperationResultCreated))
+
+ secrets := &corev1.SecretList{}
+ Expect(fakeClient.List(ctx, secrets)).To(Succeed())
+ Expect(secrets.Items).To(HaveLen(1))
+ Expect(secrets.Items[0].Data["DB_SCHEMA"]).To(Equal([]byte("custom-prefix")))
+ })
+ })
+
+ When("TCP has dataStoreSchema set in status, but not in spec", func() {
+ // this test case ensures that existing TCPs (created in a CRD version without
+ // the dataStoreSchema field) correctly adopt the spec field from the status.
+
+ It("should create the datastore secret with the correct schema name and update the TCP spec", func() {
+ By("updating the TCP status")
+ Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(tcp), tcp)).To(Succeed())
+ tcp.Status.Storage.Setup.Schema = "existing-schema-name"
+ Expect(fakeClient.Status().Update(ctx, tcp)).To(Succeed())
+
+ By("handling the resource")
+ op, err := resources.Handle(ctx, dsc, tcp)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(op).To(Equal(controllerutil.OperationResultCreated))
+
+ By("checking the secret")
+ secrets := &corev1.SecretList{}
+ Expect(fakeClient.List(ctx, secrets)).To(Succeed())
+ Expect(secrets.Items).To(HaveLen(1))
+ Expect(secrets.Items[0].Data["DB_SCHEMA"]).To(Equal([]byte("existing-schema-name")))
+
+ By("checking the TCP spec")
+ // we have to check the modified struct here (instead of retrieving the object
+ // via the fakeClient), as the TCP resource update is not done by the resources.
+ // Instead, the TCP controller will handle TCP updates after handling all resources
+ tcp.Spec.DataStoreSchema = "existing-schema-name"
+ })
+ })
+})

internal/resources/datastore/datastore_suite_test.go

@@ -0,0 +1,23 @@
+// Copyright 2022 Clastix Labs
+// SPDX-License-Identifier: Apache-2.0
+
+package datastore_test
+
+import (
+ "testing"
+
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+ "k8s.io/apimachinery/pkg/runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+var (
+ fakeClient client.Client
+ scheme *runtime.Scheme = runtime.NewScheme()
+)
+
+func TestDatastore(t *testing.T) {
+ RegisterFailHandler(Fail)
+ RunSpecs(t, "Datastore Suite")
+}

internal/webhook/handlers/handlers_suite_test.go

@@ -0,0 +1,16 @@
+// Copyright 2022 Clastix Labs
+// SPDX-License-Identifier: Apache-2.0
+
+package handlers_test
+
+import (
+ "testing"
+
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+)
+
+func TestHandlers(t *testing.T) {
+ RegisterFailHandler(Fail)
+ RunSpecs(t, "Handlers Suite")
+}

internal/webhook/handlers/tcp_defaults.go

@@ -6,6 +6,7 @@ package handlers
 import (
  "context"
  "fmt"
+ "strings"
 
  "github.com/pkg/errors"
  "gomodules.xyz/jsonpatch/v2"
@@ -23,47 +24,40 @@ type TenantControlPlaneDefaults struct {
 
 func (t TenantControlPlaneDefaults) OnCreate(object runtime.Object) AdmissionResponse {
  return func(ctx context.Context, req admission.Request) ([]jsonpatch.JsonPatchOperation, error) {
- tcp := object.(*kamajiv1alpha1.TenantControlPlane) //nolint:forcetypeassert
+ original := object.(*kamajiv1alpha1.TenantControlPlane) //nolint:forcetypeassert
 
- if len(tcp.Spec.DataStore) == 0 {
- operations, err := utils.JSONPatch(tcp, func() {
- tcp.Spec.DataStore = t.DefaultDatastore
- })
- if err != nil {
- return nil, errors.Wrap(err, "cannot create patch responses upon Tenant Control Plane creation")
- }
+ defaulted := original.DeepCopy()
+ t.defaultUnsetFields(defaulted)
 
- return operations, nil
+ operations, err := utils.JSONPatch(original, defaulted)
+ if err != nil {
+ return nil, errors.Wrap(err, "cannot create patch responses upon Tenant Control Plane creation")
  }
 
- if tcp.Spec.ControlPlane.Deployment.Replicas == nil {
- tcp.Spec.ControlPlane.Deployment.Replicas = pointer.To(int32(2))
- }
-
- return nil, nil
+ return operations, nil
  }
 }
 
 func (t TenantControlPlaneDefaults) OnDelete(runtime.Object) AdmissionResponse {
  return utils.NilOp()
 }
 
-func (t TenantControlPlaneDefaults) OnUpdate(object runtime.Object, oldObject runtime.Object) AdmissionResponse {
- return func(ctx context.Context, req admission.Request) ([]jsonpatch.JsonPatchOperation, error) {
- newTCP, oldTCP := object.(*kamajiv1alpha1.TenantControlPlane), oldObject.(*kamajiv1alpha1.TenantControlPlane) //nolint:forcetypeassert
-
- if oldTCP.Spec.DataStore == newTCP.Spec.DataStore {
- return nil, nil
- }
+func (t TenantControlPlaneDefaults) OnUpdate(runtime.Object, runtime.Object) AdmissionResponse {
+ // all immutability requirements are handled trough CEL annotations on the TenantControlPlaneSpec type
+ return utils.NilOp()
+}
 
- if len(newTCP.Spec.DataStore) == 0 {
- return nil, fmt.Errorf("DataStore is a required field")
- }
+func (t TenantControlPlaneDefaults) defaultUnsetFields(tcp *kamajiv1alpha1.TenantControlPlane) {
+ if len(tcp.Spec.DataStore) == 0 {
+ tcp.Spec.DataStore = t.DefaultDatastore
+ }
 
-  if newTCP.Spec.ControlPlane.Deployment.Replicas == nil {
-  newTCP.Spec.ControlPlane.Deployment.Replicas = pointer.To(int32(2))
-  }
+ if tcp.Spec.ControlPlane.Deployment.Replicas == nil {
+ tcp.Spec.ControlPlane.Deployment.Replicas = pointer.To(int32(2))
+ }
 
- return nil, nil
+ if len(tcp.Spec.DataStoreSchema) == 0 {
+ dss := strings.ReplaceAll(fmt.Sprintf("%s_%s", tcp.GetNamespace(), tcp.GetName()), "-", "_")
+ tcp.Spec.DataStoreSchema = dss
  }
 }