Fixes #38391 - Allow displaying all host interfaces through API
Currently, when you add an interfce to an existing host, for example with
hammer host interface create --host <hostname> --identifier test --managed false
and the try to GET request to display the interfaces with
curl --request GET --insecure --user admin:changeme https://`hostname -f`/api/v2/hosts/10/enc
you get an error about undefined method to_export. For details see the linked issue.
When creating interface with --managed false, the default class of the interface seems to be Nic::Base. Including the module exportable in the superclass gets rid of the error, the unmanaged interface is instead displayed as nil in the report. It should be possible to designate displayable attributes with attr_exportable if we agree that this would bring value to the report.
@ofedoren thank you for the feedback. I found out that methods in hosts controller allow nil interface type, which seems strange to me. The issue is fixed when I hardcode in the default type. I wonder if we should go with this approach or instead make type a required parameter. I suspect that the latter could possibly break workflows of some users. There is also question of what we should do with existing nil type interfaces. Would it be safe to change the type with a migration?
Thanks, @adamlazik1, but I still think there is something off:
#4833 was meant to fix an issue where we allowed to update the type of already created interface. This should remain. But at the same time it introduced
allow_nil_type, which we are now trying to forbid 'cause it leads to eithernilorNic::Basetype. With this PR we are going to makeNic::Managedto be the actual default type. I'd say we need to dropallow_nil_type.
allow_nil_typeis essentiallyhost.nil?, which seems to mean that we allow nil type in case the host is just being created. But even in the API docs we say it will be the default type if not provided 🤷.Since we won't allow neithier nil nor base, we also need to update this line: https://github.com/theforeman/foreman/blob/develop/app/models/nic/base.rb#L349
Could we also add a test to simulate the issue is or at least is described in the RM ticket?
Sure, I removed allow_nil_type and and added a test to ensure that a default interface type is being assigned.
Thank you for reminding me. Updated the commit message.