From f06d079d94e14a67488d867ac4ab6cc11a48dce1 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 3 Mar 2026 10:15:05 +0000 Subject: [PATCH 01/24] start --- internal/controller/clickhouse/config.go | 4 ++ internal/controller/clickhouse/constants.go | 5 ++ internal/controller/clickhouse/templates.go | 10 +++- .../clickhouse/templates/base.yaml.tmpl | 7 +++ internal/controllerutil/common.go | 9 +++ test/e2e/clickhouse_e2e_test.go | 57 +++++++++++++++++++ 6 files changed, 91 insertions(+), 1 deletion(-) diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index d6eee8d..2dd67ca 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -176,6 +176,8 @@ type baseConfigParams struct { UsersXMLPath string UsersZookeeperPath string UDFZookeeperPath string + NamedCollectionsPath string + NamedCollectionsKeyEnv string ClusterSecretEnv string ManagementPort uint16 @@ -262,6 +264,8 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 UsersXMLPath: UsersFileName, UsersZookeeperPath: KeeperPathUsers, UDFZookeeperPath: KeeperPathUDF, + NamedCollectionsPath: KeeperPathNamedCollections, + NamedCollectionsKeyEnv: EnvNamedCollectionsKey, ClusterSecretEnv: EnvClusterSecret, ManagementPort: PortManagement, diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index c049e2e..b0de330 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -36,6 +36,7 @@ const ( KeeperPathUsers = "/clickhouse/access" KeeperPathUDF = "/clickhouse/user_defined" KeeperPathDistributedDDL = "/clickhouse/task_queue/ddl" + KeeperPathNamedCollections = "/clickhouse/named_collections" ContainerName = "clickhouse-server" DefaultRevisionHistory = 10 @@ -49,11 +50,13 @@ const ( EnvDefaultUserPassword = "CLICKHOUSE_DEFAULT_USER_PASSWORD" EnvKeeperIdentity = "CLICKHOUSE_KEEPER_IDENTITY" EnvClusterSecret = "CLICKHOUSE_CLUSTER_SECRET" + EnvNamedCollectionsKey = "CLICKHOUSE_NAMED_COLLECTIONS_KEY" SecretKeyInterserverPassword = "interserver-password" SecretKeyManagementPassword = "management-password" SecretKeyKeeperIdentity = "keeper-identity" SecretKeyClusterSecret = "cluster-secret" + SecretKeyNamedCollectionsKey = "named-collections-key" ) var ( @@ -71,5 +74,7 @@ var ( {Key: SecretKeyInterserverPassword, Env: EnvInterserverPassword}, {Key: SecretKeyKeeperIdentity, Env: EnvKeeperIdentity}, {Key: SecretKeyClusterSecret, Env: EnvClusterSecret}, + {Key: SecretKeyNamedCollectionsKey, Env: EnvNamedCollectionsKey}, } + secretsToGenerateHex = []string{SecretKeyNamedCollectionsKey} ) diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 59eeb3d..31cdd6d 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -5,6 +5,7 @@ import ( "maps" "net" "path" + "slices" "strconv" appsv1 "k8s.io/api/apps/v1" @@ -137,8 +138,15 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret) boo } } + for _, key := range secretsToGenerateHex { + if _, ok := secret.Data[key]; !ok { + changed = true + secret.Data[key] = []byte(controllerutil.GenerateHexKey(16)) + } + } + for key := range secret.Data { - if _, ok := secretsToGenerate[key]; !ok { + if _, ok := secretsToGenerate[key]; !ok && !slices.Contains(secretsToGenerateHex, key) { changed = true delete(secret.Data, key) diff --git a/internal/controller/clickhouse/templates/base.yaml.tmpl b/internal/controller/clickhouse/templates/base.yaml.tmpl index 0f6222c..c668426 100644 --- a/internal/controller/clickhouse/templates/base.yaml.tmpl +++ b/internal/controller/clickhouse/templates/base.yaml.tmpl @@ -44,6 +44,13 @@ user_directories: zookeeper_path: {{ .UsersZookeeperPath }} user_defined_zookeeper_path: {{ .UDFZookeeperPath }} +named_collections_storage: + type: keeper_encrypted + key_hex: + "@from_env": {{ .NamedCollectionsKeyEnv }} + algorithm: aes_128_ctr + path: {{ .NamedCollectionsPath }} + {{- /* SSL settings */}} openSSL: {{ yaml .OpenSSL | indent 2 }} diff --git a/internal/controllerutil/common.go b/internal/controllerutil/common.go index 9d97e86..5ba03db 100644 --- a/internal/controllerutil/common.go +++ b/internal/controllerutil/common.go @@ -167,6 +167,15 @@ const ( length = 32 ) +// GenerateHexKey generates a random key and returns hex string. +func GenerateHexKey(byteLen int) string { + b := make([]byte, byteLen) + if _, err := rand.Read(b); err != nil { + panic(fmt.Sprintf("read random source: %v", err)) + } + return hex.EncodeToString(b) +} + // GeneratePassword generates a random password of fixed length using a predefined alphabet. func GeneratePassword() string { password := make([]byte, length) diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index b145e80..5e927fd 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -109,6 +109,63 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { Entry("scale up to 2 replicas", v1.ClickHouseClusterSpec{Replicas: ptr.To[int32](2)}), ) + It("should support named collections in Keeper with encryption", func(ctx context.Context) { + cr := v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: fmt.Sprintf("named-colls-%d", rand.Uint32()), //nolint:gosec + }, + Spec: v1.ClickHouseClusterSpec{ + Replicas: ptr.To[int32](1), + ContainerTemplate: v1.ContainerTemplateSpec{ + Image: v1.ContainerImage{Tag: ClickHouseBaseVersion}, + }, + DataVolumeClaimSpec: &defaultStorage, + KeeperClusterRef: &corev1.LocalObjectReference{Name: keeper.Name}, + }, + } + By("creating cluster CR") + Expect(k8sClient.Create(ctx, &cr)).To(Succeed()) + DeferCleanup(func(ctx context.Context) { + Expect(k8sClient.Delete(ctx, &cr)).To(Succeed()) + }) + WaitClickHouseUpdatedAndReady(ctx, &cr, time.Minute, false) + + By("logging named collections secret") + var secret corev1.Secret + Expect(k8sClient.Get(ctx, types.NamespacedName{ + Namespace: cr.Namespace, + Name: cr.SecretName(), + }, &secret)).To(Succeed()) + if key, ok := secret.Data["named-collections-key"]; ok { + By(fmt.Sprintf("named collections key (hex): %s", string(key))) + By(fmt.Sprintf("view with: kubectl get secret %s -n %s -o jsonpath='{.data.named-collections-key}' | base64 -d", cr.SecretName(), cr.Namespace)) + } + + By("logging generated config") + var configMap corev1.ConfigMap + cfgName := cr.ConfigMapNameByReplicaID(v1.ClickHouseReplicaID{ShardID: 0, Index: 0}) + Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cfgName}, &configMap)).To(Succeed()) + for key, data := range configMap.Data { + if strings.Contains(key, "config") && !strings.Contains(key, "users") { + By(fmt.Sprintf("%s:\n%s", key, data)) + } + } + + By("connecting to cluster") + chClient, err := testutil.NewClickHouseClient(ctx, config, &cr) + Expect(err).NotTo(HaveOccurred()) + defer chClient.Close() + + By("creating named collection") + Expect(chClient.Exec(ctx, "CREATE NAMED COLLECTION e2e_test_named_coll AS test_key = 'test_value'")).To(Succeed()) + + By("verifying named collection exists") + var name string + Expect(chClient.QueryRow(ctx, "SELECT name FROM system.named_collections WHERE name = 'e2e_test_named_coll'", &name)).To(Succeed()) + Expect(name).To(Equal("e2e_test_named_coll")) + }) + DescribeTable("ClickHouse cluster updates", func( ctx context.Context, baseReplicas int, From e4f509e69f7c3fc84cf7025066bf5e393ed966eb Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 3 Mar 2026 10:31:12 +0000 Subject: [PATCH 02/24] fix style --- internal/controller/clickhouse/constants.go | 11 +++++++---- internal/controller/clickhouse/templates.go | 2 +- internal/controllerutil/common.go | 1 + test/e2e/clickhouse_e2e_test.go | 20 +++++++++++++++++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index b0de330..7ac9c8e 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -32,10 +32,10 @@ const ( LogPath = "/var/log/clickhouse-server/" - DefaultClusterName = "default" - KeeperPathUsers = "/clickhouse/access" - KeeperPathUDF = "/clickhouse/user_defined" - KeeperPathDistributedDDL = "/clickhouse/task_queue/ddl" + DefaultClusterName = "default" + KeeperPathUsers = "/clickhouse/access" + KeeperPathUDF = "/clickhouse/user_defined" + KeeperPathDistributedDDL = "/clickhouse/task_queue/ddl" KeeperPathNamedCollections = "/clickhouse/named_collections" ContainerName = "clickhouse-server" @@ -57,6 +57,9 @@ const ( SecretKeyKeeperIdentity = "keeper-identity" SecretKeyClusterSecret = "cluster-secret" SecretKeyNamedCollectionsKey = "named-collections-key" + + // NamedCollectionsKeyByteLen is the AES-128 key size in bytes (16 bytes = 32 hex chars). + NamedCollectionsKeyByteLen = 16 ) var ( diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 31cdd6d..bfed40d 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -141,7 +141,7 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret) boo for _, key := range secretsToGenerateHex { if _, ok := secret.Data[key]; !ok { changed = true - secret.Data[key] = []byte(controllerutil.GenerateHexKey(16)) + secret.Data[key] = []byte(controllerutil.GenerateHexKey(NamedCollectionsKeyByteLen)) } } diff --git a/internal/controllerutil/common.go b/internal/controllerutil/common.go index 5ba03db..e13c355 100644 --- a/internal/controllerutil/common.go +++ b/internal/controllerutil/common.go @@ -173,6 +173,7 @@ func GenerateHexKey(byteLen int) string { if _, err := rand.Read(b); err != nil { panic(fmt.Sprintf("read random source: %v", err)) } + return hex.EncodeToString(b) } diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index 5e927fd..afb8074 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -124,6 +124,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { KeeperClusterRef: &corev1.LocalObjectReference{Name: keeper.Name}, }, } + By("creating cluster CR") Expect(k8sClient.Create(ctx, &cr)).To(Succeed()) DeferCleanup(func(ctx context.Context) { @@ -132,20 +133,28 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { WaitClickHouseUpdatedAndReady(ctx, &cr, time.Minute, false) By("logging named collections secret") + var secret corev1.Secret Expect(k8sClient.Get(ctx, types.NamespacedName{ Namespace: cr.Namespace, Name: cr.SecretName(), }, &secret)).To(Succeed()) + if key, ok := secret.Data["named-collections-key"]; ok { - By(fmt.Sprintf("named collections key (hex): %s", string(key))) - By(fmt.Sprintf("view with: kubectl get secret %s -n %s -o jsonpath='{.data.named-collections-key}' | base64 -d", cr.SecretName(), cr.Namespace)) + By("named collections key (hex): " + string(key)) + + viewCmd := "kubectl get secret " + cr.SecretName() + " -n " + cr.Namespace + + " -o jsonpath='{.data.named-collections-key}' | base64 -d" + By("view with: " + viewCmd) } By("logging generated config") + var configMap corev1.ConfigMap + cfgName := cr.ConfigMapNameByReplicaID(v1.ClickHouseReplicaID{ShardID: 0, Index: 0}) Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cfgName}, &configMap)).To(Succeed()) + for key, data := range configMap.Data { if strings.Contains(key, "config") && !strings.Contains(key, "users") { By(fmt.Sprintf("%s:\n%s", key, data)) @@ -153,16 +162,21 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { } By("connecting to cluster") + chClient, err := testutil.NewClickHouseClient(ctx, config, &cr) Expect(err).NotTo(HaveOccurred()) + defer chClient.Close() By("creating named collection") Expect(chClient.Exec(ctx, "CREATE NAMED COLLECTION e2e_test_named_coll AS test_key = 'test_value'")).To(Succeed()) By("verifying named collection exists") + var name string - Expect(chClient.QueryRow(ctx, "SELECT name FROM system.named_collections WHERE name = 'e2e_test_named_coll'", &name)).To(Succeed()) + + query := "SELECT name FROM system.named_collections WHERE name = 'e2e_test_named_coll'" + Expect(chClient.QueryRow(ctx, query, &name)).To(Succeed()) Expect(name).To(Equal("e2e_test_named_coll")) }) From 3e48fbd7484923da57a2747934965da2f7351053 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 3 Mar 2026 11:41:08 +0000 Subject: [PATCH 03/24] fix compat tests --- internal/controller/clickhouse/config.go | 2 ++ internal/controller/clickhouse/constants.go | 32 +++++++++++++++++++ internal/controller/clickhouse/templates.go | 10 +++--- .../clickhouse/templates/base.yaml.tmpl | 2 ++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index 2dd67ca..53269bd 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -176,6 +176,7 @@ type baseConfigParams struct { UsersXMLPath string UsersZookeeperPath string UDFZookeeperPath string + NamedCollectionsInKeeper bool NamedCollectionsPath string NamedCollectionsKeyEnv string @@ -264,6 +265,7 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 UsersXMLPath: UsersFileName, UsersZookeeperPath: KeeperPathUsers, UDFZookeeperPath: KeeperPathUDF, + NamedCollectionsInKeeper: SupportsNamedCollectionsInKeeper(r.Cluster.Spec.ContainerTemplate.Image.Tag), NamedCollectionsPath: KeeperPathNamedCollections, NamedCollectionsKeyEnv: EnvNamedCollectionsKey, diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 7ac9c8e..8c4fb25 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -1,9 +1,15 @@ package clickhouse import ( + "strconv" + "strings" + "github.com/blang/semver/v4" ) +// MinVersionNamedCollectionsKeeper is the minimum ClickHouse version that supports keeper_encrypted for named collections. +const MinVersionNamedCollectionsKeeper = "25.12.0" + const ( PortManagement = 9001 PortNative = 9000 @@ -62,6 +68,32 @@ const ( NamedCollectionsKeyByteLen = 16 ) +// SupportsNamedCollectionsInKeeper returns true if the given image tag (e.g. "25.11", "26.1") supports keeper_encrypted. +func SupportsNamedCollectionsInKeeper(imageTag string) bool { + if imageTag == "latest" { + return true + } + + parts := strings.SplitN(strings.TrimPrefix(imageTag, "v"), ".", 3) + if len(parts) < 2 { + return false + } + + major, err1 := strconv.ParseUint(parts[0], 10, 64) + minor, err2 := strconv.ParseUint(parts[1], 10, 64) + if err1 != nil || err2 != nil { + return false + } + + v, err := semver.Parse(strconv.FormatUint(major, 10) + "." + strconv.FormatUint(minor, 10) + ".0") + if err != nil { + return false + } + + min, _ := semver.Parse(MinVersionNamedCollectionsKeeper) + return !v.LT(min) +} + var ( breakingStatefulSetVersion, _ = semver.Parse("0.0.1") secretsToGenerate = map[string]string{ diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index bfed40d..5dd2702 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -138,10 +138,12 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret) boo } } - for _, key := range secretsToGenerateHex { - if _, ok := secret.Data[key]; !ok { - changed = true - secret.Data[key] = []byte(controllerutil.GenerateHexKey(NamedCollectionsKeyByteLen)) + if SupportsNamedCollectionsInKeeper(cr.Spec.ContainerTemplate.Image.Tag) { + for _, key := range secretsToGenerateHex { + if _, ok := secret.Data[key]; !ok { + changed = true + secret.Data[key] = []byte(controllerutil.GenerateHexKey(NamedCollectionsKeyByteLen)) + } } } diff --git a/internal/controller/clickhouse/templates/base.yaml.tmpl b/internal/controller/clickhouse/templates/base.yaml.tmpl index c668426..b594a17 100644 --- a/internal/controller/clickhouse/templates/base.yaml.tmpl +++ b/internal/controller/clickhouse/templates/base.yaml.tmpl @@ -43,6 +43,7 @@ user_directories: replicated: zookeeper_path: {{ .UsersZookeeperPath }} user_defined_zookeeper_path: {{ .UDFZookeeperPath }} +{{- if .NamedCollectionsInKeeper }} named_collections_storage: type: keeper_encrypted @@ -50,6 +51,7 @@ named_collections_storage: "@from_env": {{ .NamedCollectionsKeyEnv }} algorithm: aes_128_ctr path: {{ .NamedCollectionsPath }} +{{- end }} {{- /* SSL settings */}} openSSL: From 10e109b9d0e12b5c9b6a969c7cf54514f9649fe0 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 3 Mar 2026 12:07:49 +0000 Subject: [PATCH 04/24] style --- internal/controller/clickhouse/constants.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 8c4fb25..5548244 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -8,7 +8,7 @@ import ( ) // MinVersionNamedCollectionsKeeper is the minimum ClickHouse version that supports keeper_encrypted for named collections. -const MinVersionNamedCollectionsKeeper = "25.12.0" +const MinVersionNamedCollectionsKeeper = "25.12" const ( PortManagement = 9001 @@ -70,28 +70,32 @@ const ( // SupportsNamedCollectionsInKeeper returns true if the given image tag (e.g. "25.11", "26.1") supports keeper_encrypted. func SupportsNamedCollectionsInKeeper(imageTag string) bool { - if imageTag == "latest" { - return true + if imageTag == "" || imageTag == "latest" || strings.Contains(imageTag, "@") { + return false } - parts := strings.SplitN(strings.TrimPrefix(imageTag, "v"), ".", 3) + parts := strings.Split(strings.TrimPrefix(imageTag, "v"), ".") if len(parts) < 2 { return false } major, err1 := strconv.ParseUint(parts[0], 10, 64) minor, err2 := strconv.ParseUint(parts[1], 10, 64) + if err1 != nil || err2 != nil { return false } - v, err := semver.Parse(strconv.FormatUint(major, 10) + "." + strconv.FormatUint(minor, 10) + ".0") + parsedVersion := strconv.FormatUint(major, 10) + "." + strconv.FormatUint(minor, 10) + + v, err := semver.Parse(parsedVersion) if err != nil { return false } - min, _ := semver.Parse(MinVersionNamedCollectionsKeeper) - return !v.LT(min) + minVersion, _ := semver.Parse(MinVersionNamedCollectionsKeeper) + + return !v.LT(minVersion) } var ( From e0f2b48965a267a784091ffb1ee45a019dd5b4f4 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 3 Mar 2026 20:17:23 +0000 Subject: [PATCH 05/24] fix review --- internal/controller/clickhouse/constants.go | 7 +++++-- test/e2e/clickhouse_e2e_test.go | 21 ++++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 5548244..0537776 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -88,12 +88,15 @@ func SupportsNamedCollectionsInKeeper(imageTag string) bool { parsedVersion := strconv.FormatUint(major, 10) + "." + strconv.FormatUint(minor, 10) - v, err := semver.Parse(parsedVersion) + v, err := semver.ParseTolerant(parsedVersion) if err != nil { return false } - minVersion, _ := semver.Parse(MinVersionNamedCollectionsKeeper) + minVersion, err := semver.ParseTolerant(MinVersionNamedCollectionsKeeper) + if err != nil { + return false + } return !v.LT(minVersion) } diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index afb8074..b05fc20 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "math/rand/v2" + "path" "strconv" "strings" "time" @@ -141,11 +142,7 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }, &secret)).To(Succeed()) if key, ok := secret.Data["named-collections-key"]; ok { - By("named collections key (hex): " + string(key)) - - viewCmd := "kubectl get secret " + cr.SecretName() + " -n " + cr.Namespace + - " -o jsonpath='{.data.named-collections-key}' | base64 -d" - By("view with: " + viewCmd) + By(fmt.Sprintf("named collections key present; length=%d bytes", len(key))) } By("logging generated config") @@ -161,6 +158,20 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { } } + By("verifying named_collections_storage is rendered with keeper_encrypted") + + baseConfigKey := controllerutil.PathToName(path.Join(chctrl.ConfigPath, chctrl.ConfigFileName)) + baseConfig, ok := configMap.Data[baseConfigKey] + Expect(ok).To(BeTrue(), "base config %q not found in ConfigMap", baseConfigKey) + Expect(baseConfig).To(ContainSubstring("named_collections_storage:"), + "operator must render named_collections_storage into config") + Expect(baseConfig).To(ContainSubstring("type: keeper_encrypted"), + "named_collections_storage must use keeper_encrypted") + Expect(baseConfig).To(ContainSubstring(chctrl.EnvNamedCollectionsKey), + "named_collections_storage must reference CLICKHOUSE_NAMED_COLLECTIONS_KEY") + Expect(baseConfig).To(ContainSubstring("path: "+chctrl.KeeperPathNamedCollections), + "named_collections_storage must use Keeper path for named collections") + By("connecting to cluster") chClient, err := testutil.NewClickHouseClient(ctx, config, &cr) From e79beaccce52078c581c2167d74e0edbf750e10f Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 3 Mar 2026 22:20:03 +0000 Subject: [PATCH 06/24] fix --- .../controller/clickhouse/constants_test.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 internal/controller/clickhouse/constants_test.go diff --git a/internal/controller/clickhouse/constants_test.go b/internal/controller/clickhouse/constants_test.go new file mode 100644 index 0000000..05fc768 --- /dev/null +++ b/internal/controller/clickhouse/constants_test.go @@ -0,0 +1,22 @@ +package clickhouse + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("SupportsNamedCollectionsInKeeper", func() { + It("returns true for 25.12+", func() { + Expect(SupportsNamedCollectionsInKeeper("25.12")).To(BeTrue()) + Expect(SupportsNamedCollectionsInKeeper("25.12.1")).To(BeTrue()) + Expect(SupportsNamedCollectionsInKeeper("26.1")).To(BeTrue()) + Expect(SupportsNamedCollectionsInKeeper("27.0")).To(BeTrue()) + }) + + It("returns false for versions before 25.12", func() { + Expect(SupportsNamedCollectionsInKeeper("25.11")).To(BeFalse()) + Expect(SupportsNamedCollectionsInKeeper("25.8")).To(BeFalse()) + Expect(SupportsNamedCollectionsInKeeper("25.3")).To(BeFalse()) + Expect(SupportsNamedCollectionsInKeeper("latest")).To(BeFalse()) + }) +}) From 0761dbed2b58f1db70c999555587389fcdc0a6d7 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 17 Mar 2026 16:47:37 +0000 Subject: [PATCH 07/24] fix test --- internal/controller/clickhouse/templates.go | 3 +++ test/e2e/compatibility_e2e_test.go | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 5dd2702..6f9f3b1 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -482,6 +482,9 @@ func templateContainer(cr *v1.ClickHouseCluster, id v1.ClickHouseReplicaID, volu } for _, secret := range secretsToEnvMapping { + if secret.Key == SecretKeyNamedCollectionsKey && !SupportsNamedCollectionsInKeeper(cr.Spec.ContainerTemplate.Image.Tag) { + continue + } container.Env = append(container.Env, corev1.EnvVar{ Name: secret.Env, ValueFrom: &corev1.EnvVarSource{ diff --git a/test/e2e/compatibility_e2e_test.go b/test/e2e/compatibility_e2e_test.go index 22db98f..0aae310 100644 --- a/test/e2e/compatibility_e2e_test.go +++ b/test/e2e/compatibility_e2e_test.go @@ -2,6 +2,8 @@ package e2e import ( "context" + "fmt" + "math/rand/v2" "os" "strings" "time" @@ -33,10 +35,11 @@ var _ = Context("Compatibility", Label("compatibility"), func() { Expect(err).NotTo(HaveOccurred()) By("running on Kubernetes " + serverVersion.GitVersion) + suffix := rand.Uint32() //nolint:gosec keeper := v1.KeeperCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, - Name: "compat-keeper-" + version, + Name: fmt.Sprintf("compat-keeper-%s-%d", version, suffix), }, Spec: v1.KeeperClusterSpec{ Replicas: new(int32(3)), @@ -51,7 +54,7 @@ var _ = Context("Compatibility", Label("compatibility"), func() { clickhouse := v1.ClickHouseCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, - Name: "compat-ch-" + version, + Name: fmt.Sprintf("compat-ch-%s-%d", version, suffix), }, Spec: v1.ClickHouseClusterSpec{ Replicas: new(int32(3)), From 0ebfa59b9bac9675140ad35cf1dcfe64485f2c07 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Tue, 17 Mar 2026 17:03:19 +0000 Subject: [PATCH 08/24] replace image tag with version --- internal/controller/clickhouse/config.go | 2 +- internal/controller/clickhouse/constants.go | 2 +- internal/controller/clickhouse/sync.go | 9 ++++++++- internal/controller/clickhouse/templates.go | 12 +++++++----- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index 53269bd..0eebb90 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -265,7 +265,7 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 UsersXMLPath: UsersFileName, UsersZookeeperPath: KeeperPathUsers, UDFZookeeperPath: KeeperPathUDF, - NamedCollectionsInKeeper: SupportsNamedCollectionsInKeeper(r.Cluster.Spec.ContainerTemplate.Image.Tag), + NamedCollectionsInKeeper: SupportsNamedCollectionsInKeeper(r.keeper.Status.Version), NamedCollectionsPath: KeeperPathNamedCollections, NamedCollectionsKeyEnv: EnvNamedCollectionsKey, diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 0537776..7253d3c 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -68,7 +68,7 @@ const ( NamedCollectionsKeyByteLen = 16 ) -// SupportsNamedCollectionsInKeeper returns true if the given image tag (e.g. "25.11", "26.1") supports keeper_encrypted. +// SupportsNamedCollectionsInKeeper returns true if the given version (e.g. "25.11", "26.1") supports keeper_encrypted. func SupportsNamedCollectionsInKeeper(imageTag string) bool { if imageTag == "" || imageTag == "latest" || strings.Contains(imageTag, "@") { return false diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index 3789958..88f3394 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -190,6 +190,13 @@ func (r *clickhouseReconciler) sync(ctx context.Context, log ctrlutil.Logger) (c } func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log ctrlutil.Logger) (*ctrl.Result, error) { + if err := r.GetClient().Get(ctx, types.NamespacedName{ + Namespace: r.Cluster.Namespace, + Name: r.Cluster.Spec.KeeperClusterRef.Name, + }, &r.keeper); err != nil { + return nil, fmt.Errorf("get keeper cluster: %w", err) + } + service := templateHeadlessService(r.Cluster) if _, err := r.ReconcileService(ctx, log, service, v1.EventActionReconciling); err != nil { return nil, fmt.Errorf("reconcile service resource: %w", err) @@ -239,7 +246,7 @@ func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log return nil, fmt.Errorf("get ClickHouse cluster secret %q: %w", r.Cluster.SecretName(), getErr) } - isSecretUpdated := templateClusterSecrets(r.Cluster, &r.secret) + isSecretUpdated := templateClusterSecrets(r.Cluster, &r.secret, r.keeper.Status.Version) if err := ctrl.SetControllerReference(r.Cluster, &r.secret, r.GetScheme()); err != nil { return nil, fmt.Errorf("set controller reference for cluster secret %q: %w", r.Cluster.SecretName(), err) } diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 6f9f3b1..8923859 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -105,7 +105,7 @@ func templatePodDisruptionBudget(cr *v1.ClickHouseCluster, shardID int32) *polic return pdb } -func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret) bool { +func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, keeperVersion string) bool { secret.Name = cr.SecretName() secret.Namespace = cr.Namespace secret.Type = corev1.SecretTypeOpaque @@ -138,7 +138,7 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret) boo } } - if SupportsNamedCollectionsInKeeper(cr.Spec.ContainerTemplate.Image.Tag) { + if SupportsNamedCollectionsInKeeper(keeperVersion) { for _, key := range secretsToGenerateHex { if _, ok := secret.Data[key]; !ok { changed = true @@ -302,7 +302,7 @@ func templatePodSpec(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (corev1 return corev1.PodSpec{}, fmt.Errorf("build volumes: %w", err) } - container, err := templateContainer(cr, id, volumeMounts) + container, err := templateContainer(r, id, volumeMounts) if err != nil { return corev1.PodSpec{}, fmt.Errorf("template container: %w", err) } @@ -410,7 +410,8 @@ func templatePodSpec(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (corev1 return podSpec, nil } -func templateContainer(cr *v1.ClickHouseCluster, id v1.ClickHouseReplicaID, volumeMounts []corev1.VolumeMount) (corev1.Container, error) { +func templateContainer(r *clickhouseReconciler, id v1.ClickHouseReplicaID, volumeMounts []corev1.VolumeMount) (corev1.Container, error) { + cr := r.Cluster containerTemplate := cr.Spec.ContainerTemplate.DeepCopy() protocols := buildProtocols(cr) @@ -482,9 +483,10 @@ func templateContainer(cr *v1.ClickHouseCluster, id v1.ClickHouseReplicaID, volu } for _, secret := range secretsToEnvMapping { - if secret.Key == SecretKeyNamedCollectionsKey && !SupportsNamedCollectionsInKeeper(cr.Spec.ContainerTemplate.Image.Tag) { + if secret.Key == SecretKeyNamedCollectionsKey && !SupportsNamedCollectionsInKeeper(r.keeper.Status.Version) { continue } + container.Env = append(container.Env, corev1.EnvVar{ Name: secret.Env, ValueFrom: &corev1.EnvVarSource{ From 61b7129eed6c98ff9770902a4d7bcdaa8c607b4c Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Wed, 18 Mar 2026 17:51:18 +0000 Subject: [PATCH 09/24] new template for named collections --- internal/controller/clickhouse/config.go | 139 +++++++++++------- internal/controller/clickhouse/constants.go | 16 +- .../controller/clickhouse/constants_test.go | 28 ++-- internal/controller/clickhouse/templates.go | 16 +- .../clickhouse/templates/base.yaml.tmpl | 9 -- .../templates/named_collections.yaml.tmpl | 6 + 6 files changed, 135 insertions(+), 79 deletions(-) create mode 100644 internal/controller/clickhouse/templates/named_collections.yaml.tmpl diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index 0eebb90..8b9c09f 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -24,6 +24,8 @@ var ( networkConfigTemplateStr string //go:embed templates/log_tables.yaml.tmpl logTablesConfigTemplateStr string + //go:embed templates/named_collections.yaml.tmpl + namedCollectionsConfigTemplateStr string //go:embed templates/users.yaml.tmpl userConfigTemplateStr string //go:embed templates/client.yaml.tmpl @@ -34,35 +36,47 @@ var ( func init() { for _, templateSpec := range []struct { - Path string - Filename string - Raw string - Generator configGeneratorFunc + Path string + Filename string + Raw string + Generator configGeneratorFunc + MinKeeperVersion string }{{ - Path: ConfigPath, - Filename: ConfigFileName, - Raw: baseConfigTemplateStr, - Generator: baseConfigGenerator, + Path: ConfigPath, + Filename: ConfigFileName, + Raw: baseConfigTemplateStr, + Generator: baseConfigGenerator, + MinKeeperVersion: "", }, { - Path: path.Join(ConfigPath, ConfigDPath), - Filename: "00-network.yaml", - Raw: networkConfigTemplateStr, - Generator: networkConfigGenerator, + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-network.yaml", + Raw: networkConfigTemplateStr, + Generator: networkConfigGenerator, + MinKeeperVersion: "", }, { - Path: path.Join(ConfigPath, ConfigDPath), - Filename: "00-logs-tables.yaml", - Raw: logTablesConfigTemplateStr, - Generator: logTablesConfigGenerator, + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-logs-tables.yaml", + Raw: logTablesConfigTemplateStr, + Generator: logTablesConfigGenerator, + MinKeeperVersion: "", }, { - Path: ConfigPath, - Filename: UsersFileName, - Raw: userConfigTemplateStr, - Generator: userConfigGenerator, + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-named-collections.yaml", + Raw: namedCollectionsConfigTemplateStr, + Generator: namedCollectionsConfigGenerator, + MinKeeperVersion: MinVersionNamedCollectionsKeeper, }, { - Path: ClientConfigPath, - Filename: ClientConfigFileName, - Raw: clientConfigTemplateStr, - Generator: clientConfigGenerator, + Path: ConfigPath, + Filename: UsersFileName, + Raw: userConfigTemplateStr, + Generator: userConfigGenerator, + MinKeeperVersion: "", + }, { + Path: ClientConfigPath, + Filename: ClientConfigFileName, + Raw: clientConfigTemplateStr, + Generator: clientConfigGenerator, + MinKeeperVersion: "", }} { tmpl := template.New("").Funcs(template.FuncMap{ "yaml": func(v any) (string, error) { @@ -97,27 +111,26 @@ func init() { } generators = append(generators, &templateConfigGenerator{ - filename: templateSpec.Filename, - path: templateSpec.Path, - template: tmpl, - generator: templateSpec.Generator, + filename: templateSpec.Filename, + path: templateSpec.Path, + template: tmpl, + generator: templateSpec.Generator, + minKeeperVersion: templateSpec.MinKeeperVersion, }) } generators = append(generators, &extraConfigGenerator{ - Name: ExtraConfigFileName, - ConfigSubPath: ConfigDPath, - Getter: func(r *clickhouseReconciler) []byte { - return r.Cluster.Spec.Settings.ExtraConfig.Raw - }, + Name: ExtraConfigFileName, + ConfigSubPath: ConfigDPath, + Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraConfig.Raw }, + minKeeperVersion: "", }, &extraConfigGenerator{ - Name: ExtraUsersConfigFileName, - ConfigSubPath: UsersDPath, - Getter: func(r *clickhouseReconciler) []byte { - return r.Cluster.Spec.Settings.ExtraUsersConfig.Raw - }, + Name: ExtraUsersConfigFileName, + ConfigSubPath: UsersDPath, + Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraUsersConfig.Raw }, + minKeeperVersion: "", }) } @@ -125,15 +138,17 @@ type configGenerator interface { Filename() string Path() string ConfigKey() string + MinKeeperVersion() string Exists(r *clickhouseReconciler) bool Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) } type templateConfigGenerator struct { - filename string - path string - template *template.Template - generator configGeneratorFunc + filename string + path string + template *template.Template + generator configGeneratorFunc + minKeeperVersion string } func (g *templateConfigGenerator) Filename() string { @@ -148,6 +163,10 @@ func (g *templateConfigGenerator) ConfigKey() string { return controllerutil.PathToName(path.Join(g.path, g.filename)) } +func (g *templateConfigGenerator) MinKeeperVersion() string { + return g.minKeeperVersion +} + func (g *templateConfigGenerator) Exists(*clickhouseReconciler) bool { return true } @@ -176,9 +195,6 @@ type baseConfigParams struct { UsersXMLPath string UsersZookeeperPath string UDFZookeeperPath string - NamedCollectionsInKeeper bool - NamedCollectionsPath string - NamedCollectionsKeyEnv string ClusterSecretEnv string ManagementPort uint16 @@ -265,9 +281,6 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 UsersXMLPath: UsersFileName, UsersZookeeperPath: KeeperPathUsers, UDFZookeeperPath: KeeperPathUDF, - NamedCollectionsInKeeper: SupportsNamedCollectionsInKeeper(r.keeper.Status.Version), - NamedCollectionsPath: KeeperPathNamedCollections, - NamedCollectionsKeyEnv: EnvNamedCollectionsKey, ClusterSecretEnv: EnvClusterSecret, ManagementPort: PortManagement, @@ -284,6 +297,25 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 return builder.String(), nil } +type namedCollectionsConfigParams struct { + NamedCollectionsPath string + NamedCollectionsKeyEnv string +} + +func namedCollectionsConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, _ v1.ClickHouseReplicaID) (string, error) { + params := namedCollectionsConfigParams{ + NamedCollectionsPath: KeeperPathNamedCollections, + NamedCollectionsKeyEnv: EnvNamedCollectionsKey, + } + + builder := strings.Builder{} + if err := tmpl.Execute(&builder, params); err != nil { + return "", fmt.Errorf("template named collections config: %w", err) + } + + return builder.String(), nil +} + type networkConfigParams struct { InterserverHTTPPort uint16 InterserverHTTPUser string @@ -396,9 +428,10 @@ func clientConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, _ v } type extraConfigGenerator struct { - Name string - ConfigSubPath string - Getter func(r *clickhouseReconciler) []byte + Name string + ConfigSubPath string + Getter func(r *clickhouseReconciler) []byte + minKeeperVersion string } func (g *extraConfigGenerator) Filename() string { @@ -413,6 +446,10 @@ func (g *extraConfigGenerator) ConfigKey() string { return g.Name } +func (g *extraConfigGenerator) MinKeeperVersion() string { + return g.minKeeperVersion +} + func (g *extraConfigGenerator) Exists(r *clickhouseReconciler) bool { return len(g.Getter(r)) > 0 } diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 7253d3c..7630446 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -68,13 +68,17 @@ const ( NamedCollectionsKeyByteLen = 16 ) -// SupportsNamedCollectionsInKeeper returns true if the given version (e.g. "25.11", "26.1") supports keeper_encrypted. -func SupportsNamedCollectionsInKeeper(imageTag string) bool { - if imageTag == "" || imageTag == "latest" || strings.Contains(imageTag, "@") { +// SupportsVersion returns true if actual >= minVersion. +// Returns false for empty, "latest", or digest-based tags. +func SupportsVersion(actual, minVersion string) bool { + if minVersion == "" { + return true + } + if actual == "" || actual == "latest" || strings.Contains(actual, "@") { return false } - parts := strings.Split(strings.TrimPrefix(imageTag, "v"), ".") + parts := strings.Split(strings.TrimPrefix(actual, "v"), ".") if len(parts) < 2 { return false } @@ -93,12 +97,12 @@ func SupportsNamedCollectionsInKeeper(imageTag string) bool { return false } - minVersion, err := semver.ParseTolerant(MinVersionNamedCollectionsKeeper) + min, err := semver.ParseTolerant(minVersion) if err != nil { return false } - return !v.LT(minVersion) + return !v.LT(min) } var ( diff --git a/internal/controller/clickhouse/constants_test.go b/internal/controller/clickhouse/constants_test.go index 05fc768..35ac7be 100644 --- a/internal/controller/clickhouse/constants_test.go +++ b/internal/controller/clickhouse/constants_test.go @@ -5,18 +5,24 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("SupportsNamedCollectionsInKeeper", func() { - It("returns true for 25.12+", func() { - Expect(SupportsNamedCollectionsInKeeper("25.12")).To(BeTrue()) - Expect(SupportsNamedCollectionsInKeeper("25.12.1")).To(BeTrue()) - Expect(SupportsNamedCollectionsInKeeper("26.1")).To(BeTrue()) - Expect(SupportsNamedCollectionsInKeeper("27.0")).To(BeTrue()) +var _ = Describe("SupportsVersion", func() { + It("returns true when actual >= min", func() { + Expect(SupportsVersion("25.12", MinVersionNamedCollectionsKeeper)).To(BeTrue()) + Expect(SupportsVersion("25.12.1", MinVersionNamedCollectionsKeeper)).To(BeTrue()) + Expect(SupportsVersion("26.1", MinVersionNamedCollectionsKeeper)).To(BeTrue()) + Expect(SupportsVersion("27.0", MinVersionNamedCollectionsKeeper)).To(BeTrue()) + Expect(SupportsVersion("25.12", "")).To(BeTrue()) }) - It("returns false for versions before 25.12", func() { - Expect(SupportsNamedCollectionsInKeeper("25.11")).To(BeFalse()) - Expect(SupportsNamedCollectionsInKeeper("25.8")).To(BeFalse()) - Expect(SupportsNamedCollectionsInKeeper("25.3")).To(BeFalse()) - Expect(SupportsNamedCollectionsInKeeper("latest")).To(BeFalse()) + It("returns false when actual < min", func() { + Expect(SupportsVersion("25.11", MinVersionNamedCollectionsKeeper)).To(BeFalse()) + Expect(SupportsVersion("25.8", MinVersionNamedCollectionsKeeper)).To(BeFalse()) + Expect(SupportsVersion("25.3", MinVersionNamedCollectionsKeeper)).To(BeFalse()) + }) + + It("returns false for invalid or unknown versions", func() { + Expect(SupportsVersion("", MinVersionNamedCollectionsKeeper)).To(BeFalse()) + Expect(SupportsVersion("latest", MinVersionNamedCollectionsKeeper)).To(BeFalse()) + Expect(SupportsVersion("25.12@sha256:abc", MinVersionNamedCollectionsKeeper)).To(BeFalse()) }) }) diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 8923859..f0a3f20 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -138,13 +138,20 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, kee } } - if SupportsNamedCollectionsInKeeper(keeperVersion) { + if SupportsVersion(keeperVersion, MinVersionNamedCollectionsKeeper) { for _, key := range secretsToGenerateHex { if _, ok := secret.Data[key]; !ok { changed = true secret.Data[key] = []byte(controllerutil.GenerateHexKey(NamedCollectionsKeyByteLen)) } } + } else { + for _, key := range secretsToGenerateHex { + if _, ok := secret.Data[key]; ok { + changed = true + delete(secret.Data, key) + } + } } for key := range secret.Data { @@ -282,6 +289,11 @@ func generateConfigForSingleReplica(r *clickhouseReconciler, id v1.ClickHouseRep if !generator.Exists(r) { continue } + if minVer := generator.MinKeeperVersion(); minVer != "" { + if !SupportsVersion(r.keeper.Status.Version, minVer) { + continue + } + } data, err := generator.Generate(r, id) if err != nil { @@ -483,7 +495,7 @@ func templateContainer(r *clickhouseReconciler, id v1.ClickHouseReplicaID, volum } for _, secret := range secretsToEnvMapping { - if secret.Key == SecretKeyNamedCollectionsKey && !SupportsNamedCollectionsInKeeper(r.keeper.Status.Version) { + if secret.Key == SecretKeyNamedCollectionsKey && !SupportsVersion(r.keeper.Status.Version, MinVersionNamedCollectionsKeeper) { continue } diff --git a/internal/controller/clickhouse/templates/base.yaml.tmpl b/internal/controller/clickhouse/templates/base.yaml.tmpl index b594a17..0f6222c 100644 --- a/internal/controller/clickhouse/templates/base.yaml.tmpl +++ b/internal/controller/clickhouse/templates/base.yaml.tmpl @@ -43,15 +43,6 @@ user_directories: replicated: zookeeper_path: {{ .UsersZookeeperPath }} user_defined_zookeeper_path: {{ .UDFZookeeperPath }} -{{- if .NamedCollectionsInKeeper }} - -named_collections_storage: - type: keeper_encrypted - key_hex: - "@from_env": {{ .NamedCollectionsKeyEnv }} - algorithm: aes_128_ctr - path: {{ .NamedCollectionsPath }} -{{- end }} {{- /* SSL settings */}} openSSL: diff --git a/internal/controller/clickhouse/templates/named_collections.yaml.tmpl b/internal/controller/clickhouse/templates/named_collections.yaml.tmpl new file mode 100644 index 0000000..e620540 --- /dev/null +++ b/internal/controller/clickhouse/templates/named_collections.yaml.tmpl @@ -0,0 +1,6 @@ +named_collections_storage: + type: keeper_encrypted + key_hex: + "@from_env": {{ .NamedCollectionsKeyEnv }} + algorithm: aes_128_ctr + path: {{ .NamedCollectionsPath }} From ff93bfd289f5051d4fd297bb49a89939268795fa Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Wed, 18 Mar 2026 18:00:22 +0000 Subject: [PATCH 10/24] style --- internal/controller/clickhouse/config.go | 6 +++--- internal/controller/clickhouse/constants.go | 5 +++-- internal/controller/clickhouse/templates.go | 2 ++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index 8b9c09f..5dde6ab 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -39,7 +39,7 @@ func init() { Path string Filename string Raw string - Generator configGeneratorFunc + Generator configGeneratorFunc MinKeeperVersion string }{{ Path: ConfigPath, @@ -298,11 +298,11 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 } type namedCollectionsConfigParams struct { - NamedCollectionsPath string + NamedCollectionsPath string NamedCollectionsKeyEnv string } -func namedCollectionsConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, _ v1.ClickHouseReplicaID) (string, error) { +func namedCollectionsConfigGenerator(tmpl *template.Template, _ *clickhouseReconciler, _ v1.ClickHouseReplicaID) (string, error) { params := namedCollectionsConfigParams{ NamedCollectionsPath: KeeperPathNamedCollections, NamedCollectionsKeyEnv: EnvNamedCollectionsKey, diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 7630446..be3ff6b 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -74,6 +74,7 @@ func SupportsVersion(actual, minVersion string) bool { if minVersion == "" { return true } + if actual == "" || actual == "latest" || strings.Contains(actual, "@") { return false } @@ -97,12 +98,12 @@ func SupportsVersion(actual, minVersion string) bool { return false } - min, err := semver.ParseTolerant(minVersion) + minVer, err := semver.ParseTolerant(minVersion) if err != nil { return false } - return !v.LT(min) + return !v.LT(minVer) } var ( diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index f0a3f20..8be0a4a 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -149,6 +149,7 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, kee for _, key := range secretsToGenerateHex { if _, ok := secret.Data[key]; ok { changed = true + delete(secret.Data, key) } } @@ -289,6 +290,7 @@ func generateConfigForSingleReplica(r *clickhouseReconciler, id v1.ClickHouseRep if !generator.Exists(r) { continue } + if minVer := generator.MinKeeperVersion(); minVer != "" { if !SupportsVersion(r.keeper.Status.Version, minVer) { continue From 674a0f1e172a62da56cefd072d6410abfa35ae00 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Wed, 18 Mar 2026 18:37:33 +0000 Subject: [PATCH 11/24] fix --- internal/controller/clickhouse/config.go | 35 +++++-------------- .../clickhouse/templates/base.yaml.tmpl | 9 +++++ .../templates/named_collections.yaml.tmpl | 6 ---- 3 files changed, 17 insertions(+), 33 deletions(-) delete mode 100644 internal/controller/clickhouse/templates/named_collections.yaml.tmpl diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index 5dde6ab..dba79d1 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -24,8 +24,6 @@ var ( networkConfigTemplateStr string //go:embed templates/log_tables.yaml.tmpl logTablesConfigTemplateStr string - //go:embed templates/named_collections.yaml.tmpl - namedCollectionsConfigTemplateStr string //go:embed templates/users.yaml.tmpl userConfigTemplateStr string //go:embed templates/client.yaml.tmpl @@ -59,12 +57,6 @@ func init() { Raw: logTablesConfigTemplateStr, Generator: logTablesConfigGenerator, MinKeeperVersion: "", - }, { - Path: path.Join(ConfigPath, ConfigDPath), - Filename: "00-named-collections.yaml", - Raw: namedCollectionsConfigTemplateStr, - Generator: namedCollectionsConfigGenerator, - MinKeeperVersion: MinVersionNamedCollectionsKeeper, }, { Path: ConfigPath, Filename: UsersFileName, @@ -201,6 +193,10 @@ type baseConfigParams struct { ClusterHosts [][]string OpenSSL controller.OpenSSLConfig + + NamedCollectionsEnabled bool + NamedCollectionsPath string + NamedCollectionsKeyEnv string } type macro struct { @@ -287,30 +283,15 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 ClusterHosts: clusterHosts, OpenSSL: openSSL, - } - builder := strings.Builder{} - if err := tmpl.Execute(&builder, params); err != nil { - return "", fmt.Errorf("template base config: %w", err) - } - - return builder.String(), nil -} - -type namedCollectionsConfigParams struct { - NamedCollectionsPath string - NamedCollectionsKeyEnv string -} - -func namedCollectionsConfigGenerator(tmpl *template.Template, _ *clickhouseReconciler, _ v1.ClickHouseReplicaID) (string, error) { - params := namedCollectionsConfigParams{ - NamedCollectionsPath: KeeperPathNamedCollections, - NamedCollectionsKeyEnv: EnvNamedCollectionsKey, + NamedCollectionsEnabled: SupportsVersion(r.keeper.Status.Version, MinVersionNamedCollectionsKeeper), + NamedCollectionsPath: KeeperPathNamedCollections, + NamedCollectionsKeyEnv: EnvNamedCollectionsKey, } builder := strings.Builder{} if err := tmpl.Execute(&builder, params); err != nil { - return "", fmt.Errorf("template named collections config: %w", err) + return "", fmt.Errorf("template base config: %w", err) } return builder.String(), nil diff --git a/internal/controller/clickhouse/templates/base.yaml.tmpl b/internal/controller/clickhouse/templates/base.yaml.tmpl index 0f6222c..9b74984 100644 --- a/internal/controller/clickhouse/templates/base.yaml.tmpl +++ b/internal/controller/clickhouse/templates/base.yaml.tmpl @@ -50,3 +50,12 @@ openSSL: {{- /* Special settings, required for default config */}} display_secrets_in_show_and_select: true + +{{- if .NamedCollectionsEnabled }} +named_collections_storage: + type: keeper_encrypted + key_hex: + "@from_env": {{ .NamedCollectionsKeyEnv }} + algorithm: aes_128_ctr + path: {{ .NamedCollectionsPath }} +{{- end }} diff --git a/internal/controller/clickhouse/templates/named_collections.yaml.tmpl b/internal/controller/clickhouse/templates/named_collections.yaml.tmpl deleted file mode 100644 index e620540..0000000 --- a/internal/controller/clickhouse/templates/named_collections.yaml.tmpl +++ /dev/null @@ -1,6 +0,0 @@ -named_collections_storage: - type: keeper_encrypted - key_hex: - "@from_env": {{ .NamedCollectionsKeyEnv }} - algorithm: aes_128_ctr - path: {{ .NamedCollectionsPath }} From 6b1e7b8d57e8e6c3c106a0d7b6a2e50b8d4f2cc4 Mon Sep 17 00:00:00 2001 From: Konstantin Vedernikov Date: Wed, 18 Mar 2026 19:02:40 +0000 Subject: [PATCH 12/24] better --- internal/controller/clickhouse/config.go | 127 +++++++++++------- .../clickhouse/templates/base.yaml.tmpl | 9 -- .../clickhouse/templates/base_25_8.yaml.tmpl | 59 ++++++++ 3 files changed, 141 insertions(+), 54 deletions(-) create mode 100644 internal/controller/clickhouse/templates/base_25_8.yaml.tmpl diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index dba79d1..ce9297a 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -19,7 +19,9 @@ import ( var ( //go:embed templates/base.yaml.tmpl - baseConfigTemplateStr string + baseTemplateStr string + //go:embed templates/base_25_8.yaml.tmpl + base25_8TemplateStr string //go:embed templates/network.yaml.tmpl networkConfigTemplateStr string //go:embed templates/log_tables.yaml.tmpl @@ -33,6 +35,47 @@ var ( ) func init() { + templateFuncs := template.FuncMap{ + "yaml": func(v any) (string, error) { + data, err := yaml.Marshal(v) + return string(data), err + }, + "indent": func(countRaw any, strRaw any) (string, error) { + count, ok := countRaw.(int) + if !ok { + return "", fmt.Errorf("indent: expected int for indentation value, got %T", countRaw) + } + + str, ok := strRaw.(string) + if !ok { + return "", fmt.Errorf("indent: expected string for content value, got %T", strRaw) + } + + builder := strings.Builder{} + indentation := strings.Repeat(" ", count) + + for line := range strings.SplitSeq(str, "\n") { + if _, err := builder.WriteString(fmt.Sprintf("%s%s\n", indentation, line)); err != nil { + return "", fmt.Errorf("failed to write indented line: %w", err) + } + } + + return builder.String(), nil + }, + } + + baseTmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(baseTemplateStr)) + base25_8Tmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(base25_8TemplateStr)) + + generators = append(generators, &baseConfigGenerator{ + path: ConfigPath, + filename: ConfigFileName, + templates: []versionedTemplate{ + {minVersion: "", tmpl: baseTmpl}, + {minVersion: MinVersionNamedCollectionsKeeper, tmpl: base25_8Tmpl}, + }, + }) + for _, templateSpec := range []struct { Path string Filename string @@ -40,12 +83,6 @@ func init() { Generator configGeneratorFunc MinKeeperVersion string }{{ - Path: ConfigPath, - Filename: ConfigFileName, - Raw: baseConfigTemplateStr, - Generator: baseConfigGenerator, - MinKeeperVersion: "", - }, { Path: path.Join(ConfigPath, ConfigDPath), Filename: "00-network.yaml", Raw: networkConfigTemplateStr, @@ -70,37 +107,7 @@ func init() { Generator: clientConfigGenerator, MinKeeperVersion: "", }} { - tmpl := template.New("").Funcs(template.FuncMap{ - "yaml": func(v any) (string, error) { - data, err := yaml.Marshal(v) - return string(data), err - }, - "indent": func(countRaw any, strRaw any) (string, error) { - count, ok := countRaw.(int) - if !ok { - return "", fmt.Errorf("indent: expected int for indentation value, got %T", countRaw) - } - - str, ok := strRaw.(string) - if !ok { - return "", fmt.Errorf("indent: expected string for content value, got %T", strRaw) - } - - builder := strings.Builder{} - indentation := strings.Repeat(" ", count) - - for line := range strings.SplitSeq(str, "\n") { - if _, err := builder.WriteString(fmt.Sprintf("%s%s\n", indentation, line)); err != nil { - return "", fmt.Errorf("failed to write indented line: %w", err) - } - } - - return builder.String(), nil - }, - }) - if _, err := tmpl.Parse(templateSpec.Raw); err != nil { - panic(fmt.Sprintf("failed to parse template %s: %v", templateSpec.Filename, err)) - } + tmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(templateSpec.Raw)) generators = append(generators, &templateConfigGenerator{ filename: templateSpec.Filename, @@ -135,6 +142,38 @@ type configGenerator interface { Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) } +type versionedTemplate struct { + minVersion string + tmpl *template.Template +} + +type baseConfigGenerator struct { + path string + filename string + templates []versionedTemplate +} + +func (g *baseConfigGenerator) Filename() string { return g.filename } +func (g *baseConfigGenerator) Path() string { return g.path } +func (g *baseConfigGenerator) ConfigKey() string { + return controllerutil.PathToName(path.Join(g.path, g.filename)) +} +func (g *baseConfigGenerator) MinKeeperVersion() string { return "" } +func (g *baseConfigGenerator) Exists(*clickhouseReconciler) bool { return true } +func (g *baseConfigGenerator) Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { + keeperVersion := r.keeper.Status.Version + + var tmpl *template.Template + + for _, vt := range g.templates { + if vt.minVersion == "" || SupportsVersion(keeperVersion, vt.minVersion) { + tmpl = vt.tmpl + } + } + + return executeBaseConfig(tmpl, r, id) +} + type templateConfigGenerator struct { filename string path string @@ -194,9 +233,8 @@ type baseConfigParams struct { OpenSSL controller.OpenSSLConfig - NamedCollectionsEnabled bool - NamedCollectionsPath string - NamedCollectionsKeyEnv string + NamedCollectionsPath string + NamedCollectionsKeyEnv string } type macro struct { @@ -209,7 +247,7 @@ type keeperNode struct { Secure bool } -func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { +func executeBaseConfig(tmpl *template.Template, r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { keeperNodes := make([]keeperNode, 0, r.keeper.Replicas()) for _, host := range r.keeper.Hostnames() { if r.keeper.Spec.Settings.TLS.Enabled { @@ -284,9 +322,8 @@ func baseConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, id v1 OpenSSL: openSSL, - NamedCollectionsEnabled: SupportsVersion(r.keeper.Status.Version, MinVersionNamedCollectionsKeeper), - NamedCollectionsPath: KeeperPathNamedCollections, - NamedCollectionsKeyEnv: EnvNamedCollectionsKey, + NamedCollectionsPath: KeeperPathNamedCollections, + NamedCollectionsKeyEnv: EnvNamedCollectionsKey, } builder := strings.Builder{} diff --git a/internal/controller/clickhouse/templates/base.yaml.tmpl b/internal/controller/clickhouse/templates/base.yaml.tmpl index 9b74984..0f6222c 100644 --- a/internal/controller/clickhouse/templates/base.yaml.tmpl +++ b/internal/controller/clickhouse/templates/base.yaml.tmpl @@ -50,12 +50,3 @@ openSSL: {{- /* Special settings, required for default config */}} display_secrets_in_show_and_select: true - -{{- if .NamedCollectionsEnabled }} -named_collections_storage: - type: keeper_encrypted - key_hex: - "@from_env": {{ .NamedCollectionsKeyEnv }} - algorithm: aes_128_ctr - path: {{ .NamedCollectionsPath }} -{{- end }} diff --git a/internal/controller/clickhouse/templates/base_25_8.yaml.tmpl b/internal/controller/clickhouse/templates/base_25_8.yaml.tmpl new file mode 100644 index 0000000..2ee0884 --- /dev/null +++ b/internal/controller/clickhouse/templates/base_25_8.yaml.tmpl @@ -0,0 +1,59 @@ +path: {{ .Path }} +logger: +{{ yaml .Log | indent 2 }} +macros: + {{- range $i, $m := .Macros }} + {{ $m.Name }}: {{ $m.Value }} + {{- end }} + +{{- /* Replication settings */}} +zookeeper: + nodes: + {{- range $index, $node := .KeeperNodes }} + - host: {{ $node.Host }} + port: {{ $node.Port }} + {{- if $node.Secure }} + secure: 1 + {{- end }} + {{- end }} + identity: + "@from_env": {{ .KeeperIdentityEnv }} + +remote_servers: + default: + secret: + "@from_env": {{ .ClusterSecretEnv }} + shard: + {{- $ctx := .}} + {{- range $shard, $replicas := .ClusterHosts }} + - internal_replication: true + replica: + {{- range $i, $replica := $replicas }} + - host: {{ $replica }} + port: {{ $ctx.ManagementPort }} + {{- end }} + {{- end }} + +distributed_ddl: + path: {{ .DistributedDDLPath }} + profile: {{ .DistributedDDLProfileName }} +user_directories: + users_xml: + path: {{ .UsersXMLPath }} + replicated: + zookeeper_path: {{ .UsersZookeeperPath }} +user_defined_zookeeper_path: {{ .UDFZookeeperPath }} + +{{- /* SSL settings */}} +openSSL: +{{ yaml .OpenSSL | indent 2 }} + +{{- /* Special settings, required for default config */}} +display_secrets_in_show_and_select: true + +named_collections_storage: + type: keeper_encrypted + key_hex: + "@from_env": {{ .NamedCollectionsKeyEnv }} + algorithm: aes_128_ctr + path: {{ .NamedCollectionsPath }} From 6b52f7f025cc764cb7f945dbe322519c608187d3 Mon Sep 17 00:00:00 2001 From: Pervakov Grigorii Date: Mon, 16 Mar 2026 20:28:24 +0100 Subject: [PATCH 13/24] fix(docs): .containerTemplate.resources docstring (#135) --- api/v1alpha1/common.go | 3 +-- config/crd/bases/clickhouse.com_clickhouseclusters.yaml | 5 ++--- config/crd/bases/clickhouse.com_keeperclusters.yaml | 5 ++--- .../templates/crd/clickhouseclusters.clickhouse.com.yaml | 4 +--- dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml | 4 +--- docs/api_reference.md | 2 +- 6 files changed, 8 insertions(+), 15 deletions(-) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index c7e1cdb..55bc7c6 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -248,8 +248,7 @@ type ContainerTemplateSpec struct { // +kubebuilder:validation:Enum="Always";"Never";"IfNotPresent" ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` - // Resources is the resource requirements for the container. - // This field cannot be updated once the cluster is created. + // Resources is the resource requirements for the server container. // +optional Resources corev1.ResourceRequirements `json:"resources,omitempty"` diff --git a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml index 8400893..770801e 100644 --- a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml +++ b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml @@ -264,9 +264,8 @@ spec: - IfNotPresent type: string resources: - description: |- - Resources is the resource requirements for the container. - This field cannot be updated once the cluster is created. + description: Resources is the resource requirements for the server + container. properties: claims: description: |- diff --git a/config/crd/bases/clickhouse.com_keeperclusters.yaml b/config/crd/bases/clickhouse.com_keeperclusters.yaml index 566bb8a..74eed62 100644 --- a/config/crd/bases/clickhouse.com_keeperclusters.yaml +++ b/config/crd/bases/clickhouse.com_keeperclusters.yaml @@ -263,9 +263,8 @@ spec: - IfNotPresent type: string resources: - description: |- - Resources is the resource requirements for the container. - This field cannot be updated once the cluster is created. + description: Resources is the resource requirements for the server + container. properties: claims: description: |- diff --git a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml index 0428e9c..ef6e3db 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -251,9 +251,7 @@ spec: - IfNotPresent type: string resources: - description: |- - Resources is the resource requirements for the container. - This field cannot be updated once the cluster is created. + description: Resources is the resource requirements for the server container. properties: claims: description: |- diff --git a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml index 54b5d09..712fb08 100644 --- a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml @@ -251,9 +251,7 @@ spec: - IfNotPresent type: string resources: - description: |- - Resources is the resource requirements for the container. - This field cannot be updated once the cluster is created. + description: Resources is the resource requirements for the server container. properties: claims: description: |- diff --git a/docs/api_reference.md b/docs/api_reference.md index b08197a..ee0b653 100644 --- a/docs/api_reference.md +++ b/docs/api_reference.md @@ -152,7 +152,7 @@ ContainerTemplateSpec describes the container configuration overrides for the cl |-------|------|-------------|----------|---------| | `image` | [ContainerImage](#containerimage) | Image is the container image to be deployed. | true | | | `imagePullPolicy` | [PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#pullpolicy-v1-core) | ImagePullPolicy for the image, which defaults to IfNotPresent. | false | | -| `resources` | [ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#resourcerequirements-v1-core) | Resources is the resource requirements for the container.
This field cannot be updated once the cluster is created. | false | | +| `resources` | [ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#resourcerequirements-v1-core) | Resources is the resource requirements for the server container. | false | | | `volumeMounts` | [VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#volumemount-v1-core) array | VolumeMounts is the list of volume mounts for the container. | false | | | `env` | [EnvVar](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#envvar-v1-core) array | Env is the list of environment variables to set in the container. | false | | | `securityContext` | [SecurityContext](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#securitycontext-v1-core) | SecurityContext defines the security options the container should be run with.
If set, the fields of SecurityContext override the equivalent fields of PodSecurityContext.
More info: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ | false | | From cebaa5222c7a5a555c6c36ee38321504dc0d35f4 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Thu, 19 Mar 2026 16:20:54 +0100 Subject: [PATCH 14/24] fix review --- internal/controller/clickhouse/config.go | 171 +++++++++--------- internal/controller/clickhouse/constants.go | 86 ++++----- .../controller/clickhouse/constants_test.go | 28 --- .../controller/clickhouse/controller_test.go | 9 +- internal/controller/clickhouse/sync.go | 2 +- internal/controller/clickhouse/templates.go | 49 ++--- .../clickhouse/templates/base_25_8.yaml.tmpl | 59 ------ .../templates/named_collections.yaml.tmpl | 6 + internal/controllerutil/common.go | 8 +- internal/upgrade/checker.go | 17 +- internal/upgrade/fetcher.go | 2 +- 11 files changed, 167 insertions(+), 270 deletions(-) delete mode 100644 internal/controller/clickhouse/constants_test.go delete mode 100644 internal/controller/clickhouse/templates/base_25_8.yaml.tmpl create mode 100644 internal/controller/clickhouse/templates/named_collections.yaml.tmpl diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index ce9297a..c437625 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -15,13 +15,14 @@ import ( "github.com/ClickHouse/clickhouse-operator/internal/controller" "github.com/ClickHouse/clickhouse-operator/internal/controller/keeper" "github.com/ClickHouse/clickhouse-operator/internal/controllerutil" + "github.com/ClickHouse/clickhouse-operator/internal/upgrade" ) var ( //go:embed templates/base.yaml.tmpl baseTemplateStr string - //go:embed templates/base_25_8.yaml.tmpl - base25_8TemplateStr string + //go:embed templates/named_collections.yaml.tmpl + namedCollectionsTemplateStr string //go:embed templates/network.yaml.tmpl networkConfigTemplateStr string //go:embed templates/log_tables.yaml.tmpl @@ -65,71 +66,67 @@ func init() { } baseTmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(baseTemplateStr)) - base25_8Tmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(base25_8TemplateStr)) generators = append(generators, &baseConfigGenerator{ path: ConfigPath, filename: ConfigFileName, - templates: []versionedTemplate{ - {minVersion: "", tmpl: baseTmpl}, - {minVersion: MinVersionNamedCollectionsKeeper, tmpl: base25_8Tmpl}, - }, + tmpl: baseTmpl, }) for _, templateSpec := range []struct { - Path string - Filename string - Raw string - Generator configGeneratorFunc - MinKeeperVersion string + Path string + Filename string + Raw string + Generator configGeneratorFunc + MinVersion upgrade.ClickHouseVersion }{{ - Path: path.Join(ConfigPath, ConfigDPath), - Filename: "00-network.yaml", - Raw: networkConfigTemplateStr, - Generator: networkConfigGenerator, - MinKeeperVersion: "", + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-network.yaml", + Raw: networkConfigTemplateStr, + Generator: networkConfigGenerator, + }, { + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-logs-tables.yaml", + Raw: logTablesConfigTemplateStr, + Generator: logTablesConfigGenerator, }, { - Path: path.Join(ConfigPath, ConfigDPath), - Filename: "00-logs-tables.yaml", - Raw: logTablesConfigTemplateStr, - Generator: logTablesConfigGenerator, - MinKeeperVersion: "", + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-named-collections.yaml", + Raw: namedCollectionsTemplateStr, + Generator: namedCollectionsConfigGenerator, + MinVersion: MinVersionNamedCollections, }, { - Path: ConfigPath, - Filename: UsersFileName, - Raw: userConfigTemplateStr, - Generator: userConfigGenerator, - MinKeeperVersion: "", + Path: ConfigPath, + Filename: UsersFileName, + Raw: userConfigTemplateStr, + Generator: userConfigGenerator, }, { - Path: ClientConfigPath, - Filename: ClientConfigFileName, - Raw: clientConfigTemplateStr, - Generator: clientConfigGenerator, - MinKeeperVersion: "", + Path: ClientConfigPath, + Filename: ClientConfigFileName, + Raw: clientConfigTemplateStr, + Generator: clientConfigGenerator, }} { tmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(templateSpec.Raw)) generators = append(generators, &templateConfigGenerator{ - filename: templateSpec.Filename, - path: templateSpec.Path, - template: tmpl, - generator: templateSpec.Generator, - minKeeperVersion: templateSpec.MinKeeperVersion, + filename: templateSpec.Filename, + path: templateSpec.Path, + template: tmpl, + generator: templateSpec.Generator, + minVersion: templateSpec.MinVersion, }) } generators = append(generators, &extraConfigGenerator{ - Name: ExtraConfigFileName, - ConfigSubPath: ConfigDPath, - Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraConfig.Raw }, - minKeeperVersion: "", + Name: ExtraConfigFileName, + ConfigSubPath: ConfigDPath, + Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraConfig.Raw }, }, &extraConfigGenerator{ - Name: ExtraUsersConfigFileName, - ConfigSubPath: UsersDPath, - Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraUsersConfig.Raw }, - minKeeperVersion: "", + Name: ExtraUsersConfigFileName, + ConfigSubPath: UsersDPath, + Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraUsersConfig.Raw }, }) } @@ -137,20 +134,14 @@ type configGenerator interface { Filename() string Path() string ConfigKey() string - MinKeeperVersion() string Exists(r *clickhouseReconciler) bool Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) } -type versionedTemplate struct { - minVersion string - tmpl *template.Template -} - type baseConfigGenerator struct { - path string - filename string - templates []versionedTemplate + path string + filename string + tmpl *template.Template } func (g *baseConfigGenerator) Filename() string { return g.filename } @@ -158,28 +149,17 @@ func (g *baseConfigGenerator) Path() string { return g.path } func (g *baseConfigGenerator) ConfigKey() string { return controllerutil.PathToName(path.Join(g.path, g.filename)) } -func (g *baseConfigGenerator) MinKeeperVersion() string { return "" } func (g *baseConfigGenerator) Exists(*clickhouseReconciler) bool { return true } func (g *baseConfigGenerator) Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { - keeperVersion := r.keeper.Status.Version - - var tmpl *template.Template - - for _, vt := range g.templates { - if vt.minVersion == "" || SupportsVersion(keeperVersion, vt.minVersion) { - tmpl = vt.tmpl - } - } - - return executeBaseConfig(tmpl, r, id) + return executeBaseConfig(g.tmpl, r, id) } type templateConfigGenerator struct { - filename string - path string - template *template.Template - generator configGeneratorFunc - minKeeperVersion string + filename string + path string + template *template.Template + generator configGeneratorFunc + minVersion upgrade.ClickHouseVersion } func (g *templateConfigGenerator) Filename() string { @@ -194,11 +174,11 @@ func (g *templateConfigGenerator) ConfigKey() string { return controllerutil.PathToName(path.Join(g.path, g.filename)) } -func (g *templateConfigGenerator) MinKeeperVersion() string { - return g.minKeeperVersion -} +func (g *templateConfigGenerator) Exists(r *clickhouseReconciler) bool { + if g.minVersion != (upgrade.ClickHouseVersion{}) { + return versionAtLeast(r.keeper.Status.Version, g.minVersion) + } -func (g *templateConfigGenerator) Exists(*clickhouseReconciler) bool { return true } @@ -232,9 +212,6 @@ type baseConfigParams struct { ClusterHosts [][]string OpenSSL controller.OpenSSLConfig - - NamedCollectionsPath string - NamedCollectionsKeyEnv string } type macro struct { @@ -321,9 +298,6 @@ func executeBaseConfig(tmpl *template.Template, r *clickhouseReconciler, id v1.C ClusterHosts: clusterHosts, OpenSSL: openSSL, - - NamedCollectionsPath: KeeperPathNamedCollections, - NamedCollectionsKeyEnv: EnvNamedCollectionsKey, } builder := strings.Builder{} @@ -445,11 +419,30 @@ func clientConfigGenerator(tmpl *template.Template, r *clickhouseReconciler, _ v return builder.String(), nil } +type namedCollectionsConfigParams struct { + NamedCollectionsKeyEnv string + NamedCollectionsPath string +} + +func namedCollectionsConfigGenerator(tmpl *template.Template, _ *clickhouseReconciler, _ v1.ClickHouseReplicaID) (string, error) { + params := namedCollectionsConfigParams{ + NamedCollectionsKeyEnv: EnvNamedCollectionsKey, + NamedCollectionsPath: KeeperPathNamedCollections, + } + + builder := strings.Builder{} + if err := tmpl.Execute(&builder, params); err != nil { + return "", fmt.Errorf("template named collections config: %w", err) + } + + return builder.String(), nil +} + type extraConfigGenerator struct { - Name string - ConfigSubPath string - Getter func(r *clickhouseReconciler) []byte - minKeeperVersion string + Name string + ConfigSubPath string + Getter func(r *clickhouseReconciler) []byte + minVersion upgrade.ClickHouseVersion } func (g *extraConfigGenerator) Filename() string { @@ -464,11 +457,11 @@ func (g *extraConfigGenerator) ConfigKey() string { return g.Name } -func (g *extraConfigGenerator) MinKeeperVersion() string { - return g.minKeeperVersion -} - func (g *extraConfigGenerator) Exists(r *clickhouseReconciler) bool { + if g.minVersion != (upgrade.ClickHouseVersion{}) && !versionAtLeast(r.keeper.Status.Version, g.minVersion) { + return false + } + return len(g.Getter(r)) > 0 } diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index be3ff6b..cefd8bc 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -1,14 +1,16 @@ package clickhouse import ( - "strconv" - "strings" + "fmt" "github.com/blang/semver/v4" + + "github.com/ClickHouse/clickhouse-operator/internal/controllerutil" + "github.com/ClickHouse/clickhouse-operator/internal/upgrade" ) -// MinVersionNamedCollectionsKeeper is the minimum ClickHouse version that supports keeper_encrypted for named collections. -const MinVersionNamedCollectionsKeeper = "25.12" +// MinVersionNamedCollections is the minimum ClickHouse version that supports keeper_encrypted for named collections. +var MinVersionNamedCollections = upgrade.ClickHouseVersion{Major: 25, Minor: 12} const ( PortManagement = 9001 @@ -68,60 +70,50 @@ const ( NamedCollectionsKeyByteLen = 16 ) -// SupportsVersion returns true if actual >= minVersion. -// Returns false for empty, "latest", or digest-based tags. -func SupportsVersion(actual, minVersion string) bool { - if minVersion == "" { - return true - } - - if actual == "" || actual == "latest" || strings.Contains(actual, "@") { - return false - } - - parts := strings.Split(strings.TrimPrefix(actual, "v"), ".") - if len(parts) < 2 { +// versionAtLeast returns true if the actual version string is >= min. +// Returns false for empty, unparseable, or unknown version strings. +func versionAtLeast(actual string, min upgrade.ClickHouseVersion) bool { + v, err := upgrade.ParseBareVersion(actual) + if err != nil { return false } - major, err1 := strconv.ParseUint(parts[0], 10, 64) - minor, err2 := strconv.ParseUint(parts[1], 10, 64) - - if err1 != nil || err2 != nil { - return false - } + return v.Compare(min) >= 0 +} - parsedVersion := strconv.FormatUint(major, 10) + "." + strconv.FormatUint(minor, 10) +type secretSpec struct { + Key string + Env string + Format string + Generate func() any + Enabled func(clickhouseVersion string) bool +} - v, err := semver.ParseTolerant(parsedVersion) - if err != nil { - return false +func (s *secretSpec) generate() []byte { + var arg any + if s.Generate != nil { + arg = s.Generate() + } else { + arg = controllerutil.GeneratePassword() } - minVer, err := semver.ParseTolerant(minVersion) - if err != nil { - return false - } + return fmt.Appendf(nil, s.Format, arg) +} - return !v.LT(minVer) +func (s *secretSpec) enabled(clickhouseVersion string) bool { + return s.Enabled == nil || s.Enabled(clickhouseVersion) } var ( breakingStatefulSetVersion, _ = semver.Parse("0.0.1") - secretsToGenerate = map[string]string{ - SecretKeyInterserverPassword: "%s", - SecretKeyManagementPassword: "%s", - SecretKeyKeeperIdentity: "clickhouse:%s", - SecretKeyClusterSecret: "%s", - } - secretsToEnvMapping = []struct { - Key string - Env string - }{ - {Key: SecretKeyInterserverPassword, Env: EnvInterserverPassword}, - {Key: SecretKeyKeeperIdentity, Env: EnvKeeperIdentity}, - {Key: SecretKeyClusterSecret, Env: EnvClusterSecret}, - {Key: SecretKeyNamedCollectionsKey, Env: EnvNamedCollectionsKey}, + clusterSecrets = []secretSpec{ + {Key: SecretKeyInterserverPassword, Env: EnvInterserverPassword, Format: "%s"}, + {Key: SecretKeyManagementPassword, Format: "%s"}, + {Key: SecretKeyKeeperIdentity, Env: EnvKeeperIdentity, Format: "clickhouse:%s"}, + {Key: SecretKeyClusterSecret, Env: EnvClusterSecret, Format: "%s"}, + {Key: SecretKeyNamedCollectionsKey, Env: EnvNamedCollectionsKey, Format: "%x", + Generate: func() any { return controllerutil.GenerateRandomBytes(NamedCollectionsKeyByteLen) }, + Enabled: func(v string) bool { return versionAtLeast(v, MinVersionNamedCollections) }, + }, } - secretsToGenerateHex = []string{SecretKeyNamedCollectionsKey} ) diff --git a/internal/controller/clickhouse/constants_test.go b/internal/controller/clickhouse/constants_test.go deleted file mode 100644 index 35ac7be..0000000 --- a/internal/controller/clickhouse/constants_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package clickhouse - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("SupportsVersion", func() { - It("returns true when actual >= min", func() { - Expect(SupportsVersion("25.12", MinVersionNamedCollectionsKeeper)).To(BeTrue()) - Expect(SupportsVersion("25.12.1", MinVersionNamedCollectionsKeeper)).To(BeTrue()) - Expect(SupportsVersion("26.1", MinVersionNamedCollectionsKeeper)).To(BeTrue()) - Expect(SupportsVersion("27.0", MinVersionNamedCollectionsKeeper)).To(BeTrue()) - Expect(SupportsVersion("25.12", "")).To(BeTrue()) - }) - - It("returns false when actual < min", func() { - Expect(SupportsVersion("25.11", MinVersionNamedCollectionsKeeper)).To(BeFalse()) - Expect(SupportsVersion("25.8", MinVersionNamedCollectionsKeeper)).To(BeFalse()) - Expect(SupportsVersion("25.3", MinVersionNamedCollectionsKeeper)).To(BeFalse()) - }) - - It("returns false for invalid or unknown versions", func() { - Expect(SupportsVersion("", MinVersionNamedCollectionsKeeper)).To(BeFalse()) - Expect(SupportsVersion("latest", MinVersionNamedCollectionsKeeper)).To(BeFalse()) - Expect(SupportsVersion("25.12@sha256:abc", MinVersionNamedCollectionsKeeper)).To(BeFalse()) - }) -}) diff --git a/internal/controller/clickhouse/controller_test.go b/internal/controller/clickhouse/controller_test.go index d982830..925d984 100644 --- a/internal/controller/clickhouse/controller_test.go +++ b/internal/controller/clickhouse/controller_test.go @@ -225,9 +225,12 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { }) It("should generate all secret values", func() { - for key := range secretsToGenerate { - Expect(secrets.Items[0].Data).To(HaveKey(key)) - Expect(secrets.Items[0].Data[key]).To(Not(BeEmpty())) + for _, spec := range clusterSecrets { + if spec.Enabled != nil { + continue + } + Expect(secrets.Items[0].Data).To(HaveKey(spec.Key)) + Expect(secrets.Items[0].Data[spec.Key]).To(Not(BeEmpty())) } }) diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index 88f3394..1bb9273 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -246,7 +246,7 @@ func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log return nil, fmt.Errorf("get ClickHouse cluster secret %q: %w", r.Cluster.SecretName(), getErr) } - isSecretUpdated := templateClusterSecrets(r.Cluster, &r.secret, r.keeper.Status.Version) + isSecretUpdated := templateClusterSecrets(r.Cluster, &r.secret, r.Cluster.Status.Version) if err := ctrl.SetControllerReference(r.Cluster, &r.secret, r.GetScheme()); err != nil { return nil, fmt.Errorf("set controller reference for cluster secret %q: %w", r.Cluster.SecretName(), err) } diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 8be0a4a..6dae341 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -5,7 +5,6 @@ import ( "maps" "net" "path" - "slices" "strconv" appsv1 "k8s.io/api/apps/v1" @@ -105,7 +104,7 @@ func templatePodDisruptionBudget(cr *v1.ClickHouseCluster, shardID int32) *polic return pdb } -func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, keeperVersion string) bool { +func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, clickhouseVersion string) bool { secret.Name = cr.SecretName() secret.Namespace = cr.Namespace secret.Type = corev1.SecretTypeOpaque @@ -131,34 +130,29 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, kee secret.Data = map[string][]byte{} } - for key, template := range secretsToGenerate { - if _, ok := secret.Data[key]; !ok { - changed = true - secret.Data[key] = fmt.Appendf(nil, template, controllerutil.GeneratePassword()) - } - } + knownKeys := make(map[string]struct{}, len(clusterSecrets)) + for i := range clusterSecrets { + spec := &clusterSecrets[i] + knownKeys[spec.Key] = struct{}{} - if SupportsVersion(keeperVersion, MinVersionNamedCollectionsKeeper) { - for _, key := range secretsToGenerateHex { - if _, ok := secret.Data[key]; !ok { + if !spec.enabled(clickhouseVersion) { + if _, ok := secret.Data[spec.Key]; ok { changed = true - secret.Data[key] = []byte(controllerutil.GenerateHexKey(NamedCollectionsKeyByteLen)) + delete(secret.Data, spec.Key) } + + continue } - } else { - for _, key := range secretsToGenerateHex { - if _, ok := secret.Data[key]; ok { - changed = true - delete(secret.Data, key) - } + if _, ok := secret.Data[spec.Key]; !ok { + changed = true + secret.Data[spec.Key] = spec.generate() } } for key := range secret.Data { - if _, ok := secretsToGenerate[key]; !ok && !slices.Contains(secretsToGenerateHex, key) { + if _, ok := knownKeys[key]; !ok { changed = true - delete(secret.Data, key) } } @@ -291,12 +285,6 @@ func generateConfigForSingleReplica(r *clickhouseReconciler, id v1.ClickHouseRep continue } - if minVer := generator.MinKeeperVersion(); minVer != "" { - if !SupportsVersion(r.keeper.Status.Version, minVer) { - continue - } - } - data, err := generator.Generate(r, id) if err != nil { return nil, fmt.Errorf("generate config file %s: %w", generator.Path(), err) @@ -496,19 +484,20 @@ func templateContainer(r *clickhouseReconciler, id v1.ClickHouseReplicaID, volum }, } - for _, secret := range secretsToEnvMapping { - if secret.Key == SecretKeyNamedCollectionsKey && !SupportsVersion(r.keeper.Status.Version, MinVersionNamedCollectionsKeeper) { + for i := range clusterSecrets { + spec := &clusterSecrets[i] + if spec.Env == "" || !spec.enabled(r.keeper.Status.Version) { continue } container.Env = append(container.Env, corev1.EnvVar{ - Name: secret.Env, + Name: spec.Env, ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{ Name: cr.SecretName(), }, - Key: secret.Key, + Key: spec.Key, }, }, }) diff --git a/internal/controller/clickhouse/templates/base_25_8.yaml.tmpl b/internal/controller/clickhouse/templates/base_25_8.yaml.tmpl deleted file mode 100644 index 2ee0884..0000000 --- a/internal/controller/clickhouse/templates/base_25_8.yaml.tmpl +++ /dev/null @@ -1,59 +0,0 @@ -path: {{ .Path }} -logger: -{{ yaml .Log | indent 2 }} -macros: - {{- range $i, $m := .Macros }} - {{ $m.Name }}: {{ $m.Value }} - {{- end }} - -{{- /* Replication settings */}} -zookeeper: - nodes: - {{- range $index, $node := .KeeperNodes }} - - host: {{ $node.Host }} - port: {{ $node.Port }} - {{- if $node.Secure }} - secure: 1 - {{- end }} - {{- end }} - identity: - "@from_env": {{ .KeeperIdentityEnv }} - -remote_servers: - default: - secret: - "@from_env": {{ .ClusterSecretEnv }} - shard: - {{- $ctx := .}} - {{- range $shard, $replicas := .ClusterHosts }} - - internal_replication: true - replica: - {{- range $i, $replica := $replicas }} - - host: {{ $replica }} - port: {{ $ctx.ManagementPort }} - {{- end }} - {{- end }} - -distributed_ddl: - path: {{ .DistributedDDLPath }} - profile: {{ .DistributedDDLProfileName }} -user_directories: - users_xml: - path: {{ .UsersXMLPath }} - replicated: - zookeeper_path: {{ .UsersZookeeperPath }} -user_defined_zookeeper_path: {{ .UDFZookeeperPath }} - -{{- /* SSL settings */}} -openSSL: -{{ yaml .OpenSSL | indent 2 }} - -{{- /* Special settings, required for default config */}} -display_secrets_in_show_and_select: true - -named_collections_storage: - type: keeper_encrypted - key_hex: - "@from_env": {{ .NamedCollectionsKeyEnv }} - algorithm: aes_128_ctr - path: {{ .NamedCollectionsPath }} diff --git a/internal/controller/clickhouse/templates/named_collections.yaml.tmpl b/internal/controller/clickhouse/templates/named_collections.yaml.tmpl new file mode 100644 index 0000000..e620540 --- /dev/null +++ b/internal/controller/clickhouse/templates/named_collections.yaml.tmpl @@ -0,0 +1,6 @@ +named_collections_storage: + type: keeper_encrypted + key_hex: + "@from_env": {{ .NamedCollectionsKeyEnv }} + algorithm: aes_128_ctr + path: {{ .NamedCollectionsPath }} diff --git a/internal/controllerutil/common.go b/internal/controllerutil/common.go index 6ed39ab..a9e62bf 100644 --- a/internal/controllerutil/common.go +++ b/internal/controllerutil/common.go @@ -168,14 +168,14 @@ const ( length = 32 ) -// GenerateHexKey generates a random key and returns hex string. -func GenerateHexKey(byteLen int) string { - b := make([]byte, byteLen) +// GenerateRandomBytes generates n cryptographically random bytes. +func GenerateRandomBytes(n int) []byte { + b := make([]byte, n) if _, err := rand.Read(b); err != nil { panic(fmt.Sprintf("read random source: %v", err)) } - return hex.EncodeToString(b) + return b } // GeneratePassword generates a random password of fixed length using a predefined alphabet. diff --git a/internal/upgrade/checker.go b/internal/upgrade/checker.go index 3f2e4c6..a1b7791 100644 --- a/internal/upgrade/checker.go +++ b/internal/upgrade/checker.go @@ -68,20 +68,21 @@ func (ch ClickHouseVersion) Release() ClickHouseRelease { } } -func compareVersions(a, b ClickHouseVersion) int { - if c := cmp.Compare(a.Major, b.Major); c != 0 { +// Compare returns -1, 0, or 1 comparing v to other. +func (v ClickHouseVersion) Compare(other ClickHouseVersion) int { + if c := cmp.Compare(v.Major, other.Major); c != 0 { return c } - if c := cmp.Compare(a.Minor, b.Minor); c != 0 { + if c := cmp.Compare(v.Minor, other.Minor); c != 0 { return c } - if c := cmp.Compare(a.Patch, b.Patch); c != 0 { + if c := cmp.Compare(v.Patch, other.Patch); c != 0 { return c } - return cmp.Compare(a.Build, b.Build) + return cmp.Compare(v.Build, other.Build) } // ParseVersion parses a raw version string into a ClickHouseVersion struct. @@ -226,7 +227,7 @@ func (t *Checker) CheckUpdates(currentRaw string, channel string) (CheckResult, func getMinorUpdate(releases *ReleaseData, current ClickHouseVersion) (ClickHouseVersion, bool) { for _, vers := range releases.Releases { if latest, ok := vers[current.Release()]; ok { - if compareVersions(latest, current) > 0 { + if latest.Compare(current) > 0 { return latest, true } } @@ -254,7 +255,7 @@ func getMajorUpdates(releases *ReleaseData, current ClickHouseVersion, channel s continue } - if compareVersions(version, current) > 0 { + if version.Compare(current) > 0 { majorUpdates = append(majorUpdates, version) } } @@ -267,7 +268,7 @@ func getMajorUpdates(releases *ReleaseData, current ClickHouseVersion, channel s findUpgrades(channelStable) } - slices.SortFunc(majorUpdates, compareVersions) + slices.SortFunc(majorUpdates, ClickHouseVersion.Compare) return majorUpdates } diff --git a/internal/upgrade/fetcher.go b/internal/upgrade/fetcher.go index 404431a..ed0a3af 100644 --- a/internal/upgrade/fetcher.go +++ b/internal/upgrade/fetcher.go @@ -189,7 +189,7 @@ func filterLatestVersions(versions map[string][]ClickHouseVersion) releaseMap { latest[channel] = map[ClickHouseRelease]ClickHouseVersion{} for _, v := range vers { if s, ok := latest[channel][v.Release()]; ok { - if compareVersions(s, v) < 0 { + if s.Compare(v) < 0 { latest[channel][v.Release()] = v } } else { From b7176a1ffc606b0ba0c773f84929164a638ac316 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 11:02:01 +0100 Subject: [PATCH 15/24] remove extra fixes --- api/v1alpha1/common.go | 1 + config/crd/bases/clickhouse.com_clickhouseclusters.yaml | 5 +++-- config/crd/bases/clickhouse.com_keeperclusters.yaml | 5 +++-- .../templates/crd/clickhouseclusters.clickhouse.com.yaml | 4 +++- dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml | 4 +++- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 55bc7c6..1f27cae 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -249,6 +249,7 @@ type ContainerTemplateSpec struct { ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` // Resources is the resource requirements for the server container. + // This field cannot be updated once the cluster is created. // +optional Resources corev1.ResourceRequirements `json:"resources,omitempty"` diff --git a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml index 770801e..8400893 100644 --- a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml +++ b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml @@ -264,8 +264,9 @@ spec: - IfNotPresent type: string resources: - description: Resources is the resource requirements for the server - container. + description: |- + Resources is the resource requirements for the container. + This field cannot be updated once the cluster is created. properties: claims: description: |- diff --git a/config/crd/bases/clickhouse.com_keeperclusters.yaml b/config/crd/bases/clickhouse.com_keeperclusters.yaml index 74eed62..566bb8a 100644 --- a/config/crd/bases/clickhouse.com_keeperclusters.yaml +++ b/config/crd/bases/clickhouse.com_keeperclusters.yaml @@ -263,8 +263,9 @@ spec: - IfNotPresent type: string resources: - description: Resources is the resource requirements for the server - container. + description: |- + Resources is the resource requirements for the container. + This field cannot be updated once the cluster is created. properties: claims: description: |- diff --git a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml index ef6e3db..3b08730 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -251,7 +251,9 @@ spec: - IfNotPresent type: string resources: - description: Resources is the resource requirements for the server container. + description: |- + Resources is the resource requirements for the container. + This field cannot be updated once the cluster is created. properties: claims: description: |- diff --git a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml index 712fb08..bb59ebf 100644 --- a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml @@ -251,7 +251,9 @@ spec: - IfNotPresent type: string resources: - description: Resources is the resource requirements for the server container. + description: |- + Resources is the resource requirements for the container. + This field cannot be updated once the cluster is created. properties: claims: description: |- From 4ebde2953647a8eb68d5a67d26423f91edd7cf50 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 11:33:43 +0100 Subject: [PATCH 16/24] return back some changes --- api/v1alpha1/common.go | 2 +- .../crd/clickhouseclusters.clickhouse.com.yaml | 2 +- .../crd/keeperclusters.clickhouse.com.yaml | 4 ++-- internal/upgrade/checker.go | 18 +++++++++++------- internal/upgrade/fetcher.go | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 1f27cae..c7e1cdb 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -248,7 +248,7 @@ type ContainerTemplateSpec struct { // +kubebuilder:validation:Enum="Always";"Never";"IfNotPresent" ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` - // Resources is the resource requirements for the server container. + // Resources is the resource requirements for the container. // This field cannot be updated once the cluster is created. // +optional Resources corev1.ResourceRequirements `json:"resources,omitempty"` diff --git a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml index 3b08730..245b08c 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -253,7 +253,7 @@ spec: resources: description: |- Resources is the resource requirements for the container. - This field cannot be updated once the cluster is created. + This field cannot be updated once the cluster is created. properties: claims: description: |- diff --git a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml index bb59ebf..54b5d09 100644 --- a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml @@ -252,8 +252,8 @@ spec: type: string resources: description: |- - Resources is the resource requirements for the container. - This field cannot be updated once the cluster is created. + Resources is the resource requirements for the container. + This field cannot be updated once the cluster is created. properties: claims: description: |- diff --git a/internal/upgrade/checker.go b/internal/upgrade/checker.go index a1b7791..c201d21 100644 --- a/internal/upgrade/checker.go +++ b/internal/upgrade/checker.go @@ -70,19 +70,23 @@ func (ch ClickHouseVersion) Release() ClickHouseRelease { // Compare returns -1, 0, or 1 comparing v to other. func (v ClickHouseVersion) Compare(other ClickHouseVersion) int { - if c := cmp.Compare(v.Major, other.Major); c != 0 { + return compareVersions(v, other) +} + +func compareVersions(a, b ClickHouseVersion) int { + if c := cmp.Compare(a.Major, b.Major); c != 0 { return c } - if c := cmp.Compare(v.Minor, other.Minor); c != 0 { + if c := cmp.Compare(a.Minor, b.Minor); c != 0 { return c } - if c := cmp.Compare(v.Patch, other.Patch); c != 0 { + if c := cmp.Compare(a.Patch, b.Patch); c != 0 { return c } - return cmp.Compare(v.Build, other.Build) + return cmp.Compare(a.Build, b.Build) } // ParseVersion parses a raw version string into a ClickHouseVersion struct. @@ -227,7 +231,7 @@ func (t *Checker) CheckUpdates(currentRaw string, channel string) (CheckResult, func getMinorUpdate(releases *ReleaseData, current ClickHouseVersion) (ClickHouseVersion, bool) { for _, vers := range releases.Releases { if latest, ok := vers[current.Release()]; ok { - if latest.Compare(current) > 0 { + if compareVersions(latest, current) > 0 { return latest, true } } @@ -255,7 +259,7 @@ func getMajorUpdates(releases *ReleaseData, current ClickHouseVersion, channel s continue } - if version.Compare(current) > 0 { + if compareVersions(version, current) > 0 { majorUpdates = append(majorUpdates, version) } } @@ -268,7 +272,7 @@ func getMajorUpdates(releases *ReleaseData, current ClickHouseVersion, channel s findUpgrades(channelStable) } - slices.SortFunc(majorUpdates, ClickHouseVersion.Compare) + slices.SortFunc(majorUpdates, compareVersions) return majorUpdates } diff --git a/internal/upgrade/fetcher.go b/internal/upgrade/fetcher.go index ed0a3af..404431a 100644 --- a/internal/upgrade/fetcher.go +++ b/internal/upgrade/fetcher.go @@ -189,7 +189,7 @@ func filterLatestVersions(versions map[string][]ClickHouseVersion) releaseMap { latest[channel] = map[ClickHouseRelease]ClickHouseVersion{} for _, v := range vers { if s, ok := latest[channel][v.Release()]; ok { - if s.Compare(v) < 0 { + if compareVersions(s, v) < 0 { latest[channel][v.Release()] = v } } else { From e77dc2afc9e43cd86b5ece0c719f1273b6c87432 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 12:55:54 +0100 Subject: [PATCH 17/24] better --- internal/controller/clickhouse/constants.go | 8 ++++---- internal/controller/clickhouse/controller_test.go | 1 + internal/controller/clickhouse/templates.go | 2 ++ internal/upgrade/checker.go | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index cefd8bc..43280a5 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -10,7 +10,7 @@ import ( ) // MinVersionNamedCollections is the minimum ClickHouse version that supports keeper_encrypted for named collections. -var MinVersionNamedCollections = upgrade.ClickHouseVersion{Major: 25, Minor: 12} +var MinVersionNamedCollections = upgrade.ClickHouseVersion{Major: 25, Minor: 12} //nolint:mnd const ( PortManagement = 9001 @@ -71,14 +71,14 @@ const ( ) // versionAtLeast returns true if the actual version string is >= min. -// Returns false for empty, unparseable, or unknown version strings. -func versionAtLeast(actual string, min upgrade.ClickHouseVersion) bool { +// Returns false for empty, unparsable, or unknown version strings. +func versionAtLeast(actual string, minVersion upgrade.ClickHouseVersion) bool { v, err := upgrade.ParseBareVersion(actual) if err != nil { return false } - return v.Compare(min) >= 0 + return v.Compare(minVersion) >= 0 } type secretSpec struct { diff --git a/internal/controller/clickhouse/controller_test.go b/internal/controller/clickhouse/controller_test.go index 925d984..177dbc3 100644 --- a/internal/controller/clickhouse/controller_test.go +++ b/internal/controller/clickhouse/controller_test.go @@ -229,6 +229,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { if spec.Enabled != nil { continue } + Expect(secrets.Items[0].Data).To(HaveKey(spec.Key)) Expect(secrets.Items[0].Data[spec.Key]).To(Not(BeEmpty())) } diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index 6dae341..b0a6d09 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -138,6 +138,7 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, cli if !spec.enabled(clickhouseVersion) { if _, ok := secret.Data[spec.Key]; ok { changed = true + delete(secret.Data, spec.Key) } @@ -153,6 +154,7 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, cli for key := range secret.Data { if _, ok := knownKeys[key]; !ok { changed = true + delete(secret.Data, key) } } diff --git a/internal/upgrade/checker.go b/internal/upgrade/checker.go index c201d21..1e3d954 100644 --- a/internal/upgrade/checker.go +++ b/internal/upgrade/checker.go @@ -69,8 +69,8 @@ func (ch ClickHouseVersion) Release() ClickHouseRelease { } // Compare returns -1, 0, or 1 comparing v to other. -func (v ClickHouseVersion) Compare(other ClickHouseVersion) int { - return compareVersions(v, other) +func (ch ClickHouseVersion) Compare(other ClickHouseVersion) int { + return compareVersions(ch, other) } func compareVersions(a, b ClickHouseVersion) int { From c04ed9ab1527e2ebbba9641b2c07c2000f4cb69f Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 13:21:27 +0100 Subject: [PATCH 18/24] add version to tests --- internal/controller/clickhouse/config_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/controller/clickhouse/config_test.go b/internal/controller/clickhouse/config_test.go index 7f9008c..2d36be2 100644 --- a/internal/controller/clickhouse/config_test.go +++ b/internal/controller/clickhouse/config_test.go @@ -37,6 +37,9 @@ var _ = Describe("ConfigGenerator", func() { Spec: v1.KeeperClusterSpec{ Replicas: ptr.To[int32](3), }, + Status: v1.KeeperClusterStatus{ + Version: "25.12.1.1", + }, }, } From 25f780892ace31023df6009a20c5fe59580d5c81 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 14:24:24 +0100 Subject: [PATCH 19/24] new config file into e2e test --- test/e2e/clickhouse_e2e_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index 99e77c7..7414858 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -163,16 +163,16 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { By("verifying named_collections_storage is rendered with keeper_encrypted") - baseConfigKey := controllerutil.PathToName(path.Join(chctrl.ConfigPath, chctrl.ConfigFileName)) - baseConfig, ok := configMap.Data[baseConfigKey] - Expect(ok).To(BeTrue(), "base config %q not found in ConfigMap", baseConfigKey) - Expect(baseConfig).To(ContainSubstring("named_collections_storage:"), + ncConfigKey := controllerutil.PathToName(path.Join(chctrl.ConfigPath, chctrl.ConfigDPath, "00-named-collections.yaml")) + ncConfig, ok := configMap.Data[ncConfigKey] + Expect(ok).To(BeTrue(), "named collections config %q not found in ConfigMap", ncConfigKey) + Expect(ncConfig).To(ContainSubstring("named_collections_storage:"), "operator must render named_collections_storage into config") - Expect(baseConfig).To(ContainSubstring("type: keeper_encrypted"), + Expect(ncConfig).To(ContainSubstring("type: keeper_encrypted"), "named_collections_storage must use keeper_encrypted") - Expect(baseConfig).To(ContainSubstring(chctrl.EnvNamedCollectionsKey), + Expect(ncConfig).To(ContainSubstring(chctrl.EnvNamedCollectionsKey), "named_collections_storage must reference CLICKHOUSE_NAMED_COLLECTIONS_KEY") - Expect(baseConfig).To(ContainSubstring("path: "+chctrl.KeeperPathNamedCollections), + Expect(ncConfig).To(ContainSubstring("path: "+chctrl.KeeperPathNamedCollections), "named_collections_storage must use Keeper path for named collections") By("connecting to cluster") From 3bf93db23ffb9f3fc95572f7bd2afeec73808ec1 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 14:29:55 +0100 Subject: [PATCH 20/24] lint --- test/e2e/clickhouse_e2e_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index 7414858..1c42235 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -163,7 +163,8 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { By("verifying named_collections_storage is rendered with keeper_encrypted") - ncConfigKey := controllerutil.PathToName(path.Join(chctrl.ConfigPath, chctrl.ConfigDPath, "00-named-collections.yaml")) + ncPath := path.Join(chctrl.ConfigPath, chctrl.ConfigDPath, "00-named-collections.yaml") + ncConfigKey := controllerutil.PathToName(ncPath) ncConfig, ok := configMap.Data[ncConfigKey] Expect(ok).To(BeTrue(), "named collections config %q not found in ConfigMap", ncConfigKey) Expect(ncConfig).To(ContainSubstring("named_collections_storage:"), From 41042db190ae638a4b62cca02df6d1029d8628e3 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 17:14:09 +0100 Subject: [PATCH 21/24] fix review issues --- internal/controller/clickhouse/config.go | 64 ++++++++----------- internal/controller/clickhouse/config_test.go | 2 +- internal/controller/clickhouse/constants.go | 11 ++-- internal/controller/clickhouse/sync.go | 8 ++- internal/controller/clickhouse/templates.go | 10 +-- test/e2e/clickhouse_e2e_test.go | 35 +++------- 6 files changed, 57 insertions(+), 73 deletions(-) diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index c437625..e8df983 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -15,7 +15,6 @@ import ( "github.com/ClickHouse/clickhouse-operator/internal/controller" "github.com/ClickHouse/clickhouse-operator/internal/controller/keeper" "github.com/ClickHouse/clickhouse-operator/internal/controllerutil" - "github.com/ClickHouse/clickhouse-operator/internal/upgrade" ) var ( @@ -74,11 +73,11 @@ func init() { }) for _, templateSpec := range []struct { - Path string - Filename string - Raw string - Generator configGeneratorFunc - MinVersion upgrade.ClickHouseVersion + Path string + Filename string + Raw string + Generator configGeneratorFunc + Enabled func(r *clickhouseReconciler) bool }{{ Path: path.Join(ConfigPath, ConfigDPath), Filename: "00-network.yaml", @@ -90,11 +89,13 @@ func init() { Raw: logTablesConfigTemplateStr, Generator: logTablesConfigGenerator, }, { - Path: path.Join(ConfigPath, ConfigDPath), - Filename: "00-named-collections.yaml", - Raw: namedCollectionsTemplateStr, - Generator: namedCollectionsConfigGenerator, - MinVersion: MinVersionNamedCollections, + Path: path.Join(ConfigPath, ConfigDPath), + Filename: "00-named-collections.yaml", + Raw: namedCollectionsTemplateStr, + Generator: namedCollectionsConfigGenerator, + Enabled: func(r *clickhouseReconciler) bool { + return versionAtLeast(r.keeper.Status.Version, MinVersionNamedCollections) + }, }, { Path: ConfigPath, Filename: UsersFileName, @@ -109,11 +110,11 @@ func init() { tmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(templateSpec.Raw)) generators = append(generators, &templateConfigGenerator{ - filename: templateSpec.Filename, - path: templateSpec.Path, - template: tmpl, - generator: templateSpec.Generator, - minVersion: templateSpec.MinVersion, + filename: templateSpec.Filename, + path: templateSpec.Path, + template: tmpl, + generator: templateSpec.Generator, + enabled: templateSpec.Enabled, }) } @@ -134,7 +135,7 @@ type configGenerator interface { Filename() string Path() string ConfigKey() string - Exists(r *clickhouseReconciler) bool + Enabled(r *clickhouseReconciler) bool Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) } @@ -149,17 +150,17 @@ func (g *baseConfigGenerator) Path() string { return g.path } func (g *baseConfigGenerator) ConfigKey() string { return controllerutil.PathToName(path.Join(g.path, g.filename)) } -func (g *baseConfigGenerator) Exists(*clickhouseReconciler) bool { return true } +func (g *baseConfigGenerator) Enabled(*clickhouseReconciler) bool { return true } func (g *baseConfigGenerator) Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { return executeBaseConfig(g.tmpl, r, id) } type templateConfigGenerator struct { - filename string - path string - template *template.Template - generator configGeneratorFunc - minVersion upgrade.ClickHouseVersion + filename string + path string + template *template.Template + generator configGeneratorFunc + enabled func(r *clickhouseReconciler) bool } func (g *templateConfigGenerator) Filename() string { @@ -174,12 +175,8 @@ func (g *templateConfigGenerator) ConfigKey() string { return controllerutil.PathToName(path.Join(g.path, g.filename)) } -func (g *templateConfigGenerator) Exists(r *clickhouseReconciler) bool { - if g.minVersion != (upgrade.ClickHouseVersion{}) { - return versionAtLeast(r.keeper.Status.Version, g.minVersion) - } - - return true +func (g *templateConfigGenerator) Enabled(r *clickhouseReconciler) bool { + return g.enabled == nil || g.enabled(r) } func (g *templateConfigGenerator) Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { @@ -442,7 +439,6 @@ type extraConfigGenerator struct { Name string ConfigSubPath string Getter func(r *clickhouseReconciler) []byte - minVersion upgrade.ClickHouseVersion } func (g *extraConfigGenerator) Filename() string { @@ -457,16 +453,12 @@ func (g *extraConfigGenerator) ConfigKey() string { return g.Name } -func (g *extraConfigGenerator) Exists(r *clickhouseReconciler) bool { - if g.minVersion != (upgrade.ClickHouseVersion{}) && !versionAtLeast(r.keeper.Status.Version, g.minVersion) { - return false - } - +func (g *extraConfigGenerator) Enabled(r *clickhouseReconciler) bool { return len(g.Getter(r)) > 0 } func (g *extraConfigGenerator) Generate(r *clickhouseReconciler, _ v1.ClickHouseReplicaID) (string, error) { - if !g.Exists(r) { + if !g.Enabled(r) { return "", errors.New("extra config generator called, but no extra config provided") } diff --git a/internal/controller/clickhouse/config_test.go b/internal/controller/clickhouse/config_test.go index 2d36be2..6748cc1 100644 --- a/internal/controller/clickhouse/config_test.go +++ b/internal/controller/clickhouse/config_test.go @@ -45,7 +45,7 @@ var _ = Describe("ConfigGenerator", func() { for _, generator := range generators { It("should generate config: "+generator.Filename(), func() { - Expect(generator.Exists(&ctx)).To(BeTrue()) + Expect(generator.Enabled(&ctx)).To(BeTrue()) data, err := generator.Generate(&ctx, v1.ClickHouseReplicaID{}) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/controller/clickhouse/constants.go b/internal/controller/clickhouse/constants.go index 43280a5..0c62439 100644 --- a/internal/controller/clickhouse/constants.go +++ b/internal/controller/clickhouse/constants.go @@ -5,6 +5,7 @@ import ( "github.com/blang/semver/v4" + v1 "github.com/ClickHouse/clickhouse-operator/api/v1alpha1" "github.com/ClickHouse/clickhouse-operator/internal/controllerutil" "github.com/ClickHouse/clickhouse-operator/internal/upgrade" ) @@ -86,7 +87,7 @@ type secretSpec struct { Env string Format string Generate func() any - Enabled func(clickhouseVersion string) bool + Enabled func(status *v1.ClickHouseCluster) bool } func (s *secretSpec) generate() []byte { @@ -100,8 +101,8 @@ func (s *secretSpec) generate() []byte { return fmt.Appendf(nil, s.Format, arg) } -func (s *secretSpec) enabled(clickhouseVersion string) bool { - return s.Enabled == nil || s.Enabled(clickhouseVersion) +func (s *secretSpec) enabled(cluster *v1.ClickHouseCluster) bool { + return s.Enabled == nil || s.Enabled(cluster) } var ( @@ -113,7 +114,9 @@ var ( {Key: SecretKeyClusterSecret, Env: EnvClusterSecret, Format: "%s"}, {Key: SecretKeyNamedCollectionsKey, Env: EnvNamedCollectionsKey, Format: "%x", Generate: func() any { return controllerutil.GenerateRandomBytes(NamedCollectionsKeyByteLen) }, - Enabled: func(v string) bool { return versionAtLeast(v, MinVersionNamedCollections) }, + Enabled: func(status *v1.ClickHouseCluster) bool { + return versionAtLeast(status.Spec.ClusterDomain, MinVersionNamedCollections) + }, }, } ) diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index 1bb9273..a23a132 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -197,6 +197,12 @@ func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log return nil, fmt.Errorf("get keeper cluster: %w", err) } + if r.keeper.Status.Version == "" { + log.Info("keeper version is not yet known, waiting") + + return &ctrl.Result{RequeueAfter: chctrl.RequeueOnRefreshTimeout}, nil + } + service := templateHeadlessService(r.Cluster) if _, err := r.ReconcileService(ctx, log, service, v1.EventActionReconciling); err != nil { return nil, fmt.Errorf("reconcile service resource: %w", err) @@ -246,7 +252,7 @@ func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log return nil, fmt.Errorf("get ClickHouse cluster secret %q: %w", r.Cluster.SecretName(), getErr) } - isSecretUpdated := templateClusterSecrets(r.Cluster, &r.secret, r.Cluster.Status.Version) + isSecretUpdated := templateClusterSecrets(r.Cluster, &r.secret, r.Cluster) if err := ctrl.SetControllerReference(r.Cluster, &r.secret, r.GetScheme()); err != nil { return nil, fmt.Errorf("set controller reference for cluster secret %q: %w", r.Cluster.SecretName(), err) } diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index b0a6d09..a695280 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -104,7 +104,7 @@ func templatePodDisruptionBudget(cr *v1.ClickHouseCluster, shardID int32) *polic return pdb } -func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, clickhouseVersion string) bool { +func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, cluster *v1.ClickHouseCluster) bool { secret.Name = cr.SecretName() secret.Namespace = cr.Namespace secret.Type = corev1.SecretTypeOpaque @@ -135,7 +135,7 @@ func templateClusterSecrets(cr *v1.ClickHouseCluster, secret *corev1.Secret, cli spec := &clusterSecrets[i] knownKeys[spec.Key] = struct{}{} - if !spec.enabled(clickhouseVersion) { + if !spec.enabled(cluster) { if _, ok := secret.Data[spec.Key]; ok { changed = true @@ -283,7 +283,7 @@ func templateStatefulSet(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (*a func generateConfigForSingleReplica(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (map[string]string, error) { configFiles := map[string]string{} for _, generator := range generators { - if !generator.Exists(r) { + if !generator.Enabled(r) { continue } @@ -488,7 +488,7 @@ func templateContainer(r *clickhouseReconciler, id v1.ClickHouseReplicaID, volum for i := range clusterSecrets { spec := &clusterSecrets[i] - if spec.Env == "" || !spec.enabled(r.keeper.Status.Version) { + if spec.Env == "" || !spec.enabled(r.Cluster) { continue } @@ -651,7 +651,7 @@ func buildVolumes(r *clickhouseReconciler, id v1.ClickHouseReplicaID) ([]corev1. configVolumes := map[string]corev1.Volume{} for _, generator := range generators { - if !generator.Exists(r) { + if !generator.Enabled(r) { continue } diff --git a/test/e2e/clickhouse_e2e_test.go b/test/e2e/clickhouse_e2e_test.go index 1c42235..bade92b 100644 --- a/test/e2e/clickhouse_e2e_test.go +++ b/test/e2e/clickhouse_e2e_test.go @@ -136,45 +136,28 @@ var _ = Describe("ClickHouse controller", Label("clickhouse"), func() { }) WaitClickHouseUpdatedAndReady(ctx, &cr, time.Minute, false) - By("logging named collections secret") + By("verifying named collections secret key exists") var secret corev1.Secret Expect(k8sClient.Get(ctx, types.NamespacedName{ Namespace: cr.Namespace, Name: cr.SecretName(), }, &secret)).To(Succeed()) + Expect(secret.Data).To(HaveKey(chctrl.SecretKeyNamedCollectionsKey)) + Expect(secret.Data[chctrl.SecretKeyNamedCollectionsKey]).NotTo(BeEmpty()) - if key, ok := secret.Data["named-collections-key"]; ok { - By(fmt.Sprintf("named collections key present; length=%d bytes", len(key))) - } - - By("logging generated config") + By("verifying named collections config exists") var configMap corev1.ConfigMap cfgName := cr.ConfigMapNameByReplicaID(v1.ClickHouseReplicaID{ShardID: 0, Index: 0}) Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cfgName}, &configMap)).To(Succeed()) - for key, data := range configMap.Data { - if strings.Contains(key, "config") && !strings.Contains(key, "users") { - By(fmt.Sprintf("%s:\n%s", key, data)) - } - } - - By("verifying named_collections_storage is rendered with keeper_encrypted") - - ncPath := path.Join(chctrl.ConfigPath, chctrl.ConfigDPath, "00-named-collections.yaml") - ncConfigKey := controllerutil.PathToName(ncPath) - ncConfig, ok := configMap.Data[ncConfigKey] - Expect(ok).To(BeTrue(), "named collections config %q not found in ConfigMap", ncConfigKey) - Expect(ncConfig).To(ContainSubstring("named_collections_storage:"), - "operator must render named_collections_storage into config") - Expect(ncConfig).To(ContainSubstring("type: keeper_encrypted"), - "named_collections_storage must use keeper_encrypted") - Expect(ncConfig).To(ContainSubstring(chctrl.EnvNamedCollectionsKey), - "named_collections_storage must reference CLICKHOUSE_NAMED_COLLECTIONS_KEY") - Expect(ncConfig).To(ContainSubstring("path: "+chctrl.KeeperPathNamedCollections), - "named_collections_storage must use Keeper path for named collections") + ncConfigKey := controllerutil.PathToName( + path.Join(chctrl.ConfigPath, chctrl.ConfigDPath, "00-named-collections.yaml"), + ) + Expect(configMap.Data).To(HaveKey(ncConfigKey)) + Expect(configMap.Data[ncConfigKey]).NotTo(BeEmpty()) By("connecting to cluster") From f5eea12ccdf0457dc370f2bc47178f78387b3988 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 17:21:03 +0100 Subject: [PATCH 22/24] fix review --- internal/controller/clickhouse/sync.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index a23a132..19cae56 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -190,13 +190,6 @@ func (r *clickhouseReconciler) sync(ctx context.Context, log ctrlutil.Logger) (c } func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log ctrlutil.Logger) (*ctrl.Result, error) { - if err := r.GetClient().Get(ctx, types.NamespacedName{ - Namespace: r.Cluster.Namespace, - Name: r.Cluster.Spec.KeeperClusterRef.Name, - }, &r.keeper); err != nil { - return nil, fmt.Errorf("get keeper cluster: %w", err) - } - if r.keeper.Status.Version == "" { log.Info("keeper version is not yet known, waiting") From a2b4c8e94023c3feb949006ae6e6796779cf56c4 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 18:05:22 +0100 Subject: [PATCH 23/24] better --- .../controller/clickhouse/controller_test.go | 1 + internal/controller/clickhouse/sync.go | 39 ++++++++++++++----- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/internal/controller/clickhouse/controller_test.go b/internal/controller/clickhouse/controller_test.go index 177dbc3..5f4f024 100644 --- a/internal/controller/clickhouse/controller_test.go +++ b/internal/controller/clickhouse/controller_test.go @@ -88,6 +88,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Status: metav1.ConditionTrue, Reason: string(v1.KeeperConditionReasonStandaloneReady), }) + keeper.Status.Version = "26.1.1.1" Expect(suite.Client.Status().Update(ctx, keeper)).To(Succeed()) }) diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index 19cae56..ac27246 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -171,7 +171,7 @@ func (r *clickhouseReconciler) sync(ctx context.Context, log ctrlutil.Logger) (c return ctrl.Result{}, err } - if !stepResult.IsZero() { + if stepResult != nil && !stepResult.IsZero() { stepLog.Debug("reconcile step result", "result", stepResult) ctrlutil.UpdateResult(&result, stepResult) } @@ -190,12 +190,6 @@ func (r *clickhouseReconciler) sync(ctx context.Context, log ctrlutil.Logger) (c } func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log ctrlutil.Logger) (*ctrl.Result, error) { - if r.keeper.Status.Version == "" { - log.Info("keeper version is not yet known, waiting") - - return &ctrl.Result{RequeueAfter: chctrl.RequeueOnRefreshTimeout}, nil - } - service := templateHeadlessService(r.Cluster) if _, err := r.ReconcileService(ctx, log, service, v1.EventActionReconciling); err != nil { return nil, fmt.Errorf("reconcile service resource: %w", err) @@ -237,6 +231,19 @@ func (r *clickhouseReconciler) reconcileCommonResources(ctx context.Context, log } } + var keeperForVersion v1.KeeperCluster + if err := r.GetClient().Get(ctx, types.NamespacedName{ + Namespace: r.Cluster.Namespace, + Name: r.Cluster.Spec.KeeperClusterRef.Name, + }, &keeperForVersion); err != nil { + return nil, fmt.Errorf("get keeper cluster %q: %w", r.Cluster.Spec.KeeperClusterRef.Name, err) + } + + if keeperForVersion.Status.Version == "" { + log.Info("keeper version is not yet known, waiting") + return &ctrl.Result{RequeueAfter: chctrl.RequeueOnRefreshTimeout}, nil + } + getErr := r.GetClient().Get(ctx, types.NamespacedName{ Namespace: r.Cluster.Namespace, Name: r.Cluster.SecretName(), @@ -364,7 +371,7 @@ func (r *clickhouseReconciler) reconcileActiveReplicaStatus(ctx context.Context, pinged := false version := "" - if !hasError { + if !hasError && r.commander != nil { ctx, cancel := context.WithTimeout(ctx, chctrl.LoadReplicaStateTimeout) defer cancel() @@ -403,6 +410,10 @@ func (r *clickhouseReconciler) reconcileActiveReplicaStatus(ctx context.Context, } } + if r.commander == nil { + return &ctrl.Result{RequeueAfter: chctrl.RequeueOnRefreshTimeout}, nil + } + return nil, nil } @@ -485,6 +496,11 @@ func (r *clickhouseReconciler) reconcileReplicateSchema(ctx context.Context, log return nil, nil } + if r.commander == nil { + log.Info("commander not initialized, skipping database replication") + return &ctrl.Result{RequeueAfter: chctrl.RequeueOnRefreshTimeout}, nil + } + hasNotSynced := false replicaDatabases := ctrlutil.ExecuteParallel(readyReplicas, func(id v1.ClickHouseReplicaID) (v1.ClickHouseReplicaID, map[string]databaseDescriptor, error) { if err := r.commander.EnsureDefaultDatabaseEngine(ctx, log, id); err != nil { @@ -606,6 +622,11 @@ func (r *clickhouseReconciler) reconcileCleanUp(ctx context.Context, log ctrluti replicasToRemove[id.ShardID][id.Index] = state } + if len(replicasToRemove) > 0 && r.commander == nil { + log.Info("commander not initialized, deferring scale-down cleanup that requires shard sync") + return &ctrl.Result{RequeueAfter: chctrl.RequeueOnRefreshTimeout}, nil + } + shardsInSync := ctrlutil.ExecuteParallel(slices.Collect(maps.Keys(replicasToRemove)), func(shardID int32) (int32, struct{}, error) { log.Info("Pre scale-down shard sync", "shard_id", shardID) @@ -653,7 +674,7 @@ func (r *clickhouseReconciler) reconcileCleanUp(ctx context.Context, log ctrluti } } - if r.Cluster.Spec.Settings.EnableDatabaseSync { + if r.Cluster.Spec.Settings.EnableDatabaseSync && r.commander != nil { if err := r.commander.CleanupDatabaseReplicas(ctx, log, runningStaleReplicas); err != nil { log.Warn("failed to cleanup database replicas", "error", err) From 42ed945791934b4c019ac1597f3d127f6d9f6d61 Mon Sep 17 00:00:00 2001 From: scanhex12 Date: Fri, 20 Mar 2026 23:49:29 +0100 Subject: [PATCH 24/24] fix review --- internal/controller/clickhouse/config.go | 27 +++++-------------- internal/controller/clickhouse/config_test.go | 6 ++--- .../controller/clickhouse/controller_test.go | 4 +++ internal/controller/clickhouse/sync.go | 6 +++-- 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/internal/controller/clickhouse/config.go b/internal/controller/clickhouse/config.go index e8df983..842493a 100644 --- a/internal/controller/clickhouse/config.go +++ b/internal/controller/clickhouse/config.go @@ -66,10 +66,11 @@ func init() { baseTmpl := template.Must(template.New("").Funcs(templateFuncs).Parse(baseTemplateStr)) - generators = append(generators, &baseConfigGenerator{ - path: ConfigPath, - filename: ConfigFileName, - tmpl: baseTmpl, + generators = append(generators, &templateConfigGenerator{ + path: ConfigPath, + filename: ConfigFileName, + template: baseTmpl, + generator: executeBaseConfig, }) for _, templateSpec := range []struct { @@ -94,7 +95,7 @@ func init() { Raw: namedCollectionsTemplateStr, Generator: namedCollectionsConfigGenerator, Enabled: func(r *clickhouseReconciler) bool { - return versionAtLeast(r.keeper.Status.Version, MinVersionNamedCollections) + return versionAtLeast(r.Cluster.Status.Version, MinVersionNamedCollections) }, }, { Path: ConfigPath, @@ -139,22 +140,6 @@ type configGenerator interface { Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) } -type baseConfigGenerator struct { - path string - filename string - tmpl *template.Template -} - -func (g *baseConfigGenerator) Filename() string { return g.filename } -func (g *baseConfigGenerator) Path() string { return g.path } -func (g *baseConfigGenerator) ConfigKey() string { - return controllerutil.PathToName(path.Join(g.path, g.filename)) -} -func (g *baseConfigGenerator) Enabled(*clickhouseReconciler) bool { return true } -func (g *baseConfigGenerator) Generate(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (string, error) { - return executeBaseConfig(g.tmpl, r, id) -} - type templateConfigGenerator struct { filename string path string diff --git a/internal/controller/clickhouse/config_test.go b/internal/controller/clickhouse/config_test.go index 6748cc1..a8ae130 100644 --- a/internal/controller/clickhouse/config_test.go +++ b/internal/controller/clickhouse/config_test.go @@ -31,15 +31,15 @@ var _ = Describe("ConfigGenerator", func() { }, }, }, + Status: v1.ClickHouseClusterStatus{ + Version: "25.12.1.1", + }, }, }, keeper: v1.KeeperCluster{ Spec: v1.KeeperClusterSpec{ Replicas: ptr.To[int32](3), }, - Status: v1.KeeperClusterStatus{ - Version: "25.12.1.1", - }, }, } diff --git a/internal/controller/clickhouse/controller_test.go b/internal/controller/clickhouse/controller_test.go index 5f4f024..87693ce 100644 --- a/internal/controller/clickhouse/controller_test.go +++ b/internal/controller/clickhouse/controller_test.go @@ -59,6 +59,9 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { "test-annotation": "test-val", }, }, + Status: v1.ClickHouseClusterStatus{ + Version: "26.1.1.1", + }, } ) @@ -88,6 +91,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() { Status: metav1.ConditionTrue, Reason: string(v1.KeeperConditionReasonStandaloneReady), }) + // Unblocks CommonResources (secrets/commander); version-gated ClickHouse config uses ClickHouseCluster.status.version. keeper.Status.Version = "26.1.1.1" Expect(suite.Client.Status().Update(ctx, keeper)).To(Succeed()) }) diff --git a/internal/controller/clickhouse/sync.go b/internal/controller/clickhouse/sync.go index ac27246..10b96b2 100644 --- a/internal/controller/clickhouse/sync.go +++ b/internal/controller/clickhouse/sync.go @@ -171,7 +171,7 @@ func (r *clickhouseReconciler) sync(ctx context.Context, log ctrlutil.Logger) (c return ctrl.Result{}, err } - if stepResult != nil && !stepResult.IsZero() { + if !stepResult.IsZero() { stepLog.Debug("reconcile step result", "result", stepResult) ctrlutil.UpdateResult(&result, stepResult) } @@ -341,7 +341,9 @@ func (r *clickhouseReconciler) reconcileClusterRevisions(ctx context.Context, lo } r.versionProbe = probeResult - r.Cluster.Status.Version = r.versionProbe.Version + if probeResult.Version != "" { + r.Cluster.Status.Version = probeResult.Version + } return nil, nil }