Adds SSE-KMS and SSE-C config to S3 Objstore #3064

jalev · 2020-08-23T14:59:32Z

I added CHANGELOG entry for this change.
Change is not relevant to the end user.

Changes

Adds S3 SSE via a sse_config block. It's a reimplementation of #2170 since it looks like it was abandoned. It solves #946.

There are 3 types of SSE you can use: SSE-S3, SSE-KMS, and SSE-C.

  sse_config:
    # Whether or not we use SSE at all. This should be set at the very minimum to enable SSE-S3.
    enabled: false
    # If someone wants to use their KMS key, we let them set that up by passing in kms_key_id. This sets up SSE-KMS
    kms_key_id: ""
    # kms_encryption_context provides additional variables to use for encryption/decryption. It doesn't need to be set as AWS provides a default encryption context, but you _can_ set it if you want something more.
    kms_encryption_context: {}
    # encryption_key is what it says on the tin. This sets up SSE-C.
    encryption_key: ""

Verification

I can verify if this works when I get back to work. We're currently running into 403 errors from not providing a KMS key ID, so when we don't get 403s I'll know it works.

Signed-off-by: James Bach <james.bach@transferwise.com>

bwplotka

Amazing, this is what we need. I have minor suggestions though, otherwise LGTM!

Thanks for helping here and reimplementing old PR 🤗

docs/storage.md

Signed-off-by: James Bach <james.bach@transferwise.com>

jalev · 2020-08-24T10:54:53Z

Prior to KMS configuration:

level=info ts=2020-08-24T09:44:57.119043821Z caller=http.go:81 service=http/server component=compact msg="internal server shutdown" err="error executing compaction: compaction: group 0@13480198280331570948: upload of 01EGFXMP8HXRQRX2WEPB7E317Q failed: upload meta file to debug dir: upload file /var/thanos/compact/compact/0@13480198280331570948/01EGFXMP8HXRQRX2WEPB7E317Q/meta.json as debug/metas/01EGFXMP8HXRQRX2WEPB7E317Q.json: upload s3 object: Access Denied"
level=info ts=2020-08-24T09:44:57.119069085Z caller=intrumentation.go:66 msg="changing probe status" status=not-healthy reason="error executing compaction: compaction: group 0@13480198280331570948: upload of 01EGFXMP8HXRQRX2WEPB7E317Q failed: upload meta file to debug dir: upload file /var/thanos/compact/compact/0@13480198280331570948/01EGFXMP8HXRQRX2WEPB7E317Q/meta.json as debug/metas/01EGFXMP8HXRQRX2WEPB7E317Q.json: upload s3 object: Access Denied"

So, for testing (with KMS), I use the the following policy that lets Thanos actually use the KMS key + upload to buckets:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "VisualEditor0",
            "Effect": "Allow",
            "Action": [
                "s3:PutObject",
                "s3:GetObject",
                "s3:AbortMultipartUpload",
                "s3:ListBucket",
                "s3:DeleteObject",
                "s3:PutObjectAcl",
                "s3:ListMultipartUploadParts"
            ],
            "Resource": [
                "arn:aws:s3:::secret-bucket",
                "arn:aws:s3:::secret-bucket/*"
            ]
        },
        {
            "Sid": "VisualEditor1",
            "Effect": "Allow",
            "Action": [
                "kms:Encrypt",
                "kms:GenerateDataKey",
                "kms:Decrypt"
            ],
            "Resource": [
                "arn:aws:kms:eu-central-1:1234567890abc:key/12345678-12ab-34cd-56ef-1234567890ab"
            ]
        }
    ]
}

And configure the key (using custom helm chart, ignore that but take note of the sse_config):

  service:
    args:
      - "compact"
      - "--log.level=debug"
      - "--log.format=logfmt"
      - "--http-address=0.0.0.0:10902"
      - |
        --objstore.config=
          type: S3
          config:
            bucket: secret-bucket
            endpoint: s3.eu-central-1.amazonaws.com
            sse_config:
              type: SSE-KMS
              kms_key_id: 12345678-12ab-34cd-56ef-1234567890ab
      - "--data-dir=/var/thanos/compact"
      - "--consistency-delay=30m"
      - "--retention.resolution-raw=0d"
      - "--retention.resolution-5m=0d"
      - "--retention.resolution-1h=0d"
      - "--block-sync-concurrency=20"
      - "--compact.concurrency=1"
      - "--wait"

