Red Hat Satellite engineering is moving the tracking of its product development work on Satellite to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "Satellite project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs will be migrated starting at the end of May. If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "Satellite project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/SAT-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1087372 - unify API parameters and return values
Summary: unify API parameters and return values
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: API
Version: Nightly
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: Unspecified
Assignee: Bryan Kearney
QA Contact: sthirugn@redhat.com
URL: http://projects.theforeman.org/issues...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-04-14 09:31 UTC by Tomas Lestach
Modified: 2019-09-26 18:16 UTC (History)
12 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-09-11 12:20:48 UTC
Target Upstream Version:
Embargoed:

Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Foreman Issue Tracker 5178 0 None None None 2016-04-22 16:12:44 UTC

Description Tomas Lestach 2014-04-14 09:31:05 UTC 
Description of problem:
Calling
@api = ApipieBindings::API.new({
  :uri => HammerCLI::Settings.get(:foreman, :host),
  :username => HammerCLI::Settings.get(:foreman, :username),
  :password => HammerCLI::Settings.get(:foreman, :password),
  :api_version => 2
})
@api.resource(entity_type).call(:create, entity_hash)

expects different level of hash encapsulation for different entity_types:

* for entity_type :organizations :
@api.resource(:organizations).call(:create, entity_hash)
the entity_hash is expected to be a simple hash of organization attributes
{:description=>"Imported 'Red Hat IT' organization from Red Hat Satellite 5", :name=>"Red Hat IT"}

* for entity_type :users :
@api.resource(:users).call(:create, entity_hash)
the entity_hash is expected to be a simple hash of user attributes embedded/wrapped into a extra higher level hash with a single :user key:
{:user => user_hash}
{:user=>{:location_ids=>[], :firstname=>"another", :role_ids=>[], :lastname=>"another", :login=>"another", :mail=>"root@localhost", :auth_source_id=>1, :organization_ids=>[5], :password=>"another_gfolvwxr"}}


Similarly the return values of these API calls are not consistent:

* when creating an organization, the return value is an organization hash embedded in a higher level hash with one "organization" key
{"organization" => organization_hash}
{"organization"=>{"apply_info_task_id"=>nil, "updated_at"=>"2014-04-14T09:21:50Z", "description"=>"Imported 'Red Hat IT' organization from Red Hat Satellite 5", "default_info"=>{"system"=>[], "distributor"=>[]}, "id"=>2, "deletion_task_id"=>nil, "owner_auto_attach_all_systems_task_id"=>nil, "ancestry"=>nil, "created_at"=>"2014-04-14T09:21:49Z", "service_level"=>nil, "title"=>"Red Hat IT", "name"=>"Red Hat IT", "label"=>"Red_Hat_IT", "ignore_types"=>[], "service_levels"=>[]}}

* when creating a new user, the API return value is a simple user_hash.
{"locations"=>[], "lastname"=>"another", "firstname"=>"another", "roles"=>[{"name"=>"Anonymous", "id"=>8}], "updated_at"=>"2014-04-14T09:26:32Z", "admin"=>nil, "last_login_on"=>nil, "usergroups"=>[], "organizations"=>[{"title"=>"Red Hat IT", "name"=>"Red Hat IT", "id"=>5}], "created_at"=>"2014-04-14T09:26:32Z", "mail"=>"root@localhost", "id"=>4, "auth_source_id"=>1, "auth_source_name"=>"Internal", "login"=>"another", "auth_source_internal"=>{"type"=>"AuthSourceInternal", "name"=>"Internal", "id"=>1}}


Version-Release number of selected component (if applicable):
sat6-Satellite-6.0.3-RHEL-6-20140404.0-Satellite-x86_64-dvd1.iso


