From 393ca8bb8bd343e9870bc07cec3a598465f6a40f Mon Sep 17 00:00:00 2001 From: Murad Biashimov Date: Mon, 8 Jul 2024 21:34:30 +0200 Subject: [PATCH] fix(clickhouseuser): password reset --- CHANGELOG.md | 1 + controllers/basic_controller.go | 4 ++ controllers/clickhousegrant_controller.go | 2 +- controllers/clickhouseuser_controller.go | 14 +------ go.mod | 2 +- go.sum | 4 +- tests/clickhouseuser_test.go | 51 ++++++++++++++++++----- 7 files changed, 51 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d81d75d..bed889d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/controllers/basic_controller.go b/controllers/basic_controller.go index 3533ec57..6d6f2c25 100644 --- a/controllers/basic_controller.go +++ b/controllers/basic_controller.go @@ -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. diff --git a/controllers/clickhousegrant_controller.go b/controllers/clickhousegrant_controller.go index a0ce655a..17a191dd 100644 --- a/controllers/clickhousegrant_controller.go +++ b/controllers/clickhousegrant_controller.go @@ -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" diff --git a/controllers/clickhouseuser_controller.go b/controllers/clickhouseuser_controller.go index 6d161282..470fbc10 100644 --- a/controllers/clickhouseuser_controller.go +++ b/controllers/clickhouseuser_controller.go @@ -4,8 +4,6 @@ package controllers import ( "context" - "crypto/rand" - "encoding/base64" "fmt" "strconv" @@ -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 } @@ -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] -} diff --git a/go.mod b/go.mod index f710084b..10be15cd 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 1111b04c..83e0f37d 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/tests/clickhouseuser_test.go b/tests/clickhouseuser_test.go index d8c9c9c9..de1c7cf0 100644 --- a/tests/clickhouseuser_test.go +++ b/tests/clickhouseuser_test.go @@ -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" @@ -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") @@ -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) }