Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuring SSE for S3 backed storage #1486

Open
kenanwarren opened this issue Jun 10, 2022 · 10 comments · May be fixed by #3914
Open

Allow configuring SSE for S3 backed storage #1486

kenanwarren opened this issue Jun 10, 2022 · 10 comments · May be fixed by #3914
Labels
enhancement New feature or request good first issue Good for newcomers keepalive Label to exempt Issues / PRs from stale workflow

Comments

@kenanwarren
Copy link

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.

@kenanwarren kenanwarren changed the title Allow for configuring SSE for S3 backed storage Allow configuring SSE for S3 backed storage Jun 10, 2022
@mdisibio
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Member

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.

@mdisibio mdisibio added this to the v1.5 milestone Jun 17, 2022
@cristiangsp cristiangsp modified the milestones: v1.5, v2.0 Sep 15, 2022
@schematis
Copy link

schematis commented Oct 14, 2022

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
Copy link
Contributor

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.

@github-actions github-actions bot added the stale Used for stale issues / PRs label Dec 14, 2022
@joe-elliott
Copy link
Member

Going to mark this keepalive and "good first issue". This should just require wiring up some minio config here:

func createCore(cfg *Config, hedge bool) (*minio.Core, error) {

@joe-elliott joe-elliott added enhancement New feature or request good first issue Good for newcomers keepalive Label to exempt Issues / PRs from stale workflow and removed stale Used for stale issues / PRs labels Dec 14, 2022
@mghildiy
Copy link
Contributor

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

@jmtt89
Copy link

jmtt89 commented May 21, 2024

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
Copy link

navanin commented Jul 12, 2024

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
Copy link
Contributor

im going to try to take a swing at this

@KyriosGN0 KyriosGN0 linked a pull request Jul 26, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers keepalive Label to exempt Issues / PRs from stale workflow
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

9 participants