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

heck for dependencies between check_source and failing services #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

heck for dependencies between check_source and failing services #73

wants to merge 1 commit into from

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