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

[dev.icinga.com #9211] Empty filters are being rendered to SQL which leads to syntax errors #1634

Closed
icinga-migration opened this issue May 1, 2015 · 6 comments
Labels
area/framework Affects third party integration/development bug Something isn't working
Milestone

Comments

@icinga-migration
Copy link

This issue has been migrated from Redmine: https://dev.icinga.com/issues/9211

Created by jamesweakley on 2015-05-01 06:13:31 +00:00

Assignee: elippmann
Status: Resolved (closed on 2015-05-22 14:55:04 +00:00)
Target Version: 2.0.0-rc1
Last Update: 2015-05-22 14:55:04 +00:00 (in Redmine)


Did a fresh installation with a Postgres backend, using the following packages:

postgresql93-server.x86_64 9.3.6-1PGDG.rhel6 yum.postgresql.org icinga2.x86_64 2.3.4-1.el6 icinga2_yum_repo
icinga2-bin.x86_64 2.3.4-1.el6 icinga2_yum_repo icinga2-common.x86_64 2.3.4-1.el6 icinga2_yum_repo
icinga2-ido-pgsql.x86_64 2.3.4-1.el6 icinga2_yum_repo icingaweb2.noarch 2.0.0-3.beta3.el6 icinga2_yum_repo
icingaweb2-common.noarch 2.0.0-3.beta3.el6 icinga2_yum_repo icingaweb2-vendor-HTMLPurifier.noarch 4.6.0-1.el6 icinga2_yum_repo
icingaweb2-vendor-JShrink.noarch 1.0.1-1.el6 icinga2_yum_repo icingaweb2-vendor-Parsedown.noarch 1.0.0-1.el6 icinga2_yum_repo
icingaweb2-vendor-dompdf.noarch 0.6.1-1.el6 icinga2_yum_repo icingaweb2-vendor-lessphp.noarch 0.4.0-1.el6 icinga2_yum_repo
php-Icinga.noarch 2.0.0-3.beta3.el6 @icinga2_yum_repo

Received an error, see attached.

I have created a fix and will send a pull request shortly.

Attachments

  • sql_error.png jamesweakley - 2015-05-01 06:11:03 +00:00
  • sql_error.txt jamesweakley - 2015-05-01 06:12:30 +00:00 - From screenshot, but in text
  • stack_trace.txt jamesweakley - 2015-05-01 06:12:49 +00:00 - From screenshot, but in text

Changesets

2015-05-22 11:53:57 +00:00 by elippmann 83a6e85

lib: Don't render empty filters to SQL

Else we are presented with syntax errors.

fixes #9211
@icinga-migration
Copy link
Author

Updated by jamesweakley on 2015-05-01 06:32:02 +00:00

Pull request: #26

@icinga-migration
Copy link
Author

Updated by tgelf on 2015-05-01 10:09:05 +00:00

  • Status changed from New to Feedback
  • Assigned to set to jamesweakley

Good catch, thank you. May I ask whether you used the available Puppet modules for installation? I'd like to find the root cause for this and similar issues. I guess you have something like...

monitoring/hosts/filter = ""
monitoring/services/filter = ""

...in your roles.ini. While Icinga Web should be able to cope with this I do not really want such useless config to be generated at all.

Best,
Thomas

@icinga-migration
Copy link
Author

Updated by jamesweakley on 2015-05-01 21:45:25 +00:00

Yep, nailed it, that's exactly what I have due to using puppet for installation.

I spent a while digging around the code base trying to find where the empty filter was finding its way into the clause, being completely new to icinga I didn't even think of those files and didn't realise that:
$this->applyRestriction('monitoring/services/filter', $service);
refered to the values in roles.ini.

In the end I gave up and worked around it downstream as you can see.

I'll look at wrapping the host and service filter stuff in:
https://github.com/Icinga/puppet-icingaweb2/blob/master/manifests/config/roles.pp
with if statements to skip the line if the values are undef, and send a pull request for that too.

In future, I'll also include a unit test case with the icingaweb2 pull requests.

@icinga-migration
Copy link
Author

Updated by jamesweakley on 2015-05-03 03:46:16 +00:00

Pull request for puppet fix:
Icinga/puppet-icingaweb2#13

@icinga-migration
Copy link
Author

Updated by elippmann on 2015-05-22 11:53:32 +00:00

  • Subject changed from SQL error in /monitoring/service/show on fresh install to Empty filters are being rendered to SQL which leads to syntax errors
  • Category changed from UI to Framework
  • Status changed from Feedback to Assigned
  • Assigned to changed from jamesweakley to elippmann
  • Target Version set to 2.0.0-rc1
  • Done % changed from 100 to 0

Hi,

I changed the subject because it's bug in our framework. We're rendering empty filters which leads to syntax errors. I'll fix it in a second.

Cheers,
Eric

@icinga-migration
Copy link
Author

Updated by elippmann on 2015-05-22 14:55:04 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 0 to 100

Applied in changeset 83a6e85.

@icinga-migration icinga-migration added bug Something isn't working area/framework Affects third party integration/development labels Jan 17, 2017
@icinga-migration icinga-migration added this to the 2.0.0-rc1 milestone Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/framework Affects third party integration/development bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant