Repository navigation
CLDSRV-1000: Configure KMSs as a kms array and build a client for every entry - #6330
Conversation
The kms array replaces the kmip and kmsAWS blocks. It is optional: without it, S3KMS and S3BACKEND still select mem, file or scality, the KMSs that need no settings. In this commit only the default entry gets a KMS client; the other entries are validated, and their clients are built in the next commit. The default config.json drops its kmip and kmsAWS blocks and gets no kms array, so local runs and unit tests select the KMS through S3KMS or S3BACKEND. The kmsAWS block was a placeholder. The S3KMIP_* environment variables still apply to the default entry when it is a kmip entry: its own settings override them, and it can take its host from S3KMIP_HOSTS. S3KMS=kmip without a kms array now fails at start-up; it could not encrypt anyway, as the key ARNs need a provider name. kmip-ft-tests, which took its provider name from the kmip block, now reads it from a kms entry in its own config file.
… next to the default Keys naming the other KMSs are refused on PutObject and PutBucketEncryption.
Hello dvasilas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 4 files with indirect coverage changes @@ Coverage Diff @@
## development/9.6 #6330 +/- ##
===================================================
+ Coverage 87.89% 88.38% +0.48%
===================================================
Files 206 206
Lines 14193 14221 +28
===================================================
+ Hits 12475 12569 +94
+ Misses 1718 1652 -66
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
| // if type was extracted, it is a scality arnPrefix, it needs validation | ||
| // might throw if arn malformed or backend not available | ||
| // for any request (PUT or GET) | ||
| const error = validateKeyDetail(detail, availableBackends); |
There was a problem hiding this comment.
Given scality/Arsenal#2738:
| const error = validateKeyDetail(detail, availableBackends, key); |
There was a problem hiding this comment.
I will do another cloudserver PR to bump Arsenal and make this change.
We will likely need to target cloudserver development/9.5 because it also uses Arsenal 8.6.
| // otherwise prepend the current KMS client arnPrefix | ||
| if (isScalityKmsArn(configuredMasterKeyId)) { | ||
| const detail = extractDetailFromArn(configuredMasterKeyId); | ||
| const error = validateKeyDetail(detail, availableBackends); |
There was a problem hiding this comment.
Same thing here:
| const error = validateKeyDetail(detail, availableBackends, configuredMasterKeyId); |
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
This pull request does not target the following hotfix branch(es) so they
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-1000. Goodbye dvasilas. |
Context (S3C-11459): Allow a deployment to configure multiple KMSs, one of them the default. Users select a KMS through the key ARN.
This PR makes cloudserver accept several KMS definitions and build a client for each.
Every operation still uses the default KMS: a key whose ARN names another configured KMS is refused, as an unknown ARN is today.
Changes:
kmskey. Federation and Zenko must render the array when they move to cloudserver 9.6.kmiporkmsAWSkey or akmsobject in the configuration stops cloudserver at start-up.S3KMSandS3BACKENDselect the KMS only when there is nokmsarray, and the options aremem,fileorscality, as today. With the array,S3KMSis ignored (S3BACKENDsets the other backends, but not the KMS).S3KMS=kmiporS3KMS=awswithout the array fails at start-up: neither can be configured from the environment alone. A KMIP KMS would have no provider name, so it could not encrypt before this change either, and an AWS KMS would have no endpoint or credentials.S3KMIP_*variables still apply to the default entry when it is a KMIP entry, with today's precedence: the entry's settings override them, and the entry can take its host fromS3KMIP_HOSTS. They never apply to other entries.