Repository navigation
Conversation
`spec.tiers` registers S3, RustFS and MinIO remote tiers through the RustFS tier admin API, so bucket lifecycle transitions can reference a tier managed by the same Tenant. Tiers are reconciled before buckets. Credentials come from `tiers[].credsSecret` (`accesskey`/`secretkey`). RustFS never returns a tier secret key, so the applied Secret resourceVersion is recorded in `status.provisioning.tiers`; a Secret change, a different Secret, or an access key listed by RustFS that differs from the Secret updates the credentials with the edit API. Type, endpoint, bucket, prefix and region cannot change on a registered tier: a live tier with the same name and other settings is reported as `TierConflict` without writes, and a matching one is adopted. RustFS rejects a remote prefix that already holds objects with 409; that is reported as `TierConflict` too, and `force: true` registers the tier anyway. Removed tiers are retained, like other provisioned items. Closes rustfs#280
Buckets accept `encryption` with `mode: SSE-S3` (S3 `AES256`) or `mode: SSE-KMS` (`aws:kms`, optional `kmsKeyId`), and `state: Absent` to remove the configuration. Ownership follows objectLockConfiguration and lifecycle: omission makes no encryption calls, a matching live configuration is adopted without a write, and a different one is replaced or removed only while its hash matches the configuration last recorded in status; otherwise the bucket reports BucketEncryptionConflict. SSE-KMS requires `spec.encryption.enabled`; without it RustFS answers PutBucketEncryption with 500 "KMS service not initialized", which would otherwise keep the bucket Pending forever. RustFS records its default KMS key when none is sent, so ownership is the hash of the verified live configuration, and SSE-KMS without `kmsKeyId` matches any key. RustFS answers GetBucketEncryption without a configuration with 400 ServerSideEncryptionConfigurationNotFoundError rather than 404; the client treats that code as absent. Closes rustfs#284
GatewayJ
left a comment
There was a problem hiding this comment.
Please fix the SSE-S3 provider validation and default KMS key reconciliation before merging, and add regression coverage for the scenarios in the two inline comments.
Reviewed commit: 701d2dfd05be24785e3867ac69737a914c06dd93.
Validation: make -j8 test fmt-check release-metadata-check passed, including 796 existing Rust tests; make -j8 clippy passed. Two additional local KMS regression tests failed as described inline. The SSE-S3 upload failure was established from the RustFS 1.0.2-preview.2 server source; live object PUT/GET validation remains outstanding.
| .await | ||
| if desired | ||
| .as_ref() | ||
| .is_some_and(|desired| desired.algorithm == AdminEncryptionAlgorithm::AwsKms) |
There was a problem hiding this comment.
[P1] Validate the SSE-S3 encryption provider before applying the bucket default
This guard only checks AwsKms. A Tenant with KMS disabled and no RUSTFS_SSE_S3_MASTER_KEY can therefore receive an AES256 bucket default and be reported Ready, even though subsequent object uploads that use that default cannot obtain an encryption provider.
RustFS 1.0.2-preview.2 accepts the bucket configuration in execute_put_bucket_encryption. The object write path selects the local SSE-S3 provider when KMS is unavailable, which returns InvalidRequest when the required master key is missing.
Reproduction steps:
- Use a RustFS 1.0.2-preview.2 Tenant with KMS disabled and no
RUSTFS_SSE_S3_MASTER_KEY. - Declare a bucket with
encryption: {mode: SSE-S3}. - Reconcile the Tenant. The configuration APIs succeed, and the Operator reports the bucket Ready.
- Upload an object without explicit encryption headers, for example:
printf 'encryption probe\n' > /tmp/encryption-probe
aws --endpoint-url "$RUSTFS_ENDPOINT" s3api put-object \
--bucket secure-bucket --key probe --body /tmp/encryption-probeThe server code path returns HTTP 400 InvalidRequest because the SSE-S3 master key is missing. This scenario was verified by tracing the server source; I have not executed the upload against a live RustFS instance.
Please validate the SSE-S3 provider prerequisites before applying the configuration: enabled Tenant KMS, or a supported configuration supplying a valid RUSTFS_SSE_S3_MASTER_KEY through a Secret. Update the example to include those prerequisites and add actual object PUT/GET coverage. The existing sse_s3_bucket_encryption_is_applied_to_a_new_bucket_and_owned test only exercises the configuration APIs.
| && match desired.kms_master_key_id.as_deref() { | ||
| Some(key_id) => live.kms_master_key_id.as_deref() == Some(key_id), | ||
| None => { | ||
| desired.algorithm == AdminEncryptionAlgorithm::AwsKms | ||
| || live.kms_master_key_id.is_none() |
There was a problem hiding this comment.
[P2] Resolve the default KMS key instead of accepting every live key
When kmsKeyId is omitted, this branch accepts any live AwsKms key. The matching branch at lines 3217–3225 then returns without a PUT and replaces encryptionLastAppliedHash with the live hash. This does not implement the documented behavior that omission selects the RustFS default key.
Reproduction steps:
- Configure valid Tenant KMS with
spec.encryption.defaultKeyId: rustfs-default-key, and provision an existing KMS key namedold-explicit-key. - Declare a bucket with
encryption: {mode: SSE-KMS, kmsKeyId: old-explicit-key}and let reconciliation complete. - Remove only
kmsKeyId, keepingmode: SSE-KMS. - Reconcile again and read the bucket encryption configuration.
The bucket remains Ready with old-explicit-key; no encryption PUT selects rustfs-default-key.
The same wildcard also hides external key changes: after the Operator records the default key's hash, change the live key to foreign-key while leaving kmsKeyId omitted. Reconciliation reports ProvisioningConfigured and adopts the foreign hash, so the previous ownership record no longer detects that change.
I reproduced both cases in additional local tests using the existing BucketControlCapture mock and previous ownership status:
review_removing_explicit_kms_key_uses_the_default_key:
actual: Some("old-explicit-key")
expected: Some("rustfs-default-key")
review_default_kms_key_drift_preserves_the_ownership_record:
actual: "ProvisioningConfigured"
expected: "BucketEncryptionConflict"
Please resolve and compare the effective default key, recognize a change from an explicit key to the default, and preserve the last applied ownership hash when an external key change conflicts with the desired configuration. Add regression coverage for explicit-to-default changes, Tenant default-key changes, and external key changes.
Closes #284
Depends on #282 (this branch is stacked on it); only the last commit, 701d2df, belongs to this PR.
What
spec.buckets[].encryptiondeclares the bucket's default server-side encryption:Ownership follows
objectLockConfigurationandlifecycle:encryptionis omitted, the operator makes no encryption calls.Absent, only while its hash matchesencryptionLastAppliedHash. Otherwise the bucket reportsBucketEncryptionConflictand is left untouched.encryptionDesiredHashandencryptionLastAppliedHashare recorded in the bucket status.Validation:
Presentrequiresmode;AbsentforbidsmodeandkmsKeyId;kmsKeyIdrequiresSSE-KMS.kmsKeyIdis 1–1024 characters and must match^[A-Za-z0-9._:/+=@-]+$.SSE-KMSis rejected up front unlessspec.encryption.enabled. Without it, RustFS answers 500KMS service not initializedand the bucket would stay Pending.Default encryption only applies to objects written after it is set. Existing objects are not re-encrypted or rewritten.
RustFS behaviour handled (1.0.2-preview.2)
GET ?encryptionwithout a configuration returns 400ServerSideEncryptionConfigurationNotFoundError, not 404. The client treats it as absent.Rule, known elements only, entity references rejected.PUTwithaws:kmsand no key ID stores the default KMS key. Ownership is therefore the hash of the configuration read back after the PUT, andSSE-KMSwithoutkmsKeyIdmatches any live key.DELETE ?encryptionis idempotent (204).Changes
rustfs-admin:BucketEncryptionConfigurationwith XML encode and parse, plusget_bucket_encryption,put_bucket_encryptionanddelete_bucket_encryption.BucketEncryptionSpec, two status hashes, and the reasonsBucketEncryptionApplyFailedandBucketEncryptionConflict.reconcile_bucket_encryptionruns after the bucket policy and before lifecycle.Testing
make pre-commit: fmt, clippy, unit tests, e2e check, console test, lint, build and format.Absentremoves only an owned configuration;AES256with no PUT;BucketKeyEnabled) givesBucketEncryptionConflictand is not overwritten;state: Absentremoves the configuration.