Skip to content

Commit

Permalink
fix(clickhouseuser): password reset
Browse files Browse the repository at this point in the history
  • Loading branch information
byashimov committed Jul 9, 2024
1 parent 500da2f commit 393ca8b
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Ignore `http.StatusBadRequest` on `ClickhouseGrant` deletion
- Retry conflict error when k8s object saved to the storage
- Fix `ClickhouseUser`: password was reset due to an incorrect processing cycle

## v0.22.0 - 2024-07-02

Expand Down
4 changes: 4 additions & 0 deletions controllers/basic_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ func (i *instanceReconcilerHelper) reconcile(ctx context.Context, o v1alpha1.Aiv
return false, nil
}

if isAlreadyProcessed(o) && IsAlreadyRunning(o) {
return false, nil
}

// Create or update.
// Even if reconcile fails, we need to update the object in kube
// to save conditions and other data.
Expand Down
2 changes: 1 addition & 1 deletion controllers/clickhousegrant_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ package controllers
import (
"context"
"fmt"
"net/http"
"slices"
"strconv"
"strings"

"github.com/aiven/aiven-go-client/v2"
avngen "github.com/aiven/go-client-codegen"
Expand Down
14 changes: 1 addition & 13 deletions controllers/clickhouseuser_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ package controllers

import (
"context"
"crypto/rand"
"encoding/base64"
"fmt"
"strconv"

Expand Down Expand Up @@ -121,8 +119,7 @@ func (h *clickhouseUserHandler) get(ctx context.Context, avn *aiven.Client, avnG
// while password is returned on create only.
// And all other GET methods return empty password, even this one.
// So the only way to have a secret here is to reset it manually
password := randPassword(maxUserPasswordLength)
_, err = avn.ClickhouseUser.ResetPassword(ctx, user.Spec.Project, user.Spec.ServiceName, user.Status.UUID, password)
password, err := avn.ClickhouseUser.ResetPassword(ctx, user.Spec.Project, user.Spec.ServiceName, user.Status.UUID, nil)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -169,12 +166,3 @@ func (h *clickhouseUserHandler) convert(i client.Object) (*v1alpha1.ClickhouseUs
}
return user, nil
}

const maxUserPasswordLength = 24

func randPassword(len int) string {
buff := make([]byte, len)
_, _ = rand.Read(buff)
str := base64.StdEncoding.EncodeToString(buff)
return str[:len]
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.22

require (
github.com/ClickHouse/clickhouse-go/v2 v2.26.0
github.com/aiven/aiven-go-client/v2 v2.23.0
github.com/aiven/aiven-go-client/v2 v2.23.1-0.20240708122045-b60dda2ba032
github.com/aiven/go-api-schemas v1.78.0
github.com/aiven/go-client-codegen v0.13.0
github.com/dave/jennifer v1.7.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ github.com/ClickHouse/ch-go v0.61.5 h1:zwR8QbYI0tsMiEcze/uIMK+Tz1D3XZXLdNrlaOpeE
github.com/ClickHouse/ch-go v0.61.5/go.mod h1:s1LJW/F/LcFs5HJnuogFMta50kKDO0lf9zzfrbl0RQg=
github.com/ClickHouse/clickhouse-go/v2 v2.26.0 h1:j4/y6NYaCcFkJwN/TU700ebW+nmsIy34RmUAAcZKy9w=
github.com/ClickHouse/clickhouse-go/v2 v2.26.0/go.mod h1:iDTViXk2Fgvf1jn2dbJd1ys+fBkdD1UMRnXlwmhijhQ=
github.com/aiven/aiven-go-client/v2 v2.23.0 h1:vNxae4tzkFTPQcAYwJ1e6XU9uOfqS3spgjyWHk5Nhwo=
github.com/aiven/aiven-go-client/v2 v2.23.0/go.mod h1:Ox/NZj1lJPT8LQya3kYq6bjQ4GdHpcauB/f+6DJygEE=
github.com/aiven/aiven-go-client/v2 v2.23.1-0.20240708122045-b60dda2ba032 h1:Z6yrLuINkO+Wc3aVf0atS9JmokB8D7+1lxmOXGBzGks=
github.com/aiven/aiven-go-client/v2 v2.23.1-0.20240708122045-b60dda2ba032/go.mod h1:KdHfLIlIRZIfCSEBd39j1Q81jlSb6Nd+oCQKqERfnuA=
github.com/aiven/go-api-schemas v1.78.0 h1:XAOfoP5kH3sevzLuVYqDHsUkbInouybjgWj0ugn+W1s=
github.com/aiven/go-api-schemas v1.78.0/go.mod h1:FYR22WcKLisZ1CYqyyGk6XqNSyxfAUtaQd+P2ydwc5A=
github.com/aiven/go-client-codegen v0.13.0 h1:fPB0D4Y/jp3IyfrwRLWeZ2Ipa2T97zkudJcnnGuHUys=
Expand Down
51 changes: 41 additions & 10 deletions tests/clickhouseuser_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package tests

import (
"context"
"crypto/tls"
"fmt"
"testing"

"github.com/ClickHouse/clickhouse-go/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -78,6 +81,30 @@ func TestClickhouseUser(t *testing.T) {
assert.EqualValues(t, secret.Data["USERNAME"], user.ObjectMeta.Name)
assert.EqualValues(t, secret.Data["CLICKHOUSEUSER_USERNAME"], user.ObjectMeta.Name)

// Secrets validation
pinger := func() error {
return pingClickhouse(
ctx,
secret.Data["CLICKHOUSEUSER_HOST"],
secret.Data["CLICKHOUSEUSER_PORT"],
secret.Data["CLICKHOUSEUSER_USERNAME"],
secret.Data["CLICKHOUSEUSER_PASSWORD"],
)
}
assert.NoError(t, pinger())

// 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.
// And we make sure that controller can delete user itself
assert.NoError(t, s.Delete(user, func() error {
_, err = avnClient.ClickhouseUser.Get(ctx, cfg.Project, chName, user.Status.UUID)
return err
}))

// User has been deleted, no access
assert.ErrorContains(t, pinger(), "Authentication failed: password is incorrect, or there is no user with such name.")

// GIVEN
// New manifest with 'username' field set
updatedUserName := randName("clickhouse-user")
Expand All @@ -97,22 +124,26 @@ func TestClickhouseUser(t *testing.T) {
require.NoError(t, s.Apply(ymlUsernameSet))
require.NoError(t, s.GetRunning(updatedUser, "metadata-name")) // GetRunning must be called with the metadata name

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

// THEN
// 'username' field is preferred over 'metadata.name'
assert.NotEqual(t, updatedUserName, updatedUser.ObjectMeta.Name)
assert.NotEqual(t, userAvn.Name, updatedUser.ObjectMeta.Name)
assert.NotEqual(t, updatedUserAvn.Name, updatedUser.ObjectMeta.Name)
assert.Equal(t, updatedUserName, updatedUser.Spec.Username)
assert.Equal(t, userAvn.Name, updatedUser.Spec.Username)
assert.Equal(t, updatedUserAvn.Name, updatedUser.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.
// And we make sure that controller can delete user itself
assert.NoError(t, s.Delete(user, func() error {
_, err = avnClient.ClickhouseUser.Get(ctx, cfg.Project, chName, user.Status.UUID)
func pingClickhouse[T string | []byte](ctx context.Context, host, port, username, password T) error {
conn, err := clickhouse.Open(&clickhouse.Options{
Protocol: clickhouse.Native,
Addr: []string{fmt.Sprintf("%s:%s", host, port)},
Auth: clickhouse.Auth{Username: string(username), Password: string(password)},
TLS: &tls.Config{InsecureSkipVerify: true},
})
if err != nil {
return err
}))
}
return conn.Ping(ctx)
}

0 comments on commit 393ca8b

Please sign in to comment.