level=debug ts=2020-08-24T10:33:14.97343253Z caller=objstore.go:158 group="0@{cluster=\"secret-environment.transferwise.com\", prometheus=\"prometheus\", prometheus_shard=\"0\", replica=\"prometheus-k8s-shard-0-1.secret-environment.transferwise.com\"}" groupKey=0@11189359859295756877 msg="uploaded file" from=/var/thanos/compact/compact/0@11189359859295756877/01EGG0DS7MV3TDG8HGNTVA4W3F/meta.json dst=debug/metas/01EGG0DS7MV3TDG8HGNTVA4W3F.json bucket="tracing: secret-bucket"
level=info ts=2020-08-24T10:33:16.778498888Z caller=fetcher.go:453 component=block.BaseFetcher msg="successfully synchronized block metadata" duration=244.381563ms cached=168 returned=168 partial=1
level=debug ts=2020-08-24T10:33:16.801469782Z caller=objstore.go:158 group="0@{cluster=\"secret-environment.transferwise.com\", prometheus=\"prometheus\", prometheus_shard=\"0\", replica=\"prometheus-k8s-shard-0-1.secret-environment.transferwise.com\"}" groupKey=0@11189359859295756877 msg="uploaded file" from=/var/thanos/compact/compact/0@11189359859295756877/01EGG0DS7MV3TDG8HGNTVA4W3F/chunks/000001 dst=01EGG0DS7MV3TDG8HGNTVA4W3F/chunks/000001 bucket="tracing: secret-bucket"
level=debug ts=2020-08-24T10:33:18.556145359Z caller=objstore.go:158 group="0@{cluster=\"secret-environment.transferwise.com\", prometheus=\"prometheus\", prometheus_shard=\"0\", replica=\"prometheus-k8s-shard-0-1.secret-environment.transferwise.com\"}" groupKey=0@11189359859295756877 msg="uploaded file" from=/var/thanos/compact/compact/0@11189359859295756877/01EGG0DS7MV3TDG8HGNTVA4W3F/chunks/000002 dst=01EGG0DS7MV3TDG8HGNTVA4W3F/chunks/000002 bucket="tracing: secret-bucket"
level=debug ts=2020-08-24T10:33:20.047301162Z caller=objstore.go:158 group="0@{cluster=\"secret-environment.transferwise.com\", prometheus=\"prometheus\", prometheus_shard=\"0\", replica=\"prometheus-k8s-shard-0-1.secret-environment.transferwise.com\"}" groupKey=0@11189359859295756877 msg="uploaded file" from=/var/thanos/compact/compact/0@11189359859295756877/01EGG0DS7MV3TDG8HGNTVA4W3F/index dst=01EGG0DS7MV3TDG8HGNTVA4W3F/index bucket="tracing: secret-bucket"
level=debug ts=2020-08-24T10:33:20.101614981Z caller=objstore.go:158 group="0@{cluster=\"secret-environment.transferwise.com\", prometheus=\"prometheus\", prometheus_shard=\"0\", replica=\"prometheus-k8s-shard-0-1.secret-environment.transferwise.com\"}" groupKey=0@11189359859295756877 msg="uploaded file" from=/var/thanos/compact/compact/0@11189359859295756877/01EGG0DS7MV3TDG8HGNTVA4W3F/meta.json dst=01EGG0DS7MV3TDG8HGNTVA4W3F/meta.json bucket="tracing: secret-bucket"

And to test with an unhappy case of a misconfigured key:

level=error ts=2020-08-24T10:52:58.150471275Z caller=compact.go:382 msg="retriable error" err="compaction: group 0@11189359859295756877: upload of 01EGG1HVYM3329P0P5BQCNVD7X failed: upload meta file to debug dir: upload file /var/thanos/compact/compact/0@11189359859295756877/01EGG1HVYM3329P0P5BQCNVD7X/meta.json as debug/metas/01EGG1HVYM3329P0P5BQCNVD7X.json: upload s3 object: Key 'arn:aws:kms:eu-central-1:1234567890abc:key/12345678-12ab-34cd-56ef-BROKENKEYOHNO' does not exist"

I should add the policy I used to the documentation.

bwplotka

Amazing. Small suggestions only and happy to merge this! 🤗

bwplotka · 2020-08-24T12:32:40Z

CHANGELOG.md

