grafana / tempo

Grafana Tempo is a high volume, minimal dependency distributed tracing backend.
https://grafana.com/oss/tempo/
GNU Affero General Public License v3.0
3.88k stars 504 forks source link

Allow configuring SSE for S3 backed storage #1486

Open kenanwarren opened 2 years ago

kenanwarren commented 2 years ago

Is your feature request related to a problem? Please describe. When using S3 backed storage you can't configure the internal S3 client to use SSE.

Describe the solution you'd like Allow for passing in a configuration option to Tempo to leverage SSE. It looks like the client used by Tempo supports SSE out of the box.

Describe alternatives you've considered None that I can think of other than leveraging another tracing system that supports SSE.

Additional context We self-host Tempo/Loki/Grafana/Mimir (We're kicking the tires on Mimir, Thanos is what's used in production) internally at my current company. Mimir, Loki, and Thanos all support encryption at rest via passing in an SSE flag in the storage configuration.

mdisibio commented 2 years ago

Hi, thanks for raising this issue. We are definitely interested in keeping parity between Tempo and Mimir/Loki. For context, S3 has the ability to set default SSE on the bucket itself without changes to the clients. Would that also work for your use case?

kenanwarren commented 2 years ago

@mdisibio Ah that would work for our use case. I was unsure if it would cause an issue with the querying side of Tempo; glad to know it wouldn't! Thanks for the quick response.

joe-elliott commented 2 years ago

Let's leave this open, I believe someone will want similar functionality in the future. Also, I think it worthwhile to simply wire up all of these features to help future operators.

schematis commented 1 year ago

Running into this as well. There's an organizational level requirement that all objects in our s3 buckets (and the buckets themselves) are encrypted at rest and objects are prevented from being stored that do not meet that requirement.

Config similar to Loki with the ability to set it once for all storage would be super.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. The next time this stale check runs, the stale label will be removed if there is new activity. The issue will be closed after 15 days if there is no new activity. Please apply keepalive label to exempt this Issue.

joe-elliott commented 1 year ago

Going to mark this keepalive and "good first issue". This should just require wiring up some minio config here: https://github.com/grafana/tempo/blob/5179b5519e6003b5b3bcd6409e576cfcb2be1895/tempodb/backend/s3/s3.go#L332

mghildiy commented 1 year ago

Seems it is now default encryption on AWS as per this.

jmtt89 commented 3 months ago

hi, like reference, in my organization enforce the S3 encryption policy using this BucketPolicy

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "enforce-encryption",
            "Effect": "Deny",
            "Principal": "*",
            "Action": "s3:PutObject",
            "Resource": "arn:aws:s3:::<BUCKET-NAME>/*",
            "Condition": {
                "StringNotEquals": {
                    "s3:x-amz-server-side-encryption": "<ENCRYPTION-METHOD>"
                }
            }
        }
    ]
}

so i can't use tempo right now directly with S3.

navanin commented 1 month ago

Hi! I would really like to see movements on this issue, as our company would really like to use encryption of buckets containing traces.

KyriosGN0 commented 1 month ago

im going to try to take a swing at this