Project

General

Profile

Bug #25188

API hosts/X/facts still inconsistent

Added by Martin Bacovsky about 1 month ago. Updated about 1 month ago.

Status:
Closed
Priority:
High
Category:
API
Target version:
Difficulty:
Triaged:
Yes
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

This issue is related to https://projects.theforeman.org/issues/20891 and its fix.

The original inconsistency was partly fixed, but the API call is still not consistent and makes processing of the data more complex (https://projects.theforeman.org/issues/25167)

Current state:

Facts:

# curl  "-HContent-Type: application/json" "-d{\"per_page\":5}" -XGET http://localhost:3000/api/v2/hosts/4/facts
{
  "error": {"message":"Unable to authenticate user "}
}
[root@centos7-luna-devel ~]# curl -u admin:changeme "-HContent-Type: application/json" "-d{\"per_page\":5}" -XGET http://localhost:3000/api/v2/hosts/4/facts
{
  "total": 153,
  "subtotal": 6,
  "page": 1,
  "per_page": 5,
  "search": " host = 4",
  "sort": {
    "by": null,
    "order": null
  },
  "results": {"centos7.pichi.example.com":{"dmi":null,"distribution":null,"cpu":null,"dmi::chassis":null,"dmi::bios":null},"dmi":null,"distribution":null,"cpu":null,"dmi::chassis":null,"dmi::bios":null}
}

Interfaces:

# curl -u admin:changeme "-HContent-Type: application/json" "-d{\"per_page\":5}" -XGET http://localhost:3000/api/v2/hosts/4/interfaces
{
  "total": 1,
  "subtotal": 1,
  "page": 1,
  "per_page": 5,
  "search": null,
  "sort": {
    "by": null,
    "order": null
  },
  "results": [{"subnet_id":1,"subnet_name":"pichi.example.com","subnet6_id":null,"subnet6_name":null,"domain_id":1,"domain_name":"pichi.example.com","created_at":"2018-10-08 23:18:59 UTC","updated_at":"2018-10-08 23:20:47 UTC","managed":true,"identifier":"eth0","id":4,"name":"centos7.pichi.example.com","ip":"192.168.121.172","ip6":null,"mac":"52:54:00:c8:6b:01","mtu":1500,"fqdn":"centos7.pichi.example.com","primary":true,"provision":true,"type":"interface","virtual":false}]
}
As can be seen on the example above the facts are still not consistent and has new problems:
  • to be consistent results needs to be list (one item per fact) - currently hash
  • the number of items needs to match the metadata
  • due to effort to keep the API backward compatible two types of data are now mixed in the hash - { Host => Facts } and { Fact name => Fact value }. This makes processing of the API response difficult as it is not easy to distinguish what is what. Can we assume Fact Values can't be hashes? Facts are usually processed in a loop as it has not predefined names so currently we need to filter out the unwanted content

Proposed solution:

it seems we can not provide consistent (list) yet compatible {hash} response. So I'd propose to revert the changes and either leave it for API v3 or add new endpoint e.g. hosts/:id/fact_list and mark hosts/:id/facts as deprecated in the API docs.


Related issues

Related to Hammer CLI - Bug #25167: Hammer error: Error: undefined method `collect’ forResolved
Related to Foreman - Bug #20891: API hosts/X/facts inconsistent and superfluous hostname hash in results responseClosed2017-09-11

Associated revisions

Revision f5a53ca5 (diff)
Added by Martin Bacovsky about 1 month ago

Fixes #25188 - Remove unbound facts from /hosts/x/facts

Revert "Fixes #20891 - Remove hostname from /hosts/x/facts results"
This reverts commit 8ebbbecf36922c7a1f1c27610ddb1d14fe0467fa.

The previous fix made the API results backward incompatible
and difficult to parse and had to be reverted.

History

#1 Updated by Martin Bacovsky about 1 month ago

  • Related to Bug #25167: Hammer error: Error: undefined method `collect’ for added

#2 Updated by Martin Bacovsky about 1 month ago

  • Related to Bug #20891: API hosts/X/facts inconsistent and superfluous hostname hash in results response added

#3 Updated by Lukas Zapletal about 1 month ago

  • Triaged changed from No to Yes

#4 Updated by The Foreman Bot about 1 month ago

  • Assignee set to Martin Bacovsky
  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/6143 added

#5 Updated by Tomer Brisker about 1 month ago

  • Fixed in Releases 1.19.1, 1.20.0 added

#6 Updated by Martin Bacovsky about 1 month ago

  • Status changed from Ready For Testing to Closed

Also available in: Atom PDF