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

ref(config): Rename config to SENTRY_TRUSTED_RELAY_PKS #19240

Closed
wants to merge 1 commit into from

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Jun 9, 2020

Renames the setting for trusted Relays to more sensitive language.

Requires https://github.com/getsentry/getsentry/pull/3988

@jan-auer jan-auer requested a review from mitsuhiko June 9, 2020 11:14
@jan-auer jan-auer self-assigned this Jun 9, 2020
@jan-auer
Copy link
Member Author

jan-auer commented Jun 9, 2020

Ok, onpremise needs updating, too.

Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do something like a48d4c1 where we both deprecate this old setting and also move it into config.yml.

@jan-auer
Copy link
Member Author

jan-auer commented Jun 9, 2020

Is there any benefit we get from moving it? There are plenty of other related settings in the python file, such as SENTRY_RELAY_OPEN_REGISTRATION which we'd all have to move.

Edit: It generally seems that all of the settings that are similar to this one are in the Python file. I'm only asking as I'd like to keep this patch as straight-forward as possible.

@BYK
Copy link
Member

BYK commented Jun 9, 2020

Is there any benefit we get from moving it?

  • You get automatic backwards compatibility (and the GCB build here would be fixed)
  • Most of the customizable config is already moved to config.yml file and although there isn't a concerted effort behind this, we state that sentry.conf.py file is mostly deprecated

There are plenty of other related settings in the python file, such as SENTRY_RELAY_OPEN_REGISTRATION which we'd all have to move.

We can do them one by one (or in this PR, doesn't matter much) under a relay.* namespace.

@jan-auer
Copy link
Member Author

jan-auer commented Jun 9, 2020

Most of the customizable config is already moved to config.yml

I see. In that case, I'll give this a spin later.

we state that sentry.conf.py file is mostly deprecated

I think we should start adding large comments in server.py so that folks don't add more settings to that file. I still see new settings being added every now and then, although it has reduced over the last years.

@mitsuhiko
Copy link
Member

This setting was never supposed to be set by non sentry.io installations. We expose this setting on a per-org basis in the org settings to enable external relays.

If we think there is a value to have this config be set by other parties than us I think we can move it, but that was not the original motivation at least.

@jan-auer
Copy link
Member Author

Discussed offline a bit more, and we'll move to config.yml since this is required in the onpremise setup flow to get internal Relays working.

@BYK
Copy link
Member

BYK commented Oct 29, 2020

Closing due to staleness and conflicts.

@BYK BYK closed this Oct 29, 2020
@BYK BYK deleted the ref/rename-relay-whitelist branch October 29, 2020 20:12
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants