Skip to content

Conversation

@afeefghannam89
Copy link
Member

fixes #63

@widhalmt
Copy link
Member

widhalmt commented Apr 11, 2019

Thanks for the Pull request. I don't know why this has slipped through my attention - sorry.

I'm sorry, but I think, we need some changes:

  • You hit a bug where a file named 1 is created. This is fixed in master but you added the file to your pull request.
  • You are using standard credentials for the API. Hopefully only users on test-systems use them. We need to get real credentials. Maybe this is one of the things easier achieved with a rewrite in python and you should wait for the rewrite.
  • You should use the ca.crt file of Icinga to verify the connection instead of using -k. The file should be available on every Icinga host. While I don't think it would really pose a security risk, it's just better to verify every connection.
  • I haven't tried if this is already the case but I think, collecting the zones right after startup is better so the user can start, give the zones and then just let the script do it's job. If you ask for them later, the user can't just start the script but hast to wait then the zone part is reached. On bigger systems it can take a while to complete.

Could you please do the changes so I can merge your pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check for dependencies between check_source and failing services

2 participants