Bug 1337947

Summary: hammer ignores location-id if organization-id is specified
Product: Red Hat Satellite Reporter: Peter Vreman <peter.vreman>
Component: Organizations and LocationsAssignee: Ivan Necas <inecas>
Status: CLOSED ERRATA QA Contact: Peter Ondrejka <pondrejk>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 6.2.0CC: bbuckingham, bkearney, brubisch, inecas, jcallaha, xdmoon
Target Milestone: UnspecifiedKeywords: Triaged
Target Release: Unused   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-02-21 16:54:17 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:
Bug Depends On:    
Bug Blocks: 1122832    

Description Peter Vreman 2016-05-20 14:06:48 UTC
Description of problem:
Hammer does not check all parameters if multiple filters are supplied, e.g. supplying both organization-id and location-id then the location-id is ignored. There is also no error that both are mutual exclusive.

See below a test that location-id is completly ignored, even the validation check of the id is not done:

[crash] root@li-lc-1578:~# hammer -d -c/opt/hoici/etc/sat6/hammer-hoici.yaml environment list --organization-id=3 --location-id=999999
[ INFO 2016-05-20 14:01:43 Init] Initialization of Hammer CLI (0.5.1.5) has started...
[DEBUG 2016-05-20 14:01:43 Init] Running at ruby 2.2.2-p95
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli_config.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/csv.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/foreman.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/foreman_bootdisk.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/foreman_discovery.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/foreman_docker.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/foreman_remote_execution.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/foreman_tasks.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/gutterball.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/import.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /etc/hammer/cli.modules.d/katello.yml has been loaded
[ INFO 2016-05-20 14:01:43 Init] Configuration from the file /opt/hoici/etc/sat6/hammer-hoici.yaml has been loaded
[DEBUG 2016-05-20 14:01:43 Connection] Registered: foreman
[DEBUG 2016-05-20 14:01:43 API] Global headers: {
        :content_type => "application/json",
              :accept => "application/json;version=2",
    "Accept-Language" => "en"
}
[ INFO 2016-05-20 14:01:44 Modules] Extension module hammer_cli_foreman (0.5.1.4) loaded
[ INFO 2016-05-20 14:01:44 Modules] Extension module hammer_cli_foreman_bootdisk (0.1.3) loaded
[ INFO 2016-05-20 14:01:44 Modules] Extension module hammer_cli_foreman_discovery (0.0.2.1) loaded
[ WARN 2016-05-20 14:01:44 HammerCLIForemanRemoteExecution::JobInvocation] Resource 'job_invocations' does not exist in the API
[ WARN 2016-05-20 14:01:44 HammerCLIForemanRemoteExecution::JobTemplate] Resource 'job_templates' does not exist in the API
[ WARN 2016-05-20 14:01:44 HammerCLIForemanRemoteExecution::TemplateInput] Resource 'template_inputs' does not exist in the API
[ WARN 2016-05-20 14:01:44 HammerCLIForemanRemoteExecution::ForeignInputSet] Resource 'foreign_input_sets' does not exist in the API
[ WARN 2016-05-20 14:01:44 HammerCLIForemanRemoteExecution::RemoteExecutionFeature] Resource 'remote_execution_features' does not exist in the API
[ INFO 2016-05-20 14:01:44 HammerCLI::MainCommand] subcommand organization (HammerCLIForeman::Organization) was removed.
[ INFO 2016-05-20 14:01:44 HammerCLI::MainCommand] subcommand organization (HammerCLIKatello::Organization) was created.
[ INFO 2016-05-20 14:01:45 Modules] Extension module hammer_cli_gutterball (1.0.1) loaded
[ INFO 2016-05-20 14:01:45 Modules] Extension module hammer_cli_import (0.10.23) loaded
[ INFO 2016-05-20 14:01:45 Modules] Extension module hammer_cli_katello (0.0.22.6) loaded
[DEBUG 2016-05-20 14:01:45 Init] Using locale 'en'
[DEBUG 2016-05-20 14:01:45 Init] 'mo' files for locale domain 'hammer-cli' loaded from '/opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.5.1.5/locale'
[DEBUG 2016-05-20 14:01:45 Init] 'mo' files for locale domain 'hammer-cli-foreman' loaded from '/opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli_foreman-0.5.1.4/locale'
[DEBUG 2016-05-20 14:01:45 Init] 'mo' files for locale domain 'hammer_cli_foreman_docker' loaded from '/opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli_foreman_docker-0.0.4/locale'
[DEBUG 2016-05-20 14:01:45 Init] 'mo' files for locale domain 'hammer-cli-katello' loaded from '/opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli_katello-0.0.22.6/locale'
[ INFO 2016-05-20 14:01:45 HammerCLI::MainCommand] Called with options: {"option_debug"=>true, "option_config"=>"/opt/hoici/etc/sat6/hammer-hoici.yaml"}
[ INFO 2016-05-20 14:01:45 HammerCLIForeman::Environment] Called with options: {}
[ INFO 2016-05-20 14:01:45 HammerCLIForeman::Environment::ListCommand] Called with options: {"option_location_id"=>999999, "option_organization_id"=>3}
[ INFO 2016-05-20 14:01:45 API] GET /api/locations/999999/environments
[DEBUG 2016-05-20 14:01:45 API] Params: {
    "organization_id" => 3,
               "page" => 1,
           "per_page" => 1000
}
[DEBUG 2016-05-20 14:01:45 API] Headers: {
    :params => {
        "organization_id" => 3,
                   "page" => 1,
               "per_page" => 1000
    }
}
[DEBUG 2016-05-20 14:01:46 API] Response: {
       "total" => 15,
    "subtotal" => 15,
        "page" => 1,
    "per_page" => 1000,
      "search" => nil,
        "sort" => {
           "by" => nil,
        "order" => nil
    },
     "results" => [
        [ 0] {
            "created_at" => "2016-05-19 20:16:27 UTC",
            "updated_at" => "2016-05-19 20:16:27 UTC",
                  "name" => "KT_Hilti_Library_hg_crash_57",
                    "id" => 29
        },
....
                    :status => "200 OK",
                      :vary => "Accept-Encoding",
          :content_encoding => "gzip",
            :content_length => "510",
              :content_type => "application/json; charset=utf-8"
}
---|---------------------------------------------------
ID | NAME
---|---------------------------------------------------
29 | KT_Hilti_Library_hg_crash_57
...
1  | production
---|---------------------------------------------------


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. Run hammer with both organization-id and location-id=999999
2.
3.

Actual results:
No errors, results for the whole organization returned

Expected results:
- Results that match both organization-id and location-id are returned
- Location-id is checked on validation

Additional info:

Comment 1 Peter Vreman 2016-05-20 14:12:51 UTC
I see that in the API request URL there is a '/api/locations/999999/environments'

That means the errors are:
- Server does not check URL paramters (e.g. location_id) for validity
- Server shall returns empty result as there is no environment associated with location 999999

Comment 2 Peter Vreman 2016-05-20 14:13:48 UTC
Might be related to https://bugzilla.redhat.com/show_bug.cgi?id=1257586 that is for the UI

Comment 5 Ivan Necas 2016-05-25 07:27:09 UTC
Created redmine issue http://projects.theforeman.org/issues/15174 from this bug

Comment 6 Ivan Necas 2016-05-25 07:29:56 UTC
It seems the fix should be trivial, I've opened a pr https://github.com/theforeman/foreman/pull/3549

Comment 7 Bryan Kearney 2016-05-25 08:16:43 UTC
Upstream bug assigned to inecas

Comment 8 Bryan Kearney 2016-05-25 08:16:45 UTC
Upstream bug assigned to inecas

Comment 9 Peter Vreman 2016-05-25 08:25:19 UTC
Patch https://github.com/theforeman/foreman/pull/3549 confirmed to work

[crash] root@li-lc-1578:~# hammer -c/opt/hoici/etc/sat6/hammer-hoici.yaml environment list --organization-id=3 --location-id=999999
Location with id 999999 not found

Comment 10 Bryan Kearney 2016-05-25 16:17:01 UTC
Moving to POST since upstream bug http://projects.theforeman.org/issues/15174 has been closed

Comment 14 Peter Ondrejka 2016-10-24 11:49:16 UTC
Tested in Satellite 6.3 snap 5, various combinations of organization and location settings now resolve correctly when listing environments:

[root@4-example ~]# hammer environment list --organization "Default Organization"
---|----------------------------------------------
ID | NAME                                         
---|----------------------------------------------
6  | in_Deforg                                    
9  | in_deforg_testloc                            
5  | KT_Default_Organization_Library_capsuleview_2
2  | KT_Default_Organization_Library_foo_3        
---|----------------------------------------------
[root@4-example ~]# hammer environment list --organization "Default Organization" --location testloc
---|------------------
ID | NAME             
---|------------------
9  | in_deforg_testloc
---|------------------
[root@4-example ~]# hammer environment list --location testloc
---|------------------
ID | NAME             
---|------------------
9  | in_deforg_testloc
8  | in_testloc       
---|------------------

However, Hammer doesn't complain if organization is specified twice, e.g.:

[root@4-example ~]# hammer environment list --organization "Default Organization" --organization "sillycorp"
---|-------------
ID | NAME        
---|-------------
7  | in_sillycorp

The output could be misunderstood as being in both orgs, while in fact hammer just accepts the last specified org. Ivan, do you prefer a separate BZ for this?

Comment 15 Peter Ondrejka 2016-10-25 08:27:56 UTC
(In reply to Peter Ondrejka from comment #14)
> Ivan, do you prefer a separate BZ for this?

Never mind, I found out there is already for this issue (http://projects.theforeman.org/issues/16206)

Moving to verified as per comment #14

Comment 16 Satellite Program 2018-02-21 16:54:17 UTC
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA.
> 
> For information on the advisory, and where to find the updated files, follow the link below.
> 
> If the solution does not work for you, open a new bug report.
> 
> https://access.redhat.com/errata/RHSA-2018:0336