Project

General

Profile

Bug #27340

Attribute set Rails relation validators cause discovery to fail

Added by Lukas Zapletal about 1 year ago. Updated 12 months ago.

Status:
Closed
Priority:
Normal
Category:
Discovery plugin
Target version:

Description

The initial discovery upload fails with 422 ActiveRecord::RecordNotSaved: Failed to save the new associated discovery_attribute_set. The problem is that Rails presence validator errors out when host_id is not set when discovered host is not yet saved. This happens because update call calls save when host_id is not yet set.

As a consequence, discovery_attribute_set is not set however discovered host is saved with null reference, therefore subsequent discovery fact upload already has host.id set therefore this should save. Unfortunately due to the other validator (:discovery_attribute_set, :presence => true) it won't because then it fails with a different error 422 Validation failed: Discovery attribute set can't be blank.

This patch removes the unnecessary validators because the code is ready today that attributes association can be blank. This was added because when discovery fails, it is set to blank anyway. This could be prevented by utilizing SQL transactions, however we've been there and it caused huge pains in our orchestration and it was reverted. I left a note about this in the codebase.

Associated revisions

Revision daaa178a (diff)
Added by Lukas Zapletal about 1 year ago

Fixes #27340 - attribute set validators removed

Revision cfb86fa4 (diff)
Added by Lukas Zapletal about 1 year ago

Refs #27340 - fixed broken unit tests

Revision 02e0f3c7 (diff)
Added by Ondřej Ezr about 1 year ago

Refs #27340 - fix integration tests

Author: Ondřej Ezr <>

Revision 2156100b (diff)
Added by Lukas Zapletal about 1 year ago

Refs #27340 - fixed begin-rescue in a unit test

History

#1 Updated by The Foreman Bot about 1 year ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman_discovery/pull/482 added

#2 Updated by The Foreman Bot about 1 year ago

  • Fixed in Releases Discovery Plugin 16.0 added

#3 Updated by Anonymous about 1 year ago

  • Status changed from Ready For Testing to Closed

#4 Updated by Lukas Zapletal about 1 year ago

  • Target version changed from Discovery Plugin 16.0 to Discovery Plugin 15.0
  • Fixed in Releases Discovery Plugin 15.0.1 added
  • Fixed in Releases deleted (Discovery Plugin 16.0)

#5 Updated by The Foreman Bot about 1 year ago

  • Pull request https://github.com/theforeman/foreman_discovery/pull/484 added

#6 Updated by The Foreman Bot 12 months ago

  • Pull request https://github.com/theforeman/foreman_discovery/pull/488 added

Also available in: Atom PDF