This is a request to unify to API parameter and return value encapsulation.
Comment 1 RHEL Program Management 2014-04-14 09:45:44 UTC 
Since this issue was entered in Red Hat Bugzilla, the release flag has been
set to ? to ensure that it is properly evaluated for this release.
Comment 3 Tom McKay 2014-04-14 13:32:55 UTC 
Created redmine issue http://projects.theforeman.org/issues/5178 from this bug
Comment 4 Adam Price 2014-05-12 13:42:41 UTC 
@tlestach, what are you looking at that's describing what's expected? thanks!

i agree this should be fixed, but it should be fixed in the documented places as well as just what the code accepts.

thanks!
Comment 5 David Davis 2014-05-12 13:59:58 UTC 
For the parameters, foreman is requiring POST/PUT calls to be nested (users) while Katello isn't (organizations). The v2 spec examples say that the attributes should NOT be nested:

http://theforeman.org/manuals/1.5/index.html#5.1.1CRUDRequestExamples


Regarding what's being returned for organizations and users, both of those are coming from foreman so I don't know what's going on there.
Comment 6 Tomas Lestach 2014-05-13 07:26:19 UTC 
(In reply to Adam Price from comment #4)
> @tlestach, what are you looking at that's describing what's expected? thanks!

Expected is to unify the parameters and return values - either to choose the nested variant or the not nested - and use it globally - for the whole API interface.
My personal preference would be to use the not nested version.

> i agree this should be fixed, but it should be fixed in the documented
> places as well as just what the code accepts.

Sure.
Comment 7 jmagen@redhat.com 2014-05-13 13:22:00 UTC 
submitted fix at https://github.com/theforeman/foreman/pull/1441
Comment 9 Adam Saleh 2014-05-26 14:02:44 UTC 
Fix still not merged, the scope of the bug is unclear (some of the foreman issues seem to reference just /users endpoint, other suggests larger rewrite)
Comment 11 David Davis 2014-06-17 15:46:47 UTC 
Technically the second part of this bug (nested hashes with root nodes in JSON) is still an issue. I opened a separate bug for it though:

https://bugzilla.redhat.com/show_bug.cgi?id=1109964
Comment 13 Bryan Kearney 2014-07-15 16:01:53 UTC 
Moving to POST since upstream bug http://projects.theforeman.org/issues/5178 has been closed
-------------
Joseph Magen
PR 4181 is docs only, so it's not related to this.

This bug was caused by password [filtered] attributes not being wrapped, so POST /users didn't work unless the Hash was wrapped. Now it should work.

https://github.com/theforeman/foreman/pull/1441
-------------
Joseph Magen
Applied in changeset commit:c980e9a8917cc9fdee5835732428462e22fa3f5b.
Comment 16 sthirugn@redhat.com 2014-08-25 19:26:17 UTC 
In Snap6, this is what I get calling org and user create apis:

Org:
# curl -X POST -H "Accept:application/json" -H "Content-Type: application/json" -u admin:changeme --insecure -d '{"name":"testorgapiAug25"}' https://sat6host/katello/api/v2/organizations

{"ancestry":null,"apply_info_task_id":null,"created_at":"2014-08-25T18:11:45Z","default_info":{"system":[],"distributor":[]},"description":null,"id":11,"ignore_types":[],"katello_default":true,"label":"testorgapiAug25","name":"testorgapiAug25","title":"testorgapiAug25","updated_at":"2014-08-25T18:11:46Z","service_levels":[],"service_level":null}


User:
#curl -X POST -H "Accept:application/json" -H "Content-Type: application/json" -u admin:changeme --insecure -d '{"user":{"login":"testuseraug25","mail":"test","password":"testuseraug25","auth_source_id":"1"}}' https://sat6host.redhat.com/katello/api/v2/users

{"id":6,"login":"testuseraug25","firstname":null,"lastname":null,"mail":"test","admin":null,"auth_source_id":1,"auth_source_name":"Internal","last_login_on":null,"created_at":"2014-08-25T19:19:53Z","updated_at":"2014-08-25T19:19:53Z","default_location":null,"default_organization":null,"locations":[],"organizations":[],"auth_source_internal":{"id":1,"type":"AuthSourceInternal","name":"Internal"},"roles":[{"name":"Anonymous","id":11}],"usergroups":[]}
Comment 17 sthirugn@redhat.com 2014-08-25 19:35:02 UTC 
Verified as per my Comment 16 above.

Version Tested:
GA Snap 6 - Satellite-6.0.4-RHEL-6-20140820.1

* apr-util-ldap-1.3.9-3.el6_0.1.x86_64
* candlepin-0.9.23-1.el6_5.noarch
* candlepin-common-1.0.1-1.el6_5.noarch
* candlepin-scl-1-5.el6_4.noarch
* candlepin-scl-quartz-2.1.5-5.el6_4.noarch
* candlepin-scl-rhino-1.7R3-1.el6_4.noarch
* candlepin-scl-runtime-1-5.el6_4.noarch
* candlepin-selinux-0.9.23-1.el6_5.noarch
* candlepin-tomcat6-0.9.23-1.el6_5.noarch
* elasticsearch-0.90.10-6.el6sat.noarch
* foreman-1.6.0.41-1.el6sat.noarch
* foreman-compute-1.6.0.41-1.el6sat.noarch
* foreman-gce-1.6.0.41-1.el6sat.noarch
* foreman-libvirt-1.6.0.41-1.el6sat.noarch
* foreman-ovirt-1.6.0.41-1.el6sat.noarch
* foreman-postgresql-1.6.0.41-1.el6sat.noarch
* foreman-proxy-1.6.0.29-1.el6sat.noarch
* foreman-selinux-1.6.0.7-1.el6sat.noarch
* foreman-vmware-1.6.0.41-1.el6sat.noarch
* katello-1.5.0-29.el6sat.noarch
* katello-ca-1.0-1.noarch
* katello-certs-tools-1.5.6-1.el6sat.noarch
* katello-installer-0.0.60-1.el6sat.noarch
* openldap-2.4.23-34.el6_5.1.x86_64
* openldap-devel-2.4.23-34.el6_5.1.x86_64
* pulp-katello-0.3-3.el6sat.noarch
* pulp-nodes-common-2.4.0-0.30.beta.el6sat.noarch
* pulp-nodes-parent-2.4.0-0.30.beta.el6sat.noarch
* pulp-puppet-plugins-2.4.0-0.30.beta.el6sat.noarch
* pulp-puppet-tools-2.4.0-0.30.beta.el6sat.noarch
* pulp-rpm-plugins-2.4.0-0.30.beta.el6sat.noarch
* pulp-selinux-2.4.0-0.30.beta.el6sat.noarch
* pulp-server-2.4.0-0.30.beta.el6sat.noarch
* python-ldap-2.3.10-1.el6.x86_64
* ruby193-rubygem-net-ldap-0.3.1-3.el6sat.noarch
* ruby193-rubygem-runcible-1.1.0-2.el6sat.noarch
Comment 18 Tomas Lestach 2014-08-26 08:06:43 UTC 
Checking encapsulation for input parameters:

* POST /katello/api/organizations now really expects individual parameters, not an organization hash
https://<sat6-fqdn>/apidoc/v2/organizations/create.html
- correct

* POST /api/users still expects user hash, not individual parameters
https://<sat6-fqdn>/apidoc/v2/users/create.html
- still an issue

* checking just API documentation of create APIs, where a hash is expected:
 - POST /api/architectures 
 - POST /api/auth_source_ldaps
 - POST /api/bookmarks 
 - POST /api/common_parameters 
 - POST /api/compute_profiles 
 - POST /api/compute_resources 
 - POST /api/config_groups 
 - POST /api/config_templates 
 - POST /api/domains 
 - POST /api/environments 
 - POST /api/filters 
 - POST /api/hostgroups 
 - POST /api/hosts 
 - POST /api/locations 
 - POST /api/media 
 - POST /api/models 
 - POST /api/operatingsystems 
 - POST /api/ptables 
 - POST /api/puppetclasses 
 - POST /api/realms 
 - POST /api/reports 
 - POST /api/roles 
 - POST /api/smart_proxies 
 - POST /api/smart_variables 
 - POST /api/subnets 
 - POST /api/usergroups 
 - and POST /api/users
(I see 27 APIs, where the extra encapsulation is still an issue)

Encapsulation of return values:
* organization isn't encapsulated in an extra organization hash for the create API - correct
(not sure about the rest, it up to QA to verify that)

seen on compose: Satellite-6.0.4-RHEL-6-20140823.0-Satellite-x86_64
Comment 19 sthirugn@redhat.com 2014-08-27 15:12:03 UTC 
Failed with respect to Comment 18.

Version Tested:
GA Snap 6 - Satellite-6.0.4-RHEL-6-20140820.1
Comment 20 Dominic Cleal 2014-08-27 15:17:49 UTC 
Regarding API documentation, this should be regarded as WONTFIX (http://projects.theforeman.org/issues/4181 upstream).

The API will support unwrapped or wrapped as inputs (which is the subject of this bug), Foreman's API documentation will remain wrapped (not the subject of this bug).
Comment 23 jmagen@redhat.com 2014-09-01 07:45:02 UTC 
I agree with Dominic per comment #20. It's an issue that has been discussed and agree that Foreman's API docs will remain wrapped (i.e. require a hash) for POST/PUT, but show unwrapped for GET requests.
Comment 24 Tomas Lestach 2014-09-01 09:17:38 UTC 
I see, both approaches (unwrapped and wrapped parameter variant) get accepted for POST /api/users:
(Satellite-6.0.4-RHEL-6-20140823.0-Satellite-x86_64)

1.)
$ curl -s -H "Content-Type:application/json" -H "Accept:application/json,version=2" -k -u admin:changeme -X POST -d'{"login":"new_user", "mail":"root@localhost", "password":"secret", "auth_source_id":1}' https://$(hostname)/api/users
{"id":14,"login":"new_user","firstname":null,"lastname":null,"mail":"root@localhost","admin":null,"auth_source_id":1,"auth_source_name":"Internal","last_login_on":null,"created_at":"2014-09-01T08:59:24Z","updated_at":"2014-09-01T08:59:24Z","default_location":null,"default_organization":null,"locations":[],"organizations":[],"auth_source_internal":{"id":1,"type":"AuthSourceInternal","name":"Internal"},"roles":[{"name":"Anonymous","id":11}],"usergroups":[]}

2.)
$ curl -s -H "Content-Type:application/json" -H "Accept:application/json,version=2" -k -u admin:changeme -X POST -d'{"user":{"login":"new_user2", "mail":"root@localhost", "password":"secret", "auth_source_id":1}}' https://$(hostname)/api/users
{"id":15,"login":"new_user2","firstname":null,"lastname":null,"mail":"root@localhost","admin":null,"auth_source_id":1,"auth_source_name":"Internal","last_login_on":null,"created_at":"2014-09-01T09:03:33Z","updated_at":"2014-09-01T09:03:33Z","default_location":null,"default_organization":null,"locations":[],"organizations":[],"auth_source_internal":{"id":1,"type":"AuthSourceInternal","name":"Internal"},"roles":[{"name":"Anonymous","id":11}],"usergroups":[]}


Let's hope the behavior is the same for all the other entities.
As it was decided to keep API documentation as is, I'll respect that. :-)

I'm ok with switching the BZ to VERIFIED.
Comment 25 Daniel Lobato Garcia 2014-09-01 14:46:54 UTC 
Agreed with comment #20 too, either move to WONTFIX or VERIFIED.
Comment 26 sthirugn@redhat.com 2014-09-02 13:33:28 UTC 
Verified - As per Comment 17 through Comment 25 above.
Comment 27 Bryan Kearney 2014-09-11 12:20:48 UTC 
This was delivered with Satellite 6.0 which was released on 10 September 2014.
Note You need to log in before you can comment on or make changes to this bug.