panko-api --limit flag misbehaved when using non-admin user

[Tzu-Chia Wang]

OS: Ubuntu 16.04.3
Openstack vesion: Ocata
Panko version: stable/ocata, master(Jul 28, 2017)
Event storage backends: MariaDB

Problem:

  $ ceilometer event-list --limit 5
  $ openstack event list --limit 5

  both commands above with environment variables:

     export OS_PROJECT_NAME=demo
     export OS_USERNAME=demo

  gave back only one event.

  I had checked in the database that the events are there and if the number
  given to limit flag was
  large enough, more events would show up.

Self Examination:

  First, adding 'admin' user to the 'demo' project and using environment
  variables:

     export OS_PROJECT_NAME=demo
     export OS_USERNAME=admin

  gave back correct results with 5 events.

  So, I went into source files and found that the problem might reside in
  the file:

     panko/storage/impl_sqlalchemy.py

  I checked the mysql query spawn in the function 'get_event'.

  when using 'admin' as the user, the query is:

    SELECT event.id AS event_id
    FROM event JOIN event_type ON event_type.id = event.event_type_id
    WHERE EXISTS (SELECT *
    FROM (SELECT anon_2.trait_text_event_id AS trait_text_event_id
    FROM (SELECT trait_text.event_id AS trait_text_event_id
    FROM trait_text
    WHERE NOT (EXISTS (SELECT *
    FROM (SELECT trait_text.event_id AS event_id
    FROM trait_text
    WHERE trait_text.key = :key_1) AS anon_3
    WHERE trait_text.event_id = anon_3.event_id)) UNION
    SELECT trait_text.event_id AS trait_text_event_id
    FROM trait_text, event
    WHERE trait_text.key = :key_2 AND trait_text.value = :value_1
    AND event.id = trait_text.event_id) AS anon_2) AS anon_1
    WHERE event.id = anon_1.trait_text_event_id) ORDER BY event.generated ASC,
    event.message_id ASC
    LIMIT :param_1

  when using 'demo' as the user, the query is:

    SELECT event.id AS event_id
    FROM event JOIN event_type ON event_type.id = event.event_type_id JOIN
    (SELECT trait_text.event_id AS ev_id
    FROM trait_text, (SELECT trait_text.event_id AS ev_id
    FROM trait_text
    WHERE trait_text.key = :key_1 AND trait_text.value = :value_1)
    AS anon_2, (SELECT trait_text.event_id AS ev_id
    FROM trait_text
    WHERE trait_text.key = :key_2 AND trait_text.value = :value_2) AS anon_3
    WHERE trait_text.key = :key_1 AND trait_text.value = :value_1 AND
    anon_2.ev_id = anon_3.ev_id)
    AS anon_1 ON anon_1.ev_id = event.id ORDER BY event.generated ASC,
    event.message_id ASC
    LIMIT :param_1

  I execute the query directly in MariaDB CLI, and get back list with tons of
  duplicate event IDs.
  So I simply put the keyword 'DISTINCT' after the first 'SELECT':

    SELECT DISTINCT event.id AS event_id

  then the command gave back the correct result with 5 events.

  Therefore, I'm guessing that the problem resides between line 292-307 of
  the file.

  ( https://github.com/openstack/panko/blob/master/panko/storage/impl_sqlalchemy.py#L292-L307 )

  This part is what makes two queries using user 'admin and 'demo' different.

[SU, HAO-CHEN (astacksu)]

I have tried to modify the code in this file.
Currently I find it working correctly.

[SU, HAO-CHEN (astacksu)]

this one is the patch file for the above change

[SU, HAO-CHEN (astacksu)]

https://review.openstack.org/#/c/494772/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/494772
Committed: https://git.openstack.org/cgit/openstack/panko/commit/?id=99d591df950271594ee049caa3ff22304437a228
Submitter: Jenkins
Branch: master

commit 99d591df950271594ee049caa3ff22304437a228
Author: astacksu <email address hidden>
Date: Fri Aug 18 10:17:19 2017 +0800

    Fix api's event listing query to sqlalchemy with non-admin user

    Closes-Bug: #1710812

    Original problem:

      Event storage backend is sqlalchemy(mariaDB).
      When listing events by:

             openstack event list [--limit n]
         or ceilometer event-list [--limit n]

      using non-admin user, the number of events returned does not match the
      limit 'n'. It has much fewer events than it should have. Even when
      limit is not given, the default limit (100 if not changed) will apply
      and that the returned result is still wrong.

    Problems in code:

      In the file 'panko/storage/impl_sqlalchemy.py
      The subquery for trait filtering(project_id, user_id mostly) which
      generated in function 'get_events' into variable 'trait_subq' (start
      around line 269) does not seem behave correctly.

      In the original code, the subquery is being wrapped into a deeper
      layered subquery when a new trait is applied. The resulting
      complicated subquery makes results have duplicate 'event_id's. And
      duplicate ids still count for the amount of the result, resulting in
      much fewer number of events than the one 'limit' flag sets.

    Fixing:

      This part(line 269-299) which generates the subquery is modified.
      For each trait, the table the trait is in is adding to the 'from' part
      of the query. As for the 'where' section, the trait is filtered, and
      the event_id should be the same as other ones.

      The function at the beginning is changed from '_build_trait_query' to
      '_get_model_and_conditions' which used by the part mentioned above.

      'sqlalchemy.orm.aliased' is imported to aliase tables since the same
      table may be selected multiple times, and it is required to apply
      'aliased' to the table model before adding them to the query.

    Test:

      $ tox -e py27-mysql

      The difference is at function 'test_get_event_multiple_trait_filter'
      in file '/tests/functional/storage/test_storage_scenarios.py'.
      The order of the traits to filter can affect how the original code
      produce the subquery, which is the cause of duplicate event_id.

    Patch:

      1. Fix coding style according to Julien Danjou's suggestion.

      2. Add test code. Actually, modify existing one.

    Change-Id: Ic303e4ed7af43c1d31559895c56a29b2c5ea85fc

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/panko 4.0.0 release.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/542142

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/542142
Committed: https://git.openstack.org/cgit/openstack/panko/commit/?id=4d238c29b749e33807935206dadcc64479421e59
Submitter: Zuul
Branch: stable/pike

commit 4d238c29b749e33807935206dadcc64479421e59
Author: astacksu <email address hidden>
Date: Fri Aug 18 10:17:19 2017 +0800

    Fix api's event listing query to sqlalchemy with non-admin user

    Closes-Bug: #1710812

    Original problem:

      Event storage backend is sqlalchemy(mariaDB).
      When listing events by:

             openstack event list [--limit n]
         or ceilometer event-list [--limit n]

      using non-admin user, the number of events returned does not match the
      limit 'n'. It has much fewer events than it should have. Even when
      limit is not given, the default limit (100 if not changed) will apply
      and that the returned result is still wrong.

    Problems in code:

      In the file 'panko/storage/impl_sqlalchemy.py
      The subquery for trait filtering(project_id, user_id mostly) which
      generated in function 'get_events' into variable 'trait_subq' (start
      around line 269) does not seem behave correctly.

      In the original code, the subquery is being wrapped into a deeper
      layered subquery when a new trait is applied. The resulting
      complicated subquery makes results have duplicate 'event_id's. And
      duplicate ids still count for the amount of the result, resulting in
      much fewer number of events than the one 'limit' flag sets.

    Fixing:

      This part(line 269-299) which generates the subquery is modified.
      For each trait, the table the trait is in is adding to the 'from' part
      of the query. As for the 'where' section, the trait is filtered, and
      the event_id should be the same as other ones.

      The function at the beginning is changed from '_build_trait_query' to
      '_get_model_and_conditions' which used by the part mentioned above.

      'sqlalchemy.orm.aliased' is imported to aliase tables since the same
      table may be selected multiple times, and it is required to apply
      'aliased' to the table model before adding them to the query.

    Test:

      $ tox -e py27-mysql

      The difference is at function 'test_get_event_multiple_trait_filter'
      in file '/tests/functional/storage/test_storage_scenarios.py'.
      The order of the traits to filter can affect how the original code
      produce the subquery, which is the cause of duplicate event_id.

    Patch:

      1. Fix coding style according to Julien Danjou's suggestion.

      2. Add test code. Actually, modify existing one.

    Change-Id: Ic303e4ed7af43c1d31559895c56a29b2c5ea85fc
    (cherry picked from commit 99d591df950271594ee049caa3ff22304437a228)