Project

General

Profile

Bug #25331

Fact imports erranously cause audits to be created

Added by Timo Goebel 22 days ago. Updated 18 days ago.

Status:
Closed
Priority:
High
Category:
Audit Log
Target version:
-

Description

For very fact import an audit record is written although nothing changes.

#<Audited::Audit id: 8238963, auditable_id: 30056, auditable_type: "Nic::Managed", user_id: 4, user_type: nil, username: "API Admin", action: "update", audited_changes: {"attrs"=>[{"mtu"=>"1500", "netmask"=>"255.255.224.0", "netmask6"=>"ffff:ffff:ffff:ffff::", "network"=>"172.22.0.0", "network6"=>"fe80::"}, {"mtu"=>1500, "netmask"=>"255.255.224.0", "netmask6"=>"ffff:ffff:ffff:ffff::", "network"=>"172.22.0.0", "network6"=>"fe80::"}]}, version: 7, comment: nil, associated_id: 23973, associated_type: "Host::Base", request_uuid: "7263349c-f216-415a-8721-f7464856031e", created_at: "2018-10-29 10:08:25", remote_address: "1.2.3.4", auditable_name: "xxxxx...", associated_name: "xxxx...">

Related issues

Related to Foreman - Feature #25364: Disable all auditing during high-load client-initiated API callsReady For Testing

Associated revisions

Revision 6e9be602 (diff)
Added by Timo Goebel 18 days ago

fixes #25331 - do not audit fact import

History

#1 Updated by Lukas Zapletal 22 days ago

  • Priority changed from Normal to High
  • Status changed from New to Need more information

Timo, thanks. Is this per fact upload or per single change of a fact? The latter looks like important issue for performance.

#2 Updated by Timo Goebel 22 days ago

  • Status changed from Need more information to New

This is per fact upload as far as I traced it.

#3 Updated by Timo Goebel 22 days ago

I have slowed down the effect by sorting the hash in /usr/share/foreman/app/models/host/base.rb

-iface.attrs = attributes
+iface.attrs = attributes.sort.to_h

The diff in the example above is that mtu seems to toggle between integer and string.

Tomer proposed that we don't write any audits at all during fact import. While that might be a good approach we should also not update the interface unless something really changed.

#4 Updated by The Foreman Bot 22 days ago

  • Assignee set to Lukas Zapletal
  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/6178 added

#5 Updated by The Foreman Bot 22 days ago

  • Pull request https://github.com/theforeman/foreman/pull/6179 added

#6 Updated by Lukas Zapletal 21 days ago

  • Triaged changed from No to Yes

#7 Updated by The Foreman Bot 21 days ago

  • Pull request https://github.com/theforeman/foreman/pull/6182 added

#8 Updated by Tomer Brisker 20 days ago

  • Fixed in Releases 1.19.1, 1.20.0 added

#9 Updated by Tomer Brisker 19 days ago

  • Subject changed from for every fact import an audit record is written to Fact imports erranously cause audits to be created

clarifying title for the release note.

#10 Updated by Lukas Zapletal 19 days ago

  • Related to Feature #25364: Disable all auditing during high-load client-initiated API calls added

#11 Updated by The Foreman Bot 19 days ago

  • Pull request https://github.com/theforeman/foreman/pull/6198 added

#12 Updated by Timo Goebel 18 days ago

  • Status changed from Ready For Testing to Closed

Also available in: Atom PDF