Bug 1087372
| Summary: | unify API parameters and return values | ||
|---|---|---|---|
| Product: | Red Hat Satellite | Reporter: | Tomas Lestach <tlestach> |
| Component: | API | Assignee: | Bryan Kearney <bkearney> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | sthirugn <sthirugn> |
| Severity: | high | Docs Contact: | |
| Priority: | high | ||
| Version: | Nightly | CC: | adprice, asaleh, cwelton, daviddavis, dcleal, dlobatog, erezende, jmagen, jmontleo, mmccune, sthirugn, tlestach |
| Target Milestone: | Unspecified | ||
| Target Release: | Unused | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| URL: | http://projects.theforeman.org/issues/5178 | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2014-09-11 12:20:48 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
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. Created redmine issue http://projects.theforeman.org/issues/5178 from this bug @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! 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. (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. submitted fix at https://github.com/theforeman/foreman/pull/1441 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) 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 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. 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":[]}
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 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 Failed with respect to Comment 18. Version Tested: GA Snap 6 - Satellite-6.0.4-RHEL-6-20140820.1 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). 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. 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.
Agreed with comment #20 too, either move to WONTFIX or VERIFIED. Verified - As per Comment 17 through Comment 25 above. This was delivered with Satellite 6.0 which was released on 10 September 2014. |
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.