[flyteadmin] Add support for KMS SSE to S3 backend

[ddl-rliu]

Tracking issue

Part of a group:

1. Add support for KMS SSE to S3 backend stow#11
2. [flyteadmin] Add support for KMS SSE to S3 backend #4897
3. Add S3 server-side-encryption headers for pre-signed URLs flytekit#2193

Why are the changes needed?

S3 Stow implementation does not yet support setting ServerSideEncryption (SSE). We are particularly interested in the AWS Key Management Service (KMS) case.

What changes were proposed in this pull request?

Adds a new extra_args key (optional string field) to the stow config, which contains the keys/value like ServerSideEncryption: x, SSEKMSKeyId: x. It is passed through the storage.yaml section.

How was this patch tested?

See flyteorg/stow#11

Tested on a Flyte deployment, against an S3 bucket with policy denying any request without "s3:x-amz-server-side-encryption": "aws:kms".

(Pdb) rsp.url
'https://...s3.us-west-2.amazonaws.com/...&X-Amz-SignedHeaders=content-md5%3Bhost%3B
x-amz-server-side-encryption%3Bx-amz-server-side-encryption-aws-kms-key-id
&x-amz-server-side-encryption=aws%3Akms&x-amz-server-side-encryption-aws-kms-key-id=...
&X-Amz-Signature=...'
(Pdb) rsp.status_code
200
(Pdb) rsp.headers
{'x-amz-id-2': '...', 'x-amz-request-id': '...', 'Date': 'Thu, 15 Feb 2024 23:28:48 GMT', 
'x-amz-server-side-encryption': 'aws:kms', 'x-amz-server-side-encryption-aws-kms-key-id': ''...', 
'Server': 'AmazonS3', ...}

Related PRs

1. Add support for KMS SSE to S3 backend stow#11
2. [flyteadmin] Add support for KMS SSE to S3 backend #4897
3. Add S3 server-side-encryption headers for pre-signed URLs flytekit#2193

Docs link

[welcome]

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

• Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
• Sign off your commits (Reference: DCO Guide).

[ddl-ebrown]

Does this need to be surfaced in the Helm chart in any way? Or can this already be handled through the configmap values as they're currently defined?

[ddl-rliu]

Let me know if I missed the question, but what I did to get this to work in my deployment was to manually edit the flyte-admin-base-config configmap - key storage.yaml storage.connection.extra-args and value "{\"ServerSideEncryption\": \"aws:kms\", \"SSEKMSKeyId\": \"kmsId\"}"

[ddl-ebrown]

Cool, if it automatically flowed in through the config map without anything special, that's great!

I think you might want to add it to the values.yaml for documentation IFF all the other possible settings are there. If there's just a link to the Go config struct already then you probably don't need to.

[ddl-rliu]

https://github.com/ddl-rliu/flyte/pull/1/files Drafted this PR with what I think would be the right changes to the documentation/values.yaml/etc.

[ddl-ebrown]

Just a reminder to bump to latest stow

[ddl-ebrown]

I think you need to go get to update the stow version once https://github.com/flyteorg/stow/pull/11/files merges, right?

[ddl-rliu]

Yep, will hold off on this PR until the stow change merges

[ddl-rliu]

Need to add this to flyte propeller too + associated configmap storage.yaml changes

[ddl-ebrown]

Filed #4949 to discuss the ETags aren't MD5 issue

[ddl-rliu]

See flyteorg/stow#11 (comment)