feat(clickhouseuser,clickhousedatabase): support UTF-8 names for user…

… and database names (#756)

api/v1alpha1/clickhousedatabase_types.go

@@ -7,8 +7,15 @@ import (
 )
 
 // ClickhouseDatabaseSpec defines the desired state of ClickhouseDatabase
+// +kubebuilder:validation:XValidation:rule="!has(oldSelf.databaseName) || has(self.databaseName)", message="databaseName is required once set"
 type ClickhouseDatabaseSpec struct {
 	ServiceDependant `json:",inline"`
+
+	// Specifies the Clickhouse database name. Defaults to `metadata.name` if omitted.
+	// Note: `metadata.name` is ASCII-only. For UTF-8 names, use `spec.databaseName`, but ASCII is advised for compatibility.
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	DatabaseName string `json:"databaseName,omitempty"`
 }
 
 // ClickhouseDatabaseStatus defines the observed state of ClickhouseDatabase
@@ -21,6 +28,7 @@ type ClickhouseDatabaseStatus struct {
 // +kubebuilder:subresource:status
 
 // ClickhouseDatabase is the Schema for the databases API
+// +kubebuilder:printcolumn:name="Database name",type="string",JSONPath=".spec.databaseName"
 // +kubebuilder:printcolumn:name="Service Name",type="string",JSONPath=".spec.serviceName"
 // +kubebuilder:printcolumn:name="Project",type="string",JSONPath=".spec.project"
 type ClickhouseDatabase struct {
@@ -33,6 +41,16 @@ type ClickhouseDatabase struct {
 
 var _ AivenManagedObject = &ClickhouseDatabase{}
 
+func (in *ClickhouseDatabase) GetDatabaseName() string {
+	// Default to Spec.Username and use ObjectMeta.Name if empty.
+	// ObjectMeta.Name doesn't support UTF-8 characters, Spec.Username does.
+	name := in.Spec.DatabaseName
+	if name == "" {
+		name = in.ObjectMeta.Name
+	}
+	return name
+}
+
 func (*ClickhouseDatabase) NoSecret() bool {
 	return true
 }

api/v1alpha1/clickhouserole_types.go

@@ -11,7 +11,6 @@ type ClickhouseRoleSpec struct {
 	ServiceDependant `json:",inline"`
 
 	// +kubebuilder:validation:MaxLength=255
-	// +kubebuilder:validation:Pattern="^[a-zA-Z_][0-9a-zA-Z_]*$"
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	// The role that is to be created
 	Role string `json:"role"`

api/v1alpha1/clickhouseuser_types.go

@@ -6,10 +6,26 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// TODO: Enable `username` validation rule (add a '+' to the XValidation:rule line below).
+//
+// Currently controller-gen has a bug that prevents the line below from working correctly.
+// We use XValidation on connInfoSecretTargetDisabled which is on the same level in the generated CRD yaml as
+// username. Kubebuilder has CRD flattening which merges the validation rules into a single allOf array
+// which generates invalid CRD yaml that results in a "spec.validation.openAPIV3Schema.properties[spec].allOf[0].x-kubernetes-validations:
+// Forbidden: must be empty to be structural" error when trying to install the CRDs.
+
+// kubebuilder:validation:XValidation:rule="!has(oldSelf.username) || has(self.username)",message="Username is required once set"
+
 // ClickhouseUserSpec defines the desired state of ClickhouseUser
 type ClickhouseUserSpec struct {
 	ServiceDependant `json:",inline"`
 	SecretFields     `json:",inline"`
+
+	// Name of the Clickhouse user. Defaults to `metadata.name` if omitted.
+	// Note: `metadata.name` is ASCII-only. For UTF-8 names, use `spec.username`, but ASCII is advised for compatibility.
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	Username string `json:"username,omitempty"`
 }
 
 // ClickhouseUserStatus defines the observed state of ClickhouseUser
@@ -27,6 +43,7 @@ type ClickhouseUserStatus struct {
 
 // ClickhouseUser is the Schema for the clickhouseusers API.
 // Info "Exposes secret keys": `CLICKHOUSEUSER_HOST`, `CLICKHOUSEUSER_PORT`, `CLICKHOUSEUSER_USER`, `CLICKHOUSEUSER_PASSWORD`
+// +kubebuilder:printcolumn:name="Username",type="string",JSONPath=".spec.username"
 // +kubebuilder:printcolumn:name="Service Name",type="string",JSONPath=".spec.serviceName"
 // +kubebuilder:printcolumn:name="Project",type="string",JSONPath=".spec.project"
 // +kubebuilder:printcolumn:name="Connection Information Secret",type="string",JSONPath=".spec.connInfoSecretTarget.name"
@@ -40,6 +57,16 @@ type ClickhouseUser struct {
 
 var _ AivenManagedObject = &ClickhouseUser{}
 
+func (in *ClickhouseUser) GetUsername() string {
+	// Default to Spec.Username and use ObjectMeta.Name if empty.
+	// ObjectMeta.Name doesn't support UTF-8 characters, Spec.Username does.
+	name := in.Spec.Username
+	if name == "" {
+		name = in.ObjectMeta.Name
+	}
+	return name
+}
+
 func (in *ClickhouseUser) NoSecret() bool {
 	return in.Spec.ConnInfoSecretTargetDisabled != nil && *in.Spec.ConnInfoSecretTargetDisabled
 }

charts/aiven-operator-crds/templates/aiven.io_clickhousedatabases.yaml

@@ -15,6 +15,9 @@ spec:
   scope: Namespaced
   versions:
     - additionalPrinterColumns:
+        - jsonPath: .spec.databaseName
+          name: Database name
+          type: string
         - jsonPath: .spec.serviceName
           name: Service Name
           type: string
@@ -59,6 +62,15 @@ spec:
                     - key
                     - name
                   type: object
+                databaseName:
+                  description: |-
+                    Specifies the Clickhouse database name. Defaults to `metadata.name` if omitted.
+                    Note: `metadata.name` is ASCII-only. For UTF-8 names, use `spec.databaseName`, but ASCII is advised for compatibility.
+                  maxLength: 63
+                  type: string
+                  x-kubernetes-validations:
+                    - message: Value is immutable
+                      rule: self == oldSelf
                 project:
                   description: Identifies the project this resource belongs to
                   maxLength: 63
@@ -81,6 +93,9 @@ spec:
                 - project
                 - serviceName
               type: object
+              x-kubernetes-validations:
+                - message: databaseName is required once set
+                  rule: "!has(oldSelf.databaseName) || has(self.databaseName)"
             status:
               description: ClickhouseDatabaseStatus defines the observed state of ClickhouseDatabase
               properties:

charts/aiven-operator-crds/templates/aiven.io_clickhouseroles.yaml

@@ -73,7 +73,6 @@ spec:
                 role:
                   description: The role that is to be created
                   maxLength: 255
-                  pattern: ^[a-zA-Z_][0-9a-zA-Z_]*$
                   type: string
                   x-kubernetes-validations:
                     - message: Value is immutable

charts/aiven-operator-crds/templates/aiven.io_clickhouseusers.yaml

@@ -15,6 +15,9 @@ spec:
   scope: Namespaced
   versions:
     - additionalPrinterColumns:
+        - jsonPath: .spec.username
+          name: Username
+          type: string
         - jsonPath: .spec.serviceName
           name: Service Name
           type: string
@@ -123,6 +126,15 @@ spec:
                   x-kubernetes-validations:
                     - message: Value is immutable
                       rule: self == oldSelf
+                username:
+                  description: |-
+                    Name of the Clickhouse user. Defaults to `metadata.name` if omitted.
+                    Note: `metadata.name` is ASCII-only. For UTF-8 names, use `spec.username`, but ASCII is advised for compatibility.
+                  maxLength: 63
+                  type: string
+                  x-kubernetes-validations:
+                    - message: Value is immutable
+                      rule: self == oldSelf
               required:
                 - project
                 - serviceName

config/crd/bases/aiven.io_clickhousedatabases.yaml

@@ -15,6 +15,9 @@ spec:
   scope: Namespaced
   versions:
     - additionalPrinterColumns:
+        - jsonPath: .spec.databaseName
+          name: Database name
+          type: string
         - jsonPath: .spec.serviceName
           name: Service Name
           type: string
@@ -59,6 +62,15 @@ spec:
                     - key
                     - name
                   type: object
+                databaseName:
+                  description: |-
+                    Specifies the Clickhouse database name. Defaults to `metadata.name` if omitted.
+                    Note: `metadata.name` is ASCII-only. For UTF-8 names, use `spec.databaseName`, but ASCII is advised for compatibility.
+                  maxLength: 63
+                  type: string
+                  x-kubernetes-validations:
+                    - message: Value is immutable
+                      rule: self == oldSelf
                 project:
                   description: Identifies the project this resource belongs to
                   maxLength: 63
@@ -81,6 +93,9 @@ spec:
                 - project
                 - serviceName
               type: object
+              x-kubernetes-validations:
+                - message: databaseName is required once set
+                  rule: "!has(oldSelf.databaseName) || has(self.databaseName)"
             status:
               description: ClickhouseDatabaseStatus defines the observed state of ClickhouseDatabase
               properties:

config/crd/bases/aiven.io_clickhouseroles.yaml

@@ -73,7 +73,6 @@ spec:
                 role:
                   description: The role that is to be created
                   maxLength: 255
-                  pattern: ^[a-zA-Z_][0-9a-zA-Z_]*$
                   type: string
                   x-kubernetes-validations:
                     - message: Value is immutable

config/crd/bases/aiven.io_clickhouseusers.yaml

@@ -15,6 +15,9 @@ spec:
   scope: Namespaced
   versions:
     - additionalPrinterColumns:
+        - jsonPath: .spec.username
+          name: Username
+          type: string
         - jsonPath: .spec.serviceName
           name: Service Name
           type: string
@@ -123,6 +126,15 @@ spec:
                   x-kubernetes-validations:
                     - message: Value is immutable
                       rule: self == oldSelf
+                username:
+                  description: |-
+                    Name of the Clickhouse user. Defaults to `metadata.name` if omitted.
+                    Note: `metadata.name` is ASCII-only. For UTF-8 names, use `spec.username`, but ASCII is advised for compatibility.
+                  maxLength: 63
+                  type: string
+                  x-kubernetes-validations:
+                    - message: Value is immutable
+                      rule: self == oldSelf
               required:
                 - project
                 - serviceName

controllers/clickhousedatabase_controller.go

@@ -50,9 +50,10 @@ func (h *ClickhouseDatabaseHandler) createOrUpdate(ctx context.Context, avn *aiv
 		return err
 	}
 
-	_, err = avn.ClickhouseDatabase.Get(ctx, db.Spec.Project, db.Spec.ServiceName, db.Name)
+	dbName := db.GetDatabaseName()
+	_, err = avn.ClickhouseDatabase.Get(ctx, db.Spec.Project, db.Spec.ServiceName, dbName)
 	if isNotFound(err) {
-		err = avn.ClickhouseDatabase.Create(ctx, db.Spec.Project, db.Spec.ServiceName, db.Name)
+		err = avn.ClickhouseDatabase.Create(ctx, db.Spec.Project, db.Spec.ServiceName, dbName)
 	}
 
 	if err != nil {
@@ -79,7 +80,8 @@ func (h *ClickhouseDatabaseHandler) delete(ctx context.Context, avn *aiven.Clien
 		return false, err
 	}
 
-	err = avn.ClickhouseDatabase.Delete(ctx, db.Spec.Project, db.Spec.ServiceName, db.Name)
+	dbName := db.GetDatabaseName()
+	err = avn.ClickhouseDatabase.Delete(ctx, db.Spec.Project, db.Spec.ServiceName, dbName)
 	if err != nil && !isNotFound(err) {
 		return false, err
 	}
@@ -93,7 +95,8 @@ func (h *ClickhouseDatabaseHandler) get(ctx context.Context, avn *aiven.Client,
 		return nil, err
 	}
 
-	_, err = avn.ClickhouseDatabase.Get(ctx, db.Spec.Project, db.Spec.ServiceName, db.Name)
+	dbName := db.GetDatabaseName()
+	_, err = avn.ClickhouseDatabase.Get(ctx, db.Spec.Project, db.Spec.ServiceName, dbName)
 	if err != nil {
 		return nil, err
 	}

controllers/clickhouserole_controller.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"regexp"
 	"strconv"
 	"strings"
 
@@ -21,12 +20,6 @@ import (
 	"github.com/aiven/aiven-operator/api/v1alpha1"
 )
 
-// https://clickhouse.com/docs/en/sql-reference/syntax#identifiers
-var (
-	identifierRegex      = regexp.MustCompile(`^[a-zA-Z_][0-9a-zA-Z_]+$`)
-	errInvalidIdentifier = errors.New("invalid identifier")
-)
-
 // default database used for statements that do not target a particular database
 // think CREATE ROLE / GRANT / etc...
 const defaultDatabase = "system"
@@ -133,12 +126,15 @@ func (h *clickhouseRoleHandler) convert(i client.Object) (*v1alpha1.ClickhouseRo
 	return role, nil
 }
 
-func runQuery(ctx context.Context, avn *aiven.Client, r *v1alpha1.ClickhouseRole, query string) error {
-	if !identifierRegex.MatchString(r.Spec.Role) {
-		return fmt.Errorf("%w: %q", errInvalidIdentifier, r.Spec.Role)
-	}
+// Escapes database identifiers like table or column names
+func escape(identifier string) string {
+	// See https://github.com/ClickHouse/clickhouse-go/blob/8ad6ec6b95d8b0c96d00115bc2d69ff13083f94b/lib/column/column.go#L32
+	replacer := strings.NewReplacer("`", "\\`", "\\", "\\\\")
+	return "`" + replacer.Replace(identifier) + "`"
+}
 
-	q := fmt.Sprintf("%s %s", query, r.Spec.Role)
+func runQuery(ctx context.Context, avn *aiven.Client, r *v1alpha1.ClickhouseRole, query string) error {
+	q := fmt.Sprintf("%s %s", query, escape(r.Spec.Role))
 	_, err := avn.ClickHouseQuery.Query(ctx, r.Spec.Project, r.Spec.ServiceName, defaultDatabase, q)
 	return err
 }

controllers/clickhouseuser_controller.go

@@ -53,12 +53,29 @@ func (h *clickhouseUserHandler) createOrUpdate(ctx context.Context, avn *aiven.C
 		return err
 	}
 
-	r, err := avn.ClickhouseUser.Create(ctx, user.Spec.Project, user.Spec.ServiceName, user.Name)
+	list, err := avn.ClickhouseUser.List(ctx, user.Spec.Project, user.Spec.ServiceName)
 	if err != nil {
 		return err
 	}
 
-	user.Status.UUID = r.User.UUID
+	var uuid string
+	for _, u := range list.Users {
+		if u.Name == user.GetUsername() {
+			uuid = u.UUID
+			break
+		}
+	}
+
+	if uuid == "" {
+		r, err := avn.ClickhouseUser.Create(ctx, user.Spec.Project, user.Spec.ServiceName, user.GetUsername())
+		if err != nil {
+			return err
+		}
+
+		uuid = r.User.UUID
+	}
+
+	user.Status.UUID = uuid
 
 	meta.SetStatusCondition(&user.Status.Conditions,
 		getInitializedCondition("Created",
@@ -119,12 +136,12 @@ func (h *clickhouseUserHandler) get(ctx context.Context, avn *aiven.Client, avnG
 		prefix + "HOST":     s.URIParams["host"],
 		prefix + "PORT":     s.URIParams["port"],
 		prefix + "PASSWORD": password,
-		prefix + "USERNAME": user.Name,
+		prefix + "USERNAME": user.GetUsername(),
 		// todo: remove in future releases
 		"HOST":     s.URIParams["host"],
 		"PORT":     s.URIParams["port"],
 		"PASSWORD": password,
-		"USERNAME": user.Name,
+		"USERNAME": user.GetUsername(),
 	}
 
 	secret := newSecret(user, stringData, false)

docs/docs/api-reference/clickhouse.md

@@ -16,13 +16,22 @@ title: "Clickhouse"
         key: token
 
       connInfoSecretTarget:
-        name: clickhouse-secret
-        prefix: MY_SECRET_PREFIX_
+        name: my-clickhouse
         annotations:
           foo: bar
         labels:
           baz: egg
 
+      tags:
+        env: test
+        instance: foo
+
+      userConfig:
+        ip_filter:
+          - network: 0.0.0.0/32
+            description: bar
+          - network: 10.20.0.0/16
+
       project: my-aiven-project
       cloudName: google-europe-west1
       plan: startup-16