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

Ensure source systems may not send Event.Type.INCIDENT_START events #751

Open
3 tasks
hmpf opened this issue Apr 11, 2024 · 4 comments
Open
3 tasks

Ensure source systems may not send Event.Type.INCIDENT_START events #751

hmpf opened this issue Apr 11, 2024 · 4 comments
Assignees
Labels
bug Something is not working as expected discussion Requires developer feedback/discussion before implementation enhancement New feature or request

Comments

@hmpf
Copy link
Contributor

hmpf commented Apr 11, 2024

INCIDENT_START is created by the backend when it receives a new incident and should not be created by any other process.

Needed:

  • Make test that api may not create such an event
  • Remove Type.INCIDENT_START from ALLOWED_TYPES_FOR_SOURCE_SYSTEMS
  • Simplify EventViewSet.validate_event_type_for_incident

.. plus eventual surprises..

This is really a case where making the test first is a good way to proceed!

@hmpf hmpf added bug Something is not working as expected enhancement New feature or request labels Apr 11, 2024
@hmpf
Copy link
Contributor Author

hmpf commented Apr 11, 2024

.. it might be useful to have a readonly endpoint that shows which event types are available for use with the API for the logged-in user.

@johannaengland johannaengland self-assigned this Apr 25, 2024
@johannaengland
Copy link
Contributor

While I am at it: Should it be allowed to post stateless events? Because when creating a stateless incident a stateless event is automatically created.

@johannaengland
Copy link
Contributor

And I was also thinking:
Why should we explicitly forbid source systems to send start events?
Because currently when encountering an event that doesn't fit (e.g. a start event, a double reopen, ...) we simply register the event without it influencing the incidents (see https://github.com/Uninett/Argus/blob/master/src/argus/incident/views.py#L507-L517).
Why do we need to change this?

@johannaengland johannaengland added the discussion Requires developer feedback/discussion before implementation label Jun 11, 2024
@lunkwill42
Copy link
Member

And I was also thinking: Why should we explicitly forbid source systems to send start events? Because currently when encountering an event that doesn't fit (e.g. a start event, a double reopen, ...) we simply register the event without it influencing the incidents (see https://github.com/Uninett/Argus/blob/master/src/argus/incident/views.py#L507-L517). Why do we need to change this?

I assume these are the lines you're referring to (remember to use the permalink option when linking to GitHub code, or your link will be outdated as soon as the branch changes):

if incident.events.filter(type=event_type).exists():
self._raise_type_validation_error(f"The incident already has a related event of type '{event_type}'.")
if incident.stateful:
if event_type in {Event.Type.INCIDENT_START, Event.Type.INCIDENT_END}:
validate_incident_has_no_relation_to_event_type()
if event_type in {Event.Type.INCIDENT_END, Event.Type.CLOSE} and not incident.open:
self._raise_type_validation_error("The incident is already closed.")
elif event_type == Event.Type.REOPEN and incident.open:
self._raise_type_validation_error("The incident is already open.")
else:

According to the linked code, I assume we can create the test mentioned by @hmpf's initial comment, and use this to verify that this isn't actually an issue. Case closed. Maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working as expected discussion Requires developer feedback/discussion before implementation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants