feat: support encrypted storage policy generation#269
Open
GrigoryPervakov wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class support for opt-in at-rest encryption by generating a dedicated ClickHouse encrypted storage policy (and key management) when spec.settings.encryption is set, and documents/validates the new behavior via CRD CEL rules plus tests.
Changes:
- Introduce
spec.settings.encryptionand generate encrypted wrapper disks + an encrypted storage policy in ClickHouse config. - Extend managed Secret generation/injection with a disk encryption key and add CEL-based immutability/validation constraints in the CRD.
- Add/adjust unit, webhook envtest, and e2e coverage plus storage/configuration documentation updates.
Reviewed changes
Copilot reviewed 14 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test/e2e/clickhouse_e2e_test.go |
Adds e2e verifying encrypted storage policy presence in system.storage_policies. |
Makefile |
Filters CRD schema checker info-level output in check-crd-compat (to avoid CEL-cost notes failing the gate). |
internal/webhook/v1alpha1/webhook_suite_test.go |
Makes teardown nil-safe for webhook test environment. |
internal/webhook/v1alpha1/common.go |
Adds admission validation for additional disk names ending with -encrypted. |
internal/webhook/v1alpha1/clickhousecluster_webhook_test.go |
Adds webhook/envtest cases for encryption defaulting and immutability constraints. |
internal/controller/clickhouse/templates/storage_jbod.yaml.tmpl |
Extends storage config template to emit encrypted disks + encrypted policy when enabled. |
internal/controller/clickhouse/constants.go |
Introduces encryption-related constants and secret/env key names plus key generation spec. |
internal/controller/clickhouse/config.go |
Extends storage config generator to produce encrypted wrapper disks/policy params and enable template when encryption is set. |
internal/controller/clickhouse/config_test.go |
Adds unit test validating generated YAML structure for encryption disks/policies. |
docs/reference/api-reference.mdx |
Documents the new encryption field in the generated API reference. |
docs/guides/storage.mdx |
Adds at-rest encryption guide section and updates immutability/validation reference. |
docs/guides/configuration.mdx |
Documents disk-encryption-key in the Secret contract and links to storage guide encryption section. |
dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml |
Updates packaged CRD with new encryption schema/defaults and CEL validations. |
dist/chart-cluster/values.yaml |
Sets a default settings: {} in chart-cluster values. |
config/crd/bases/clickhouse.com_clickhouseclusters.yaml |
Updates base CRD manifest with new encryption schema/defaults and CEL validations. |
api/v1alpha1/zz_generated.deepcopy.go |
Regenerates deepcopy implementations for the new EncryptionSettings field/type. |
api/v1alpha1/clickhousecluster_types.go |
Adds EncryptionSettings type, settings default, and CEL validations (immutability + name rules). |
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+546
to
+549
| var policies int64 | ||
| Expect(chClient.QueryRowReplica(ctx, v1.ClickHouseReplicaID{}, `SELECT count()::Int64 | ||
| FROM system.storage_policies WHERE | ||
| policy_name = 'encrypted' AND arrayAll(x -> endsWith(x,'_encrypted'), disks)`, &policies)).To(Succeed()) |
Comment on lines
+110
to
+112
| if strings.HasSuffix(tmpl.Name, "-encrypted") { | ||
| errs = append(errs, fmt.Errorf("additionalVolumeClaimTemplates[%d].metadata.name %q is reserved: names ending in \"-encrypted\" collide with generated encrypted disk names", i, tmpl.Name)) | ||
| } |
| // The set of disks is fixed at creation. | ||
| // +optional | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MaxItems=128 |
| // +optional | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MaxItems=128 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(t, !has(t.metadata) || !t.metadata.name.endsWith('-encrypted'))",message="names ending in '-encrypted' are reserved for generated encrypted disk names" |
Comment on lines
207
to
209
| | Additional disk named `default` | Rejected — reserved by the ClickHouse default disk | | ||
| | Additional disk name ending in `-encrypted` | Rejected — collides with generated encrypted disk names | | ||
| | Additional disk named `clickhouse-storage-volume` | Rejected — collides with the primary data volume name | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Roadmap item: easily configurable at-rest encryption via ClickHouse disks. Encrypting table data today means hand-writing
storage_configurationwith encrypted disks and distributing the key yourself.What
spec.settings.encryption(presence enables feature): the operator generates an opt-inencryptedstorage policy that wraps every data disk (built-indefaultand JBOD disks) with ClickHouseencrypteddisks (AES_128_CTR). Thedefaultpolicy stays plaintext, tables opt in withSETTINGS storage_policy = 'encrypted';encryption.policyNameoverrides the name.disk-encryption-key), injected asCLICKHOUSE_DISK_ENCRYPTION_KEY, or user-supplied viaexternalSecret.policyNameis immutable and must not bedefault, additional disk names must not end in-encrypted(they collide with generated wrapper names).additionalVolumeClaimTemplatesis capped at 32 to fit the CEL cost budget.check-crd-compat: ignore info-level checker output (CEL cost notes fail the gate otherwise).externalSecretkey reference; unit/envtest coverage for generation and admission rules; e2e verifying the policy, a data round-trip, and part placement on the encrypted disk.Related Issues
Related to #65