Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When updating poller name, duplicate name protection may be over zealous #4455

Closed
ctrowat opened this issue Nov 3, 2021 · 4 comments · Fixed by #4456
Closed

When updating poller name, duplicate name protection may be over zealous #4455

ctrowat opened this issue Nov 3, 2021 · 4 comments · Fixed by #4456
Labels
bug Undesired behaviour gui UI related issue resolved A fixed issue
Milestone

Comments

@ctrowat
Copy link
Contributor

ctrowat commented Nov 3, 2021

Describe the bug

When editing properties of remote pollers the query for detecting duplicate poller hostnames and database hostnames is missing critical brackets

To Reproduce

Deploy Cacti with a remote poller, try to edit any property of that remote poller. I receive an error to "Please enter a non-duplicate hostname" as well as "Please enter a non-duplicate database hostname".

Expected behavior

The remote poller gets renamed.

Initial analysis

The query building that starts here:
https://github.com/Cacti/cacti/blob/1.2.x/pollers.php#L386

Ends with a query being written like this:

SELECT id FROM poller WHERE id != 2 AND dbhost IN ('192.168.1.2') OR ((dbhost = 'cacti-poller-01' OR dbhost LIKE 'cacti-poller-01%' OR dbhost = 'cacti-poller-01.example.com'))

When the poller already exists in the database with the id of 2 it is returned in this query because of the 'OR' expression not being bracketed together with the dbhost part of the clause.

I believe the query should be written as follows:

SELECT id FROM poller WHERE id != 2 AND (dbhost IN ('192.168.1.2') OR ((dbhost = 'cacti-poller-01' OR dbhost LIKE 'cacti-poller-01%' OR dbhost = 'cacti-poller-01.example.com')))

Note the extra brackets that begin after the 'AND'

@ctrowat ctrowat added bug Undesired behaviour unverified Some days we don't have a clue labels Nov 3, 2021
@netniV
Copy link
Member

netniV commented Nov 3, 2021

That sounds reasonable, can you submit a PR for this?

@ctrowat
Copy link
Contributor Author

ctrowat commented Nov 3, 2021

I can try to put a PR together for this - I'm just finishing up some other tasks at the moment but recognize that I definitely should make an effort to contribute back to a project that gives us so much.

I am curious about like 397, where it adds the wildcard to the host portion of the name. I don't feel like this is right, if you had a remote poller named cacti-poller.example.com then later added cacti-poller-2.example.com it would falsely call that a duplicate. Perhaps it should match only on the first section or the FQDN, and not just match anything that starts with the first portion of the host name (in this case cacti-poller%)?

If I submit a fix for this, would it be okay for me to remove that line?

@netniV
Copy link
Member

netniV commented Nov 4, 2021

I'm not sure why it's so inclusive. When you highlighted the code, my first thought was that it could match extra numbers or other suffixes of additional hosts but there must have been a user case for it.

Feel free to amend the query as you see right, it will get peer reviewed anyway:)

@ctrowat
Copy link
Contributor Author

ctrowat commented Nov 5, 2021

I have submitted PR #4456

@netniV netniV added gui UI related issue resolved A fixed issue and removed unverified Some days we don't have a clue labels Nov 7, 2021
@netniV netniV added this to the v1.2.20 milestone Nov 7, 2021
@netniV netniV linked a pull request Nov 7, 2021 that will close this issue
@netniV netniV changed the title Unable to edit properties of remote pollers - please enter a non-duplicate hostname/database hostname When updating poller name, matches may be too wide in scope Nov 7, 2021
@netniV netniV changed the title When updating poller name, matches may be too wide in scope When updating poller name, duplicate name protection may be over zealous Apr 3, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour gui UI related issue resolved A fixed issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants