-
Notifications
You must be signed in to change notification settings - Fork 94
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
fix(server): Validate release and environment attrs in sessions #1018
Conversation
@@ -468,3 +468,25 @@ def test_session_auto_ip(mini_sentry, relay_with_processing, sessions_consumer): | |||
# Can't test ip_address since it's not posted to Kafka. Just test that it is accepted. | |||
session = sessions_consumer.get_session() | |||
assert session | |||
|
|||
|
|||
def test_session_invalid_release(mini_sentry, relay_with_processing, sessions_consumer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a test_session_invalid_environment
test here too
"sid": "8333339f-5675-4f89-a9a0-1c935255ab58", | ||
"timestamp": timestamp.isoformat(), | ||
"started": timestamp.isoformat(), | ||
"attrs": {"release": "latest"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And maybe some tests for erroneous releases like the ones mentioned here -> https://github.com/getsentry/sentry/pull/26279/files#diff-21815168d041846d793a0e86869d5eb8391601025fb32017363e3359e93188b5R649
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could do, but since this is in integration tests, I see little point in testing sentry-release-parser
.
Validates the
release
andenvironment
attributes in sessions. Invalidreleases drop the session, since releases are required. Invalid environments are
simply removed.