Skip to content

Commit

Permalink
feat(clickhouseuser): add 'username' to spec
Browse files Browse the repository at this point in the history
Previously the username was set from Metadata.name, which doesn't support UTF-8:

    The ClickhouseUser "testuser-🦄" is invalid: metadata.name: Invalid
    value: "testuser-🦄": a lowercase RFC 1123

Clickhouse usernames have UTF-8 support. This can be verified by running:

    CREATE USER 'testuser-🦄' IDENTIFIED BY 'testpassword'
  • Loading branch information
rriski committed Jun 18, 2024
1 parent e94dade commit 4883263
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 57 deletions.
18 changes: 18 additions & 0 deletions api/v1alpha1/clickhouseuser_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,16 @@ import (
)

// ClickhouseUserSpec defines the desired state of ClickhouseUser
// +kubebuilder:validation:XValidation:rule="!has(oldSelf.username) || has(self.username)",message="Username is required once set"
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
Expand All @@ -27,6 +34,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"
Expand All @@ -40,6 +48,16 @@ type ClickhouseUser struct {

var _ AivenManagedObject = &ClickhouseUser{}

func (in *ClickhouseUser) GetName() 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
}
Expand Down
26 changes: 21 additions & 5 deletions charts/aiven-operator-crds/templates/aiven.io_clickhouseusers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ spec:
scope: Namespaced
versions:
- additionalPrinterColumns:
- jsonPath: .spec.username
name: Username
type: string
- jsonPath: .spec.serviceName
name: Service Name
type: string
Expand Down Expand Up @@ -49,6 +52,15 @@ spec:
metadata:
type: object
spec:
allOf:
- x-kubernetes-validations:
- message:
connInfoSecretTargetDisabled can only be set during resource
creation.
rule: has(oldSelf.connInfoSecretTargetDisabled) == has(self.connInfoSecretTargetDisabled)
- x-kubernetes-validations:
- message: Username is required once set
rule: "!has(oldSelf.username) || has(self.username)"
description: ClickhouseUserSpec defines the desired state of ClickhouseUser
properties:
authSecretRef:
Expand Down Expand Up @@ -123,15 +135,19 @@ 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
type: object
x-kubernetes-validations:
- message:
connInfoSecretTargetDisabled can only be set during resource
creation.
rule: has(oldSelf.connInfoSecretTargetDisabled) == has(self.connInfoSecretTargetDisabled)
status:
description: ClickhouseUserStatus defines the observed state of ClickhouseUser
properties:
Expand Down
26 changes: 21 additions & 5 deletions config/crd/bases/aiven.io_clickhouseusers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ spec:
scope: Namespaced
versions:
- additionalPrinterColumns:
- jsonPath: .spec.username
name: Username
type: string
- jsonPath: .spec.serviceName
name: Service Name
type: string
Expand Down Expand Up @@ -49,6 +52,15 @@ spec:
metadata:
type: object
spec:
allOf:
- x-kubernetes-validations:
- message:
connInfoSecretTargetDisabled can only be set during resource
creation.
rule: has(oldSelf.connInfoSecretTargetDisabled) == has(self.connInfoSecretTargetDisabled)
- x-kubernetes-validations:
- message: Username is required once set
rule: "!has(oldSelf.username) || has(self.username)"
description: ClickhouseUserSpec defines the desired state of ClickhouseUser
properties:
authSecretRef:
Expand Down Expand Up @@ -123,15 +135,19 @@ 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
type: object
x-kubernetes-validations:
- message:
connInfoSecretTargetDisabled can only be set during resource
creation.
rule: has(oldSelf.connInfoSecretTargetDisabled) == has(self.connInfoSecretTargetDisabled)
status:
description: ClickhouseUserStatus defines the observed state of ClickhouseUser
properties:
Expand Down
2 changes: 1 addition & 1 deletion controllers/clickhouseuser_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ 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)
r, err := avn.ClickhouseUser.Create(ctx, user.Spec.Project, user.Spec.ServiceName, user.GetName())
if err != nil {
return err
}
Expand Down
19 changes: 18 additions & 1 deletion docs/docs/api-reference/clickhouseuser.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,29 @@ title: "ClickhouseUser"

connInfoSecretTarget:
name: clickhouse-user-secret
prefix: MY_SECRET_PREFIX_
annotations:
foo: bar
labels:
baz: egg

project: my-aiven-project
serviceName: my-clickhouse
username: example-username

---

apiVersion: aiven.io/v1alpha1
kind: Clickhouse
metadata:
name: my-clickhouse
spec:
authSecretRef:
name: aiven-token
key: token

