search

Icinga / icingaweb2

A lightweight and extensible web interface to keep an eye on your environment. Analyse problems and act on them.
https://icinga.com/get-started/
GNU General Public License v2.0
806 stars 280 forks source link

View who removed the comment, downtime or acknowledgement in the history views #2281

Closed icinga-migration closed 4 years ago

icinga-migration commented 8 years ago

This issue has been migrated from Redmine: https://dev.icinga.com/issues/11101

Created by ClemensBW on 2016-02-05 18:41:43 +00:00

Assignee: (none) Status: New Target Version: (none) Last Update: 2016-02-26 20:58:54 +00:00 (in Redmine)

Hello,

if I (Clemens) create a host comments and a other user (wan) delete this, i don't can see that in the history. Can you plase add a mouseover for that or a detail text, who delete the comment?

(dito for acknowledgments, think this is a little bit profoundly)

Attachments

Parent Task: #11180

icinga-migration commented 8 years ago

Updated by elippmann on 2016-02-18 09:26:05 +00:00

Hi,

Thanks for the report. Icinga 1.x and 2.x do not know who deleted the comment because there is no parameter to the remove comment command for passing the author of the removal. Afaik this is not going to be implemented in the near future.

Best, Eric

icinga-migration commented 8 years ago

Updated by elippmann on 2016-02-18 09:27:28 +00:00

Of course, Web 2 could maintain this because we know the authenticated user but this requires much effort.

icinga-migration commented 8 years ago

Updated by mfriedrich on 2016-02-26 18:34:35 +00:00

Logging such data including sensitive information such as the ip address or name can cause trouble in several european countries and their data retention laws. We had that issue with the core/classicui a while back as well where Ricardo added a patch by afaik Telekom request. See #5689 for details.

icinga-migration commented 8 years ago

Updated by tgelf on 2016-02-26 19:47:13 +00:00

I absolutely believe you, Michael. But that's ridiculous, I wouldn't care. We log the name for all other actions like creating comments. If someone doesn't agree with a software logging the fact that he deleted a comment he could simply stop deleting them.

Another option for him would be to explain his problems to his employer. I'm pretty sure they are willing to pay for a migration to another software not logging the fact that he deletes comments. If they are used to cope with such individuals they will also have no problems with paying someone to compile dedicated patched versions to make their employees feel better.

So if someone pops up with such feelings please sent him to me. I REALLY love to join that kind of conversations ;)

From a technical point of view, Eric has all said. It's a very old command definition for the command pipe, defined as...

DEL_HOST_COMMENT;

...and similar. I see no problem with allowing also supporting:

DEL_HOST_COMMENT;;

We should try whether Icinga 1.x and 2.x would silently accept such commands. If they are happy with the known fields and silently discard the part that would be perfect. Otherwise we must use the version awareness of Icinga Web 2 to treat new 2.x versions different than older ones. Same for the 2.x API.

So, from my side: GO for this feature. I see no valid reason against this, it should IMO have been there since a very long time. However, it could involve quite some amount of work. Start with all DEL_* commands, it involved IDO and more, the next one then discovers that this would also be great for flag-toggling actions (e.g. disable service checks) and so on.

Seen from that point of view it could make more sense to postpone it unless we got a new data persistence backend. Depending on what we opt for we could then delegate logging user actions to the web frontend. Would allow for more granular control and more flexibility. And the troll you mentioned before would have to spend less for patched web versions than for patched & custom-compiled cores ;)

Cheers, Thomas

icinga-migration commented 8 years ago

Updated by mfriedrich on 2016-02-26 20:58:55 +00:00

Oh I just stumbled over this issue when looking for a different one with acknowledgements. It reminded me of that old discussion so I thought I'd share it for those loving their data retention laws. I don't care about them either, each additional config flag adding a different behaviour is like hell.

For the technical idea:

Changing the external command API is something I will not recommend nor implement neither in Icinga 1.x nor Icinga 2.x. Not only for ensuring compatibility with the existing specification and documentation, but also for implementation reasons. Adding additional values at the end and hoping it would work is dangerous. Furthermore the external command implementation is cumbersome. Error handling is bad either. In terms of Web2 you really don't want that feature of logging such extra-data only with a specific Icinga2 comnand pipe version.

The external command pipe is a dead horse, let it rest. Instead I'd suggest to use the Icinga 2 API as command transport, and discuss how those additional fields can be passed into the JSON body for post requests. I'll happily add such "action logging" to it if it makes your life easier.

One thing we should keep in mind - where to store these action logs, first inside the icinga2 memory, then inside the DB IDO history. That should be discussed if it requires schema extensions. Probably a good idea for a history (api) backend.

Kind regards, Michael

K0nne commented 4 years ago

Hello, are there any plans to add this feature to the new IcingaDB?

nilmerg commented 4 years ago

I can't recall any concrete plans, but I've seen intriguing columns in the history tables of the aforementioned types. Take a look for yourselves. :zipper_mouth_face:

nilmerg commented 4 years ago

Actually, since v2.12 Icinga is able to process this just fine with the API. (https://github.com/Icinga/icinga2/pull/7645) Seems to have been forgotten to consider in Icinga DB Web. Should be no problem to get this into the final release. :smiley:

Thanks for chasing this up. :+1: