Support additional SSE methods for S3

[taharah]

Thanos, Prometheus and Golang version used

Thanos: 0.3.2
Prometheus: 2.7.2
Golang: 1.11.5

What happened

Currently, the S3 bucket client only supports using the default key for SSE [1].

What you expected to happen

The S3 bucket client should be able to also support SSE-KMS and SSE-C encryption [2].

How to reproduce it (as minimally and precisely as possible):

N/A

Full logs to relevant components

N/A

Anything else we need to know

I took a look at the code and this does introduce the opportunity to modify the existing SSE configuration in order to be a little cleaner as well as support the additional encryption methods.

For instance, the current configuration is this:

type: S3
config:
  encrypt_sse: false

I think it would be nice if we changed the configuration to something like the following:

type: S3
config:
  server_side_encryption:
    enabled: false
    kms_key_id: ""  # Used for SSE-KMS encrytpion
    encryption_key: ""  # Used for SSE-C encryption

NOTE: I've included only the relevant portions in the above examples.

However, the change could be made passive by just adding the parameters to the existing configuration at the top level.

[adrien-f]

Hi 👋 !

This is an interesting feature. So we need to support three cases:

• If no kms_key_id and no encryption_key but enabled then NewSSE
• If kms_key_id and enabled then NewSSEKMS. Do you know what's the context parameter for ?
• If encryption_key and enabled then NewSSEC

Seems clear enough ! What do you think ?

[taharah]

@adrien-f that looks good to me and what I was thinking. Additionally, the KMS encryption context is an optional set of key/value pairs that you can send with the requests.

[jujugrrr]

hi, it seems like a basic feature if you use S3 with server side encryption. @adrien-f Do you think it would be possible to work on this?

@taharah did you just use the default sse after all?

[bwplotka]

Valid request, help wanted (:

👍

[jujugrrr]

I can help with testing if needed, but I have limited go experience

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.

[jujugrrr]

Still a good feature to have :-)

[stale]

This issue/PR has been automatically marked as stale because it has not had recent activity. Please comment on status otherwise the issue will be closed in a week. Thank you for your contributions.