+```yaml
+
+---
+sse_config:


pkg/objstore/s3/s3.go

Signed-off-by: James Bach <james.bach@transferwise.com>

bwplotka

LGTM!

Thanks for your patience! 💪

pkg/objstore/s3/s3.go

bwplotka · 2020-08-24T13:24:28Z

pkg/objstore/s3/s3.go

@@ -173,8 +192,32 @@ func NewBucketWithConfig(logger log.Logger, config Config, component string) (*B
 	client.SetAppInfo(fmt.Sprintf("thanos-%s", component), fmt.Sprintf("%s (%s)", version.Version, runtime.Version()))

 	var sse encrypt.ServerSide
-	if config.SSEEncryption {
-		sse = encrypt.NewSSE()
+	if config.SSEConfig.Type != "" {


Instead of this we can have just empty case for ""

Not a blocker (:

Signed-off-by: James Bach <james.bach@transferwise.com>

jalev · 2020-08-24T13:40:04Z

Going to wait for tests to run and then ready to merge 👌

bwplotka · 2020-08-25T11:13:29Z

OMG link checking job is too annoying, let's kill it for now.... Related issue: #3060

bwplotka · 2020-08-25T11:13:50Z

Thanks!

bwplotka

We are missing an important things here.

encrypt_sse: true should fail and not be silently ignored. This will ensure users will not get surprised. Will add that change in separate PR (:

jalev added 2 commits August 23, 2020 16:00

Adds SSE config to S3 Objstore

70ac68a

Signed-off-by: James Bach <james.bach@transferwise.com>

Document sse_config block in storage.md

4b70610

Signed-off-by: James Bach <james.bach@transferwise.com>

jalev changed the title ~~Adds SSE config to S3 Objstore~~ Adds SSE-KMS and SSE-C config to S3 Objstore Aug 23, 2020

jalev added 3 commits August 23, 2020 15:24

run make docs

666be08

Signed-off-by: James Bach <james.bach@transferwise.com>

Add SSE config options to changelog

16c2832

Signed-off-by: James Bach <james.bach@transferwise.com>

Update changelog

104c7a4

Signed-off-by: James Bach <james.bach@transferwise.com>

bwplotka approved these changes Aug 23, 2020

View reviewed changes

docs/storage.md Outdated Show resolved Hide resolved

jalev added 3 commits August 24, 2020 09:22

Define explicit SSE 'Type'

9608dc1

Signed-off-by: James Bach <james.bach@transferwise.com>

Update docs again for real

370466e

Signed-off-by: James Bach <james.bach@transferwise.com>

Fix bad logic for SSE-KMS

e7a5408

Signed-off-by: James Bach <james.bach@transferwise.com>

bwplotka approved these changes Aug 24, 2020

View reviewed changes

jalev added 4 commits August 24, 2020 12:45

Adds example policy for KMS documentation

81d03e8

Signed-off-by: James Bach <james.bach@transferwise.com>

Changes SSE strings into constants

1c8efe2

Signed-off-by: James Bach <james.bach@transferwise.com>

Make SSE error message prettier

0b2d3a3

Signed-off-by: James Bach <james.bach@transferwise.com>

Export consant with comments

9079289

Signed-off-by: James Bach <james.bach@transferwise.com>

bwplotka approved these changes Aug 24, 2020

View reviewed changes

Group consts in pkg/s3

2c52cc2

Signed-off-by: James Bach <james.bach@transferwise.com>

bwplotka merged commit 61b8382 into thanos-io:master Aug 25, 2020

bwplotka reviewed Aug 26, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds SSE-KMS and SSE-C config to S3 Objstore #3064

Adds SSE-KMS and SSE-C config to S3 Objstore #3064

jalev commented Aug 23, 2020 •

edited

bwplotka left a comment

jalev commented Aug 24, 2020

bwplotka left a comment

bwplotka Aug 24, 2020

bwplotka left a comment

bwplotka Aug 24, 2020

jalev commented Aug 24, 2020 •

edited

bwplotka commented Aug 25, 2020

bwplotka commented Aug 25, 2020

bwplotka left a comment

Adds SSE-KMS and SSE-C config to S3 Objstore #3064

Adds SSE-KMS and SSE-C config to S3 Objstore #3064

Conversation

jalev commented Aug 23, 2020 • edited

Changes

Verification

bwplotka left a comment

Choose a reason for hiding this comment

jalev commented Aug 24, 2020

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka Aug 24, 2020

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka Aug 24, 2020

Choose a reason for hiding this comment

jalev commented Aug 24, 2020 • edited

bwplotka commented Aug 25, 2020

bwplotka commented Aug 25, 2020

bwplotka left a comment

Choose a reason for hiding this comment

jalev commented Aug 23, 2020 •

edited

jalev commented Aug 24, 2020 •

edited