diff --git a/console-web/app/(dashboard)/tenants/[namespace]/[name]/tenant-detail-client.tsx b/console-web/app/(dashboard)/tenants/[namespace]/[name]/tenant-detail-client.tsx index e55a3fd..f3534fe 100644 --- a/console-web/app/(dashboard)/tenants/[namespace]/[name]/tenant-detail-client.tsx +++ b/console-web/app/(dashboard)/tenants/[namespace]/[name]/tenant-detail-client.tsx @@ -162,6 +162,9 @@ export function TenantDetailClient({ namespace, name, initialTab, initialYamlEdi }) const [encLocal, setEncLocal] = useState({ keyDirectory: "", + masterKeySecretName: "", + masterKeySecretKey: "local-master-key", + allowInsecureDevDefaults: false, }) const [encDefaultKeyId, setEncDefaultKeyId] = useState("") const [encKmsSecretName, setEncKmsSecretName] = useState("") @@ -464,10 +467,20 @@ export function TenantDetailClient({ namespace, name, initialTab, initialYamlEdi if (data.local) { setEncLocal({ keyDirectory: data.local.keyDirectory || "", + masterKeySecretName: data.local.masterKeySecretRef?.name || "", + masterKeySecretKey: data.local.masterKeySecretRef?.key || "local-master-key", + allowInsecureDevDefaults: data.local.allowInsecureDevDefaults || false, + }) + } else { + setEncLocal({ + keyDirectory: "", + masterKeySecretName: "", + masterKeySecretKey: "local-master-key", + allowInsecureDevDefaults: false, }) } setEncDefaultKeyId(data.defaultKeyId || "") - setEncKmsSecretName(data.kmsSecretName || "") + setEncKmsSecretName(data.backend === "vault" ? data.kmsSecretName || "" : "") } catch (e) { const err = e as ApiError toast.error(err.message || t("Failed to load encryption config")) @@ -483,21 +496,40 @@ export function TenantDetailClient({ namespace, name, initialTab, initialYamlEdi toast.warning(t("Vault endpoint is required")) return } + if ( + encEnabled && + encBackend === "local" && + !encLocal.allowInsecureDevDefaults && + (!encLocal.masterKeySecretName.trim() || !encLocal.masterKeySecretKey.trim()) + ) { + toast.warning(t("Local KMS master key Secret name and key are required")) + return + } setEncSaving(true) try { const body: UpdateEncryptionRequest = { enabled: encEnabled, backend: encBackend, - kmsSecretName: encKmsSecretName || undefined, + kmsSecretName: encBackend === "vault" ? encKmsSecretName.trim() || undefined : undefined, defaultKeyId: encDefaultKeyId.trim() || undefined, } if (encBackend === "vault") { body.vault = { - endpoint: encVault.endpoint, + endpoint: encVault.endpoint.trim(), } } else { body.local = { - keyDirectory: encLocal.keyDirectory || undefined, + keyDirectory: encLocal.keyDirectory.trim() || undefined, + masterKeySecretRef: + !encLocal.allowInsecureDevDefaults && + encLocal.masterKeySecretName.trim() && + encLocal.masterKeySecretKey.trim() + ? { + name: encLocal.masterKeySecretName.trim(), + key: encLocal.masterKeySecretKey.trim(), + } + : undefined, + allowInsecureDevDefaults: encLocal.allowInsecureDevDefaults, } } const res = await api.updateEncryption(namespace, name, body) @@ -1117,12 +1149,52 @@ export function TenantDetailClient({ namespace, name, initialTab, initialYamlEdi
setEncLocal((l) => ({ ...l, keyDirectory: e.target.value }))} />
+
+ + setEncLocal((l) => ({ ...l, masterKeySecretName: e.target.value }))} + disabled={encLocal.allowInsecureDevDefaults} + /> +
+
+ + setEncLocal((l) => ({ ...l, masterKeySecretKey: e.target.value }))} + disabled={encLocal.allowInsecureDevDefaults} + /> +
+
+ + + setEncLocal((l) => ({ + ...l, + allowInsecureDevDefaults: e.target.checked, + })) + } + className="h-4 w-4 rounded border-border" + /> +
+

+ {t( + "Local KMS requires a master key Secret unless insecure development defaults are explicitly enabled.", + )} +

)} @@ -1142,11 +1214,12 @@ export function TenantDetailClient({ namespace, name, initialTab, initialYamlEdi placeholder={`${t("Optional")} – ${t("Secret containing vault-token")}`} value={encKmsSecretName} onChange={(e) => setEncKmsSecretName(e.target.value)} + disabled={encBackend !== "vault"} />

{encBackend === "vault" ? t("Required for Vault: Secret must contain key 'vault-token'.") - : t("Not required for Local backend.")} + : t("Only used for Vault. Local KMS uses masterKeySecretRef.")}

diff --git a/console-web/i18n/locales/en-US.json b/console-web/i18n/locales/en-US.json index faf1f48..b426bcd 100755 --- a/console-web/i18n/locales/en-US.json +++ b/console-web/i18n/locales/en-US.json @@ -236,7 +236,12 @@ "KMS Secret Name": "KMS Secret Name", "Secret containing vault-token": "Secret containing vault-token", "Required for Vault: Secret must contain key 'vault-token'.": "Required for Vault: Secret must contain key 'vault-token'.", - "Not required for Local backend.": "Not required for Local backend.", + "Only used for Vault. Local KMS uses masterKeySecretRef.": "Only used for Vault. Local KMS uses masterKeySecretRef.", + "Master Key Secret Name": "Master Key Secret Name", + "Master Key Secret Key": "Master Key Secret Key", + "Allow insecure development defaults": "Allow insecure development defaults", + "Local KMS requires a master key Secret unless insecure development defaults are explicitly enabled.": "Local KMS requires a master key Secret unless insecure development defaults are explicitly enabled.", + "Local KMS master key Secret name and key are required": "Local KMS master key Secret name and key are required", "Failed to load encryption config": "Failed to load encryption config", "Encryption config updated": "Encryption config updated", "Loading encryption config...": "Loading encryption config...", diff --git a/console-web/i18n/locales/zh-CN.json b/console-web/i18n/locales/zh-CN.json index afd42b6..a36ca4e 100755 --- a/console-web/i18n/locales/zh-CN.json +++ b/console-web/i18n/locales/zh-CN.json @@ -233,7 +233,12 @@ "KMS Secret Name": "KMS Secret 名称", "Secret containing vault-token and TLS certs": "包含 vault-token 和 TLS 证书的 Secret", "Secret must contain key 'vault-token'. Optional: 'vault-ca-cert', 'vault-client-cert', 'vault-client-key'.": "Secret 必须包含 'vault-token' 键。可选:'vault-ca-cert'、'vault-client-cert'、'vault-client-key'。", - "Not required for Local backend.": "本地后端不需要 Secret。", + "Only used for Vault. Local KMS uses masterKeySecretRef.": "仅用于 Vault。Local KMS 使用 masterKeySecretRef。", + "Master Key Secret Name": "Master Key Secret 名称", + "Master Key Secret Key": "Master Key Secret 键", + "Allow insecure development defaults": "允许开发环境不安全默认值", + "Local KMS requires a master key Secret unless insecure development defaults are explicitly enabled.": "Local KMS 需要 master key Secret,除非显式启用开发环境不安全默认值。", + "Local KMS master key Secret name and key are required": "Local KMS master key Secret 名称和键不能为空", "Auth Type": "认证方式", "Not yet implemented in backend": "后端尚未实现", "Retry (Seconds)": "重试间隔(秒)", diff --git a/console-web/types/api.ts b/console-web/types/api.ts index 4f374d2..cff5cbe 100644 --- a/console-web/types/api.ts +++ b/console-web/types/api.ts @@ -396,6 +396,11 @@ export interface VaultInfo { export interface LocalKmsInfo { keyDirectory: string | null + masterKeySecretRef: { + name: string + key: string + } | null + allowInsecureDevDefaults: boolean } export interface SecurityContextInfo { @@ -423,6 +428,11 @@ export interface UpdateEncryptionRequest { } local?: { keyDirectory?: string + masterKeySecretRef?: { + name: string + key: string + } + allowInsecureDevDefaults?: boolean } kmsSecretName?: string defaultKeyId?: string diff --git a/deploy/k8s-dev/operator-rbac.yaml b/deploy/k8s-dev/operator-rbac.yaml index e6a34d7..1768319 100755 --- a/deploy/k8s-dev/operator-rbac.yaml +++ b/deploy/k8s-dev/operator-rbac.yaml @@ -42,7 +42,7 @@ rules: resources: ["tokenreviews"] verbs: ["create"] - apiGroups: ["apps"] - resources: ["statefulsets"] + resources: ["statefulsets", "replicasets", "deployments"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] - apiGroups: ["cert-manager.io"] resources: ["certificates"] diff --git a/deploy/rustfs-operator/crds/tenant-crd.yaml b/deploy/rustfs-operator/crds/tenant-crd.yaml index 8e559b8..a6be36c 100644 --- a/deploy/rustfs-operator/crds/tenant-crd.yaml +++ b/deploy/rustfs-operator/crds/tenant-crd.yaml @@ -111,10 +111,44 @@ spec: description: 'Local: optional key directory override.' nullable: true properties: + allowInsecureDevDefaults: + default: false + description: |- + Explicitly allow RustFS development-only insecure KMS defaults. + + When true, the operator sets `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true`, allowing + Local KMS to start without a master key and store key material as plaintext-dev-only. + Do not use this in production. + type: boolean keyDirectory: - description: 'Absolute directory for KMS key files (default: `/data/kms-keys`).' + description: |- + Absolute directory for KMS key files. + + Must be in a subdirectory under a RustFS data PVC mount. Defaults to the first data PVC mount + under `persistence.path`, for example `/data/rustfs0/.kms-keys`. nullable: true type: string + masterKeySecretRef: + description: |- + Secret key selector for `RUSTFS_KMS_LOCAL_MASTER_KEY`. + + Required for Local KMS unless `allowInsecureDevDefaults` is explicitly set to `true`. + The referenced Secret key should contain the local master key string used to encrypt + Local KMS key files at rest. + nullable: true + properties: + key: + description: Secret data key containing the local master key string. + minLength: 1 + type: string + name: + description: Secret name in the Tenant namespace. + minLength: 1 + type: string + required: + - key + - name + type: object type: object vault: description: 'Vault: HTTP(S) endpoint (required when `backend: vault`).' @@ -127,7 +161,14 @@ spec: - endpoint type: object type: object + x-kubernetes-validations: + - message: Local KMS requires local.masterKeySecretRef unless local.allowInsecureDevDefaults is true + rule: '!self.enabled || self.backend != ''local'' || (has(self.local) && (has(self.local.masterKeySecretRef) || (has(self.local.allowInsecureDevDefaults) && self.local.allowInsecureDevDefaults == true)))' env: + description: |- + Additional RustFS container environment variables. + + `RUSTFS_KMS_*` is reserved; configure KMS through `spec.encryption`. items: description: EnvVar represents an environment variable present in a Container. properties: @@ -426,8 +467,10 @@ spec: Typical use-case: a StatefulSet Pod gets stuck in Terminating when the node goes down. Setting this to `ForceDelete` allows the operator to force delete the Pod object so the StatefulSet controller can recreate it elsewhere. + Force deletion requires the Node object to be deleted or marked with an effective + `node.kubernetes.io/out-of-service` taint that the target Pod does not tolerate. - Values: DoNothing | Delete | ForceDelete + Values: DoNothing | Delete | ForceDelete | DeleteStatefulSetPod | DeleteDeploymentPod | DeleteBothStatefulSetAndDeploymentPod enum: - DoNothing - Delete @@ -1725,7 +1768,9 @@ spec: description: Status of the condition (True, False, Unknown) type: string type: - description: Type of condition (Ready, Progressing, Degraded) + description: |- + Type of condition (Ready, Reconciling, Degraded, SpecValid, CredentialsReady, KmsReady, + TlsReady, PoolsReady, WorkloadsReady, ProvisioningReady) type: string required: - message diff --git a/deploy/rustfs-operator/crds/tenant.yaml b/deploy/rustfs-operator/crds/tenant.yaml index 8e559b8..a6be36c 100755 --- a/deploy/rustfs-operator/crds/tenant.yaml +++ b/deploy/rustfs-operator/crds/tenant.yaml @@ -111,10 +111,44 @@ spec: description: 'Local: optional key directory override.' nullable: true properties: + allowInsecureDevDefaults: + default: false + description: |- + Explicitly allow RustFS development-only insecure KMS defaults. + + When true, the operator sets `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true`, allowing + Local KMS to start without a master key and store key material as plaintext-dev-only. + Do not use this in production. + type: boolean keyDirectory: - description: 'Absolute directory for KMS key files (default: `/data/kms-keys`).' + description: |- + Absolute directory for KMS key files. + + Must be in a subdirectory under a RustFS data PVC mount. Defaults to the first data PVC mount + under `persistence.path`, for example `/data/rustfs0/.kms-keys`. nullable: true type: string + masterKeySecretRef: + description: |- + Secret key selector for `RUSTFS_KMS_LOCAL_MASTER_KEY`. + + Required for Local KMS unless `allowInsecureDevDefaults` is explicitly set to `true`. + The referenced Secret key should contain the local master key string used to encrypt + Local KMS key files at rest. + nullable: true + properties: + key: + description: Secret data key containing the local master key string. + minLength: 1 + type: string + name: + description: Secret name in the Tenant namespace. + minLength: 1 + type: string + required: + - key + - name + type: object type: object vault: description: 'Vault: HTTP(S) endpoint (required when `backend: vault`).' @@ -127,7 +161,14 @@ spec: - endpoint type: object type: object + x-kubernetes-validations: + - message: Local KMS requires local.masterKeySecretRef unless local.allowInsecureDevDefaults is true + rule: '!self.enabled || self.backend != ''local'' || (has(self.local) && (has(self.local.masterKeySecretRef) || (has(self.local.allowInsecureDevDefaults) && self.local.allowInsecureDevDefaults == true)))' env: + description: |- + Additional RustFS container environment variables. + + `RUSTFS_KMS_*` is reserved; configure KMS through `spec.encryption`. items: description: EnvVar represents an environment variable present in a Container. properties: @@ -426,8 +467,10 @@ spec: Typical use-case: a StatefulSet Pod gets stuck in Terminating when the node goes down. Setting this to `ForceDelete` allows the operator to force delete the Pod object so the StatefulSet controller can recreate it elsewhere. + Force deletion requires the Node object to be deleted or marked with an effective + `node.kubernetes.io/out-of-service` taint that the target Pod does not tolerate. - Values: DoNothing | Delete | ForceDelete + Values: DoNothing | Delete | ForceDelete | DeleteStatefulSetPod | DeleteDeploymentPod | DeleteBothStatefulSetAndDeploymentPod enum: - DoNothing - Delete @@ -1725,7 +1768,9 @@ spec: description: Status of the condition (True, False, Unknown) type: string type: - description: Type of condition (Ready, Progressing, Degraded) + description: |- + Type of condition (Ready, Reconciling, Degraded, SpecValid, CredentialsReady, KmsReady, + TlsReady, PoolsReady, WorkloadsReady, ProvisioningReady) type: string required: - message diff --git a/deploy/rustfs-operator/templates/clusterrole.yaml b/deploy/rustfs-operator/templates/clusterrole.yaml index bb2bcc9..9235ceb 100755 --- a/deploy/rustfs-operator/templates/clusterrole.yaml +++ b/deploy/rustfs-operator/templates/clusterrole.yaml @@ -40,9 +40,9 @@ rules: resources: ["tokenreviews"] verbs: ["create"] - # StatefulSets for tenant pools + # StatefulSets for tenant pools and owner lookups for pod cleanup - apiGroups: ["apps"] - resources: ["statefulsets"] + resources: ["statefulsets", "replicasets", "deployments"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] # cert-manager Certificate orchestration and readiness watches diff --git a/docs/operator-user-guide.md b/docs/operator-user-guide.md index e042774..e0a7483 100644 --- a/docs/operator-user-guide.md +++ b/docs/operator-user-guide.md @@ -345,6 +345,7 @@ The operator reserves these environment variables and manages them automatically - `RUSTFS_ADDRESS` - `RUSTFS_CONSOLE_ADDRESS` - `RUSTFS_CONSOLE_ENABLE` +- `RUSTFS_KMS_*` variables; use `spec.encryption` instead. - TLS-related RustFS variables when Tenant TLS is enabled. For a single-pool single-node single-disk Tenant, `RUSTFS_VOLUMES` is rendered as the local data path, for example `/data/rustfs0`. Multi-pool tenants and other layouts render peer DNS URLs through the Tenant headless Service and are validated by RustFS at runtime. @@ -358,7 +359,7 @@ For a single-pool single-node single-disk Tenant, `RUSTFS_VOLUMES` is rendered a - `DeleteDeploymentPod`: Longhorn-compatible force delete for Deployment pods stuck on down nodes. - `DeleteBothStatefulSetAndDeploymentPod`: Longhorn-compatible force delete for both StatefulSet and Deployment pods. -Force deletion can have data consistency implications. Use it only when the storage backend and operational procedure are designed for that failure mode. +Force deletion can have data consistency implications. Use it only when the storage backend and operational procedure are designed for that failure mode. Force deletion requires the Node object to be deleted or marked with an effective `node.kubernetes.io/out-of-service` taint that the target Pod does not tolerate, so volume detach fencing is explicit. ### 7.5 TLS @@ -425,10 +426,46 @@ Supported backends: | Backend | Purpose | |---------|---------| -| `local` | File-based local KMS key directory. The directory must be absolute, and the Tenant must have exactly one RustFS server replica across all pools. | +| `local` | File-based local KMS key directory and local master key. The directory must be absolute, must be in a subdirectory under a RustFS data PVC mount, and the Tenant must have exactly one RustFS server replica across all pools. | | `vault` | HashiCorp Vault endpoint. Requires a Secret containing `vault-token`. | -Local KMS does not use `kmsSecret`; if you set one, it is ignored. Use Vault KMS for multi-server Tenants. +Local KMS does not use `kmsSecret`; if you set one, it is ignored. Configure the local master key with `spec.encryption.local.masterKeySecretRef`, which maps to RustFS `RUSTFS_KMS_LOCAL_MASTER_KEY`. By default, Local KMS stores key files under the first data PVC mount, for example `/data/rustfs0/.kms-keys` when `persistence.path` is `/data`. Use Vault KMS for multi-server Tenants. `allowInsecureDevDefaults: true` maps to `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true` and should only be used for development because RustFS may store Local KMS key material as plaintext-dev-only. + +Upgrade note: older operator versions defaulted Local KMS to `/data/kms-keys`, which was not on a data PVC. The operator does not migrate key files automatically. For an existing Local KMS Tenant that still uses the old implicit default, or that explicitly set the legacy path, the operator blocks reconciliation or StatefulSet rollout until you copy any existing key files and `.master-key.salt` into the new PVC-backed directory, keep using the same local master key Secret, then set `spec.encryption.local.keyDirectory` explicitly to that subdirectory. + +Local example: + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: rustfs-local-kms + namespace: storage +type: Opaque +stringData: + local-master-key: "replace-with-a-strong-local-kms-master-key" +--- +apiVersion: rustfs.com/v1alpha1 +kind: Tenant +metadata: + name: rustfs-local + namespace: storage +spec: + pools: + - name: pool-0 + servers: 1 + persistence: + volumesPerServer: 1 + encryption: + enabled: true + backend: local + local: + keyDirectory: /data/rustfs0/.kms-keys + masterKeySecretRef: + name: rustfs-local-kms + key: local-master-key + defaultKeyId: tenant-default +``` Vault example: @@ -765,7 +802,7 @@ Common blocked reasons: | `CredentialSecretNotFound` | Secret exists in the Tenant namespace. | | `CredentialSecretMissingKey` | Secret contains `accesskey` and `secretkey`. | | `CredentialSecretTooShort` | Both credential values are at least 8 characters. | -| `KmsSecretNotFound` / `KmsSecretMissingKey` | KMS Secret exists and contains required keys such as `vault-token`. | +| `KmsSecretNotFound` / `KmsSecretMissingKey` | KMS Secret exists and contains required keys, such as Vault `vault-token` or the Local KMS `masterKeySecretRef.key`. | | `CertManagerCrdMissing` / `CertManagerIssuerNotFound` | cert-manager is installed and the issuer exists. | | `StatefulSetUpdateValidationFailed` | An immutable StatefulSet or pool-shape field was changed. | | `ProvisioningFailed` | Check `status.provisioning`, policy ConfigMaps, user Secrets, and RustFS admin credentials. | diff --git a/docs/operator-user-guide.zh-CN.md b/docs/operator-user-guide.zh-CN.md index fb511aa..83dd82b 100644 --- a/docs/operator-user-guide.zh-CN.md +++ b/docs/operator-user-guide.zh-CN.md @@ -347,6 +347,7 @@ Operator 会自动管理以下环境变量: - `RUSTFS_ADDRESS` - `RUSTFS_CONSOLE_ADDRESS` - `RUSTFS_CONSOLE_ENABLE` +- `RUSTFS_KMS_*` 变量;请改用 `spec.encryption` 配置 - 启用 TLS 时的 RustFS TLS 相关变量 对于单 pool 的单节点单盘 Tenant,`RUSTFS_VOLUMES` 会渲染为本地数据路径,例如 `/data/rustfs0`。多 pool Tenant 和其他布局仍会通过 Tenant headless Service 渲染 peer DNS URL,并由 RustFS 在运行时校验。 @@ -360,7 +361,7 @@ Operator 会自动管理以下环境变量: - `DeleteDeploymentPod`:Longhorn 兼容模式,强制删除 down node 上卡住的 Deployment Pod。 - `DeleteBothStatefulSetAndDeploymentPod`:Longhorn 兼容模式,同时处理 StatefulSet 和 Deployment Pod。 -强制删除可能影响数据一致性。只有当存储后端和运维流程明确支持该故障处理方式时才应启用。 +强制删除可能影响数据一致性。只有当存储后端和运维流程明确支持该故障处理方式时才应启用。强制删除要求 Node 对象已删除,或 Node 带有对目标 Pod 生效且未被该 Pod tolerate 的 `node.kubernetes.io/out-of-service` taint,确保 volume detach fencing 是显式的。 ### 7.5 TLS @@ -427,10 +428,46 @@ Tenant 加密通过 `spec.encryption` 配置。 | Backend | 用途 | |---------|------| -| `local` | 文件型本地 KMS key 目录。目录必须是绝对路径,且整个 Tenant 所有 pool 的 server 总数必须为 1。 | +| `local` | 文件型本地 KMS key 目录和本地 master key。目录必须是绝对路径,必须位于 RustFS 数据 PVC mount 下的子目录,且整个 Tenant 所有 pool 的 server 总数必须为 1。 | | `vault` | HashiCorp Vault endpoint,需要包含 `vault-token` 的 Secret。 | -Local KMS 不使用 `kmsSecret`;即使设置也会被忽略。多 server Tenant 应使用 Vault KMS。 +Local KMS 不使用 `kmsSecret`;即使设置也会被忽略。请通过 `spec.encryption.local.masterKeySecretRef` 配置本地 master key,它会映射到 RustFS 的 `RUSTFS_KMS_LOCAL_MASTER_KEY`。默认情况下,Local KMS 会把 key 文件存放在第一块数据 PVC mount 下,例如 `persistence.path` 为 `/data` 时使用 `/data/rustfs0/.kms-keys`。多 server Tenant 应使用 Vault KMS。`allowInsecureDevDefaults: true` 会映射到 `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true`,只能用于开发环境,因为 RustFS 可能用 plaintext-dev-only 方式保存 Local KMS key material。 + +升级提示:旧版本 Operator 的 Local KMS 默认目录是 `/data/kms-keys`,该路径不在数据 PVC 下。Operator 不会自动迁移 key 文件。对于仍使用旧隐式默认值,或显式设置了旧路径的已有 Local KMS Tenant,Operator 会阻断 reconcile 或 StatefulSet 滚动更新;请先把旧目录中的 key 文件和 `.master-key.salt` 复制到新的 PVC 持久化目录,继续使用同一个本地 master key Secret,然后显式设置 `spec.encryption.local.keyDirectory` 为该子目录。 + +Local 示例: + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: rustfs-local-kms + namespace: storage +type: Opaque +stringData: + local-master-key: "replace-with-a-strong-local-kms-master-key" +--- +apiVersion: rustfs.com/v1alpha1 +kind: Tenant +metadata: + name: rustfs-local + namespace: storage +spec: + pools: + - name: pool-0 + servers: 1 + persistence: + volumesPerServer: 1 + encryption: + enabled: true + backend: local + local: + keyDirectory: /data/rustfs0/.kms-keys + masterKeySecretRef: + name: rustfs-local-kms + key: local-master-key + defaultKeyId: tenant-default +``` Vault 示例: @@ -767,7 +804,7 @@ kubectl logs -n rustfs-system \ | `CredentialSecretNotFound` | Secret 是否存在于 Tenant namespace。 | | `CredentialSecretMissingKey` | Secret 是否包含 `accesskey` 和 `secretkey`。 | | `CredentialSecretTooShort` | 两个凭据值是否都至少 8 个字符。 | -| `KmsSecretNotFound` / `KmsSecretMissingKey` | KMS Secret 是否存在,并包含 `vault-token` 等必要 key。 | +| `KmsSecretNotFound` / `KmsSecretMissingKey` | KMS Secret 是否存在,并包含必要 key,例如 Vault 的 `vault-token` 或 Local KMS 的 `masterKeySecretRef.key`。 | | `CertManagerCrdMissing` / `CertManagerIssuerNotFound` | cert-manager 是否安装,issuer 是否存在。 | | `StatefulSetUpdateValidationFailed` | 是否修改了不可变 StatefulSet 字段或 pool 形态字段。 | | `ProvisioningFailed` | 检查 `status.provisioning`、policy ConfigMap、user Secret 和 RustFS 管理员凭据。 | diff --git a/src/console/handlers/encryption.rs b/src/console/handlers/encryption.rs index 0ee3c0e..ad819be 100644 --- a/src/console/handlers/encryption.rs +++ b/src/console/handlers/encryption.rs @@ -18,7 +18,7 @@ use crate::console::{ state::Claims, }; use crate::types::v1alpha1::encryption::{ - EncryptionConfig, KmsBackendType, LocalKmsConfig, VaultKmsConfig, + EncryptionConfig, KmsBackendType, LocalKmsConfig, LocalKmsMasterKeySecretRef, VaultKmsConfig, }; use crate::types::v1alpha1::tenant::Tenant; use axum::{Extension, Json, extract::Path}; @@ -26,6 +26,12 @@ use k8s_openapi::api::core::v1 as corev1; use kube::api::{Patch, PatchParams}; use kube::{Api, Client}; +fn trim_to_non_empty(value: Option) -> Option { + value + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) +} + /// GET /namespaces/:namespace/tenants/:name/encryption pub async fn get_encryption( Path((namespace, name)): Path<(String, String)>, @@ -49,8 +55,17 @@ pub async fn get_encryption( }), local: enc.local.as_ref().map(|l| LocalInfo { key_directory: l.key_directory.clone(), + master_key_secret_ref: l.master_key_secret_ref.as_ref().map(|s| { + LocalMasterKeySecretRefInfo { + name: s.name.clone(), + key: s.key.clone(), + } + }), + allow_insecure_dev_defaults: l.allow_insecure_dev_defaults, }), - kms_secret_name: enc.kms_secret.as_ref().map(|s| s.name.clone()), + kms_secret_name: (enc.backend == KmsBackendType::Vault) + .then(|| enc.kms_secret.as_ref().map(|s| s.name.clone())) + .flatten(), default_key_id: enc.default_key_id.clone(), security_context: tenant.spec.security_context.as_ref().map(|sc| { SecurityContextInfo { @@ -101,50 +116,65 @@ pub async fn update_encryption( Some("vault") => KmsBackendType::Vault, _ => KmsBackendType::Local, }; + let vault_endpoint = body + .vault + .as_ref() + .and_then(|v| trim_to_non_empty(Some(v.endpoint.clone()))); + let kms_secret_name = trim_to_non_empty(body.kms_secret_name.clone()); + let default_key_id = trim_to_non_empty(body.default_key_id.clone()); if backend == KmsBackendType::Vault { - let vault_ok = body - .vault - .as_ref() - .map(|v| !v.endpoint.is_empty()) - .unwrap_or(false); - if !vault_ok { + if vault_endpoint.is_none() { return Err(Error::BadRequest { message: "Vault backend requires vault.endpoint to be non-empty".to_string(), }); } - let secret_ok = body - .kms_secret_name - .as_ref() - .map(|s| !s.is_empty()) - .unwrap_or(false); - if !secret_ok { + if kms_secret_name.is_none() { return Err(Error::BadRequest { message: "Vault backend requires kmsSecretName".to_string(), }); } + } else { + let local = body.local.as_ref(); + let allow_insecure_dev_defaults = local + .and_then(|l| l.allow_insecure_dev_defaults) + .unwrap_or(false); + let master_key_ref_ok = local + .and_then(|l| l.master_key_secret_ref.as_ref()) + .is_some_and(|s| !s.name.trim().is_empty() && !s.key.trim().is_empty()); + if !allow_insecure_dev_defaults && !master_key_ref_ok { + return Err(Error::BadRequest { + message: "Local backend requires local.masterKeySecretRef.name/key unless local.allowInsecureDevDefaults is true".to_string(), + }); + } } let vault = if backend == KmsBackendType::Vault { - body.vault.map(|v| VaultKmsConfig { - endpoint: v.endpoint, - }) + vault_endpoint.map(|endpoint| VaultKmsConfig { endpoint }) } else { None }; let local = if backend == KmsBackendType::Local { body.local.map(|l| LocalKmsConfig { - key_directory: l.key_directory, + key_directory: trim_to_non_empty(l.key_directory), + master_key_secret_ref: l.master_key_secret_ref.map(|s| { + LocalKmsMasterKeySecretRef { + name: s.name.trim().to_string(), + key: s.key.trim().to_string(), + } + }), + allow_insecure_dev_defaults: l.allow_insecure_dev_defaults.unwrap_or(false), }) } else { None }; - let kms_secret = body - .kms_secret_name - .filter(|s| !s.is_empty()) - .map(|s| corev1::LocalObjectReference { name: s }); + let kms_secret = if backend == KmsBackendType::Vault { + kms_secret_name.map(|s| corev1::LocalObjectReference { name: s }) + } else { + None + }; Some(EncryptionConfig { enabled: true, @@ -152,7 +182,7 @@ pub async fn update_encryption( vault, local, kms_secret, - default_key_id: body.default_key_id.filter(|s| !s.is_empty()), + default_key_id, }) } else { Some(EncryptionConfig { @@ -190,3 +220,18 @@ async fn create_client(claims: &Claims) -> Result { message: format!("Failed to create K8s client: {}", e), }) } + +#[cfg(test)] +mod tests { + use super::trim_to_non_empty; + + #[test] + fn trim_to_non_empty_drops_blank_strings() { + assert_eq!(trim_to_non_empty(None), None); + assert_eq!(trim_to_non_empty(Some(" ".to_string())), None); + assert_eq!( + trim_to_non_empty(Some(" /data/rustfs0/.kms-keys ".to_string())), + Some("/data/rustfs0/.kms-keys".to_string()) + ); + } +} diff --git a/src/console/models/encryption.rs b/src/console/models/encryption.rs index f2865dc..2c4f3b4 100644 --- a/src/console/models/encryption.rs +++ b/src/console/models/encryption.rs @@ -39,6 +39,15 @@ pub struct VaultInfo { #[serde(rename_all = "camelCase")] pub struct LocalInfo { pub key_directory: Option, + pub master_key_secret_ref: Option, + pub allow_insecure_dev_defaults: bool, +} + +#[derive(Debug, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct LocalMasterKeySecretRefInfo { + pub name: String, + pub key: String, } /// SecurityContext information (TenantSpec; shown alongside encryption for convenience). @@ -73,6 +82,8 @@ pub struct UpdateVaultRequest { #[serde(rename_all = "camelCase")] pub struct UpdateLocalRequest { pub key_directory: Option, + pub master_key_secret_ref: Option, + pub allow_insecure_dev_defaults: Option, } #[derive(Debug, Deserialize, ToSchema)] diff --git a/src/context.rs b/src/context.rs index de39bc0..940285d 100755 --- a/src/context.rs +++ b/src/context.rs @@ -13,6 +13,7 @@ // limitations under the License. use crate::types; +use crate::types::v1alpha1::encryption::LocalKmsMasterKeySecretRef; use crate::types::v1alpha1::tenant::Tenant; use k8s_openapi::NamespaceResourceScope; use k8s_openapi::api::core::v1::Secret; @@ -24,6 +25,7 @@ use serde::de::DeserializeOwned; use snafu::Snafu; use snafu::futures::TryFutureExt; use std::fmt::Debug; +use std::path::{Component, Path, PathBuf}; use tracing::info; #[derive(Debug, Snafu)] @@ -75,14 +77,110 @@ pub enum Error { Serde { source: serde_json::Error }, } -/// Validates Local KMS: absolute `keyDirectory` and at most one server replica across pools. +fn local_kms_key_directory( + local: Option<&types::v1alpha1::encryption::LocalKmsConfig>, + pools: &[types::v1alpha1::pool::Pool], +) -> String { + local + .and_then(|l| l.key_directory.clone()) + .unwrap_or_else(|| { + let base_path = pools + .first() + .and_then(|pool| pool.persistence.path.as_deref()); + types::v1alpha1::persistence::default_local_kms_key_directory(base_path) + }) +} + +fn normalize_absolute_path(path: &str) -> Option { + let path = Path::new(path); + if !path.is_absolute() { + return None; + } + + let mut normalized = PathBuf::from("/"); + for component in path.components() { + match component { + Component::RootDir | Component::CurDir => {} + Component::Normal(part) => normalized.push(part), + Component::ParentDir => { + normalized.pop(); + } + Component::Prefix(_) => return None, + } + } + + Some(normalized) +} + +fn local_kms_key_dir_is_on_data_volume( + key_dir: &str, + pools: &[types::v1alpha1::pool::Pool], +) -> bool { + let Some(key_dir) = normalize_absolute_path(key_dir) else { + return false; + }; + + pools.iter().any(|pool| { + (0..pool.persistence.volumes_per_server).any(|shard| { + let mount_path = types::v1alpha1::persistence::data_volume_mount_path( + pool.persistence.path.as_deref(), + shard, + ); + normalize_absolute_path(&mount_path) + .is_some_and(|mount_path| key_dir != mount_path && key_dir.starts_with(mount_path)) + }) + }) +} + +fn local_kms_data_volume_hint(pools: &[types::v1alpha1::pool::Pool]) -> String { + pools + .first() + .map(|pool| { + types::v1alpha1::persistence::default_local_kms_key_directory( + pool.persistence.path.as_deref(), + ) + }) + .unwrap_or_else(|| types::v1alpha1::persistence::default_local_kms_key_directory(None)) +} + +fn local_kms_legacy_path_migration_message( + key_dir: &str, + pools: &[types::v1alpha1::pool::Pool], +) -> String { + let target_dir = local_kms_data_volume_hint(pools); + format!( + "Local KMS keyDirectory '{}' is the legacy non-PVC path. Copy existing key files and .master-key.salt into '{}', then set spec.encryption.local.keyDirectory to that PVC-backed subdirectory before rolling RustFS pods.", + key_dir, target_dir + ) +} + +fn reserved_kms_env_var_name(tenant: &Tenant) -> Option<&str> { + tenant + .spec + .env + .iter() + .find(|env| env.name.starts_with("RUSTFS_KMS_")) + .map(|env| env.name.as_str()) +} + +fn validate_no_reserved_kms_env(tenant: &Tenant) -> Result<(), Error> { + if let Some(env_name) = reserved_kms_env_var_name(tenant) { + return Err(Error::KmsConfigInvalid { + message: format!( + "spec.env must not set reserved KMS environment variable '{}'. Configure KMS through spec.encryption so the operator can validate persistence, migration, and Secret references.", + env_name + ), + }); + } + Ok(()) +} + +/// Validates Local KMS: absolute persistent `keyDirectory` and at most one server replica across pools. fn validate_local_kms_tenant( local: Option<&types::v1alpha1::encryption::LocalKmsConfig>, pools: &[types::v1alpha1::pool::Pool], ) -> Result<(), Error> { - let key_dir = local - .and_then(|l| l.key_directory.as_deref()) - .unwrap_or("/data/kms-keys"); + let key_dir = local_kms_key_directory(local, pools); if !key_dir.starts_with('/') { return Err(Error::KmsConfigInvalid { message: format!( @@ -91,6 +189,20 @@ fn validate_local_kms_tenant( ), }); } + if key_dir == types::v1alpha1::persistence::LEGACY_LOCAL_KMS_KEY_DIR { + return Err(Error::KmsConfigInvalid { + message: local_kms_legacy_path_migration_message(&key_dir, pools), + }); + } + if !local_kms_key_dir_is_on_data_volume(&key_dir, pools) { + return Err(Error::KmsConfigInvalid { + message: format!( + "Local KMS keyDirectory must be in a subdirectory under a RustFS data PVC mount (got \"{}\"). Use the default or a path such as \"{}\".", + key_dir, + local_kms_data_volume_hint(pools) + ), + }); + } let total_servers: i32 = pools.iter().map(|p| p.servers).sum(); if total_servers > 1 { return Err(Error::KmsConfigInvalid { @@ -100,6 +212,65 @@ fn validate_local_kms_tenant( Ok(()) } +fn validate_local_kms_master_key_ref( + local: Option<&types::v1alpha1::encryption::LocalKmsConfig>, +) -> Result, Error> { + let allow_insecure_dev_defaults = local.is_some_and(|l| l.allow_insecure_dev_defaults); + let selector = local.and_then(|l| l.master_key_secret_ref.as_ref()); + + let Some(selector) = selector else { + if allow_insecure_dev_defaults { + return Ok(None); + } + return Err(Error::KmsConfigInvalid { + message: "Local KMS requires spec.encryption.local.masterKeySecretRef unless spec.encryption.local.allowInsecureDevDefaults is true for development-only plaintext key storage".to_string(), + }); + }; + + if selector.name.is_empty() { + return Err(Error::KmsConfigInvalid { + message: "Local KMS masterKeySecretRef.name must not be empty".to_string(), + }); + } + if selector.key.is_empty() { + return Err(Error::KmsConfigInvalid { + message: "Local KMS masterKeySecretRef.key must not be empty".to_string(), + }); + } + Ok(Some(selector)) +} + +fn validate_secret_utf8_non_blank( + secret: &Secret, + secret_name: &str, + key: &str, +) -> Result<(), Error> { + let Some(value) = secret.data.as_ref().and_then(|data| data.get(key)) else { + return KmsSecretMissingKeySnafu { + secret_name: secret_name.to_string(), + key: key.to_string(), + } + .fail(); + }; + + let value = std::str::from_utf8(&value.0).map_err(|_| Error::KmsConfigInvalid { + message: format!( + "KMS Secret '{}' key '{}' must contain valid UTF-8", + secret_name, key + ), + })?; + if value.trim().is_empty() { + return Err(Error::KmsConfigInvalid { + message: format!( + "KMS Secret '{}' key '{}' must not be blank", + secret_name, key + ), + }); + } + + Ok(()) +} + fn status_semantically_equal( current: Option<&types::v1alpha1::status::Status>, next: &types::v1alpha1::status::Status, @@ -426,12 +597,15 @@ impl Context { /// Validates encryption configuration and the KMS Secret. /// /// Checks: - /// 1. Local KMS: absolute key directory and single replica (sum of pool servers). + /// 1. Local KMS: absolute key directory, single replica, and a local master key Secret + /// unless explicit development-only insecure defaults are enabled. /// 2. Vault endpoint is non-empty when backend is Vault. /// 3. KMS Secret exists and contains the correct keys for the auth type. pub async fn validate_kms_secret(&self, tenant: &Tenant) -> Result<(), Error> { use crate::types::v1alpha1::encryption::KmsBackendType; + validate_no_reserved_kms_env(tenant)?; + let Some(ref enc) = tenant.spec.encryption else { return Ok(()); }; @@ -439,14 +613,24 @@ impl Context { return Ok(()); } - // Local KMS: RustFS requires an absolute key directory; multi-replica tenants need Vault - // (or a shared filesystem) because each Pod would otherwise have its own key files. if enc.backend == KmsBackendType::Local { validate_local_kms_tenant(enc.local.as_ref(), &tenant.spec.pools)?; - } - - // Vault: non-empty endpoint and `kmsSecret` with `vault-token` (RustFS `build_vault_kms_config`). - if enc.backend == KmsBackendType::Vault { + if let Some(selector) = validate_local_kms_master_key_ref(enc.local.as_ref())? { + let secret: Secret = match self.get(&selector.name, &tenant.namespace()?).await { + Ok(secret) => secret, + Err(error) => { + return Err(map_secret_get_error( + error, + selector.name.clone(), + SecretValidationKind::Kms, + )); + } + }; + validate_secret_utf8_non_blank(&secret, &selector.name, &selector.key)?; + } + return Ok(()); + } else if enc.backend == KmsBackendType::Vault { + // Vault: non-empty endpoint and `kmsSecret` with `vault-token` (RustFS `build_vault_kms_config`). let endpoint_empty = enc .vault .as_ref() @@ -469,27 +653,25 @@ impl Context { .to_string(), }); } - } - let Some(ref secret_ref) = enc.kms_secret else { - return Ok(()); - }; - if secret_ref.name.is_empty() { - return Ok(()); - } - - let secret: Secret = match self.get(&secret_ref.name, &tenant.namespace()?).await { - Ok(secret) => secret, - Err(error) => { - return Err(map_secret_get_error( - error, - secret_ref.name.clone(), - SecretValidationKind::Kms, - )); - } - }; + let Some(secret_ref) = enc.kms_secret.as_ref() else { + return Err(Error::KmsConfigInvalid { + message: + "Vault backend requires kmsSecret referencing a Secret with key vault-token" + .to_string(), + }); + }; + let secret: Secret = match self.get(&secret_ref.name, &tenant.namespace()?).await { + Ok(secret) => secret, + Err(error) => { + return Err(map_secret_get_error( + error, + secret_ref.name.clone(), + SecretValidationKind::Kms, + )); + } + }; - if enc.backend == KmsBackendType::Vault { let has_token = secret .data .as_ref() @@ -591,18 +773,29 @@ impl Context { #[cfg(test)] mod validate_local_kms_tests { use super::Error; - use super::validate_local_kms_tenant; use super::{SecretValidationKind, map_secret_get_error}; - use crate::types::v1alpha1::encryption::LocalKmsConfig; + use super::{ + validate_local_kms_master_key_ref, validate_local_kms_tenant, validate_no_reserved_kms_env, + validate_secret_utf8_non_blank, + }; + use crate::types::v1alpha1::encryption::{LocalKmsConfig, LocalKmsMasterKeySecretRef}; use crate::types::v1alpha1::persistence::PersistenceConfig; use crate::types::v1alpha1::pool::Pool; + use k8s_openapi::ByteString; + use k8s_openapi::api::core::v1 as corev1; + use std::collections::BTreeMap; fn pool(servers: i32) -> Pool { + pool_with_path(servers, None) + } + + fn pool_with_path(servers: i32, path: Option<&str>) -> Pool { Pool { name: "p".to_string(), servers, persistence: PersistenceConfig { volumes_per_server: 4, + path: path.map(ToOwned::to_owned), ..Default::default() }, scheduling: Default::default(), @@ -620,6 +813,13 @@ mod validate_local_kms_tests { } } + fn master_key_selector() -> LocalKmsMasterKeySecretRef { + LocalKmsMasterKeySecretRef { + name: "local-kms-master-key".to_string(), + key: "local-master-key".to_string(), + } + } + #[test] fn credential_secret_get_maps_only_404_to_not_found() { let err = map_secret_get_error( @@ -660,6 +860,24 @@ mod validate_local_kms_tests { validate_local_kms_tenant(None, &[pool(1)]).unwrap(); } + #[test] + fn local_kms_accepts_key_dir_on_data_volume() { + let local = LocalKmsConfig { + key_directory: Some("/data/rustfs0/kms".to_string()), + ..Default::default() + }; + validate_local_kms_tenant(Some(&local), &[pool(1)]).unwrap(); + } + + #[test] + fn local_kms_accepts_key_dir_on_custom_data_volume() { + let local = LocalKmsConfig { + key_directory: Some("/mnt/rustfs/rustfs0/kms".to_string()), + ..Default::default() + }; + validate_local_kms_tenant(Some(&local), &[pool_with_path(1, Some("/mnt/rustfs"))]).unwrap(); + } + #[test] fn local_kms_rejects_relative_key_dir() { let local = LocalKmsConfig { @@ -670,10 +888,167 @@ mod validate_local_kms_tests { assert!(matches!(err, Error::KmsConfigInvalid { .. })); } + #[test] + fn local_kms_rejects_key_dir_outside_data_volume() { + let local = LocalKmsConfig { + key_directory: Some("/opt/rustfs-kms".to_string()), + ..Default::default() + }; + let err = validate_local_kms_tenant(Some(&local), &[pool(1)]).unwrap_err(); + assert!( + matches!(err, Error::KmsConfigInvalid { message } if message.contains("data PVC mount")) + ); + } + + #[test] + fn local_kms_rejects_legacy_key_dir_with_migration_guidance() { + let local = LocalKmsConfig { + key_directory: Some("/data/kms-keys".to_string()), + ..Default::default() + }; + let err = validate_local_kms_tenant(Some(&local), &[pool(1)]).unwrap_err(); + assert!(matches!(err, Error::KmsConfigInvalid { message } + if message.contains("legacy non-PVC path") + && message.contains("/data/rustfs0/.kms-keys") + && message.contains(".master-key.salt") + && message.contains("spec.encryption.local.keyDirectory"))); + } + + #[test] + fn local_kms_rejects_data_volume_root() { + let local = LocalKmsConfig { + key_directory: Some("/data/rustfs0".to_string()), + ..Default::default() + }; + let err = validate_local_kms_tenant(Some(&local), &[pool(1)]).unwrap_err(); + assert!( + matches!(err, Error::KmsConfigInvalid { message } if message.contains("subdirectory")) + ); + } + + #[test] + fn local_kms_rejects_prefix_match_outside_data_volume() { + let local = LocalKmsConfig { + key_directory: Some("/data/rustfs01/kms".to_string()), + ..Default::default() + }; + let err = validate_local_kms_tenant(Some(&local), &[pool(1)]).unwrap_err(); + assert!( + matches!(err, Error::KmsConfigInvalid { message } if message.contains("data PVC mount")) + ); + } + #[test] fn local_kms_rejects_multi_pool_multi_replica() { let local = LocalKmsConfig::default(); let err = validate_local_kms_tenant(Some(&local), &[pool(2), pool(2)]).unwrap_err(); assert!(matches!(err, Error::KmsConfigInvalid { .. })); } + + #[test] + fn local_kms_requires_master_key_secret_ref_by_default() { + let local = LocalKmsConfig::default(); + let err = validate_local_kms_master_key_ref(Some(&local)).unwrap_err(); + assert!(matches!(err, Error::KmsConfigInvalid { message } + if message.contains("masterKeySecretRef"))); + } + + #[test] + fn local_kms_allows_explicit_insecure_dev_defaults_without_master_key() { + let local = LocalKmsConfig { + allow_insecure_dev_defaults: true, + ..Default::default() + }; + + let selector = validate_local_kms_master_key_ref(Some(&local)).unwrap(); + + assert!(selector.is_none()); + } + + #[test] + fn local_kms_rejects_empty_master_key_secret_ref_name() { + let local = LocalKmsConfig { + master_key_secret_ref: Some(LocalKmsMasterKeySecretRef { + name: String::new(), + ..master_key_selector() + }), + ..Default::default() + }; + + let err = validate_local_kms_master_key_ref(Some(&local)).unwrap_err(); + + assert!(matches!(err, Error::KmsConfigInvalid { message } + if message.contains("name must not be empty"))); + } + + #[test] + fn local_kms_secret_key_must_be_utf8_and_non_blank() { + let mut data = BTreeMap::new(); + data.insert("blank".to_string(), ByteString(b" \n".to_vec())); + data.insert("invalid".to_string(), ByteString(vec![0xff])); + let secret = corev1::Secret { + data: Some(data), + ..Default::default() + }; + + let missing = + validate_secret_utf8_non_blank(&secret, "local-kms-master-key", "missing").unwrap_err(); + assert!(matches!(missing, Error::KmsSecretMissingKey { key, .. } if key == "missing")); + + let invalid = + validate_secret_utf8_non_blank(&secret, "local-kms-master-key", "invalid").unwrap_err(); + assert!(matches!(invalid, Error::KmsConfigInvalid { message } + if message.contains("valid UTF-8"))); + + let blank = + validate_secret_utf8_non_blank(&secret, "local-kms-master-key", "blank").unwrap_err(); + assert!(matches!(blank, Error::KmsConfigInvalid { message } + if message.contains("must not be blank"))); + } + + #[test] + fn local_kms_secret_key_accepts_non_empty_data() { + let mut data = BTreeMap::new(); + data.insert( + "local-master-key".to_string(), + ByteString(b"test-master-key".to_vec()), + ); + let secret = corev1::Secret { + data: Some(data), + ..Default::default() + }; + + validate_secret_utf8_non_blank(&secret, "local-kms-master-key", "local-master-key") + .unwrap(); + } + + #[test] + fn reserved_kms_env_is_rejected_without_encryption_spec() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.encryption = None; + tenant.spec.env = vec![corev1::EnvVar { + name: "RUSTFS_KMS_ENABLE".to_string(), + value: Some("true".to_string()), + ..Default::default() + }]; + + let err = validate_no_reserved_kms_env(&tenant).unwrap_err(); + assert!(matches!(err, Error::KmsConfigInvalid { message } + if message.contains("RUSTFS_KMS_ENABLE") + && message.contains("spec.encryption"))); + } + + #[test] + fn reserved_kms_env_rejects_unmodelled_rustfs_kms_vars() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.env = vec![corev1::EnvVar { + name: "RUSTFS_KMS_LOCAL_MASTER_KEY".to_string(), + value: Some("secret".to_string()), + ..Default::default() + }]; + + let err = validate_no_reserved_kms_env(&tenant).unwrap_err(); + assert!(matches!(err, Error::KmsConfigInvalid { message } + if message.contains("RUSTFS_KMS_LOCAL_MASTER_KEY"))); + } } diff --git a/src/reconcile.rs b/src/reconcile.rs index 228804e..9463774 100755 --- a/src/reconcile.rs +++ b/src/reconcile.rs @@ -17,11 +17,13 @@ use crate::status::{StatusBuilder, StatusError}; use crate::types::v1alpha1::status::{ConditionType, Reason, Status}; use crate::types::v1alpha1::tenant::Tenant; use crate::{context, types}; +use k8s_openapi::api::apps::v1 as appsv1; use k8s_openapi::api::core::v1 as corev1; -use kube::ResourceExt; -use kube::api::{DeleteParams, ListParams, PropagationPolicy}; +use k8s_openapi::apimachinery::pkg::apis::meta::v1 as metav1; +use kube::api::{DeleteParams, ListParams, Preconditions, PropagationPolicy}; use kube::runtime::controller::Action; use kube::runtime::events::EventType; +use kube::{Resource, ResourceExt}; use snafu::Snafu; use std::sync::Arc; use std::time::Duration; @@ -32,6 +34,8 @@ mod pool_lifecycle; mod provisioning; mod tls; +const OUT_OF_SERVICE_TAINT_KEY: &str = "node.kubernetes.io/out-of-service"; + use phases::{ cleanup_removed_decommissioned_pool_statefulsets, finalize_tenant_status, maybe_cleanup_terminating_pods, reconcile_pool_statefulsets, reconcile_rbac_resources, @@ -338,19 +342,112 @@ fn condition_marker( .map(|condition| (condition.status.clone(), condition.reason.clone())) } -fn statefulset_owned_by_tenant( - ss: &k8s_openapi::api::apps::v1::StatefulSet, - tenant: &Tenant, -) -> bool { - ss.metadata.owner_references.as_ref().is_some_and(|refs| { +fn object_owned_by_tenant(metadata: &metav1::ObjectMeta, tenant: &Tenant) -> bool { + let Some(tenant_uid) = tenant.metadata.uid.as_deref().filter(|uid| !uid.is_empty()) else { + return false; + }; + + metadata.owner_references.as_ref().is_some_and(|refs| { refs.iter().any(|owner| { - owner.kind == "Tenant" + owner.api_version == Tenant::api_version(&()) + && owner.kind == Tenant::kind(&()) && owner.name == tenant.name() - && owner.uid == tenant.metadata.uid.as_deref().unwrap_or("") + && owner.uid == tenant_uid + && owner.controller == Some(true) }) }) } +fn statefulset_owned_by_tenant(ss: &appsv1::StatefulSet, tenant: &Tenant) -> bool { + object_owned_by_tenant(&ss.metadata, tenant) +} + +fn statefulset_matches_pod_controller_and_tenant( + pod: &corev1::Pod, + statefulset: &appsv1::StatefulSet, + tenant: &Tenant, +) -> bool { + let Some((_, statefulset_uid)) = pod_controller_owner_name_and_uid(pod, "StatefulSet") else { + return false; + }; + + statefulset_owned_by_tenant(statefulset, tenant) + && statefulset.metadata.uid.as_deref() == Some(statefulset_uid.as_str()) +} + +fn replicaset_matches_pod_controller_and_tenant( + pod: &corev1::Pod, + replicaset: &appsv1::ReplicaSet, + owning_deployment: Option<&appsv1::Deployment>, + tenant: &Tenant, +) -> bool { + let Some((_, replicaset_uid)) = pod_controller_owner_name_and_uid(pod, "ReplicaSet") else { + return false; + }; + if replicaset.metadata.uid.as_deref() != Some(replicaset_uid.as_str()) { + return false; + } + if object_owned_by_tenant(&replicaset.metadata, tenant) { + return true; + } + + let Some((_, deployment_uid)) = + controller_owner_name_and_uid(replicaset.metadata.owner_references.as_ref(), "Deployment") + else { + return false; + }; + let Some(deployment) = owning_deployment else { + return false; + }; + + object_owned_by_tenant(&deployment.metadata, tenant) + && deployment.metadata.uid.as_deref() == Some(deployment_uid.as_str()) +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum NodePodDeletionSafety { + Ready, + DownUnfenced, + Fenced, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +struct PodDeletionPlan { + force: bool, + precondition_uid: Option, + event_reason: &'static str, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +enum PodCleanupDecision { + Skip, + SkipForceDeleteNeedsFencing, + Delete(PodDeletionPlan), +} + +impl PodDeletionPlan { + fn delete_params(&self) -> DeleteParams { + let preconditions = self.precondition_uid.clone().map(|uid| Preconditions { + uid: Some(uid), + resource_version: None, + }); + + if self.force { + DeleteParams { + grace_period_seconds: Some(0), + propagation_policy: Some(PropagationPolicy::Background), + preconditions, + ..DeleteParams::default() + } + } else { + DeleteParams { + preconditions, + ..DeleteParams::default() + } + } + } +} + async fn cleanup_stuck_terminating_pods_on_down_nodes( tenant: &Tenant, namespace: &str, @@ -359,6 +456,12 @@ async fn cleanup_stuck_terminating_pods_on_down_nodes( ) -> Result<(), Error> { let pods_api: kube::Api = kube::Api::namespaced(ctx.client.clone(), namespace); let nodes_api: kube::Api = kube::Api::all(ctx.client.clone()); + let statefulsets_api: kube::Api = + kube::Api::namespaced(ctx.client.clone(), namespace); + let replicasets_api: kube::Api = + kube::Api::namespaced(ctx.client.clone(), namespace); + let deployments_api: kube::Api = + kube::Api::namespaced(ctx.client.clone(), namespace); let selector = format!("rustfs.tenant={}", tenant.name()); let pods = pods_api @@ -382,13 +485,105 @@ async fn cleanup_stuck_terminating_pods_on_down_nodes( continue; } + let mut verified_tenant_owner = object_owned_by_tenant(&pod.metadata, tenant); + if let Some((statefulset_name, _statefulset_uid)) = + pod_controller_owner_name_and_uid(&pod, "StatefulSet") + { + let owned_by_tenant = match statefulsets_api.get(&statefulset_name).await { + Ok(statefulset) => { + statefulset_matches_pod_controller_and_tenant(&pod, &statefulset, tenant) + } + Err(kube::Error::Api(ae)) if ae.code == 404 => false, + Err(source) => { + return Err(Error::Context { + source: context::Error::Kube { source }, + }); + } + }; + + if !owned_by_tenant { + warn!( + tenant = %tenant.name(), + namespace = %namespace, + pod = %pod.name_any(), + statefulset = %statefulset_name, + "skipping terminating StatefulSet pod because the owner StatefulSet is not owned by this tenant" + ); + continue; + } + verified_tenant_owner = true; + } + if let Some((replicaset_name, _replicaset_uid)) = + pod_controller_owner_name_and_uid(&pod, "ReplicaSet") + { + let owned_by_tenant = match replicasets_api.get(&replicaset_name).await { + Ok(replicaset) => { + let deployment = if object_owned_by_tenant(&replicaset.metadata, tenant) { + None + } else if let Some((deployment_name, _deployment_uid)) = + controller_owner_name_and_uid( + replicaset.metadata.owner_references.as_ref(), + "Deployment", + ) + { + match deployments_api.get(&deployment_name).await { + Ok(deployment) => Some(deployment), + Err(kube::Error::Api(ae)) if ae.code == 404 => None, + Err(source) => { + return Err(Error::Context { + source: context::Error::Kube { source }, + }); + } + } + } else { + None + }; + + replicaset_matches_pod_controller_and_tenant( + &pod, + &replicaset, + deployment.as_ref(), + tenant, + ) + } + Err(kube::Error::Api(ae)) if ae.code == 404 => false, + Err(source) => { + return Err(Error::Context { + source: context::Error::Kube { source }, + }); + } + }; + + if !owned_by_tenant { + warn!( + tenant = %tenant.name(), + namespace = %namespace, + pod = %pod.name_any(), + replicaset = %replicaset_name, + "skipping terminating ReplicaSet pod because the owner chain is not owned by this tenant" + ); + continue; + } + verified_tenant_owner = true; + } + + if !verified_tenant_owner { + warn!( + tenant = %tenant.name(), + namespace = %namespace, + pod = %pod.name_any(), + "skipping terminating pod because it does not have a verified Tenant owner chain" + ); + continue; + } + let Some(node_name) = pod.spec.as_ref().and_then(|s| s.node_name.clone()) else { continue; }; - let node_is_down = match nodes_api.get(&node_name).await { - Ok(node) => is_node_down(&node), - Err(kube::Error::Api(ae)) if ae.code == 404 => true, + let node_deletion_safety = match nodes_api.get(&node_name).await { + Ok(node) => node_pod_deletion_safety(&node, &pod), + Err(kube::Error::Api(ae)) if ae.code == 404 => NodePodDeletionSafety::Fenced, Err(source) => { return Err(Error::Context { source: context::Error::Kube { source }, @@ -396,10 +591,26 @@ async fn cleanup_stuck_terminating_pods_on_down_nodes( } }; - if !node_is_down { + if node_deletion_safety == NodePodDeletionSafety::Ready { continue; } + let deletion_plan = match cleanup_decision_for_pod(&pod, &policy, node_deletion_safety) { + PodCleanupDecision::Skip => continue, + PodCleanupDecision::SkipForceDeleteNeedsFencing => { + warn!( + tenant = %tenant.name(), + namespace = %namespace, + node = %node_name, + pod = %pod.name_any(), + policy = ?policy, + "skipping pod force deletion because the node is not fenced" + ); + continue; + } + PodCleanupDecision::Delete(plan) => plan, + }; + let pod_name = pod.name_any(); warn!( tenant = %tenant.name(), @@ -409,49 +620,15 @@ async fn cleanup_stuck_terminating_pods_on_down_nodes( policy = ?policy, "terminating pod is scheduled on a down node" ); - let delete_params = match policy { - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DoNothing => continue, - // Legacy option: normal delete. - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::Delete => { - DeleteParams::default() - } - // Legacy option: explicit force delete. - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::ForceDelete - // Longhorn-compatible options: always force delete. - | crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DeleteStatefulSetPod - | crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DeleteDeploymentPod - | crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DeleteBothStatefulSetAndDeploymentPod => { - DeleteParams { - grace_period_seconds: Some(0), - propagation_policy: Some(PropagationPolicy::Background), - ..DeleteParams::default() - } - } - }; + let delete_params = deletion_plan.delete_params(); match pods_api.delete(&pod_name, &delete_params).await { Ok(_) => { - let reason = match policy { - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::ForceDelete => { - "ForceDeletedPodOnDownNode" - } - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::Delete => { - "DeletedPodOnDownNode" - } - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DeleteStatefulSetPod - | crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DeleteDeploymentPod - | crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DeleteBothStatefulSetAndDeploymentPod => { - "LonghornLikeForceDeletedPodOnDownNode" - } - crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown::DoNothing => { - "" - } - }; let _ = ctx .record( tenant, EventType::Warning, - reason, + deletion_plan.event_reason, &format!( "Pod '{}' is terminating on down node '{}'; applied policy {:?}", pod_name, node_name, policy @@ -462,6 +639,15 @@ async fn cleanup_stuck_terminating_pods_on_down_nodes( Err(kube::Error::Api(ae)) if ae.code == 404 => { // Pod already gone. } + Err(kube::Error::Api(ae)) if ae.code == 409 => { + // UID precondition failed; a new object may already exist with the same pod name. + warn!( + tenant = %tenant.name(), + namespace = %namespace, + pod = %pod_name, + "skipping pod deletion because the pod changed before delete" + ); + } Err(source) => { return Err(Error::Context { source: context::Error::Kube { source }, @@ -492,10 +678,152 @@ fn pod_matches_policy_controller_kind( } fn pod_has_owner_kind(pod: &corev1::Pod, kind: &str) -> bool { - pod.metadata - .owner_references + pod_controller_owner_name_and_uid(pod, kind).is_some() +} + +fn pod_controller_owner_name_and_uid(pod: &corev1::Pod, kind: &str) -> Option<(String, String)> { + controller_owner_name_and_uid(pod.metadata.owner_references.as_ref(), kind) +} + +fn controller_owner_name_and_uid( + owner_references: Option<&Vec>, + kind: &str, +) -> Option<(String, String)> { + owner_references.and_then(|refs| { + refs.iter() + .find(|r| r.kind == kind && r.api_version == "apps/v1" && r.controller == Some(true)) + .map(|r| (r.name.clone(), r.uid.clone())) + }) +} + +fn force_delete_requires_fencing( + policy: &crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown, +) -> bool { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + matches!( + policy, + P::ForceDelete + | P::DeleteStatefulSetPod + | P::DeleteDeploymentPod + | P::DeleteBothStatefulSetAndDeploymentPod + ) +} + +fn cleanup_decision_for_pod( + pod: &corev1::Pod, + policy: &crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown, + node_deletion_safety: NodePodDeletionSafety, +) -> PodCleanupDecision { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + if !pod_matches_policy_controller_kind(pod, policy) + || node_deletion_safety == NodePodDeletionSafety::Ready + { + return PodCleanupDecision::Skip; + } + + if force_delete_requires_fencing(policy) + && node_deletion_safety != NodePodDeletionSafety::Fenced + { + return PodCleanupDecision::SkipForceDeleteNeedsFencing; + } + + let precondition_uid = pod.metadata.uid.clone(); + match policy { + P::DoNothing => PodCleanupDecision::Skip, + P::Delete => PodCleanupDecision::Delete(PodDeletionPlan { + force: false, + precondition_uid, + event_reason: "DeletedPodOnDownNode", + }), + P::ForceDelete => PodCleanupDecision::Delete(PodDeletionPlan { + force: true, + precondition_uid, + event_reason: "ForceDeletedPodOnDownNode", + }), + P::DeleteStatefulSetPod + | P::DeleteDeploymentPod + | P::DeleteBothStatefulSetAndDeploymentPod => PodCleanupDecision::Delete(PodDeletionPlan { + force: true, + precondition_uid, + event_reason: "LonghornLikeForceDeletedPodOnDownNode", + }), + } +} + +fn node_pod_deletion_safety(node: &corev1::Node, pod: &corev1::Pod) -> NodePodDeletionSafety { + if !is_node_down(node) { + return NodePodDeletionSafety::Ready; + } + + if node_has_effective_out_of_service_taint(node, pod) { + NodePodDeletionSafety::Fenced + } else { + NodePodDeletionSafety::DownUnfenced + } +} + +fn node_has_effective_out_of_service_taint(node: &corev1::Node, pod: &corev1::Pod) -> bool { + node.spec .as_ref() - .is_some_and(|refs| refs.iter().any(|r| r.kind == kind)) + .and_then(|spec| spec.taints.as_ref()) + .is_some_and(|taints| { + taints.iter().any(|taint| { + taint.key == OUT_OF_SERVICE_TAINT_KEY + && !pod_tolerates_taint(pod, taint) + && (taint.effect == "NoExecute" || taint.effect == "NoSchedule") + }) + }) +} + +fn pod_tolerates_taint(pod: &corev1::Pod, taint: &corev1::Taint) -> bool { + pod.spec + .as_ref() + .and_then(|spec| spec.tolerations.as_ref()) + .is_some_and(|tolerations| { + tolerations + .iter() + .any(|toleration| toleration_matches_taint(toleration, taint)) + }) +} + +fn toleration_matches_taint(toleration: &corev1::Toleration, taint: &corev1::Taint) -> bool { + let effect_matches = toleration + .effect + .as_deref() + .is_none_or(|effect| effect == taint.effect.as_str()); + if !effect_matches { + return false; + } + + let key_value_matches = match toleration.operator.as_deref().unwrap_or("Equal") { + "Exists" => toleration + .key + .as_deref() + .is_none_or(|key| key.is_empty() || key == taint.key), + _ => { + toleration.key.as_deref() == Some(taint.key.as_str()) + && toleration.value.as_deref() == taint.value.as_deref() + } + }; + if !key_value_matches { + return false; + } + + if taint.effect == "NoExecute" + && let Some(seconds) = toleration.toleration_seconds + { + if seconds <= 0 { + return false; + } + + return taint.time_added.as_ref().is_none_or(|time_added| { + chrono::Utc::now() < time_added.0 + chrono::Duration::seconds(seconds) + }); + } + + true } fn is_node_down(node: &corev1::Node) -> bool { @@ -556,6 +884,7 @@ pub fn error_policy(object: Arc, error: &Error, _ctx: Arc) -> A // Use 60-second requeue to reduce event/log spam while user fixes the issue types::error::Error::ImmutableFieldModified { .. } | types::error::Error::InvalidTenantName { .. } + | types::error::Error::KmsMigrationBlocked { .. } | types::error::Error::PoolDeleteBlocked { .. } => Duration::from_secs(60), // Other type errors - use moderate requeue @@ -600,6 +929,7 @@ fn reconcile_error_reason(error: &Error) -> &'static str { types::error::Error::InvalidPoolSpec { .. } => "InvalidPoolSpec", types::error::Error::ImmutableFieldModified { .. } => "ImmutableFieldModified", types::error::Error::PoolDeleteBlocked { .. } => "PoolDeleteBlocked", + types::error::Error::KmsMigrationBlocked { .. } => "KmsMigrationBlocked", types::error::Error::NoNamespace => "NoNamespace", types::error::Error::InternalError { .. } => "InternalError", types::error::Error::SerdeJson { .. } => "SerdeJsonError", @@ -613,13 +943,207 @@ fn reconcile_error_reason(error: &Error) -> &'static str { mod tests { use super::is_node_down; use super::{ - pod_has_owner_kind, pod_matches_policy_controller_kind, should_create_rbac, - should_mark_reconcile_started, + NodePodDeletionSafety, PodCleanupDecision, cleanup_decision_for_pod, + force_delete_requires_fencing, node_pod_deletion_safety, object_owned_by_tenant, + pod_controller_owner_name_and_uid, pod_has_owner_kind, pod_matches_policy_controller_kind, + replicaset_matches_pod_controller_and_tenant, should_create_rbac, + should_mark_reconcile_started, statefulset_matches_pod_controller_and_tenant, }; use crate::types::v1alpha1::status::Status; + use k8s_openapi::api::apps::v1 as appsv1; use k8s_openapi::api::core::v1 as corev1; use k8s_openapi::apimachinery::pkg::apis::meta::v1 as metav1; + fn node_with_ready_status(status: &str) -> corev1::Node { + corev1::Node { + status: Some(corev1::NodeStatus { + conditions: Some(vec![corev1::NodeCondition { + type_: "Ready".to_string(), + status: status.to_string(), + ..Default::default() + }]), + ..Default::default() + }), + ..Default::default() + } + } + + fn node_with_out_of_service_taint() -> corev1::Node { + let mut node = node_with_ready_status("Unknown"); + node.spec = Some(corev1::NodeSpec { + taints: Some(vec![corev1::Taint { + key: super::OUT_OF_SERVICE_TAINT_KEY.to_string(), + value: Some("nodeshutdown".to_string()), + effect: "NoExecute".to_string(), + ..Default::default() + }]), + ..Default::default() + }); + node + } + + fn owner_reference(kind: &str) -> metav1::OwnerReference { + metav1::OwnerReference { + api_version: "apps/v1".to_string(), + kind: kind.to_string(), + name: "owner".to_string(), + uid: "uid".to_string(), + controller: Some(true), + ..Default::default() + } + } + + fn pod_with_owner(kind: &str) -> corev1::Pod { + corev1::Pod { + metadata: metav1::ObjectMeta { + uid: Some("pod-uid".to_string()), + deletion_timestamp: Some(metav1::Time(chrono::Utc::now())), + owner_references: Some(vec![owner_reference(kind)]), + ..Default::default() + }, + ..Default::default() + } + } + + fn statefulset_owned_by_test_tenant(uid: &str, tenant_uid: &str) -> appsv1::StatefulSet { + appsv1::StatefulSet { + metadata: metav1::ObjectMeta { + name: Some("owner".to_string()), + uid: Some(uid.to_string()), + owner_references: Some(vec![tenant_owner_reference(tenant_uid)]), + ..Default::default() + }, + ..Default::default() + } + } + + fn tenant_owner_reference(uid: &str) -> metav1::OwnerReference { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.metadata.uid = Some(uid.to_string()); + tenant.new_owner_ref() + } + + fn deployment_owner_reference(uid: &str) -> metav1::OwnerReference { + metav1::OwnerReference { + api_version: "apps/v1".to_string(), + kind: "Deployment".to_string(), + name: "owner-deployment".to_string(), + uid: uid.to_string(), + controller: Some(true), + ..Default::default() + } + } + + fn replicaset_owned_by_test_tenant(uid: &str, tenant_uid: &str) -> appsv1::ReplicaSet { + appsv1::ReplicaSet { + metadata: metav1::ObjectMeta { + name: Some("owner".to_string()), + uid: Some(uid.to_string()), + owner_references: Some(vec![tenant_owner_reference(tenant_uid)]), + ..Default::default() + }, + ..Default::default() + } + } + + fn replicaset_owned_by_deployment(uid: &str, deployment_uid: &str) -> appsv1::ReplicaSet { + appsv1::ReplicaSet { + metadata: metav1::ObjectMeta { + name: Some("owner".to_string()), + uid: Some(uid.to_string()), + owner_references: Some(vec![deployment_owner_reference(deployment_uid)]), + ..Default::default() + }, + ..Default::default() + } + } + + fn deployment_owned_by_test_tenant(uid: &str, tenant_uid: &str) -> appsv1::Deployment { + appsv1::Deployment { + metadata: metav1::ObjectMeta { + name: Some("owner-deployment".to_string()), + uid: Some(uid.to_string()), + owner_references: Some(vec![tenant_owner_reference(tenant_uid)]), + ..Default::default() + }, + ..Default::default() + } + } + + fn object_meta_with_owner_reference(owner: metav1::OwnerReference) -> metav1::ObjectMeta { + metav1::ObjectMeta { + owner_references: Some(vec![owner]), + ..Default::default() + } + } + + #[test] + fn test_object_owner_match_requires_tenant_api_version() { + let tenant = crate::tests::create_test_tenant(None, None); + let mut owner_ref = tenant_owner_reference("test-uid-123"); + + assert!(object_owned_by_tenant( + &object_meta_with_owner_reference(owner_ref.clone()), + &tenant + )); + + owner_ref.api_version = "rustfs.com/v1beta1".to_string(); + assert!(!object_owned_by_tenant( + &object_meta_with_owner_reference(owner_ref), + &tenant + )); + } + + #[test] + fn test_object_owner_match_requires_controller_tenant_ref() { + let tenant = crate::tests::create_test_tenant(None, None); + let mut non_controller_owner = tenant_owner_reference("test-uid-123"); + non_controller_owner.controller = Some(false); + let mut missing_controller_owner = tenant_owner_reference("test-uid-123"); + missing_controller_owner.controller = None; + + assert!(!object_owned_by_tenant( + &object_meta_with_owner_reference(non_controller_owner), + &tenant + )); + assert!(!object_owned_by_tenant( + &object_meta_with_owner_reference(missing_controller_owner), + &tenant + )); + } + + #[test] + fn test_object_owner_match_requires_tenant_uid() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.metadata.uid = None; + + assert!(!object_owned_by_tenant( + &object_meta_with_owner_reference(tenant_owner_reference("")), + &tenant + )); + } + + #[test] + fn test_legacy_delete_policy_does_not_make_bare_labeled_pod_owned() { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + let tenant = crate::tests::create_test_tenant(None, None); + let pod = corev1::Pod { + metadata: metav1::ObjectMeta { + name: Some("bare-labeled-pod".to_string()), + labels: Some(std::collections::BTreeMap::from([( + "rustfs.tenant".to_string(), + "test-tenant".to_string(), + )])), + ..Default::default() + }, + ..Default::default() + }; + + assert!(pod_matches_policy_controller_kind(&pod, &P::ForceDelete)); + assert!(!object_owned_by_tenant(&pod.metadata, &tenant)); + } + #[test] fn should_not_mark_reconcile_started_when_generation_is_current() { let mut tenant = crate::tests::create_test_tenant(None, None); @@ -698,54 +1222,126 @@ mod tests { #[test] fn test_is_node_down_ready_true() { - let node = corev1::Node { - status: Some(corev1::NodeStatus { - conditions: Some(vec![corev1::NodeCondition { - type_: "Ready".to_string(), - status: "True".to_string(), - ..Default::default() - }]), - ..Default::default() - }), - ..Default::default() - }; + let node = node_with_ready_status("True"); assert!(!is_node_down(&node)); } #[test] fn test_is_node_down_ready_false() { - let node = corev1::Node { - status: Some(corev1::NodeStatus { - conditions: Some(vec![corev1::NodeCondition { - type_: "Ready".to_string(), - status: "False".to_string(), - ..Default::default() - }]), - ..Default::default() - }), - ..Default::default() - }; + let node = node_with_ready_status("False"); assert!(is_node_down(&node)); } #[test] fn test_is_node_down_ready_unknown() { - let node = corev1::Node { - status: Some(corev1::NodeStatus { - conditions: Some(vec![corev1::NodeCondition { - type_: "Ready".to_string(), - status: "Unknown".to_string(), - ..Default::default() - }]), + let node = node_with_ready_status("Unknown"); + assert!(is_node_down(&node)); + } + + #[test] + fn test_node_deletion_safety_ready_unknown_without_taint_is_unfenced() { + let node = node_with_ready_status("Unknown"); + let pod = pod_with_owner("StatefulSet"); + + assert_eq!( + node_pod_deletion_safety(&node, &pod), + NodePodDeletionSafety::DownUnfenced + ); + } + + #[test] + fn test_node_deletion_safety_out_of_service_taint_is_fenced() { + let node = node_with_out_of_service_taint(); + let pod = pod_with_owner("StatefulSet"); + + assert_eq!( + node_pod_deletion_safety(&node, &pod), + NodePodDeletionSafety::Fenced + ); + } + + #[test] + fn test_node_deletion_safety_ready_true_with_out_of_service_taint_is_ready() { + let mut node = node_with_out_of_service_taint(); + node.status = Some(corev1::NodeStatus { + conditions: Some(vec![corev1::NodeCondition { + type_: "Ready".to_string(), + status: "True".to_string(), ..Default::default() - }), + }]), ..Default::default() - }; - assert!(is_node_down(&node)); + }); + let pod = pod_with_owner("StatefulSet"); + + assert_eq!( + node_pod_deletion_safety(&node, &pod), + NodePodDeletionSafety::Ready + ); + } + + #[test] + fn test_node_deletion_safety_tolerated_out_of_service_taint_is_unfenced() { + let node = node_with_out_of_service_taint(); + let mut pod = pod_with_owner("StatefulSet"); + pod.spec = Some(corev1::PodSpec { + tolerations: Some(vec![corev1::Toleration { + key: Some(super::OUT_OF_SERVICE_TAINT_KEY.to_string()), + operator: Some("Exists".to_string()), + effect: Some("NoExecute".to_string()), + ..Default::default() + }]), + ..Default::default() + }); + + assert_eq!( + node_pod_deletion_safety(&node, &pod), + NodePodDeletionSafety::DownUnfenced + ); + } + + #[test] + fn test_node_deletion_safety_expired_toleration_no_longer_blocks_fencing() { + let mut node = node_with_out_of_service_taint(); + node.spec + .as_mut() + .and_then(|spec| spec.taints.as_mut()) + .and_then(|taints| taints.first_mut()) + .expect("out-of-service taint should exist") + .time_added = Some(metav1::Time( + chrono::Utc::now() - chrono::Duration::seconds(60), + )); + let mut pod = pod_with_owner("StatefulSet"); + pod.spec = Some(corev1::PodSpec { + tolerations: Some(vec![corev1::Toleration { + key: Some(super::OUT_OF_SERVICE_TAINT_KEY.to_string()), + operator: Some("Exists".to_string()), + effect: Some("NoExecute".to_string()), + toleration_seconds: Some(1), + ..Default::default() + }]), + ..Default::default() + }); + + assert_eq!( + node_pod_deletion_safety(&node, &pod), + NodePodDeletionSafety::Fenced + ); } #[test] fn test_pod_owner_kind_helpers() { + let pod = pod_with_owner("StatefulSet"); + + assert!(pod_has_owner_kind(&pod, "StatefulSet")); + assert!(!pod_has_owner_kind(&pod, "ReplicaSet")); + assert_eq!( + pod_controller_owner_name_and_uid(&pod, "StatefulSet"), + Some(("owner".to_string(), "uid".to_string())) + ); + } + + #[test] + fn test_pod_owner_kind_requires_controller_owner() { let pod = corev1::Pod { metadata: metav1::ObjectMeta { owner_references: Some(vec![metav1::OwnerReference { @@ -753,6 +1349,7 @@ mod tests { kind: "StatefulSet".to_string(), name: "ss".to_string(), uid: "uid".to_string(), + controller: Some(false), ..Default::default() }]), ..Default::default() @@ -760,43 +1357,203 @@ mod tests { ..Default::default() }; - assert!(pod_has_owner_kind(&pod, "StatefulSet")); - assert!(!pod_has_owner_kind(&pod, "ReplicaSet")); + assert!(!pod_has_owner_kind(&pod, "StatefulSet")); } #[test] - fn test_policy_controller_kind_matching_longhorn_like() { + fn test_statefulset_owner_match_requires_tenant_and_statefulset_uid() { + let tenant = crate::tests::create_test_tenant(None, None); + let pod = pod_with_owner("StatefulSet"); + let matching_statefulset = statefulset_owned_by_test_tenant("uid", "test-uid-123"); + let wrong_statefulset_uid = statefulset_owned_by_test_tenant("other-uid", "test-uid-123"); + let wrong_tenant_uid = statefulset_owned_by_test_tenant("uid", "other-tenant-uid"); + + assert!(statefulset_matches_pod_controller_and_tenant( + &pod, + &matching_statefulset, + &tenant + )); + assert!(!statefulset_matches_pod_controller_and_tenant( + &pod, + &wrong_statefulset_uid, + &tenant + )); + assert!(!statefulset_matches_pod_controller_and_tenant( + &pod, + &wrong_tenant_uid, + &tenant + )); + } + + #[test] + fn test_replicaset_owner_match_requires_tenant_owner_chain_and_uid() { + let tenant = crate::tests::create_test_tenant(None, None); + let pod = pod_with_owner("ReplicaSet"); + let matching_replicaset = replicaset_owned_by_test_tenant("uid", "test-uid-123"); + let wrong_replicaset_uid = replicaset_owned_by_test_tenant("other-uid", "test-uid-123"); + let deployment_owned_replicaset = replicaset_owned_by_deployment("uid", "deployment-uid"); + let matching_deployment = deployment_owned_by_test_tenant("deployment-uid", "test-uid-123"); + let wrong_deployment_tenant = + deployment_owned_by_test_tenant("deployment-uid", "other-tenant-uid"); + + assert!(replicaset_matches_pod_controller_and_tenant( + &pod, + &matching_replicaset, + None, + &tenant + )); + assert!(!replicaset_matches_pod_controller_and_tenant( + &pod, + &wrong_replicaset_uid, + None, + &tenant + )); + assert!(replicaset_matches_pod_controller_and_tenant( + &pod, + &deployment_owned_replicaset, + Some(&matching_deployment), + &tenant + )); + assert!(!replicaset_matches_pod_controller_and_tenant( + &pod, + &deployment_owned_replicaset, + Some(&wrong_deployment_tenant), + &tenant + )); + assert!(!replicaset_matches_pod_controller_and_tenant( + &pod, + &deployment_owned_replicaset, + None, + &tenant + )); + } + + #[test] + fn test_force_delete_policies_require_fencing() { use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; - let ss_pod = corev1::Pod { - metadata: metav1::ObjectMeta { - deletion_timestamp: Some(metav1::Time(chrono::Utc::now())), - owner_references: Some(vec![metav1::OwnerReference { - api_version: "apps/v1".to_string(), - kind: "StatefulSet".to_string(), - name: "ss".to_string(), - uid: "uid".to_string(), - ..Default::default() - }]), - ..Default::default() - }, - ..Default::default() + assert!(force_delete_requires_fencing(&P::ForceDelete)); + assert!(force_delete_requires_fencing(&P::DeleteStatefulSetPod)); + assert!(force_delete_requires_fencing(&P::DeleteDeploymentPod)); + assert!(force_delete_requires_fencing( + &P::DeleteBothStatefulSetAndDeploymentPod + )); + assert!(!force_delete_requires_fencing(&P::Delete)); + assert!(!force_delete_requires_fencing(&P::DoNothing)); + } + + #[test] + fn test_cleanup_decision_force_delete_uses_uid_precondition() { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + let pod = pod_with_owner("StatefulSet"); + let decision = + cleanup_decision_for_pod(&pod, &P::ForceDelete, NodePodDeletionSafety::Fenced); + + let PodCleanupDecision::Delete(plan) = decision else { + panic!("expected force-delete plan"); }; + assert!(plan.force); + assert_eq!(plan.precondition_uid.as_deref(), Some("pod-uid")); + assert_eq!(plan.event_reason, "ForceDeletedPodOnDownNode"); + + let delete_params = plan.delete_params(); + assert_eq!(delete_params.grace_period_seconds, Some(0)); + assert_eq!( + delete_params + .preconditions + .as_ref() + .and_then(|preconditions| preconditions.uid.as_deref()), + Some("pod-uid") + ); + } - let deploy_pod = corev1::Pod { - metadata: metav1::ObjectMeta { - deletion_timestamp: Some(metav1::Time(chrono::Utc::now())), - owner_references: Some(vec![metav1::OwnerReference { - api_version: "apps/v1".to_string(), - kind: "ReplicaSet".to_string(), - name: "rs".to_string(), - uid: "uid".to_string(), - ..Default::default() - }]), - ..Default::default() - }, - ..Default::default() + #[test] + fn test_cleanup_decision_normal_delete_keeps_uid_precondition_without_force() { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + let pod = pod_with_owner("StatefulSet"); + let decision = + cleanup_decision_for_pod(&pod, &P::Delete, NodePodDeletionSafety::DownUnfenced); + + let PodCleanupDecision::Delete(plan) = decision else { + panic!("expected normal delete plan"); }; + assert!(!plan.force); + assert_eq!(plan.precondition_uid.as_deref(), Some("pod-uid")); + assert_eq!(plan.event_reason, "DeletedPodOnDownNode"); + + let delete_params = plan.delete_params(); + assert_eq!(delete_params.grace_period_seconds, None); + assert_eq!( + delete_params + .preconditions + .as_ref() + .and_then(|preconditions| preconditions.uid.as_deref()), + Some("pod-uid") + ); + } + + #[test] + fn test_cleanup_decision_skips_unfenced_force_delete() { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + let statefulset_pod = pod_with_owner("StatefulSet"); + let deployment_pod = pod_with_owner("ReplicaSet"); + + assert_eq!( + cleanup_decision_for_pod( + &statefulset_pod, + &P::ForceDelete, + NodePodDeletionSafety::DownUnfenced + ), + PodCleanupDecision::SkipForceDeleteNeedsFencing + ); + assert_eq!( + cleanup_decision_for_pod( + &statefulset_pod, + &P::DeleteStatefulSetPod, + NodePodDeletionSafety::DownUnfenced + ), + PodCleanupDecision::SkipForceDeleteNeedsFencing + ); + assert_eq!( + cleanup_decision_for_pod( + &deployment_pod, + &P::DeleteDeploymentPod, + NodePodDeletionSafety::DownUnfenced + ), + PodCleanupDecision::SkipForceDeleteNeedsFencing + ); + } + + #[test] + fn test_cleanup_decision_skips_ready_node_and_controller_mismatch() { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + let ss_pod = pod_with_owner("StatefulSet"); + let deploy_pod = pod_with_owner("ReplicaSet"); + + assert_eq!( + cleanup_decision_for_pod(&ss_pod, &P::Delete, NodePodDeletionSafety::Ready), + PodCleanupDecision::Skip + ); + assert_eq!( + cleanup_decision_for_pod( + &deploy_pod, + &P::DeleteStatefulSetPod, + NodePodDeletionSafety::Fenced + ), + PodCleanupDecision::Skip + ); + } + + #[test] + fn test_policy_controller_kind_matching_longhorn_like() { + use crate::types::v1alpha1::k8s::PodDeletionPolicyWhenNodeIsDown as P; + + let ss_pod = pod_with_owner("StatefulSet"); + let deploy_pod = pod_with_owner("ReplicaSet"); assert!(pod_matches_policy_controller_kind( &ss_pod, diff --git a/src/reconcile/phases.rs b/src/reconcile/phases.rs index faef084..7c475dd 100644 --- a/src/reconcile/phases.rs +++ b/src/reconcile/phases.rs @@ -93,13 +93,9 @@ pub(super) async fn validate_tenant_prerequisites( return Err(e.into()); } - // Validate encryption / KMS: Vault requires endpoint + kmsSecret (and correct keys); - // must run whenever encryption is enabled — not only when kmsSecret is set, or Vault - // without a Secret reference would skip validation entirely. - if let Some(ref enc) = tenant.spec.encryption - && enc.enabled - && let Err(e) = ctx.validate_kms_secret(tenant).await - { + // Validate encryption / KMS and reject raw RUSTFS_KMS_* env overrides even when + // spec.encryption is omitted or disabled. + if let Err(e) = ctx.validate_kms_secret(tenant).await { let status_error = StatusError::from_context_error(&e); patch_status_error(ctx, tenant, &status_error).await; return Err(e.into()); @@ -116,7 +112,7 @@ pub(super) async fn validate_tenant_prerequisites( tenant, EventType::Warning, "KmsConfigWarning", - "Local KMS backend does not use kmsSecret; the Secret reference will be ignored", + "Local KMS backend ignores kmsSecret; use spec.encryption.local.masterKeySecretRef for the local master key", ) .await; } @@ -625,8 +621,20 @@ async fn reconcile_existing_pool_statefulset( "StatefulSet update validation failed" ); - let status_error = StatusError::statefulset_update_validation_failed(&ss_name); + let status_error = if matches!(e, types::error::Error::KmsMigrationBlocked { .. }) { + StatusError::from_types_error(&e) + } else { + StatusError::statefulset_update_validation_failed(&ss_name) + }; patch_status_error(ctx, tenant, &status_error).await; + let _ = ctx + .record( + tenant, + EventType::Warning, + "StatefulSetUpdateBlocked", + &format!("StatefulSet '{}' update blocked: {}", ss_name, e), + ) + .await; return Err(e.into()); } diff --git a/src/status.rs b/src/status.rs index c444b8c..52860f5 100644 --- a/src/status.rs +++ b/src/status.rs @@ -145,6 +145,11 @@ impl StatusError { ConditionType::SpecValid, sanitize_message(message), ), + types::error::Error::KmsMigrationBlocked { message, .. } => Self::blocked( + Reason::KmsConfigInvalid, + ConditionType::KmsReady, + sanitize_message(message), + ), types::error::Error::NoNamespace => Self::transient( Reason::KubernetesApiError, ConditionType::Ready, diff --git a/src/types/error.rs b/src/types/error.rs index 8b07a6b..53883d7 100755 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -39,6 +39,9 @@ pub enum Error { #[snafu(display("invalid pool specification for tenant '{}': {}", name, message))] InvalidPoolSpec { name: String, message: String }, + #[snafu(display("KMS migration is blocked for tenant '{}': {}", name, message))] + KmsMigrationBlocked { name: String, message: String }, + #[snafu(display("serde_json error: {}", source))] SerdeJson { source: serde_json::Error }, } diff --git a/src/types/v1alpha1/encryption.rs b/src/types/v1alpha1/encryption.rs index bd2cbfa..3f7885d 100644 --- a/src/types/v1alpha1/encryption.rs +++ b/src/types/v1alpha1/encryption.rs @@ -49,24 +49,58 @@ pub struct VaultKmsConfig { pub endpoint: String, } +/// Secret key selector for the Local KMS master key. +#[derive(Deserialize, Serialize, Clone, Debug, KubeSchema, Default)] +#[serde(rename_all = "camelCase")] +pub struct LocalKmsMasterKeySecretRef { + /// Secret name in the Tenant namespace. + #[schemars(length(min = 1))] + pub name: String, + + /// Secret data key containing the local master key string. + #[schemars(length(min = 1))] + pub key: String, +} + /// Local file-based KMS: key material directory inside the container. #[derive(Deserialize, Serialize, Clone, Debug, KubeSchema, Default)] #[serde(rename_all = "camelCase")] pub struct LocalKmsConfig { - /// Absolute directory for KMS key files (default: `/data/kms-keys`). + /// Absolute directory for KMS key files. + /// + /// Must be in a subdirectory under a RustFS data PVC mount. Defaults to the first data PVC mount + /// under `persistence.path`, for example `/data/rustfs0/.kms-keys`. #[serde(default, skip_serializing_if = "Option::is_none")] pub key_directory: Option, + + /// Secret key selector for `RUSTFS_KMS_LOCAL_MASTER_KEY`. + /// + /// Required for Local KMS unless `allowInsecureDevDefaults` is explicitly set to `true`. + /// The referenced Secret key should contain the local master key string used to encrypt + /// Local KMS key files at rest. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub master_key_secret_ref: Option, + + /// Explicitly allow RustFS development-only insecure KMS defaults. + /// + /// When true, the operator sets `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true`, allowing + /// Local KMS to start without a master key and store key material as plaintext-dev-only. + /// Do not use this in production. + #[serde(default)] + pub allow_insecure_dev_defaults: bool, } /// Encryption / KMS configuration for a Tenant. /// /// Injected env vars match the RustFS server (`rustfs/src/config/cli.rs`, `init_kms_system`): -/// `RUSTFS_KMS_ENABLE`, `RUSTFS_KMS_BACKEND`, `RUSTFS_KMS_KEY_DIR`, `RUSTFS_KMS_LOCAL_KEY_DIR`, +/// `RUSTFS_KMS_ENABLE`, `RUSTFS_KMS_BACKEND`, `RUSTFS_KMS_KEY_DIR`, +/// `RUSTFS_KMS_LOCAL_MASTER_KEY`, `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS`, /// `RUSTFS_KMS_DEFAULT_KEY_ID`, `RUSTFS_KMS_VAULT_ADDRESS`, `RUSTFS_KMS_VAULT_TOKEN`. /// /// **Vault Secret:** key `vault-token` (required). /// -/// **Local:** no Secret; use a single-server tenant (operator validates replica count). +/// **Local:** use `local.masterKeySecretRef` unless explicitly enabling development-only +/// insecure defaults; use a single-server tenant (operator validates replica count). #[derive(Deserialize, Serialize, Clone, Debug, KubeSchema, Default)] #[serde(rename_all = "camelCase")] pub struct EncryptionConfig { diff --git a/src/types/v1alpha1/k8s.rs b/src/types/v1alpha1/k8s.rs index 00230bb..5411beb 100755 --- a/src/types/v1alpha1/k8s.rs +++ b/src/types/v1alpha1/k8s.rs @@ -65,7 +65,9 @@ pub enum ImagePullPolicy { /// when the node becomes unreachable. /// /// WARNING: Force-deleting pods can have data consistency implications depending on -/// your storage backend and workload semantics. +/// your storage backend and workload semantics. Force deletion requires the Node object +/// to be deleted or marked with an effective `node.kubernetes.io/out-of-service` taint +/// that the target Pod does not tolerate. #[derive(Default, Deserialize, Serialize, Clone, Debug, Display, PartialEq, Eq)] #[serde(rename_all = "PascalCase")] pub enum PodDeletionPolicyWhenNodeIsDown { @@ -121,7 +123,7 @@ impl JsonSchema for PodDeletionPolicyWhenNodeIsDown { "DeleteDeploymentPod", "DeleteBothStatefulSetAndDeploymentPod" ], - "description": "Pod deletion policy when the node hosting the Pod is down (NotReady/Unknown). Values: DoNothing | Delete | ForceDelete | DeleteStatefulSetPod | DeleteDeploymentPod | DeleteBothStatefulSetAndDeploymentPod" + "description": "Pod deletion policy when the node hosting the Pod is down (NotReady/Unknown). Force deletion requires the Node object to be deleted or marked with an effective node.kubernetes.io/out-of-service taint that the target Pod does not tolerate. Values: DoNothing | Delete | ForceDelete | DeleteStatefulSetPod | DeleteDeploymentPod | DeleteBothStatefulSetAndDeploymentPod" } } } diff --git a/src/types/v1alpha1/persistence.rs b/src/types/v1alpha1/persistence.rs index 5cbada9..e590a97 100755 --- a/src/types/v1alpha1/persistence.rs +++ b/src/types/v1alpha1/persistence.rs @@ -16,6 +16,23 @@ use k8s_openapi::api::core::v1 as corev1; use kube::KubeSchema; use serde::{Deserialize, Serialize}; +pub(crate) const DEFAULT_PERSISTENCE_PATH: &str = "/data"; +pub(crate) const LEGACY_LOCAL_KMS_KEY_DIR: &str = "/data/kms-keys"; +pub(crate) const LOCAL_KMS_KEY_DIR_NAME: &str = ".kms-keys"; + +pub(crate) fn data_volume_mount_path(base_path: Option<&str>, shard: i32) -> String { + let base_path = base_path.unwrap_or(DEFAULT_PERSISTENCE_PATH); + format!("{}/rustfs{}", base_path.trim_end_matches('/'), shard) +} + +pub(crate) fn default_local_kms_key_directory(base_path: Option<&str>) -> String { + format!( + "{}/{}", + data_volume_mount_path(base_path, 0), + LOCAL_KMS_KEY_DIR_NAME + ) +} + #[derive(Deserialize, Serialize, Clone, Debug, KubeSchema)] #[serde(rename_all = "camelCase")] pub struct PersistenceConfig { @@ -48,3 +65,34 @@ impl Default for PersistenceConfig { } } } + +#[cfg(test)] +mod tests { + use super::{data_volume_mount_path, default_local_kms_key_directory}; + + #[test] + fn data_volume_mount_path_uses_default_base_path() { + assert_eq!(data_volume_mount_path(None, 0), "/data/rustfs0"); + assert_eq!(data_volume_mount_path(None, 3), "/data/rustfs3"); + } + + #[test] + fn data_volume_mount_path_trims_trailing_slash() { + assert_eq!( + data_volume_mount_path(Some("/mnt/rustfs/"), 1), + "/mnt/rustfs/rustfs1" + ); + } + + #[test] + fn default_local_kms_key_directory_uses_first_data_volume() { + assert_eq!( + default_local_kms_key_directory(None), + "/data/rustfs0/.kms-keys" + ); + assert_eq!( + default_local_kms_key_directory(Some("/mnt/rustfs")), + "/mnt/rustfs/rustfs0/.kms-keys" + ); + } +} diff --git a/src/types/v1alpha1/status.rs b/src/types/v1alpha1/status.rs index 1abb72d..a51b290 100755 --- a/src/types/v1alpha1/status.rs +++ b/src/types/v1alpha1/status.rs @@ -245,7 +245,8 @@ pub struct ConditionInput { #[derive(Deserialize, Serialize, Clone, Debug, JsonSchema, PartialEq)] #[serde(rename_all = "camelCase")] pub struct Condition { - /// Type of condition (Ready, Reconciling, Degraded) + /// Type of condition (Ready, Reconciling, Degraded, SpecValid, CredentialsReady, KmsReady, + /// TlsReady, PoolsReady, WorkloadsReady, ProvisioningReady) #[serde(rename = "type")] pub type_: String, diff --git a/src/types/v1alpha1/tenant.rs b/src/types/v1alpha1/tenant.rs index 86fbc50..5c79562 100755 --- a/src/types/v1alpha1/tenant.rs +++ b/src/types/v1alpha1/tenant.rs @@ -91,11 +91,16 @@ pub struct TenantSpec { /// Typical use-case: a StatefulSet Pod gets stuck in Terminating when the node goes down. /// Setting this to `ForceDelete` allows the operator to force delete the Pod object so the /// StatefulSet controller can recreate it elsewhere. + /// Force deletion requires the Node object to be deleted or marked with an effective + /// `node.kubernetes.io/out-of-service` taint that the target Pod does not tolerate. /// - /// Values: DoNothing | Delete | ForceDelete + /// Values: DoNothing | Delete | ForceDelete | DeleteStatefulSetPod | DeleteDeploymentPod | DeleteBothStatefulSetAndDeploymentPod #[serde(default, skip_serializing_if = "Option::is_none")] pub pod_deletion_policy_when_node_is_down: Option, + /// Additional RustFS container environment variables. + /// + /// `RUSTFS_KMS_*` is reserved; configure KMS through `spec.encryption`. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub env: Vec, @@ -190,6 +195,7 @@ pub struct TenantSpec { /// Encryption / KMS configuration for server-side encryption. /// When enabled, the operator injects KMS environment variables and mounts /// secrets into RustFS pods so the in-process `rustfs-kms` library is configured. + #[x_kube(validation = Rule::new("!self.enabled || self.backend != 'local' || (has(self.local) && (has(self.local.masterKeySecretRef) || (has(self.local.allowInsecureDevDefaults) && self.local.allowInsecureDevDefaults == true)))").message("Local KMS requires local.masterKeySecretRef unless local.allowInsecureDevDefaults is true"))] #[serde(default, skip_serializing_if = "Option::is_none")] pub encryption: Option, diff --git a/src/types/v1alpha1/tenant/workloads.rs b/src/types/v1alpha1/tenant/workloads.rs index 89f9a85..0ef70e1 100755 --- a/src/types/v1alpha1/tenant/workloads.rs +++ b/src/types/v1alpha1/tenant/workloads.rs @@ -15,12 +15,20 @@ use super::Tenant; use crate::types; use crate::types::v1alpha1::encryption::KmsBackendType; +use crate::types::v1alpha1::persistence::{ + DEFAULT_PERSISTENCE_PATH, LEGACY_LOCAL_KMS_KEY_DIR, data_volume_mount_path, + default_local_kms_key_directory, +}; use crate::types::v1alpha1::pool::Pool; use crate::types::v1alpha1::tls::{TlsPlan, http_probe}; use k8s_openapi::api::apps::v1; use k8s_openapi::api::core::v1 as corev1; use k8s_openapi::apimachinery::pkg::apis::meta::v1 as metav1; +const LOCAL_KMS_KEY_DIR_ENV: &str = "RUSTFS_KMS_KEY_DIR"; +const LOCAL_KMS_LOCAL_KEY_DIR_ENV: &str = "RUSTFS_KMS_LOCAL_KEY_DIR"; +const LOCAL_KMS_MASTER_KEY_ENV: &str = "RUSTFS_KMS_LOCAL_MASTER_KEY"; +const KMS_ALLOW_INSECURE_DEV_DEFAULTS_ENV: &str = "RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS"; const VOLUME_CLAIM_TEMPLATE_PREFIX: &str = "vol"; const DEFAULT_RUN_AS_USER: i64 = 10001; const DEFAULT_RUN_AS_GROUP: i64 = 10001; @@ -38,10 +46,37 @@ fn is_tls_operator_managed_env_var(name: &str) -> bool { TLS_OPERATOR_MANAGED_ENV_VARS.contains(&name) } +fn is_kms_operator_managed_env_var(name: &str) -> bool { + name.starts_with("RUSTFS_KMS_") +} + fn volume_claim_template_name(shard: i32) -> String { format!("{VOLUME_CLAIM_TEMPLATE_PREFIX}-{shard}") } +fn container_env_values<'a>(container: &'a corev1::Container, name: &str) -> Vec<&'a str> { + container + .env + .as_ref() + .map(|env| { + env.iter() + .filter(|var| var.name == name) + .filter_map(|var| var.value.as_deref()) + .collect() + }) + .unwrap_or_default() +} + +fn local_kms_key_dir_env_values(container: &corev1::Container) -> Vec<&str> { + let mut values = container_env_values(container, LOCAL_KMS_KEY_DIR_ENV); + values.extend(container_env_values(container, LOCAL_KMS_LOCAL_KEY_DIR_ENV)); + values +} + +fn first_container(spec: &v1::StatefulSetSpec) -> Option<&corev1::Container> { + spec.template.spec.as_ref()?.containers.first() +} + fn stateful_name(tenant: &Tenant, pool: &Pool) -> String { format!("{}-{}", tenant.name(), pool.name) } @@ -55,7 +90,11 @@ impl Tenant { ) -> String { let tenant_name = self.name(); let headless_service = self.headless_service_name(); - let base_path = pool.persistence.path.as_deref().unwrap_or("/data"); + let base_path = pool + .persistence + .path + .as_deref() + .unwrap_or(DEFAULT_PERSISTENCE_PATH); let base_path = base_path.trim_end_matches('/'); if self.spec.pools.len() == 1 && pool.is_single_node_single_disk() { @@ -243,9 +282,10 @@ impl Tenant { /// `build_vault_kms_config`) and CLI env (`rustfs/src/config/cli.rs`): only the variables /// parsed into `Config` are set here. /// - /// Returns `(env_vars, pod_volumes, volume_mounts)` — volumes are unused (no TLS cert mounts). + /// Returns `(env_vars, pod_volumes, volume_mounts)` — Local KMS uses the data PVC. fn configure_kms( &self, + pool: &Pool, ) -> ( Vec, Vec, @@ -315,19 +355,48 @@ impl Tenant { .local .as_ref() .and_then(|l| l.key_directory.as_deref()) - .unwrap_or("/data/kms-keys"); + .map(ToOwned::to_owned) + .unwrap_or_else(|| { + default_local_kms_key_directory(pool.persistence.path.as_deref()) + }); env.push(corev1::EnvVar { - name: "RUSTFS_KMS_KEY_DIR".to_owned(), - value: Some(key_dir.to_string()), - ..Default::default() - }); - env.push(corev1::EnvVar { - name: "RUSTFS_KMS_LOCAL_KEY_DIR".to_owned(), - value: Some(key_dir.to_string()), + name: LOCAL_KMS_KEY_DIR_ENV.to_owned(), + value: Some(key_dir), ..Default::default() }); + if let Some(selector) = enc + .local + .as_ref() + .and_then(|l| l.master_key_secret_ref.as_ref()) + { + env.push(corev1::EnvVar { + name: LOCAL_KMS_MASTER_KEY_ENV.to_owned(), + value_from: Some(corev1::EnvVarSource { + secret_key_ref: Some(corev1::SecretKeySelector { + name: selector.name.clone(), + key: selector.key.clone(), + optional: Some(false), + }), + ..Default::default() + }), + ..Default::default() + }); + } + + if enc + .local + .as_ref() + .is_some_and(|l| l.allow_insecure_dev_defaults) + { + env.push(corev1::EnvVar { + name: KMS_ALLOW_INSECURE_DEV_DEFAULTS_ENV.to_owned(), + value: Some("true".to_owned()), + ..Default::default() + }); + } + if let Some(ref id) = enc.default_key_id && !id.is_empty() { @@ -362,11 +431,10 @@ impl Tenant { // Default path is /data if not specified // Volume mount names must match the volume claim template names (vol-0, vol-1, etc.) // Mount paths follow RustFS convention: /data/rustfs0, /data/rustfs1, etc. - let base_path = pool.persistence.path.as_deref().unwrap_or("/data"); let mut volume_mounts: Vec = (0..pool.persistence.volumes_per_server) .map(|i| corev1::VolumeMount { name: volume_claim_template_name(i), - mount_path: format!("{}/rustfs{}", base_path.trim_end_matches('/'), i), + mount_path: data_volume_mount_path(pool.persistence.path.as_deref(), i), ..Default::default() }) .collect(); @@ -434,12 +502,15 @@ impl Tenant { } // Merge with user-provided environment variables. - // Preserve the legacy override behavior except for TLS runtime values that - // must stay aligned with the rendered TLS mounts, probes, status, and hash. + // Preserve the legacy override behavior except for operator-managed runtime + // values that must stay aligned with rendered mounts, probes, status, and hash. for user_env in &self.spec.env { if tls_plan.enabled && is_tls_operator_managed_env_var(&user_env.name) { continue; } + if is_kms_operator_managed_env_var(&user_env.name) { + continue; + } // Remove any existing var with the same name to allow non-reserved overrides. env_vars.retain(|e| e.name != user_env.name); env_vars.push(user_env.clone()); @@ -453,7 +524,7 @@ impl Tenant { volume_mounts.append(&mut log_volume_mounts); // Configure KMS / encryption environment variables and volumes - let (kms_env, mut kms_volumes, mut kms_mounts) = self.configure_kms(); + let (kms_env, mut kms_volumes, mut kms_mounts) = self.configure_kms(pool); env_vars.extend(kms_env); pod_volumes.append(&mut kms_volumes); volume_mounts.append(&mut kms_mounts); @@ -789,6 +860,48 @@ impl Tenant { Ok(false) } + fn uses_implicit_local_kms_key_directory(&self) -> bool { + self.spec.encryption.as_ref().is_some_and(|enc| { + enc.enabled + && enc.backend == KmsBackendType::Local + && enc + .local + .as_ref() + .and_then(|local| local.key_directory.as_deref()) + .is_none() + }) + } + + fn blocked_local_kms_implicit_default_migration( + &self, + existing_spec: &v1::StatefulSetSpec, + desired_spec: &v1::StatefulSetSpec, + ) -> Option<(String, String)> { + if !self.uses_implicit_local_kms_key_directory() { + return None; + } + + let existing_container = first_container(existing_spec)?; + let existing_key_dirs = local_kms_key_dir_env_values(existing_container); + if !existing_key_dirs.contains(&LEGACY_LOCAL_KMS_KEY_DIR) { + return None; + } + + let desired_container = first_container(desired_spec)?; + let desired_key_dirs = local_kms_key_dir_env_values(desired_container); + let desired_key_dir = desired_key_dirs + .iter() + .copied() + .find(|dir| *dir != LEGACY_LOCAL_KMS_KEY_DIR) + .or_else(|| desired_key_dirs.first().copied())?; + (desired_key_dir != LEGACY_LOCAL_KMS_KEY_DIR).then(|| { + ( + LEGACY_LOCAL_KMS_KEY_DIR.to_string(), + desired_key_dir.to_string(), + ) + }) + } + /// Validates that a StatefulSet update is safe by checking for changes to /// immutable fields that would cause API rejection. /// @@ -837,6 +950,18 @@ impl Tenant { .unwrap_or(&"".to_string()) .clone(); + if let Some((existing_key_dir, desired_key_dir)) = + self.blocked_local_kms_implicit_default_migration(existing_spec, desired_spec) + { + return Err(types::error::Error::KmsMigrationBlocked { + name: self.name(), + message: format!( + "Local KMS default key directory migration for StatefulSet '{}' is blocked: existing pods use legacy non-PVC path '{}', while the desired implicit default is '{}'. Copy existing key files and .master-key.salt into '{}', then set spec.encryption.local.keyDirectory explicitly to that PVC-backed path before rolling the StatefulSet.", + ss_name, existing_key_dir, desired_key_dir, desired_key_dir + ), + }); + } + // MinIO-compatible expansion model: an existing pool's server count is // immutable. Horizontal capacity expansion must add a new pool. if existing_spec.replicas != desired_spec.replicas { @@ -939,6 +1064,9 @@ impl Tenant { #[allow(clippy::unwrap_used, clippy::expect_used)] mod tests { use super::{DEFAULT_FS_GROUP, DEFAULT_RUN_AS_GROUP, DEFAULT_RUN_AS_USER}; + use crate::types::v1alpha1::encryption::{ + EncryptionConfig, KmsBackendType, LocalKmsConfig, LocalKmsMasterKeySecretRef, + }; use crate::types::v1alpha1::logging::{LoggingConfig, LoggingMode}; use crate::types::v1alpha1::tls::{SecretKeyReference, TlsPlan}; use k8s_openapi::api::core::v1 as corev1; @@ -963,6 +1091,29 @@ mod tests { .as_deref() } + fn set_env_value(container: &mut corev1::Container, name: &str, value: &str) { + let env = container + .env + .as_mut() + .expect("container should have environment variables"); + if let Some(var) = env.iter_mut().find(|var| var.name == name) { + var.value = Some(value.to_string()); + } else { + env.push(corev1::EnvVar { + name: name.to_string(), + value: Some(value.to_string()), + ..Default::default() + }); + } + } + + fn local_master_key_selector() -> LocalKmsMasterKeySecretRef { + LocalKmsMasterKeySecretRef { + name: "local-kms-master-key".to_string(), + key: "local-master-key".to_string(), + } + } + #[test] fn disabled_tls_statefulset_keeps_http_and_has_no_tls_wiring() { let tenant = crate::tests::create_test_tenant(None, None); @@ -1129,6 +1280,374 @@ mod tests { ); } + #[test] + fn local_kms_default_key_directory_uses_data_pvc_subdirectory() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + + let pod_spec = statefulset.spec.unwrap().template.spec.unwrap(); + let container = &pod_spec.containers[0]; + assert_eq!( + env_value(container, "RUSTFS_KMS_KEY_DIR"), + Some("/data/rustfs0/.kms-keys") + ); + assert_eq!(env_value(container, "RUSTFS_KMS_LOCAL_KEY_DIR"), None); + assert!( + container + .volume_mounts + .as_ref() + .expect("data mount should be present") + .iter() + .any(|mount| mount.mount_path == "/data/rustfs0") + ); + } + + #[test] + fn local_kms_default_key_directory_uses_custom_persistence_path() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.pools[0].persistence.path = Some("/mnt/rustfs".to_string()); + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + + let pod_spec = statefulset.spec.unwrap().template.spec.unwrap(); + let container = &pod_spec.containers[0]; + assert_eq!( + env_value(container, "RUSTFS_KMS_KEY_DIR"), + Some("/mnt/rustfs/rustfs0/.kms-keys") + ); + assert!( + container + .volume_mounts + .as_ref() + .expect("data mount should be present") + .iter() + .any(|mount| mount.mount_path == "/mnt/rustfs/rustfs0") + ); + } + + #[test] + fn local_kms_custom_key_directory_is_rendered_unchanged() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + local: Some(LocalKmsConfig { + key_directory: Some("/data/rustfs0/custom-kms".to_string()), + ..Default::default() + }), + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + + let container = &statefulset.spec.unwrap().template.spec.unwrap().containers[0]; + assert_eq!( + env_value(container, "RUSTFS_KMS_KEY_DIR"), + Some("/data/rustfs0/custom-kms") + ); + assert_eq!(env_value(container, "RUSTFS_KMS_LOCAL_KEY_DIR"), None); + } + + #[test] + fn local_kms_master_key_secret_ref_is_rendered() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + local: Some(LocalKmsConfig { + master_key_secret_ref: Some(local_master_key_selector()), + ..Default::default() + }), + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + + let container = &statefulset.spec.unwrap().template.spec.unwrap().containers[0]; + let master_key_env = container + .env + .as_ref() + .expect("env should be rendered") + .iter() + .find(|var| var.name == "RUSTFS_KMS_LOCAL_MASTER_KEY") + .expect("local master key env should be rendered"); + let selector = master_key_env + .value_from + .as_ref() + .and_then(|source| source.secret_key_ref.as_ref()) + .expect("local master key should come from Secret key ref"); + + assert_eq!(selector.name, "local-kms-master-key"); + assert_eq!(selector.key, "local-master-key"); + assert_eq!(selector.optional, Some(false)); + } + + #[test] + fn local_kms_allow_insecure_dev_defaults_is_explicitly_rendered() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + local: Some(LocalKmsConfig { + allow_insecure_dev_defaults: true, + ..Default::default() + }), + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + + let container = &statefulset.spec.unwrap().template.spec.unwrap().containers[0]; + assert_eq!( + env_value(container, "RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS"), + Some("true") + ); + assert_eq!(env_value(container, "RUSTFS_KMS_LOCAL_MASTER_KEY"), None); + } + + #[test] + fn local_kms_statefulset_keeps_operator_managed_env_when_spec_env_conflicts() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + ..Default::default() + }); + tenant.spec.env = vec![ + corev1::EnvVar { + name: "RUSTFS_KMS_KEY_DIR".to_string(), + value: Some("/data/kms-keys".to_string()), + ..Default::default() + }, + corev1::EnvVar { + name: "RUSTFS_KMS_LOCAL_KEY_DIR".to_string(), + value: Some("/data/kms-keys".to_string()), + ..Default::default() + }, + corev1::EnvVar { + name: "RUSTFS_KMS_BACKEND".to_string(), + value: Some("vault".to_string()), + ..Default::default() + }, + corev1::EnvVar { + name: "CUSTOM_USER_ENV".to_string(), + value: Some("kept".to_string()), + ..Default::default() + }, + ]; + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + let pod_spec = statefulset.spec.unwrap().template.spec.unwrap(); + let container = &pod_spec.containers[0]; + let env = container.env.as_ref().expect("env should be rendered"); + + assert_eq!( + env.iter() + .filter(|var| var.name == "RUSTFS_KMS_KEY_DIR") + .count(), + 1 + ); + assert_eq!( + env_value(container, "RUSTFS_KMS_KEY_DIR"), + Some("/data/rustfs0/.kms-keys") + ); + assert_eq!(env_value(container, "RUSTFS_KMS_LOCAL_KEY_DIR"), None); + assert_eq!(env_value(container, "RUSTFS_KMS_BACKEND"), Some("local")); + assert_eq!(env_value(container, "CUSTOM_USER_ENV"), Some("kept")); + } + + #[test] + fn statefulset_drops_reserved_kms_env_even_when_encryption_is_disabled() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.encryption = None; + tenant.spec.env = vec![ + corev1::EnvVar { + name: "RUSTFS_KMS_LOCAL_MASTER_KEY".to_string(), + value: Some("secret".to_string()), + ..Default::default() + }, + corev1::EnvVar { + name: "CUSTOM_USER_ENV".to_string(), + value: Some("kept".to_string()), + ..Default::default() + }, + ]; + let pool = &tenant.spec.pools[0]; + + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet"); + let pod_spec = statefulset.spec.unwrap().template.spec.unwrap(); + let container = &pod_spec.containers[0]; + + assert_eq!(env_value(container, "RUSTFS_KMS_LOCAL_MASTER_KEY"), None); + assert_eq!(env_value(container, "CUSTOM_USER_ENV"), Some("kept")); + } + + #[test] + fn local_kms_implicit_default_migration_from_legacy_dir_is_blocked() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + let mut existing = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + let container = existing + .spec + .as_mut() + .expect("StatefulSet should have spec") + .template + .spec + .as_mut() + .expect("Pod template should have spec") + .containers + .first_mut() + .expect("Container should exist"); + set_env_value(container, "RUSTFS_KMS_KEY_DIR", "/data/kms-keys"); + set_env_value(container, "RUSTFS_KMS_LOCAL_KEY_DIR", "/data/kms-keys"); + + let err = tenant + .validate_statefulset_update(&existing, pool) + .expect_err("implicit Local KMS default migration should be blocked"); + + assert!( + matches!(err, crate::types::error::Error::KmsMigrationBlocked { message, .. } + if message.contains("/data/kms-keys") + && message.contains("/data/rustfs0/.kms-keys") + && message.contains("spec.encryption.local.keyDirectory")) + ); + } + + #[test] + fn local_kms_implicit_default_migration_detects_duplicate_legacy_env() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + let mut existing = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet with Local KMS"); + let container = existing + .spec + .as_mut() + .expect("StatefulSet should have spec") + .template + .spec + .as_mut() + .expect("Pod template should have spec") + .containers + .first_mut() + .expect("Container should exist"); + container + .env + .as_mut() + .expect("container should have environment variables") + .insert( + 0, + corev1::EnvVar { + name: "RUSTFS_KMS_KEY_DIR".to_string(), + value: Some("/data/rustfs0/custom-old-env".to_string()), + ..Default::default() + }, + ); + set_env_value(container, "RUSTFS_KMS_LOCAL_KEY_DIR", "/data/kms-keys"); + + let err = tenant + .validate_statefulset_update(&existing, pool) + .expect_err("duplicate legacy Local KMS env should still be blocked"); + + assert!(matches!( + err, + crate::types::error::Error::KmsMigrationBlocked { .. } + )); + } + + #[test] + fn local_kms_explicit_key_directory_allows_user_controlled_migration() { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].servers = 1; + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + ..Default::default() + }); + let mut existing = tenant + .new_statefulset(&tenant.spec.pools[0]) + .expect("Should create StatefulSet with Local KMS"); + let container = existing + .spec + .as_mut() + .expect("StatefulSet should have spec") + .template + .spec + .as_mut() + .expect("Pod template should have spec") + .containers + .first_mut() + .expect("Container should exist"); + set_env_value(container, "RUSTFS_KMS_KEY_DIR", "/data/kms-keys"); + set_env_value(container, "RUSTFS_KMS_LOCAL_KEY_DIR", "/data/kms-keys"); + + tenant.spec.encryption = Some(EncryptionConfig { + enabled: true, + backend: KmsBackendType::Local, + local: Some(LocalKmsConfig { + key_directory: Some("/data/rustfs0/.kms-keys".to_string()), + ..Default::default() + }), + ..Default::default() + }); + let pool = &tenant.spec.pools[0]; + + tenant + .validate_statefulset_update(&existing, pool) + .expect("explicit Local KMS keyDirectory should allow user-controlled rollout"); + } + #[test] fn mixed_pool_single_node_single_disk_uses_peer_dns_volume() { let mut tenant = crate::tests::create_test_tenant(None, None);