project: my-aiven-project
cloudName: google-europe-west1
plan: startup-16
```

## ClickhouseUser {: #ClickhouseUser }
Expand Down Expand Up @@ -58,6 +73,8 @@ ClickhouseUserSpec defines the desired state of ClickhouseUser.
- [`authSecretRef`](#spec.authSecretRef-property){: name='spec.authSecretRef-property'} (object). Authentication reference to Aiven token in a secret. See below for [nested schema](#spec.authSecretRef).
- [`connInfoSecretTarget`](#spec.connInfoSecretTarget-property){: name='spec.connInfoSecretTarget-property'} (object). Secret configuration. See below for [nested schema](#spec.connInfoSecretTarget).
- [`connInfoSecretTargetDisabled`](#spec.connInfoSecretTargetDisabled-property){: name='spec.connInfoSecretTargetDisabled-property'} (boolean, Immutable). When true, the secret containing connection information will not be created, defaults to false. This field cannot be changed after resource creation.
- [`username`](#spec.username-property){: name='spec.username-property'} (string, Immutable, MaxLength: 63). 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.

## authSecretRef {: #spec.authSecretRef }

Expand Down
17 changes: 16 additions & 1 deletion docs/docs/api-reference/examples/clickhouseuser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,26 @@ spec:

connInfoSecretTarget:
name: clickhouse-user-secret
prefix: MY_SECRET_PREFIX_
annotations:
foo: bar
labels:
baz: egg

project: my-aiven-project
serviceName: my-clickhouse
username: example-username

---

apiVersion: aiven.io/v1alpha1
kind: Clickhouse
metadata:
name: my-clickhouse
spec:
authSecretRef:
name: aiven-token
key: token

project: my-aiven-project
cloudName: google-europe-west1
plan: startup-16
87 changes: 43 additions & 44 deletions tests/clickhouseuser_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package tests

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -10,45 +9,6 @@ import (
"github.com/aiven/aiven-operator/api/v1alpha1"
)

func getClickhouseUserYaml(project, chName, userName, cloudName string) string {
return fmt.Sprintf(`
apiVersion: aiven.io/v1alpha1
kind: Clickhouse
metadata:
name: %[2]s
spec:
authSecretRef:
name: aiven-token
key: token
project: %[1]s
cloudName: %[4]s
plan: startup-16
---
apiVersion: aiven.io/v1alpha1
kind: ClickhouseUser
metadata:
name: %[3]s
spec:
authSecretRef:
name: aiven-token
key: token
connInfoSecretTarget:
name: my-ch-user-secret
annotations:
foo: bar
labels:
baz: egg
project: %[1]s
serviceName: %[2]s
`, project, chName, userName, cloudName)
}

func TestClickhouseUser(t *testing.T) {
t.Parallel()
defer recoverPanic(t)
Expand All @@ -57,9 +17,18 @@ func TestClickhouseUser(t *testing.T) {
ctx, cancel := testCtx()
defer cancel()

chName := randName("clickhouse-user")
chName := randName("clickhouse")
userName := randName("clickhouse-user")
yml := getClickhouseUserYaml(cfg.Project, chName, userName, cfg.PrimaryCloudName)
yml, err := loadExampleYaml("clickhouseuser.yaml", map[string]string{
"google-europe-west1": cfg.PrimaryCloudName,
"my-aiven-project": cfg.Project,
"clickhouse-user-secret": userName,
"my-clickhouse-user": userName,
"my-clickhouse": chName,
// Remove 'username' from the initial yaml
"username: example-username": "",
})
require.NoError(t, err)
s := NewSession(ctx, k8sClient, cfg.Project)

// Cleans test afterward
Expand Down Expand Up @@ -87,8 +56,10 @@ func TestClickhouseUser(t *testing.T) {

userAvn, err := avnClient.ClickhouseUser.Get(ctx, cfg.Project, chName, user.Status.UUID)
require.NoError(t, err)
assert.Equal(t, userName, user.GetName())
assert.Equal(t, userAvn.Name, user.GetName())

// Gets name from `metadata.name` when `username` is not set
assert.Equal(t, userName, user.ObjectMeta.Name)
assert.Equal(t, userAvn.Name, user.ObjectMeta.Name)

secret, err := s.GetSecret(user.Spec.ConnInfoSecretTarget.Name)
require.NoError(t, err)
Expand All @@ -103,6 +74,34 @@ func TestClickhouseUser(t *testing.T) {
assert.Equal(t, map[string]string{"foo": "bar"}, secret.Annotations)
assert.Equal(t, map[string]string{"baz": "egg"}, secret.Labels)

// GIVEN
// New manifest with 'username' field set
updatedUserName := randName("clickhouse-user")
ymlUsernameSet, err := loadExampleYaml("clickhouseuser.yaml", map[string]string{
"google-europe-west1": cfg.PrimaryCloudName,
"my-aiven-project": cfg.Project,
"clickhouse-user-secret": updatedUserName,
"my-clickhouse-user": "example-clickhouse",
"my-clickhouse": chName,
"example-username": updatedUserName,
})
require.NoError(t, err)

// WHEN
// Applies updated manifest
require.NoError(t, s.Apply(ymlUsernameSet))
require.NoError(t, s.GetRunning(user, updatedUserName))

userAvn, err = avnClient.ClickhouseUser.Get(ctx, cfg.Project, chName, user.Status.UUID)
require.NoError(t, err)

// THEN
// 'username' field is preferred over 'metadata.name'
assert.NotEqual(t, updatedUserName, user.ObjectMeta.Name)
assert.NotEqual(t, userAvn.Name, user.ObjectMeta.Name)
assert.Equal(t, updatedUserName, user.Spec.Username)
assert.Equal(t, userAvn.Name, user.Spec.Username)

// We need to validate deletion,
// because we can get false positive here:
// if service is deleted, user is destroyed in Aiven. No service — no user. No user — no user.
Expand Down

0 comments on commit 4883263

Please sign in to comment.