ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-5648

Zabbix API item.update doesn't update item 'name' parameter.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.0.4rc1, 2.1.0
    • Fix Version/s: 2.0.4rc1, 2.1.0
    • Component/s: API (A)
    • Labels:

      Description

      Hi!

      I tried to update Item params with item.update method of zabbix api, all params has changed succesfully, but 'name' field hasn't updated.
      Here is my json requests to zabbix:

      10: json_obj: {'params': {'itemid': u'30917', 'key_': 'failcounter[parts5.time_median]', 'name': 'Failcounter: \xd0\xa1\xd1\x82\xd0\xb0\xd1\x82. \xd1\x81\xd1\x80\xd0\xb5\xd0\xb4. \xd0\xb2\xd1\x80\xd0\xb5\xd0\xbc\xd1\x8f \xd0\xbe\xd1\x82\xd0\xb2\xd0\xb5\xd1\x82\xd0\xb0 \xd0\xbf\xd1\x80\xd0\xbe\xd0\xb5\xd0\xba\xd1\x82\xd0\xb0  parts5', 'description': 'Failcounter: \xd0\xa1\xd1\x82\xd0\xb0\xd1\x82. \xd1\x81\xd1\x80\xd0\xb5\xd0\xb4. \xd0\xb2\xd1\x80\xd0\xb5\xd0\xbc\xd1\x8f \xd0\xbe\xd1\x82\xd0\xb2\xd0\xb5\xd1\x82\xd0\xb0 \xd0\xbf\xd1\x80\xd0\xbe\xd0\xb5\xd0\xba\xd1\x82\xd0\xb0  parts5', 'delay': 300, 'trends': 30, 'value_type': 0, 'templateid': u'10307', 'type': 0, 'history': 14}, 'jsonrpc': '2.0', 'method': 'item.update', 'auth': u'2dcaf3d694a7aea4c656549728c6009e', 'id': 217}
      20: Sending: {"params": {"itemid": "30917", "key_": "failcounter[parts5.time_median]", "name": "Failcounter: \u0421\u0442\u0430\u0442. \u0441\u0440\u0435\u0434. \u0432\u0440\u0435\u043c\u044f \u043e\u0442\u0432\u0435\u0442\u0430 \u043f\u0440\u043e\u0435\u043a\u0442\u0430  parts5", "description": "Failcounter: \u0421\u0442\u0430\u0442. \u0441\u0440\u0435\u0434. \u0432\u0440\u0435\u043c\u044f \u043e\u0442\u0432\u0435\u0442\u0430 \u043f\u0440\u043e\u0435\u043a\u0442\u0430  parts5", "delay": 300, "trends": 30, "value_type": 0, "templateid": "10307", "type": 0, "history": 14}, "jsonrpc": "2.0", "method": "item.update", "auth": "2dcaf3d694a7aea4c656549728c6009e", "id": 217}
      10: Sending headers: {'Content-Type': 'application/json-rpc', 'User-Agent': 'python/zabbix_api'}
      20: Response Code: 200
      10: Response Body: {u'jsonrpc': u'2.0', u'result': {u'itemids': [u'30917']}, u'id': 217}
      

      I'm using python api for making requests. Item 'failcounter[parts5.time_median]' is templated item, and it belongs to template 'Template_App_failcounter".

        Activity

        Hide
        Oleg Egorov added a comment - - edited

        RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648 r30619

        Show
        Oleg Egorov added a comment - - edited RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648 r30619
        Hide
        Toms added a comment - - edited

        (1) This check should be performed in CItemGeneral::CheckInput(), to support check for discovery items, and item prototypes

        Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648 r30680

        Toms CLOSED. Review my changes in r30688, if OK merge.

        Show
        Toms added a comment - - edited (1) This check should be performed in CItemGeneral::CheckInput(), to support check for discovery items, and item prototypes Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648 r30680 Toms CLOSED. Review my changes in r30688, if OK merge.
        Hide
        Toms added a comment -

        TESTED

        Show
        Toms added a comment - TESTED
        Hide
        Oleg Egorov added a comment -

        Fixed in 2.0.4 r30708, 2.1.0 r30709

        Show
        Oleg Egorov added a comment - Fixed in 2.0.4 r30708, 2.1.0 r30709
        Hide
        Pavels Jelisejevs added a comment - - edited

        (1) This fix must be documented in the 2.0 API changelog.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs CLOSED.

        Pavels Jelisejevs Don't forget to document the remaining fixes when you're done.

        Show
        Pavels Jelisejevs added a comment - - edited (1) This fix must be documented in the 2.0 API changelog. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED. Pavels Jelisejevs Don't forget to document the remaining fixes when you're done.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (2) The same check should be performed for graphs, triggers, screens (both templated and plain) and all of the prototypes.

        Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30786

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (2) The same check should be performed for graphs, triggers, screens (both templated and plain) and all of the prototypes. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30786 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (3) You've added a $prototype parameter to the CGraphGeneral::checkInput() method. It's not the best solution, if you need some specific validation rules for either prototypes or graphs, just extend this method in the corresponding class.

        Please, pay extra attention to the check on line 146: it looks weird to me.

        Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (3) You've added a $prototype parameter to the CGraphGeneral::checkInput() method. It's not the best solution, if you need some specific validation rules for either prototypes or graphs, just extend this method in the corresponding class. Please, pay extra attention to the check on line 146: it looks weird to me. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (4) Don't use isset() to check the "templateid" property, because it will pass through NULLs. Use array_key_exists() instead.

        Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (4) Don't use isset() to check the "templateid" property, because it will pass through NULLs. Use array_key_exists() instead. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (5) A similar check has to be performed for template screens. There's a difference, though. In template screens, the "templateid" field refers to the template that the screen belongs to, not to the parent screen. So the "templateid" property must be set when calling create, but cannot be changed later.

        Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (5) A similar check has to be performed for template screens. There's a difference, though. In template screens, the "templateid" field refers to the template that the screen belongs to, not to the parent screen. So the "templateid" property must be set when calling create, but cannot be changed later. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801 Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (6) Since your fix affected the screen validation, may be you could also implement the correct validate methods for update and delete? See CService as an example. Keep in mind, that CTemplateScreen extends CScreen and already has some validation methods, maybe they can be unified.

        Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801
        For 'update' and 'create' (not 'delete'), because in 'delete' exist only one check

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (6) Since your fix affected the screen validation, may be you could also implement the correct validate methods for update and delete? See CService as an example. Keep in mind, that CTemplateScreen extends CScreen and already has some validation methods, maybe they can be unified. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801 For 'update' and 'create' (not 'delete'), because in 'delete' exist only one check Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (7) There's an error on the template configuration page.

        Declaration of CTemplateScreen::validateCreate() should be compatible with that of CScreen::validateCreate() [api/classes/CTemplateScreen.php:25]
        Declaration of CTemplateScreen::create() should be compatible with that of CScreen::create() [api/classes/CTemplateScreen.php:25]

        Make sure you have strict standard errors enabled.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs We always use type hinting for arrays, so instead of removing it, you need to add it where it's missing.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (7) There's an error on the template configuration page. Declaration of CTemplateScreen::validateCreate() should be compatible with that of CScreen::validateCreate() [api/classes/CTemplateScreen.php:25] Declaration of CTemplateScreen::create() should be compatible with that of CScreen::create() [api/classes/CTemplateScreen.php:25] Make sure you have strict standard errors enabled. Oleg Egorov RESOLVED Pavels Jelisejevs We always use type hinting for arrays, so instead of removing it, you need to add it where it's missing. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (8) I cannot create a plain screen:

        Cannot set "templateid" for screen. [CScreen.create -> CScreen.validateCreate]

        Oleg Egorov RESOLVED

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (8) I cannot create a plain screen: Cannot set "templateid" for screen. [CScreen.create -> CScreen.validateCreate] Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (9) Some issues regarding screens:

        1. In CScreen::validateCreate() we first need to check if the fields are correct, only then - if the screen doesn't exist.
        2. In CScreen::validateUpdate() permission checks and field checks should be performed in separate loops.
        3. Templateid is not validated when calling screen.update.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs Please review my changes in r30877. If everything is OK, it can be closed.

        Oleg Egorov CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (9) Some issues regarding screens: 1. In CScreen::validateCreate() we first need to check if the fields are correct, only then - if the screen doesn't exist. 2. In CScreen::validateUpdate() permission checks and field checks should be performed in separate loops. 3. Templateid is not validated when calling screen.update. Oleg Egorov RESOLVED Pavels Jelisejevs Please review my changes in r30877. If everything is OK, it can be closed. Oleg Egorov CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (10) In CTemplateScreen::validateUpdate():

        1. templateid must be validated after permissions;
        2. remove the old templateid check.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (10) In CTemplateScreen::validateUpdate(): 1. templateid must be validated after permissions; 2. remove the old templateid check. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (11) Some error message improvements:

        1. When calling update methods, the error should read "Cannot update" instead of "Cannot set";
        2. The error should always mention the name of the screen.

        So, for create methods it should be: "Cannot set "templateid" for screen "%1$s".
        For update methods: "Cannot update "templateid" for screen "%1$s".

        Keep in mind, that the name of the screen may not be passed to update methods, so you may have to pull it out of the database using the CZBXAPI::extendObjects() methods or in some other way.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (11) Some error message improvements: 1. When calling update methods, the error should read "Cannot update" instead of "Cannot set"; 2. The error should always mention the name of the screen. So, for create methods it should be: "Cannot set "templateid" for screen "%1$s". For update methods: "Cannot update "templateid" for screen "%1$s". Keep in mind, that the name of the screen may not be passed to update methods, so you may have to pull it out of the database using the CZBXAPI::extendObjects() methods or in some other way. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (12) In CTrigger::checkInput() templateid check must be performed before the permission check.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs Please review my changes in r30879.

        Oleg Egorov CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (12) In CTrigger::checkInput() templateid check must be performed before the permission check. Oleg Egorov RESOLVED Pavels Jelisejevs Please review my changes in r30879. Oleg Egorov CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (13) In CGraph::checkInput():

        1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid.
        2. We don't check permissions for super admins, so we don't need to make the API request for super admins.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs Before using items IDs to validate permissions, we need to validate the items and item IDs themselves. That is, the check in CGraphGeneral::checkInput() lines 66 and 73 have to be performed before permission checks.

        Also please review my changes in r30879.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs itemids also needed to be validated. Please review my changes in r30894.

        Oleg Egorov OK, CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (13) In CGraph::checkInput(): 1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid. 2. We don't check permissions for super admins, so we don't need to make the API request for super admins. Oleg Egorov RESOLVED Pavels Jelisejevs Before using items IDs to validate permissions, we need to validate the items and item IDs themselves. That is, the check in CGraphGeneral::checkInput() lines 66 and 73 have to be performed before permission checks. Also please review my changes in r30879. Oleg Egorov RESOLVED Pavels Jelisejevs itemids also needed to be validated. Please review my changes in r30894. Oleg Egorov OK, CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (14) In CGraphPrototype::checkInput():

        1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid.

        Oleg Egorov Duplicated in (13)

        Pavels Jelisejevs Sorry, I meant CGraphPrototype.

        Oleg Egorov RESOLVED

        Pavels Jelisejevs Same problem as in (13).

        Oleg Egorov RESOLVED

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (14) In CGraphPrototype::checkInput(): 1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid. Oleg Egorov Duplicated in (13) Pavels Jelisejevs Sorry, I meant CGraphPrototype. Oleg Egorov RESOLVED Pavels Jelisejevs Same problem as in (13). Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
        Hide
        Alexander Vladishev added a comment - - edited

        (15) Cannot Full clone a template: "Cannot set "templateid" for item."

        Oleg Egorov RESOLVED

        Pavels Jelisejevs Please review my changes in r30904.

        Oleg Egorov CLOSED

        Show
        Alexander Vladishev added a comment - - edited (15) Cannot Full clone a template: "Cannot set "templateid" for item." Oleg Egorov RESOLVED Pavels Jelisejevs Please review my changes in r30904. Oleg Egorov CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (16) Almost forgot: this also has to be done for applications.

        Oleg Egorov RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30863

        Pavels Jelisejevs Please review my changes in r30884.

        Oleg Egorov CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (16) Almost forgot: this also has to be done for applications. Oleg Egorov RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30863 Pavels Jelisejevs Please review my changes in r30884. Oleg Egorov CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (17) Cannot create a graph prototype:

        Undefined variable: allowedItems [api/classes/CGraphPrototype.php:890]
        Graph prototype must have at least one prototype. [CGraphGeneral.create -> CGraphPrototype.checkInput]

        Oleg Egorov This error not exist in my last revision

        RESOLVED

        Pavels Jelisejevs CLOSED.

        Show
        Pavels Jelisejevs added a comment - - edited (17) Cannot create a graph prototype: Undefined variable: allowedItems [api/classes/CGraphPrototype.php:890] Graph prototype must have at least one prototype. [CGraphGeneral.create -> CGraphPrototype.checkInput] Oleg Egorov This error not exist in my last revision RESOLVED Pavels Jelisejevs CLOSED.
        Hide
        Oleg Egorov added a comment -

        FIXED IN 2.0.4rc1 r30905, 2.1.0 (trunk) r30908

        Show
        Oleg Egorov added a comment - FIXED IN 2.0.4rc1 r30905, 2.1.0 (trunk) r30908

          People

          • Assignee:
            Oleg Egorov
            Reporter:
            Alexander Malaev
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: